Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Increase default MAX_SESS_STKCTR or provide a way to change it after build time #1565

Closed
ironashram opened this issue Feb 21, 2022 · 14 comments
Labels
status: fixed This issue is a now-fixed bug. type: feature This issue describes a feature request / wishlist.

Comments

@ironashram
Copy link

ironashram commented Feb 21, 2022

Your Feature Request

The current default value of 3 for MAX_SESS_STKCTR is quite low while dealing with a complex setup.
There are also situations where you cannot easily build haproxy yourself without a lot of additional overhead.

My current setup uses haproxy to rate-limit requests by tracking request rate using source ips as keys in stick-tables
i have stick-tables defined as dummy backends and in each one of them i track counters with tcp-request connection track-scX or http-request track-scX
then based on a few acls i decide which backend needs to be used or what needs to be done to that request/connection
this works perfectly fine but i've reached a point where i need to add more rate limiting based on base32+src/url32+src and here comes the issue with MAX_SESS_STKCTR

in order to achieve what i need i have the following options

  • drop some of the existing logic to add the new one, which i don't want to do
  • use a custom build of haproxy with a higher MAX_SESS_STKCTR, this one is indeed an option but given that i use the haproxy-ingress controller (it's based on the haproxy:alpine image) on k8s that brings to the table quite a lot of challenges and would require a lot more effort to maintain

I hope this information is enough to at least get the idea of why i'd like to have more flexibility on this and decided to open this issue, i cannot share the actual haproxy configuration

My suggestion would be to either increase the default value or even better provide a way for it to be changed after build time, like in a global config or something like that

What are you trying to do?

Track more than 3 counters without building haproxy from source

Output of haproxy -vv

/ # haproxy -vv
HA-Proxy version 2.3.14-83c5b44 2021/09/07 - https://haproxy.org/
Status: stable branch - will stop receiving fixes around Q1 2022.
Known bugs: http://www.haproxy.org/bugs/bugs-2.3.14.html
Running on: Linux 5.10.43-flatcar #1 SMP Tue Jun 15 19:46:57 -00 2021 x86_64
Build options :
  TARGET  = linux-musl
  CPU     = generic
  CC      = cc
  CFLAGS  = -O2 -g -Wall -Wextra -Wdeclaration-after-statement -fwrapv -Wno-address-of-packed-member -Wno-unused-label -Wno-sign-compare -Wno-unused-parameter -Wno-clobbered -Wno-missing-field-initializers -Wno-cast-function-type -Wtype-limits -Wshift-negative-value -Wshift-overflow=2 -Wduplicated-cond -Wnull-dereference
  OPTIONS = USE_PCRE2=1 USE_PCRE2_JIT=1 USE_GETADDRINFO=1 USE_OPENSSL=1 USE_LUA=1 USE_ZLIB=1
  DEBUG   = 

Feature list : +EPOLL -KQUEUE +NETFILTER -PCRE -PCRE_JIT +PCRE2 +PCRE2_JIT +POLL -PRIVATE_CACHE +THREAD -PTHREAD_PSHARED -BACKTRACE -STATIC_PCRE -STATIC_PCRE2 +TPROXY +LINUX_TPROXY +LINUX_SPLICE +LIBCRYPT +CRYPT_H +GETADDRINFO +OPENSSL +LUA +FUTEX +ACCEPT4 -CLOSEFROM +ZLIB -SLZ +CPU_AFFINITY +TFO +NS +DL +RT -DEVICEATLAS -51DEGREES -WURFL -SYSTEMD -OBSOLETE_LINKER +PRCTL +THREAD_DUMP -EVPORTS

Default settings :
  bufsize = 16384, maxrewrite = 1024, maxpollevents = 200

Built with multi-threading support (MAX_THREADS=64, default=16).
Built with OpenSSL version : OpenSSL 1.1.1l  24 Aug 2021
Running on OpenSSL version : OpenSSL 1.1.1l  24 Aug 2021
OpenSSL library supports TLS extensions : yes
OpenSSL library supports SNI : yes
OpenSSL library supports : TLSv1.0 TLSv1.1 TLSv1.2 TLSv1.3
Built with Lua version : Lua 5.3.6
Built with network namespace support.
Built with the Prometheus exporter as a service
Built with zlib version : 1.2.11
Running on zlib version : 1.2.11
Compression algorithms supported : identity("identity"), deflate("deflate"), raw-deflate("deflate"), gzip("gzip")
Built with transparent proxy support using: IP_TRANSPARENT IPV6_TRANSPARENT IP_FREEBIND
Built with PCRE2 version : 10.36 2020-12-04
PCRE2 library supports JIT : yes
Encrypted password support via crypt(3): yes
Built with gcc compiler version 10.3.1 20210424

Available polling systems :
      epoll : pref=300,  test result OK
       poll : pref=200,  test result OK
     select : pref=150,  test result OK
Total: 3 (3 usable), will use epoll.

Available multiplexer protocols :
(protocols marked as <default> cannot be specified using 'proto' keyword)
              h2 : mode=HTTP       side=FE|BE     mux=H2
            fcgi : mode=HTTP       side=BE        mux=FCGI
       <default> : mode=HTTP       side=FE|BE     mux=H1
       <default> : mode=TCP        side=FE|BE     mux=PASS

Available services : prometheus-exporter
Available filters :
        [SPOE] spoe
        [CACHE] cache
        [FCGI] fcgi-app
        [COMP] compression
        [TRACE] trace

@ironashram ironashram added the type: feature This issue describes a feature request / wishlist. label Feb 21, 2022
@TimWolla

This comment was marked as resolved.

@ironashram

This comment was marked as resolved.

@lukastribus
Copy link
Member

The current default value of 3 for MAX_SESS_STKCTR is quite low while dealing with a complex setup.

Please elaborate and explain your exact use case with a concrete example beyond "too low for complex setups". This is a feature/change request, explaining the context is necessary to understand the situation.

@ironashram
Copy link
Author

The current default value of 3 for MAX_SESS_STKCTR is quite low while dealing with a complex setup.

Please elaborate and explain your exact use case with a concrete example beyond "too low for complex setups". This is a feature/change request, explaining the context is necessary to understand the situation.

edited with as much information as i can share, i hope it's enough

@wtarreau
Copy link
Member

I really am not in favor of changing the default value because it increases both memory usage and CPU usage per request for every single user only for very rare niche use cases. I'm aware of some users exploiting this to its maximum to build DDoS protection infrastructure, but I consider that when you're going down that route you can afford to build your LB yourself with your optimal settings.

The essential question is: why do you really need more than 3 keys ? 3 allows for example to have counters per src, counters per URL and counters per src+url, which usually are what most users deal with. Since 2.5 you have arrays of up to 100 GPC/GPT for a single tracked key, which means up to 300 metrics per request involving up to 3 different keys without having to rebuild. Unless you can prove that you have a very compelling case that would serve the vast majority of the user base which is not covered by this, I'd prefer not to change this.

Another option is that if you think your use case is closely related to the use of your distro, you may turn to the distro to negotiate a change of the default value in their packages.

@Tristan971
Copy link
Member

Tristan971 commented Jun 7, 2022

I was originally going to send this as a question on the mailing list, good to see it here

While SCxGPC provides enough cardinality, stick-tables have a large advantage in that they directly translate into (limited, but still) prometheus metrics.

I'm aware of some users exploiting this to its maximum to build DDoS protection infrastructure
...
3 allows for example to have counters per src, counters per URL and counters per src+url, which usually are what most users deal with

This is correct in my experience, however it misses the fact that you are then dedicating all of your counters to DDoS handling (in our case, progressively stricter bucketing of malicious sources).

Once these are all exhausted, you're a bit out of luck for monitoring miscellaneous things (protocol versions, ciphers, ...).

That said, all of this can be somewhat worked around by way of the admin API and SCxGPC, but that means a somewhat significant extra piece of software+processing, which isn't quite ideal

That said this is certainly overall niche and

it increases both memory usage and CPU usage per request

Definitely not a worthy tradeoff to change the default in general then indeed...

If it's not too specific to use-cases, how bad is it CPU-wise to increase it on our own build. As in, is the order of magnitude of the impact a fraction-of-a-% of CPU core or more significant?

(the memory use of haproxy is insignificant enough that it doesn't matter to us even if it doubled to be honest)

@wtarreau
Copy link
Member

wtarreau commented Jun 7, 2022

If it's not too specific to use-cases, how bad is it CPU-wise to increase it on our own build. As in, is the order of magnitude of the impact a fraction-of-a-% of CPU core or more significant?

It's only within the percent. We're doing it in the enterprise packages because a few users want this and we're not going to ask them to rebuild. Thus it's totally manageable for local builds. You can't go higher than 10 because the command parser relies on a single digit after scXgpcY, track-scX and friends. But even at 10 it already starts to allow users to write extremely ugly configurations. This is the main risk by the way. Keep in mind that such resources are precious and that abusing them will result in horribly undebuggable configs.

@Tristan971
Copy link
Member

Tristan971 commented Jun 7, 2022

I see, thanks for clarifying that.

As for the ugly configs... The fancy DDoS setups are indeed not pretty (however there are not many other choices except hiding behind CF like everyone else but that's another topic). Anyway, for that purpose I agree 3 SCs works well enough as you mentioned.

I don't know exactly how others want to (ab)use more SCs, but here I am mostly interested in snippets like so:

acl proto_ssl_tlsv12 ssl_fc_protocol -m str -i TLSv1.2
acl proto_ssl_tlsv13 ssl_fc_protocol -m str -i TLSv1.3
http-request track-sc2 src table st_proto_tls12 if proto_ssl_tlsv12
http-request track-sc2 src table st_proto_tls13 if proto_ssl_tlsv13

then

sum(haproxy_sticktable_used{name=~"st_proto.+"}) by (name)

proto

for quickly monitoring the occasional few things I'm interested in that aren't part of the default metricset, but are ACL-able otherwise

Eventually I'll probably give up and use some intermediate piece of code using the admin socket and specific GPCs but... I don't hate this workaround enough for that yet 😄 (especially considering I'd still need some sc-inc-gpcN anyway)

@wtarreau
Copy link
Member

wtarreau commented Jun 7, 2022

That's typically an example where you could even have used a single table and a single track-sc. Just a few examples:

  • store the protocol version in use as a bit in gpt0:
http-request track-sc2 src
http-request sc-set-gpt0(2) sc_get_gpt0(2),or(1) if { ssl_fc_protocol -m str -i TLSv1.2 }
http-request sc-set-gpt0(2) sc_get_gpt0(2),or(2) if { ssl_fc_protocol -m str -i TLSv1.3 }
  • or append the version to the key:
http-request track-sc2 src,concat(-tls1.2) if { ssl_fc_protocol -m str -i TLSv1.2 }
http-request track-sc2 src,concat(-tls1.3) if { ssl_fc_protocol -m str -i TLSv1.3 }

These are not necessarily prettier, but that's just to illustrate that there are various possibilities already even without changing the number of stick pointers.

@Tristan971
Copy link
Member

Tristan971 commented Jun 7, 2022

That's typically an example where you could even have used a single table and a single track-sc

Indeed that would have been my first idea, but then those aren't prometheus metrics without me writing an extra piece of software to extract it either via the admin or dataplane api, alas... But long term those are definitely the right approach for this yes

@superstes
Copy link

I have also encountered setups where the MAX_SESS_STKCTR of 3 is too low.
From an admin perspective I do not understand why this isn't a global setting that can be increased easily.

The performance impact can't be that bad as HAPEE has also set it to 12 by default.

@Tristan971
Copy link
Member

Tristan971 commented May 18, 2024

Actually it is now a tunable, since I think sometime during 2.8? https://docs.haproxy.org/2.8/configuration.html#3.2-tune.stick-counters

@wtarreau
Copy link
Member

For memory usage reasons it used to be just an array inside a structure, and it was not reasonable to have a large array for everyone given how rare such users are. For HAPEE it's different, the list of supported systems is narrower and users are expected to have large servers and lots of RAM. Regardless, the previous limit of 12 was still considered too low by a few users who needed much more, to a point that we didn't consider reasonable even for other users, so we finally moved that out of the struct and made it dynamically allocated so that everyone sets their limit according to their RAM (and slightly increased CPU usage as well when scanning all of these).

@superstes
Copy link

Good to hear 👍
Had not yet found tune.stick-counters.

@TimWolla TimWolla added the status: fixed This issue is a now-fixed bug. label May 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: fixed This issue is a now-fixed bug. type: feature This issue describes a feature request / wishlist.
Projects
None yet
Development

No branches or pull requests

6 participants