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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20070529091415.03a14216@freepuppy>
Date:	Tue, 29 May 2007 09:14:15 -0700
From:	Stephen Hemminger <shemminger@...ux-foundation.org>
To:	"Ilpo Järvinen" <ilpo.jarvinen@...sinki.fi>"@smtp.osdl.org
Cc:	David Miller <davem@...emloft.net>, Baruch Even <baruch@...en.org>,
	Herbert Xu <herbert@...dor.apana.org.au>,
	Netdev <netdev@...r.kernel.org>
Subject: Re: [PATCH 0/9]: tcp-2.6 patchset

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

> On Sun, 27 May 2007, Ilpo Järvinen wrote:
> 
> > On Sun, 27 May 2007, Baruch Even wrote:
> > 
> > > * Ilpo J?rvinen <ilpo.jarvinen@...sinki.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@...ux-foundation.org>
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html