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

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

On Mon, Nov 04, 2019 at 11:00:06AM +0100, Cédric Dufour wrote:
> [Sorry for multiple posting. I'm having trouble with sending this
> patch via GMail *SMTP*; trying the WUI...]

no worries.

> Thanks Willy for your feedback.
> 
> I'm not sure whether I should further split stktable_fetch_expr() into
> some new sample_...() in sample.c or if there is some other *.c file where
> a generic function, including the required act_rule, would make sense.

At the moment I must confess I have no authoritative opinion, for not
having studied that area in a while. However I suspect that your new
function stktable_fetch_expr() is in fact not related to stick tables
at all and should entirely be in sample.c with a sample_ name. I think
you got confused by the fact that this action works both on a sample
and an expression, but the part you're dealing with is entirely a
sample expression.

> Also, I'm not HAProxy-savvy enough to know whether this proposal is
> multi-thread safe (is it ?).

Yes it is. All our datapath is thread-safe for now because execution
of all processing happens on the same thread from end to end. However
if you would access central resources like server states, stick-table
contents, queues or such stuff, then you'd have to take additional
precautions. I think I'll soon start to write some doc on this after
the conference.

Otherwise your patch looks good and reasonably non-intrusive so that
we can get it into 2.1 in time. I would appreciate it if you could
split it so that the new generic function is in its own patch and that
a second patch only implements support for it from sc-set-gpt0(). This
would simplify future backports if needed at any point in time, typically
if your generic function becomes required for something else in the
future.

Thanks!
Willy

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

2019-11-04 Thread Cédric Dufour 
[Sorry for multiple posting. I'm having trouble with sending this
patch via GMail *SMTP*; trying the WUI...]

Thanks Willy for your feedback.

I'm not sure whether I should further split stktable_fetch_expr() into
some new sample_...() in sample.c or if there is some other *.c file where
a generic function, including the required act_rule, would make sense.

Also, I'm not HAProxy-savvy enough to know whether this proposal is
multi-thread safe (is it ?).

[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  | 82 +++---
 3 files changed, 102 insertions(+), 25 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 session  [{if | unless} ]
 - { track-sc0 | track-sc1 | track-sc2 }  [table ]
 - sc-inc-gpc0()
 - sc-inc-gpc1()
-- sc-set-gpt0() 
+- sc-set-gpt0() {  |  }
 - set-var() 
 - unset-var()
 - silent-drop
diff --git a/include/types/action.h b/include/types/action.h
index 54a6f71a4..