Re: [PATCH v3] MINOR: stick-table: allow sc-set-gpt0 to set value from an expression

2019-11-15 Thread Willy Tarreau
On Fri, Nov 08, 2019 at 10:06:17AM +0100, Cédric Dufour wrote:
> You can go ahead with PATCH v3.

OK thanks, now merged!

> I triple-checked it against our use-case (along haproxy 2.0.5, the latest
> Ubuntu-packaged version which we base our re-packaging on) and all seems well.

You should definitely switch to haproxy.debian.net which is provided
by the same maintainers, but with *really* updated packages, unless of
course you like to live dangerously with bugs that only you and a few
other users of these packages experience :-)

In case you want to feel a shiver down your spine, here is the list
of the 100 known bugs affecting your currently packaged version:

http://www.haproxy.org/bugs/bugs-2.0.5.html

> Thank you very much for your help and merging.
> 
> Toute bonne journée par chez vous ;-)

You're welcome,
Willy



Re: [PATCH v3] MINOR: stick-table: allow sc-set-gpt0 to set value from an expression

2019-11-08 Thread Cédric Dufour
[confirmed now: (subscribing/)sending to the *list* from Thunderbird
via GMail SMTP does not work; resending via its Web UI]

Hello Willy,

On 07/11/2019 18:58, Willy Tarreau wrote:
> Hi Cédric,
>
> On Wed, Nov 06, 2019 at 06:38:53PM +0100, Cédric Dufour wrote:
>
> Just let me know if you want me to merge this one now or if you made some
> extra changes since you posted.

You can go ahead with PATCH v3.

I triple-checked it against our use-case (along haproxy 2.0.5, the latest
Ubuntu-packaged version which we base our re-packaging on) and all seems well.

> Thanks,
> Willy

Thank you very much for your help and merging.

Toute bonne journée par chez vous ;-)

Cédric


Re: [PATCH v3] MINOR: stick-table: allow sc-set-gpt0 to set value from an expression

2019-11-07 Thread Willy Tarreau
Hi Cédric,

On Wed, Nov 06, 2019 at 06:38:53PM +0100, Cédric Dufour wrote:
> [sorry for mis-threading; hoping I got the git send-mail --in-reply-to right]

Don't worry about this. The purpose of the list is precisely to act as
humans and not as pre-configured bots. Whatever you can do right which
saves others' time is fine. What you can't easily do is left to others,
that's how we're the most efficient.

> Actually, reading through your original and last comments, I realize I must
> have misunderstood the sample_expr() part and got carried away.
> 
> Unless I'm mistaken, we can use the existing sample_fetch_as_type() function
> directly (without any further addition to sample.c). Or am I missing 
> something ?

Initially when I quickly had a look at it I had the impression that it
would only process a sample fetch function but not the whole expression.
That disturbed me a little bit because I thought we had something to do
this. Now I had a second look after your comment and I think you're
right :-)

> This leaves the switch(rule->from) <-> smpt_opt_dir stuff. Since there is
> nothing about act_rule in sample.c so I felt it has no place there.

Absolutely.

> Wouldn't an extra function call just to deal with this switch() be overkill ?

Yes I think it would be. Let's just place your switch() where you need it
and simply rely on sample_fetch_as_type() to do most of the job. I don't
see what could cause trouble there. Oh I'm seeing that it's apparently
what you did in this new version of the patch. I reviewed it very quickly
but at first glance it looks OK.

> Let me know if you still think this ought to go in a separate function
> (like if anticipating set_gpt1 :-) ).

Not now, it looks OK as-is.

Just let me know if you want me to merge this one now or if you made some
extra changes since you posted.

Thanks,
Willy



[PATCH v3] MINOR: stick-table: allow sc-set-gpt0 to set value from an expression

2019-11-06 Thread Cédric Dufour
[sorry for mis-threading; hoping I got the git send-mail --in-reply-to right]

Hello Willy!

Actually, reading through your original and last comments, I realize I must
have misunderstood the sample_expr() part and got carried away.

Unless I'm mistaken, we can use the existing sample_fetch_as_type() function
directly (without any further addition to sample.c). Or am I missing something ?

This leaves the switch(rule->from) <-> smpt_opt_dir stuff. Since there is
nothing about act_rule in sample.c so I felt it has no place there.
Wouldn't an extra function call just to deal with this switch() be overkill ?
Let me know if you still think this ought to go in a separate function
(like if anticipating set_gpt1 :-) ).

Best,

Cédric

[patch]
Allow the sc-set-gpt0 action to set GPT0 to a value dynamically evaluated from
its  argument (in addition to the existing static  alternative).
---
 doc/configuration.txt  | 44 +---
 include/types/action.h |  1 +
 src/stick_table.c  | 57 +++---
 3 files changed, 79 insertions(+), 23 deletions(-)

diff --git a/doc/configuration.txt b/doc/configuration.txt
index 8dedbfc48..bea62bd98 100644
--- a/doc/configuration.txt
+++ b/doc/configuration.txt
@@ -4465,11 +4465,13 @@ http-request sc-inc-gpc1() [ { if | unless } 
 ]
   counter designated by . If an error occurs, this action silently fails
   and the actions evaluation continues.
 
-http-request sc-set-gpt0()  [ { if | unless }  ]
+http-request sc-set-gpt0() {  |  }
+  [ { if | unless }  ]
 
-  This action sets the GPT0 tag according to the sticky counter designated by
-   and the value of . The expected result is a boolean. If an error
-  occurs, this action silently fails and the actions evaluation continues.
+  This action sets the 32-bit unsigned GPT0 tag according to the sticky counter
+  designated by  and the value of /. The expected result is a
+  boolean. If an error occurs, this action silently fails and the actions
+  evaluation continues.
 
 http-request set-dst  [ { if | unless }  ]
 
@@ -4974,11 +4976,13 @@ http-response sc-inc-gpc1() [ { if | unless } 
 ]
   counter designated by . If an error occurs, this action silently fails
   and the actions evaluation continues.
 
-http-response sc-set-gpt0()  [ { if | unless }  ]
+http-response sc-set-gpt0() {  |  }
+   [ { if | unless }  ]
 
-  This action sets the GPT0 tag according to the sticky counter designated by
-   and the value of . The expected result is a boolean. If an error
-  occurs, this action silently fails and the actions evaluation continues.
+  This action sets the 32-bit unsigned GPT0 tag according to the sticky counter
+  designated by  and the value of /. The expected result is a
+  boolean. If an error occurs, this action silently fails and the actions
+  evaluation continues.
 
 http-response send-spoe-group [ { if | unless }  ]
 
@@ -9394,11 +9398,11 @@ tcp-request connection  [{if | unless} 
]
 counter designated by . If an error occurs, this action silently
 fails and the actions evaluation continues.
 
-- sc-set-gpt0() :
-This action sets the GPT0 tag according to the sticky counter 
designated
-by  and the value of . The expected result is a boolean. If
-an error occurs, this action silently fails and the actions evaluation
-continues.
+- sc-set-gpt0() {  |  }:
+This action sets the 32-bit unsigned GPT0 tag according to the sticky
+counter designated by  and the value of /. The
+expected result is a boolean. If an error occurs, this action silently
+fails and the actions evaluation continues.
 
 - set-src  :
   Is used to set the source IP address to the value of specified
@@ -9556,7 +9560,7 @@ tcp-request content  [{if | unless} ]
 - { track-sc0 | track-sc1 | track-sc2 }  [table ]
 - sc-inc-gpc0()
 - sc-inc-gpc1()
-- sc-set-gpt0() 
+- sc-set-gpt0() {  |  }
 - set-dst 
 - set-dst-port 
 - set-var() 
@@ -9820,11 +9824,11 @@ tcp-response content  [{if | unless} 
]
 counter designated by . If an error occurs, this action fails
 silently and the actions evaluation continues.
 
-- sc-set-gpt0()  :
-This action sets the GPT0 tag according to the sticky counter 
designated
-by  and the value of . The expected result is a boolean. If
-an error occurs, this action silently fails and the actions evaluation
-continues.
+- sc-set-gpt0() {  |  }
+This action sets the 32-bit unsigned GPT0 tag according to the sticky
+counter designated by  and the value of /. The
+expected result is a boolean. If an error occurs, this action silently
+fails and the actions evaluation continues.
 
 - "silent-drop" :
 This stops the evaluation of the rules and makes the client-facing
@@ -9945,7 +9949,7 @@ tcp-request sess