Re: [RFC PATCH 24/28] tcp: try to fit AccECN option with SACK

["Ilpo Järvinen" <ilpo.jarvinen@cs.helsinki.fi>]

[-- Attachment #1: Type: text/plain, Size: 1639 bytes --]

On Wed, 18 Mar 2020, Eric Dumazet wrote:

> 
> 
> On 3/18/20 2:43 AM, Ilpo Järvinen wrote:
> > From: Ilpo Järvinen <ilpo.jarvinen@cs.helsinki.fi>
> > 
> > As SACK blocks tend to eat all option space when there are
> > many holes, it is useful to compromise on sending many SACK
> > blocks in every ACK and try to fit AccECN option there
> > by reduction the number of SACK blocks. But never go below
> > two SACK blocks because of AccECN option.
> > 
> > As AccECN option is often not put to every ACK, the space
> > hijack is usually only temporary.
> > 
> > Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@cs.helsinki.fi>
> > ---
> >  net/ipv4/tcp_output.c | 15 +++++++++++++++
> >  1 file changed, 15 insertions(+)
> > 
> > diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
> > index 4cc590a47f43..0aec2c57a9cc 100644
> > --- a/net/ipv4/tcp_output.c
> > +++ b/net/ipv4/tcp_output.c
> > @@ -756,6 +756,21 @@ static int tcp_options_fit_accecn(struct tcp_out_options *opts, int required,
> >  	if (opts->num_ecn_bytes < required)
> >  		return 0;
> 
> Have you tested this patch ?
> 
> (You forgot to remove the prior 2 lines)
> 
> >  
> > +	if (opts->num_ecn_bytes < required) {

Yes and no. There was no unit test for this particular condition but
I added a few now (with and w/o timestamps). I also managed to find and 
fix a byte-order related bug related to non-fullsized option while making 
those tests.

(I didn't actually forget to remove it. I managed to add the problem 
during a botched conflict merge when I reorganized some of the code.)

Thanks for taking a look.

-- 
 i.