Re: sc-set-gpt with expression: internal error, unexpected rule->from=0, please report this bug!
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!
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!
>> 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,
Re: sc-set-gpt with expression: internal error, unexpected rule->from=0, please report this bug!
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!
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
