>> 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(<value>)" only takes values or variables
> but not fetches and "-m int gt <value>" 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.

Aurelien
From b66b401ddb36a4c686fa0df965492da204ba66a8 Mon Sep 17 00:00:00 2001
From: Aurelien DARRAGON <adarra...@haproxy.com>
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 <adarra...@haproxy.com>
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, 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;
@@ -2851,6 +2853,7 @@ static enum act_parse_ret parse_set_gpt(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

Reply via email to