netdev - Re: [PATCH net-next] gro: relax ID check in inet_gro

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: <1363882091.4431.20.camel@edumazet-glaptop>
Date:	Thu, 21 Mar 2013 09:08:11 -0700
From:	Eric Dumazet <eric.dumazet@...il.com>
To:	David Miller <davem@...emloft.net>
Cc:	netdev@...r.kernel.org, dmitry@...adcom.com, eilong@...adcom.com,
	pshelar@...ira.com, hkchu@...gle.com, maze@...gle.com
Subject: Re: [PATCH net-next] gro: relax ID check in inet_gro_receive()

On Thu, 2013-03-21 at 11:46 -0400, David Miller wrote:
> From: Eric Dumazet <eric.dumazet@...il.com>
> Date: Wed, 20 Mar 2013 21:52:33 -0700
> 
> > GRE TSO support doesn't increment the ID in the inner IP header.
> 
> Is this a fundamental limitation of doing TSO over GRO or
> were the Broadcom folks just being lazy with their firmware
> implementation?
> 

Well, I suspect this hardware is not capable of doing the proper ID
manipulation twice. (inner and outer header)

Still TSO support permits a single GRE flow going from 3Gbps to 9Gbps on
our hosts. So even if the inner IP id is 'broken', we are going to use
TSO.

Note we are limited by the receiver, as the receiver has to perform the
tcp checksum in software (bnx2x doesnt support CHECKSUM_COMPLETE yet)

Hopefully next firmware or NIC will do the right thing.

> I really don't want to apply this patch, because ipv4 frames
> even with DF set should have an incrementing ID field, in
> order to accomodate various header compression schemes.
> 
> We go out of our way to do this for normal unencapsulated TCP stream
> packets, rather than set the ID field to zero (which we did for some
> time until the compression issue was pointed out to us).

I understand your concern, but this check in GRO brings nothing at all.

Once we receive frames with 'bad IPv4 ID', should we accept them or drop
them ?

TCP stack doesn't care at receive (obviously as this ID is not a concern
for the transport layer), so GRO should do the same, as GRO is a best
effort to reduce cpu load.

I fully understand the 'tos' check because of proper ECN support, but
the ttl check or id check are totally useless and time consuming.

GRO aggregation should roughly work the same than TCP coalescing, and we
don't care of IP ID or ttl in TCP stack.

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