Re: TFO warnings
Hi Elias, On Fri, Oct 15, 2021 at 11:45:30AM +0200, Elias Abacioglu wrote: > Hi > > I have backends with `default-server tfo`. > I also have `retry-on conn-failure` on every backend except one where I > have `retry-on all-retryable-errors`. > > I still get this warning for every backend. > > [WARNING] (9572) : parsing [/etc/haproxy/haproxy.cfg:332] : backend > 'cookie_backend': server 'dogfight03' has tfo activated, the backend should > be configured with at least 'conn-failure', 'empty-response' and > 'response-timeout' or we wouldn't be able to retry the connection on > failure. > > Is this a bug or do I need to have `retry-on conn-failure empty-response > response-timeout`? > Isn't it bad to have `retry-on response-timeout`? In theory it could be bad, but with TFO you never know. The request gets sent with the SYN, so from haproxy's perspective, if the sendto() succeeds, it's gone. If the packet gets blocked, or if the data are ignored by the host, you end up with a connection that's waiting for its request while it was sent, effectively causing a response timeout. And it's not just a matter of supporting the feature or not. In my tests, TFO would randomly stop working and come back some time later, very likely as a protective measure under load. Maybe we could think about a "tfo" retry option, which would enable all those but only in case TFO was used. This would cover only the first idempotent requests of a connection that are sent using TFO without affecting other ones, and would also be easier to configure. I don't know how easy it is to implement since I don't know if we keep the info somewhere that TFO was used, but overall it should not be too complicated. Are you interested in having a look at this and trying to write a patch for this ? I can provide you some guidance if needed. Willy
Re: [PATCH] CLEANUP: assorted typo fixes in the code and comments
On Fri, Oct 15, 2021 at 04:18:21PM +0500, Ilya Shipitsin wrote: > This is 27th iteration of typo fixes Merged, thanks Ilya! Willy
Re: PATCH: add ::1 to predefined LOCALHOST acl
On Fri, Oct 15, 2021 at 04:38:29PM +0200, Björn Jacke wrote: > Hi, > > are there any objections for adding ::1 to the LOCALHOST acl? See attached > patch... No objection from me and I think it totally makes sense, of course. I've added a small description to it and merged it. Thanks Björn! Willy
Re: [PATCH] CLEANUP: Consistently `unsigned int` for bitfields
On Sat, Oct 16, 2021 at 06:24:18PM +0200, Tim Duesterhus wrote:
> see 6a0dd733906611dea958cf74b9f51bb16028ae20
>
> Found using GitHub's CodeQL scan.
> ---
> include/haproxy/stick_table-t.h | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/include/haproxy/stick_table-t.h b/include/haproxy/stick_table-t.h
> index 3b1f2b3ef..133f992b5 100644
> --- a/include/haproxy/stick_table-t.h
> +++ b/include/haproxy/stick_table-t.h
> @@ -125,8 +125,8 @@ struct stktable_data_type {
> const char *name; /* name of the data type */
> int std_type; /* standard type we can use for this data, STD_T_* */
> int arg_type; /* type of optional argument, ARG_T_* */
> - int is_array:1; /* this is an array of gpc/gpt */
> - int is_local:1; /* this is local only and never learned */
> + unsigned int is_array:1; /* this is an array of gpc/gpt */
> + unsigned int is_local:1; /* this is local only and never learned */
Please note, since last patch we've added shorter type names like "uint"
for the unsigned int, that help preserve alignment. Do you mind if I adapt
your patch ?
Willy
Re: [PATCH 1/2] CI: Add `permissions` to GitHub Actions
On Sat, Oct 16, 2021 at 06:10:26PM +0200, Tim Duesterhus wrote: > This change locks down the permissions of the access token in GitHub Actions > to > only allow reading the repository contents and nothing else. (...) This series and the coccinelle one applied, thanks Tim! Willy
