* GRO disabled with IPv4 options
@ 2017-11-16 15:12 Cristian Klein
2017-11-16 21:40 ` Herbert Xu
2017-11-16 22:34 ` Eric Dumazet
0 siblings, 2 replies; 6+ messages in thread
From: Cristian Klein @ 2017-11-16 15:12 UTC (permalink / raw)
To: netdev; +Cc: herbert, Ahmed Ali-Eldin
[CC-ing Herbert Xu, who is to 'git blame' for the code in question. :)]
Dear all,
We are working on a research prototype which, among others, adds a new
IPv4 option. During testing we noticed that the packets captured by
tcpdump shrank from 10s of KBs to the MTU, which indicates that Generic
Receive Offload (GRO) got disabled.
Upon further investigation, we found the following line in
`inet_gro_receive`:
if (*(u8 *)iph != 0x45)
goto out_unlock;
in plain English, don't do GRO if any IPv4 options are present.
Does somebody know the rationale for this? Is it because IPv4 options
are rarely used, hence implementing GRO in that case does not pay off or
are there some caveats? Specifically would it make sense to do GRO when
the IPv4 options are byte-identical in consecutive packets?
Regards,
--
Cristian Klein, PhD
Researcher @ Umeå University
http://kleinlabs.eu
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: GRO disabled with IPv4 options
2017-11-16 15:12 GRO disabled with IPv4 options Cristian Klein
@ 2017-11-16 21:40 ` Herbert Xu
2017-11-16 22:47 ` Tom Herbert
2017-11-16 22:34 ` Eric Dumazet
1 sibling, 1 reply; 6+ messages in thread
From: Herbert Xu @ 2017-11-16 21:40 UTC (permalink / raw)
To: Cristian Klein; +Cc: netdev, Ahmed Ali-Eldin
On Thu, Nov 16, 2017 at 04:12:43PM +0100, Cristian Klein wrote:
>
> Does somebody know the rationale for this? Is it because IPv4
> options are rarely used, hence implementing GRO in that case does
> not pay off or are there some caveats? Specifically would it make
Precisely. GRO is about optimising for the common case. At the
time there was no impetus to support IP options.
> sense to do GRO when the IPv4 options are byte-identical in
> consecutive packets?
Yes there is no reason why we can't do this. As long as it doesn't
penalise the non-IP-option case too much.
Cheers,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: GRO disabled with IPv4 options
2017-11-16 15:12 GRO disabled with IPv4 options Cristian Klein
2017-11-16 21:40 ` Herbert Xu
@ 2017-11-16 22:34 ` Eric Dumazet
1 sibling, 0 replies; 6+ messages in thread
From: Eric Dumazet @ 2017-11-16 22:34 UTC (permalink / raw)
To: Cristian Klein; +Cc: netdev, herbert, Ahmed Ali-Eldin
On Thu, 2017-11-16 at 16:12 +0100, Cristian Klein wrote:
> [CC-ing Herbert Xu, who is to 'git blame' for the code in question. :)]
>
> Dear all,
>
> We are working on a research prototype which, among others, adds a new
> IPv4 option. During testing we noticed that the packets captured by
> tcpdump shrank from 10s of KBs to the MTU, which indicates that Generic
> Receive Offload (GRO) got disabled.
>
> Upon further investigation, we found the following line in
> `inet_gro_receive`:
>
> if (*(u8 *)iph != 0x45)
> goto out_unlock;
>
> in plain English, don't do GRO if any IPv4 options are present.
>
> Does somebody know the rationale for this? Is it because IPv4 options
> are rarely used, hence implementing GRO in that case does not pay off or
> are there some caveats? Specifically would it make sense to do GRO when
> the IPv4 options are byte-identical in consecutive packets?
I guess nobody really uses IPV4 options, and you are the first caring
enough ;)
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: GRO disabled with IPv4 options
2017-11-16 21:40 ` Herbert Xu
@ 2017-11-16 22:47 ` Tom Herbert
2017-11-17 13:51 ` Willem de Bruijn
0 siblings, 1 reply; 6+ messages in thread
From: Tom Herbert @ 2017-11-16 22:47 UTC (permalink / raw)
To: Herbert Xu
Cc: Cristian Klein, Linux Kernel Network Developers, Ahmed Ali-Eldin
On Thu, Nov 16, 2017 at 1:40 PM, Herbert Xu <herbert@gondor.apana.org.au> wrote:
> On Thu, Nov 16, 2017 at 04:12:43PM +0100, Cristian Klein wrote:
>>
>> Does somebody know the rationale for this? Is it because IPv4
>> options are rarely used, hence implementing GRO in that case does
>> not pay off or are there some caveats? Specifically would it make
>
> Precisely. GRO is about optimising for the common case. At the
> time there was no impetus to support IP options.
>
>> sense to do GRO when the IPv4 options are byte-identical in
>> consecutive packets?
>
> Yes there is no reason why we can't do this. As long as it doesn't
> penalise the non-IP-option case too much.
>
Of course it would also be nice to have GRO support for various IPv6
extension headers, at this point we're more likely to see those rather
than IP options in real deployment!
Tom
> Cheers,
> --
> Email: Herbert Xu <herbert@gondor.apana.org.au>
> Home Page: http://gondor.apana.org.au/~herbert/
> PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: GRO disabled with IPv4 options
2017-11-16 22:47 ` Tom Herbert
@ 2017-11-17 13:51 ` Willem de Bruijn
2017-11-17 14:02 ` David Miller
0 siblings, 1 reply; 6+ messages in thread
From: Willem de Bruijn @ 2017-11-17 13:51 UTC (permalink / raw)
To: Tom Herbert
Cc: Herbert Xu, Cristian Klein, Linux Kernel Network Developers,
Ahmed Ali-Eldin
On Thu, Nov 16, 2017 at 5:47 PM, Tom Herbert <tom@herbertland.com> wrote:
> On Thu, Nov 16, 2017 at 1:40 PM, Herbert Xu <herbert@gondor.apana.org.au> wrote:
>> On Thu, Nov 16, 2017 at 04:12:43PM +0100, Cristian Klein wrote:
>>>
>>> Does somebody know the rationale for this? Is it because IPv4
>>> options are rarely used, hence implementing GRO in that case does
>>> not pay off or are there some caveats? Specifically would it make
>>
>> Precisely. GRO is about optimising for the common case. At the
>> time there was no impetus to support IP options.
>>
>>> sense to do GRO when the IPv4 options are byte-identical in
>>> consecutive packets?
>>
>> Yes there is no reason why we can't do this. As long as it doesn't
>> penalise the non-IP-option case too much.
>>
> Of course it would also be nice to have GRO support for various IPv6
> extension headers, at this point we're more likely to see those rather
> than IP options in real deployment!
ipv6_gro_receive already pulls common ones with ipv6_gso_pull_exthdrs.
And to add a counterpoint: GRO has to be resilient to malformed packets,
so there is value in keeping it simple and limited to the common case.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: GRO disabled with IPv4 options
2017-11-17 13:51 ` Willem de Bruijn
@ 2017-11-17 14:02 ` David Miller
0 siblings, 0 replies; 6+ messages in thread
From: David Miller @ 2017-11-17 14:02 UTC (permalink / raw)
To: willemdebruijn.kernel; +Cc: tom, herbert, cklein, netdev, ahmeda
From: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
Date: Fri, 17 Nov 2017 08:51:57 -0500
> ipv6_gro_receive already pulls common ones with ipv6_gso_pull_exthdrs.
> And to add a counterpoint: GRO has to be resilient to malformed packets,
> so there is value in keeping it simple and limited to the common case.
Agreed.
Also it is not exactly clear what should happen with all IP option
types when reconstructing the original packet stream on the GSO side.
It just sounds incredibly complicated to me for little to no gain.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-11-17 14:02 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-11-16 15:12 GRO disabled with IPv4 options Cristian Klein
2017-11-16 21:40 ` Herbert Xu
2017-11-16 22:47 ` Tom Herbert
2017-11-17 13:51 ` Willem de Bruijn
2017-11-17 14:02 ` David Miller
2017-11-16 22:34 ` Eric Dumazet
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).