Re: sc-set-gpt with expression: internal error, unexpected rule->from=0, please report this bug!

2023-08-14 Thread Willy Tarreau
On Thu, Aug 10, 2023 at 01:59:34PM +0200, Johannes Naab wrote:
> On 8/9/23 17:53, Aurelien DARRAGON wrote:
> >> "http-request sc-set-gpt" does work, so does "tcp-request session". I.e.
> >> the bug seems to depend on "tcp-request connection".
> >>
> > 
> > Indeed, according to both doc and code, sc-set-gpt and sc-set-gpt0 are
> > available from:
> > 
> > - tcp-request session
> > - tcp-request content
> > - tcp-response content
> > - http-request
> > - http-response
> > - http-after-response
> > 
> > But, according to the doc, they are also available from:
> > - tcp-request connection
> > 
> > But the switch-cases in parse_set_gpt(), action_set_gpt(), and
> > action_set_gpt0() from stick_table.c don't allow this case, so it looks
> > like it was forgotten indeed when the expr support was added for
> > sc-set-gpt0 in 0d7712dff0 ("MINOR: stick-table: allow sc-set-gpt0 to set
> > value from an expression").
> > 
> > We have the same issue for the sc-add-gpc action which was greatly
> > inspired from set-gpt, where the switch cases defined in parse_add_gpc()
> > and action_add_gpc() from stick_table.c don't allow tcp-request
> > connection as origin.
> > 
> > Please find the attached patches that should help solve the above issues.
> 
> Thanks. With those patches "tcp-request connection" does work as well,
> even with setting session variables.

Thanks to you two, I've applied your 3 patches now (Aurélien's code fixes
and Johannes' doc fixes).

Willy



Re: sc-set-gpt with expression: internal error, unexpected rule->from=0, please report this bug!

2023-08-10 Thread Johannes Naab
On 8/9/23 17:53, Aurelien DARRAGON wrote:
>> "http-request sc-set-gpt" does work, so does "tcp-request session". I.e.
>> the bug seems to depend on "tcp-request connection".
>>
> 
> Indeed, according to both doc and code, sc-set-gpt and sc-set-gpt0 are
> available from:
> 
> - tcp-request session
> - tcp-request content
> - tcp-response content
> - http-request
> - http-response
> - http-after-response
> 
> But, according to the doc, they are also available from:
> - tcp-request connection
> 
> But the switch-cases in parse_set_gpt(), action_set_gpt(), and
> action_set_gpt0() from stick_table.c don't allow this case, so it looks
> like it was forgotten indeed when the expr support was added for
> sc-set-gpt0 in 0d7712dff0 ("MINOR: stick-table: allow sc-set-gpt0 to set
> value from an expression").
> 
> We have the same issue for the sc-add-gpc action which was greatly
> inspired from set-gpt, where the switch cases defined in parse_add_gpc()
> and action_add_gpc() from stick_table.c don't allow tcp-request
> connection as origin.
> 
> Please find the attached patches that should help solve the above issues.

Thanks. With those patches "tcp-request connection" does work as well,
even with setting session variables.

Johannes



Re: sc-set-gpt with expression: internal error, unexpected rule->from=0, please report this bug!

2023-08-09 Thread Aurelien DARRAGON
>> I have no idea what causes it at the moment. A few things you could try,
>> in any order, to help locate the bug:
>>
>>   - check if it accepts it using "http-request sc-set-gpt" instead of
>> "tcp-request connection" so that we know if it's related to the ruleset
>> or something else ;
>>
> 
> Thanks, that seems to narrow the problem down.
> 
> "http-request sc-set-gpt" does work, so does "tcp-request session". I.e.
> the bug seems to depend on "tcp-request connection".
> 
> "session" works for me, for setting session variables it might even be
> necessary, but those might be avoidable by setting the conditional
> directly.
> (But not trivially since "sub()" only takes values or variables
> but not fetches and "-m int gt " only seem to takes direct
> values).

Indeed, according to both doc and code, sc-set-gpt and sc-set-gpt0 are
available from:

- tcp-request session
- tcp-request content
- tcp-response content
- http-request
- http-response
- http-after-response

But, according to the doc, they are also available from:
- tcp-request connection

But the switch-cases in parse_set_gpt(), action_set_gpt(), and
action_set_gpt0() from stick_table.c don't allow this case, so it looks
like it was forgotten indeed when the expr support was added for
sc-set-gpt0 in 0d7712dff0 ("MINOR: stick-table: allow sc-set-gpt0 to set
value from an expression").

We have the same issue for the sc-add-gpc action which was greatly
inspired from set-gpt, where the switch cases defined in parse_add_gpc()
and action_add_gpc() from stick_table.c don't allow tcp-request
connection as origin.

Please find the attached patches that should help solve the above issues.

AurelienFrom b66b401ddb36a4c686fa0df965492da204ba66a8 Mon Sep 17 00:00:00 2001
From: Aurelien DARRAGON 
Date: Wed, 9 Aug 2023 17:39:29 +0200
Subject: [PATCH 2/2] BUG/MINOR: stktable: allow sc-add-gpc from tcp-request
 connection

Following the previous commit's logic, we enable the use of sc-add-gpc
from tcp-request connection since it was probably forgotten in the first
place for sc-set-gpt0, and since sc-add-gpc was inspired from it, it also
lacks its.

As sc-add-gpc was implemented in 5a72d03a58 ("MINOR: stick-table: implement
the sc-add-gpc() action"), this should only be backported to 2.8
---
 src/stick_table.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/stick_table.c b/src/stick_table.c
index 363269f01..b11e94961 100644
--- a/src/stick_table.c
+++ b/src/stick_table.c
@@ -2913,6 +2913,7 @@ static enum act_return action_add_gpc(struct act_rule *rule, struct proxy *px,
 			value = (unsigned int)(rule->arg.gpc.value);
 		else {
 			switch (rule->from) {
+			case ACT_F_TCP_REQ_CON: smp_opt_dir = SMP_OPT_DIR_REQ; break;
 			case ACT_F_TCP_REQ_SES: smp_opt_dir = SMP_OPT_DIR_REQ; break;
 			case ACT_F_TCP_REQ_CNT: smp_opt_dir = SMP_OPT_DIR_REQ; break;
 			case ACT_F_TCP_RES_CNT: smp_opt_dir = SMP_OPT_DIR_RES; break;
@@ -3013,6 +3014,7 @@ static enum act_parse_ret parse_add_gpc(const char **args, int *arg, struct prox
 			return ACT_RET_PRS_ERR;
 
 		switch (rule->from) {
+		case ACT_F_TCP_REQ_CON: smp_val = SMP_VAL_FE_CON_ACC; break;
 		case ACT_F_TCP_REQ_SES: smp_val = SMP_VAL_FE_SES_ACC; break;
 		case ACT_F_TCP_REQ_CNT: smp_val = SMP_VAL_FE_REQ_CNT; break;
 		case ACT_F_TCP_RES_CNT: smp_val = SMP_VAL_BE_RES_CNT; break;
-- 
2.34.1

From 0b3586a30a8181316477140daf56dd3309b1f6f1 Mon Sep 17 00:00:00 2001
From: Aurelien DARRAGON 
Date: Wed, 9 Aug 2023 17:23:32 +0200
Subject: [PATCH 1/2] BUG/MINOR: stktable: allow sc-set-gpt(0) from tcp-request
 connection

Both the documentation and original developer intents seem to suggest
that sc-set-gpt/sc-set-gpt0 actions should be available from
tcp-request connection.

Yet because it was probably forgotten when expr support was added to
sc-set-gpt0 in 0d7712dff0 ("MINOR: stick-table: allow sc-set-gpt0 to
set value from an expression") it doesn't work and will report this
kind of errors:
 "internal error, unexpected rule->from=0, please report this bug!"

Fixing the code to comply with the documentation and the expected
behavior.

This must be backported to every stable versions.

[for < 2.5, as only sc-set-gpt0 existed back then, the patch must be
manually applied to skip irrelevant parts]
---
 src/stick_table.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/stick_table.c b/src/stick_table.c
index a2aa9c451..363269f01 100644
--- a/src/stick_table.c
+++ b/src/stick_table.c
@@ -2656,6 +2656,7 @@ static enum act_return action_set_gpt(struct act_rule *rule, struct proxy *px,
 			value = (unsigned int)(rule->arg.gpt.value);
 		else {
 			switch (rule->from) {
+			case ACT_F_TCP_REQ_CON: smp_opt_dir = SMP_OPT_DIR_REQ; break;
 			case ACT_F_TCP_REQ_SES: smp_opt_dir = SMP_OPT_DIR_REQ; break;
 			case ACT_F_TCP_REQ_CNT: smp_opt_dir = SMP_OPT_DIR_REQ; break;
 			case ACT_F_TCP_RES_CNT: smp_opt_dir = SMP_OPT_DIR_RES; break;
@@ -2724,6 +2725,7 @@ static enum act_return action_set_gpt0(struct act_rule *rule, st

Re: sc-set-gpt with expression: internal error, unexpected rule->from=0, please report this bug!

2023-08-09 Thread Johannes Naab
Hi Willy,

On 8/9/23 13:48, Willy Tarreau wrote:
> Hi Johannes,
> 
> On Wed, Aug 09, 2023 at 01:02:29PM +0200, Johannes Naab wrote:
>> Hi,
>>
>> I'm trying to use a stick table with general purpose tags (gpt) to do longer
>> term (beyond the window itself) maximum connection rate tracking:
>> - stick table with conn_rate and one gpt
>> - update/set gpt0 if the current conn_rate is greater than what is stored in 
>> the gpt.
>>
>> But I have trouble setting the gpt even from a trivial sample expression,
>> erroring during config parsing with `internal error, unexpected rule->from=0,
>> please report this bug!`.
> 
> At first glance I can't find a reason why your config would not work,
> so you've definitely discovered a bug.
> 
> I have no idea what causes it at the moment. A few things you could try,
> in any order, to help locate the bug:
> 
>   - check if it accepts it using "http-request sc-set-gpt" instead of
> "tcp-request connection" so that we know if it's related to the ruleset
> or something else ;
> 

Thanks, that seems to narrow the problem down.

"http-request sc-set-gpt" does work, so does "tcp-request session". I.e.
the bug seems to depend on "tcp-request connection".

"session" works for me, for setting session variables it might even be
necessary, but those might be avoidable by setting the conditional
directly.
(But not trivially since "sub()" only takes values or variables
but not fetches and "-m int gt " only seem to takes direct
values).

"tcp-request connection" state could be helpful to avoid TLS handshakes.


>   - please also try sc0-set-gpt(0) instead of sc-set-gpt(0,0), maybe there
> is something wrong in the latter's parser.
> 

That does not seem to make any difference.

>   - does your other test with "int(1)" as the expression also fail or did
> it work ? If it did work, maybe forcing a cat to integer on the variable
> using "var(proc.baz),add(0)" could work.
> 

Any expression fails in "tcp-request connection", even the more trivial
"int(1)", "var(proc.baz),add(0)" does fail as well.

> In any case some feedback on these points could be useful. The last two
> ones would be safe workarounds if they work.
> 
> 

For completeness a running/working config for tracking the max conn_rate
(https://xkcd.com/979/):

```
frontend foo
bind :::8080 v4v6
default_backend bar
tcp-request connection track-sc0 src table stick1

## track max conn_rate
tcp-request session set-var(sess.prev_conn_rate) sc_get_gpt(0,0,stick1)
tcp-request session set-var(sess.cur_conn_rate) sc_conn_rate(0,stick1)
tcp-request session sc-set-gpt(0,0) var(sess.cur_conn_rate) if { 
var(sess.cur_conn_rate),sub(sess.prev_conn_rate) -m int gt 0 }

http-response set-header cur-conn-rate %[var(sess.cur_conn_rate)]
http-response set-header prev-conn-rate %[var(sess.prev_conn_rate)]

backend stick1
stick-table type ipv6 size 1m expire 1h store conn_rate(10s),gpt(1)
```

Thanks!
Johannes


>> Config, output, and haproxy -vv below.
>>
>> Should this work, or do I misunderstand what sc-set-gpt can achieve?
> 
> For me it should work, and if there's a corner case that makes it
> impossible with your config, I'm not seeing it and we should report it
> in a much more user-friendly way!
> 
> Thanks!
> Willy
> 




Re: sc-set-gpt with expression: internal error, unexpected rule->from=0, please report this bug!

2023-08-09 Thread Willy Tarreau
Hi Johannes,

On Wed, Aug 09, 2023 at 01:02:29PM +0200, Johannes Naab wrote:
> Hi,
> 
> I'm trying to use a stick table with general purpose tags (gpt) to do longer
> term (beyond the window itself) maximum connection rate tracking:
> - stick table with conn_rate and one gpt
> - update/set gpt0 if the current conn_rate is greater than what is stored in 
> the gpt.
> 
> But I have trouble setting the gpt even from a trivial sample expression,
> erroring during config parsing with `internal error, unexpected rule->from=0,
> please report this bug!`.

At first glance I can't find a reason why your config would not work,
so you've definitely discovered a bug.

I have no idea what causes it at the moment. A few things you could try,
in any order, to help locate the bug:

  - check if it accepts it using "http-request sc-set-gpt" instead of
"tcp-request connection" so that we know if it's related to the ruleset
or something else ;

  - please also try sc0-set-gpt(0) instead of sc-set-gpt(0,0), maybe there
is something wrong in the latter's parser.

  - does your other test with "int(1)" as the expression also fail or did
it work ? If it did work, maybe forcing a cat to integer on the variable
using "var(proc.baz),add(0)" could work.

In any case some feedback on these points could be useful. The last two
ones would be safe workarounds if they work.


> Config, output, and haproxy -vv below.
> 
> Should this work, or do I misunderstand what sc-set-gpt can achieve?

For me it should work, and if there's a corner case that makes it
impossible with your config, I'm not seeing it and we should report it
in a much more user-friendly way!

Thanks!
Willy



sc-set-gpt with expression: internal error, unexpected rule->from=0, please report this bug!

2023-08-09 Thread Johannes Naab
Hi,

I'm trying to use a stick table with general purpose tags (gpt) to do longer 
term (beyond the window itself) maximum connection rate tracking:
- stick table with conn_rate and one gpt
- update/set gpt0 if the current conn_rate is greater than what is stored in 
the gpt.

But I have trouble setting the gpt even from a trivial sample expression, 
erroring during config parsing with `internal error, unexpected rule->from=0, 
please report this bug!`.

Config, output, and haproxy -vv below.

Should this work, or do I misunderstand what sc-set-gpt can achieve?

Best regards,
Johannes


config
```
global
log stdout format raw local0
stats socket /run/haproxy/admin.sock mode 660 level admin
stats timeout 30s

set-var proc.baz int(3)

defaults
log global
modehttp
timeout connect 5000
timeout client  5
timeout server  5

frontend foo
bind :::8080 v4v6
default_backend bar
tcp-request connection track-sc0 src table stick1
tcp-request connection sc-set-gpt(0,0) var(proc.baz)
# tcp-request connection sc-set-gpt(0,0) int(1)
http-response set-header conn-rate %[sc_get_gpt(0,0,stick1)]

## track max conn_rate
#tcp-request connection set-var(sess.prev_conn_rate) 
sc_get_gpt(0,0,stick1)
#tcp-request connection set-var(sess.cur_conn_rate) 
sc_conn_rate(0,stick1)
#tcp-request connection sc-set-gpt(0,0) var(sess.cur_conn_rate) if { 
var(sess.cur_conn_rate),sub(sess.prev_conn_rate) -m int gt 0 }

backend bar
server localhost 127.0.0.1:80

backend stick1
stick-table type ipv6 size 1m expire 1h store conn_rate(10s),gpt(1)
```

error
```
# ./haproxy -f ~/haproxy.cfg
[NOTICE]   (139304) : haproxy version is 2.9-dev2-227317-63
[NOTICE]   (139304) : path to executable is ./haproxy
[ALERT](139304) : config : parsing [/root/haproxy.cfg:19] : internal error, 
unexpected rule->from=0, please report this bug!
[ALERT](139304) : config : Error(s) found in configuration file : 
/root/haproxy.cfg
[ALERT](139304) : config : Fatal errors found in configuration.
```

`haproxy -vv` (initally on 2.6, but it still occurs in recent git)
```
HAProxy version 2.9-dev2-227317-63 2023/08/09 - https://haproxy.org/
Status: development branch - not safe for use in production.
Known bugs: https://github.com/haproxy/haproxy/issues?q=is:issue+is:open
Running on: Linux 5.15.0-73-generic #80-Ubuntu SMP Mon May 15 15:18:26 UTC 2023 
x86_64
Build options :
  TARGET  = linux-glibc
  CPU = generic
  CC  = cc
  CFLAGS  = -O2 -g -Wall -Wextra -Wundef -Wdeclaration-after-statement 
-Wfatal-errors -Wtype-limits -Wshift-negative-value -Wshift-overflow=2 
-Wduplicated-cond -Wnull-dereference -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 -Wno-string-plus-int 
-Wno-atomic-alignment
  OPTIONS = USE_OPENSSL=1 USE_SYSTEMD=1 USE_PCRE=1
  DEBUG   = -DDEBUG_STRICT -DDEBUG_MEMORY_POOLS

Feature list : -51DEGREES +ACCEPT4 +BACKTRACE -CLOSEFROM +CPU_AFFINITY +CRYPT_H 
-DEVICEATLAS +DL -ENGINE +EPOLL -EVPORTS +GETADDRINFO -KQUEUE -LIBATOMIC 
+LIBCRYPT +LINUX_SPLICE +LINUX_TPROXY -LUA -MATH -MEMORY_PROFILING +NETFILTER 
+NS -OBSOLETE_LINKER +OPENSSL -OPENSSL_WOLFSSL -OT +PCRE -PCRE2 -PCRE2_JIT 
-PCRE_JIT +POLL +PRCTL -PROCCTL -PROMEX -PTHREAD_EMULATION -QUIC 
-QUIC_OPENSSL_COMPAT +RT +SHM_OPEN +SLZ +SSL -STATIC_PCRE -STATIC_PCRE2 
+SYSTEMD +TFO +THREAD +THREAD_DUMP +TPROXY -WURFL -ZLIB

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

Built with multi-threading support (MAX_TGROUPS=16, MAX_THREADS=256, default=2).
Built with OpenSSL version : OpenSSL 3.0.2 15 Mar 2022
Running on OpenSSL version : OpenSSL 3.0.2 15 Mar 2022
OpenSSL library supports TLS extensions : yes
OpenSSL library supports SNI : yes
OpenSSL library supports : TLSv1.0 TLSv1.1 TLSv1.2 TLSv1.3
OpenSSL providers loaded : default
Built with network namespace support.
Built with libslz for stateless compression.
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 PCRE version : 8.39 2016-06-14
Running on PCRE version : 8.39 2016-06-14
PCRE library supports JIT : no (USE_PCRE_JIT not set)
Encrypted password support via crypt(3): yes
Built with gcc compiler version 11.4.0

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  cannot be specified using 'proto' keyword)
 h2 : mode=HTTP  side=FE|BE  mux=H2flags=HTX|HOL_RISK|NO_UPG
   fcgi : mode=HTTP  side=BE mux=FCGI  flags=HTX|HOL_RISK|NO_UPG
   : mode=HTT