Re: [PATCH] DOC: quic: fix misspelled tune.quic.socket-owner
Hello Willy, I understand, thank you for the explanation. Have a nice holidays ! ;) Le 07/06/2023 à 14:55, Willy Tarreau a écrit : Hello Artur, On Tue, Jun 06, 2023 at 03:18:31PM +0200, Artur wrote: About the backporting instructions I was not sure how far it should be backported. I preferred to skip it instead of giving an erroneous instruction. Maybe someone can explain if this backport instruction is really required and what to do if one is unsure about how to backport. You should see them as a time saver for the person doing the backports, that's why we like patch authors to provide as much useful information as they can. Sometimes even just adding "this patch probably needs to be backported" or "the feature was already there in 2.7 and maybe before so the patch may need to be backported at least there" will be a hint to the person that they should really check twice if they don't find it the first time. As a rule of thumb, just keep in mind that the commit message part of a patch is the one where humans talk to humans, and that anything that crosses your head and that can help decide if a patch has to be backported, could be responsible for a regression, needs to be either fixed or reverted etc is welcome. Thanks, Willy -- Best regards, Artur
Re: [PATCH] DOC: quic: fix misspelled tune.quic.socket-owner
Hello Artur, On Tue, Jun 06, 2023 at 03:18:31PM +0200, Artur wrote: > About the backporting instructions I was not sure how far it should be > backported. I preferred to skip it instead of giving an erroneous > instruction. > Maybe someone can explain if this backport instruction is really required > and what to do if one is unsure about how to backport. You should see them as a time saver for the person doing the backports, that's why we like patch authors to provide as much useful information as they can. Sometimes even just adding "this patch probably needs to be backported" or "the feature was already there in 2.7 and maybe before so the patch may need to be backported at least there" will be a hint to the person that they should really check twice if they don't find it the first time. As a rule of thumb, just keep in mind that the commit message part of a patch is the one where humans talk to humans, and that anything that crosses your head and that can help decide if a patch has to be backported, could be responsible for a regression, needs to be either fixed or reverted etc is welcome. Thanks, Willy
Re: [PATCH] DOC: quic: fix misspelled tune.quic.socket-owner
Le 06/06/2023 à 14:52, Amaury Denoyelle a écrit : Do not hesitate to give us feedback if you test QUIC support :) Yes, I will. I deployed haproxy+quic (recommended setup) on one site with some trafic so I need few days to get visitors' feedback, if any. At this point I can't see anything wrong for all the tests I've made. -- Best regards, Artur
Re: [PATCH] DOC: quic: fix misspelled tune.quic.socket-owner
Hello Tim,
Thank you for your help. I forgot to include patch description in the
body. My bad. Luckily Amaury was there. :)
About the backporting instructions I was not sure how far it should be
backported. I preferred to skip it instead of giving an erroneous
instruction.
Maybe someone can explain if this backport instruction is really
required and what to do if one is unsure about how to backport.
Le 06/06/2023 à 14:54, Tim Düsterhus a écrit :
Hi Artur,
On 6/6/23 14:42, Artur wrote:
DOC: quic: fix misspelled tune.quic.socket-owner
Commit 511ddd5 introduced tune.quic.socket-owner parameter
related to QUIC socket behaviour.
However it was misspelled in configuration.txt in 'bind' section as
tune.quic.conn-owner.
I'm not a committer, but a regular contributor. I had a look: The
patch looks pretty good, but the commit message is lacking a body,
which is required as per:
https://github.com/haproxy/haproxy/blob/a475448161b406b0b81f5b551336417b05426492/CONTRIBUTING#L562-L567
As you already found the commit that introduced the issue, it would be
a good opportunity to add a reference to the message body, something
like:
The typo was introduced in commit
511ddd5785266c149dfa593582512239480e1688 ("MINOR: quic: define config
option for socket per conn") and needs to be backported together with
that commit (2.8 and possible 2.7).
Best regards
Tim Düsterhus
--
Best regards,
Artur
Re: [PATCH] DOC: quic: fix misspelled tune.quic.socket-owner
Hi On 6/6/23 15:05, Amaury Denoyelle wrote: In fact, I already merged the patch. The commit message was already provided as the email body so I integrated it directly into the patch. Yeah, I've seen your email shortly after I sent mine. As for backport information, most of the time we skip documentation patches when they are only for spelling fixes. However as this one is directly to correct a referenced keyword we may choose to backport it. Thanks for the clarification :-) Best regards Tim Düsterhus
Re: [PATCH] DOC: quic: fix misspelled tune.quic.socket-owner
On Tue, Jun 06, 2023 at 02:54:19PM +0200, Tim Düsterhus wrote:
> Hi Artur,
> On 6/6/23 14:42, Artur wrote:
> > DOC: quic: fix misspelled tune.quic.socket-owner
> > > Commit 511ddd5 introduced tune.quic.socket-owner parameter
> > related to QUIC socket behaviour.
> > However it was misspelled in configuration.txt in 'bind' section as
> > tune.quic.conn-owner.
> >
> I'm not a committer, but a regular contributor. I had a look: The patch
> looks pretty good, but the commit message is lacking a body, which is
> required as per:
> https://github.com/haproxy/haproxy/blob/a475448161b406b0b81f5b551336417b05426492/CONTRIBUTING#L562-L567
> As you already found the commit that introduced the issue, it would be a
> good opportunity to add a reference to the message body, something like:
> The typo was introduced in commit 511ddd5785266c149dfa593582512239480e1688
> ("MINOR: quic: define config option for socket per conn") and needs to be
> backported together with that commit (2.8 and possible 2.7).
> Best regards
In fact, I already merged the patch. The commit message was already
provided as the email body so I integrated it directly into the patch.
As for backport information, most of the time we skip documentation
patches when they are only for spelling fixes. However as this one is
directly to correct a referenced keyword we may choose to backport it.
--
Amaury Denoyelle
Re: [PATCH] DOC: quic: fix misspelled tune.quic.socket-owner
Hi Artur,
On 6/6/23 14:42, Artur wrote:
DOC: quic: fix misspelled tune.quic.socket-owner
Commit 511ddd5 introduced tune.quic.socket-owner parameter
related to QUIC socket behaviour.
However it was misspelled in configuration.txt in 'bind' section as
tune.quic.conn-owner.
I'm not a committer, but a regular contributor. I had a look: The patch
looks pretty good, but the commit message is lacking a body, which is
required as per:
https://github.com/haproxy/haproxy/blob/a475448161b406b0b81f5b551336417b05426492/CONTRIBUTING#L562-L567
As you already found the commit that introduced the issue, it would be a
good opportunity to add a reference to the message body, something like:
The typo was introduced in commit
511ddd5785266c149dfa593582512239480e1688 ("MINOR: quic: define config
option for socket per conn") and needs to be backported together with
that commit (2.8 and possible 2.7).
Best regards
Tim Düsterhus
Re: [PATCH] DOC: quic: fix misspelled tune.quic.socket-owner
On Tue, Jun 06, 2023 at 02:42:37PM +0200, Artur wrote: > DOC: quic: fix misspelled tune.quic.socket-owner > Commit 511ddd5 introduced tune.quic.socket-owner parameter > related to QUIC socket behaviour. > However it was misspelled in configuration.txt in 'bind' section as > tune.quic.conn-owner. Merged, thank you very much ! Do not hesitate to give us feedback if you test QUIC support :) -- Amaury Denoyelle
[PATCH] DOC: quic: fix misspelled tune.quic.socket-owner
DOC: quic: fix misspelled tune.quic.socket-owner Commit 511ddd5 introduced tune.quic.socket-owner parameter related to QUIC socket behaviour. However it was misspelled in configuration.txt in 'bind' section as tune.quic.conn-owner. From a446bfc3cf50f58cde0bdc36e93154099771bf9e Mon Sep 17 00:00:00 2001 From: Artur Pydo Date: Tue, 6 Jun 2023 11:49:59 +0200 Subject: [PATCH] DOC: quic: fix misspelled tune.quic.socket-owner --- doc/configuration.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/configuration.txt b/doc/configuration.txt index adcd00414..b147b501c 100644 --- a/doc/configuration.txt +++ b/doc/configuration.txt @@ -4936,7 +4936,7 @@ bind / [, ...] [param*] was the FD of an accept(). Should be used carefully. - 'quic4@' -> address is resolved as IPv4 and protocol UDP is used. Note that to achieve the best performance with a - large traffic you should keep "tune.quic.conn-owner" on + large traffic you should keep "tune.quic.socket-owner" on connection. Else QUIC connections will be multiplexed over the listener socket. Another alternative would be to duplicate QUIC listener instances over several threads, -- 2.30.2
