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]

Date:	Mon, 11 Oct 2010 15:00:33 +0100
From:	Ben Hutchings <bhutchings@...arflare.com>
To:	Vladislav Zolotarov <vladz@...adcom.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

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.

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.

--
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