netdev - RE: [PATCH net-next 2/5] bnx2x: save cycles in setting gso

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: <8628FE4E7912BF47A96AE7DD7BAC0AADDDEE428246@SJEXCHCCR02.corp.ad.broadcom.com>
Date:	Mon, 11 Oct 2010 10:06:19 -0700
From:	"Vladislav Zolotarov" <vladz@...adcom.com>
To:	"Ben Hutchings" <bhutchings@...arflare.com>
cc:	"David Miller" <davem@...emloft.net>,
	"Dmitry Kravkov" <dmitry@...adcom.com>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"Eilon Greenstein" <eilong@...adcom.com>
Subject: RE: [PATCH net-next 2/5] bnx2x: save cycles in setting gso_size



> -----Original Message-----
> From: Ben Hutchings [mailto:bhutchings@...arflare.com]
> Sent: Monday, October 11, 2010 4:01 PM
> To: Vladislav Zolotarov
> Cc: David Miller; Dmitry Kravkov; netdev@...r.kernel.org; Eilon
> Greenstein
> Subject: RE: [PATCH net-next 2/5] bnx2x: save cycles in setting
> gso_size
> 
> On Mon, 2010-10-11 at 01:53 -0700, Vladislav Zolotarov wrote:
> [...]
> > Dave, it's a gSo_size, not a gRo_size and we are handling an LRO skb
> > here. It's quite confusing, I agree... ;) This is currently the way
> > to mark an LRO skb in order to properly handle the LRO skbs that
> > might still be forwarded to the stack short time after the LRO has
> > been disabled due to enabling the IP forwarding (or if there is a
> > bug in a driver).
> > See the skb_warn_if_lro() below:
> >
> > static inline bool skb_warn_if_lro(const struct sk_buff *skb)
> > {
> > 	/* LRO sets gso_size but not gso_type, whereas if GSO is really
> > 	 * wanted then gso_type will be set. */
> > 	struct skb_shared_info *shinfo = skb_shinfo(skb);
> > 	if (skb_is_nonlinear(skb) && shinfo->gso_size != 0 &&
> > 	    unlikely(shinfo->gso_type == 0)) {
> > 		__skb_warn_lro_forwarding(skb);
> > 		return true;
> > 	}
> > 	return false;
> > }
> >
> > So, the convention is to set the gSo_size to a none-zero value
> leaving
> > the gSo_type zero for an LRO skbs. It's quite hacky but this is what
> > we have for quite a while and it quite worked so far. ;) To make it
> > cleaner we might have done the following:
> [...]
> 
> The requirement (or as you call it, "convention") is to set gso_size to
> the observed MSS of the packets that have been combined.  If you don't
> do that then TCP will not know the true number of packets for purposes
> of delayed-ACK etc.

Thanks, Ben. I see that now. 
Dave, we will respin this patch series.

Thanks,
vlad

> 
> Ben.
> 
> --
> Ben Hutchings, Senior Software Engineer, Solarflare Communications
> Not speaking for my employer; that's the marketing department's job.
> They asked us to note that Solarflare product names are trademarked.
>