Re: [PATCH 0/9]: tcp-2.6 patchset

[Stephen Hemminger <shemminger@linux-foundation.org>]

On Mon, 28 May 2007 13:27:03 +0300 (EEST)
"Ilpo Järvinen" <ilpo.jarvinen@helsinki.fi> wrote:

> On Sun, 27 May 2007, Ilpo Järvinen wrote:
> 
> > On Sun, 27 May 2007, Baruch Even wrote:
> > 
> > > * Ilpo J?rvinen <ilpo.jarvinen@helsinki.fi> [070527 14:16]:
> > > > 
> > > > Thus, my original question basically culminates in this: should cc
> > > > modules be passed number of packets acked or number of skbs acked?
> > > > ...The latter makes no sense to me unless the value is intented to
> > > > be interpreted as number of timestamps acked or something along those 
> > > > lines. ...I briefly tried looking up for documentation for cc module 
> > > > interface but didn't find anything useful about this, and thus asked in 
> > > > the first place...
> > > 
> > > At least the htcp module that I wrote assumes that the number is actual
> > > number of tcp packets so GSO should be considered.
> > 
> > Thanks for the info! It is what I suspected... ...I'll write a patch for 
> > it tomorrow against net-2.6... Dave, beware that it will partially 
> > overlap with the changes made in the patch 8, so you might choose to put 
> > the patch 8 on hold until this issue is first resolved...
> > 
> > > The consequences of this bug are not too large but it does make all
> > > congestion control algorithms a lot less aggressive. On my machines GSO
> > > is disabled by default (e1000 at 100mbps & Tigon3 @ 1Gbps).
> > 
> > Agreed, that's my impression too. However, some algorithms do things
> > like > 0 checks for it, so it might disturb their dynamics even more
> > than in the "too small value" cases...
> 
> Hmm, there seems to be another case that I'm not too sure of...
> Please check the alternative I choose for SYN handling below...
> 
> ...hmm... While exploring this SYN thingie, I noticed that commit 
> 164891aadf1721fca4dce473bb0e0998181537c6 drops !FLAG_RETRANS_DATA_ACKED 
> check from rtt_sample call (when combining it with pkts_acked call).
> I hope that's intentional?!? ...the commit message didn't say anything 
> about it nor was anything in cc modules changed to accomodate that.
> 
> 
> [PATCH] [TCP]: Fix GSO ignorance of pkts_acked arg (cong.cntrl modules)
> 
> The code used to ignore GSO completely, passing either way too
> small or zero pkts_acked when GSO skb or part of it got ACKed.
> In addition, there is no need to calculate the value in the loop
> but simple arithmetics after the loop is sufficient.

Yes, thanks for fixing this. Wonder how it affects measurements.
 
> It is not very clear how SYN segments should be handled, so I
> choose to follow the previous implementation in this respect.

Since we don't invoke congestion control modules until after the SYN
handshake this is not a problem.


-- 
Stephen Hemminger <shemminger@linux-foundation.org>