* [PATCH net-next 1/3] ipv6: Clear flush_id to make GRO work
2014-09-09 18:23 [PATCH net-next 0/3] net: enable GRO for IPIP and SIT Tom Herbert
@ 2014-09-09 18:23 ` Tom Herbert
2014-09-09 18:38 ` Eric Dumazet
2014-09-09 18:23 ` [PATCH 2/3] ipip: Add gro callbacks to ipip offload Tom Herbert
` (2 subsequent siblings)
3 siblings, 1 reply; 9+ messages in thread
From: Tom Herbert @ 2014-09-09 18:23 UTC (permalink / raw)
To: davem, netdev
In TCP gro we check flush_id which is derived from the IP identifier.
In IPv4 gro path the flush_id is set with the expectation that every
matched packet increments IP identifier. In IPv6, the flush_id is
never set and thus is uinitialized. What's worse is that in IPv6
over IPv4 encapsulation, the IP identifier is taken from the outer
header which is currently not incremented on every packet for Linux
stack, so GRO in this case never matches packets (identifier is
not increasing).
This patch clears flush_id for every time for a matched packet in
IPv6 gro_receive. We need to do this each time to overwrite the
setting that would be done in IPv4 gro_receive per the outer
header in IPv6 over Ipv4 encapsulation.
Signed-off-by: Tom Herbert <therbert@google.com>
---
net/ipv6/ip6_offload.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/net/ipv6/ip6_offload.c b/net/ipv6/ip6_offload.c
index 5bcda33..929bbbcd 100644
--- a/net/ipv6/ip6_offload.c
+++ b/net/ipv6/ip6_offload.c
@@ -261,6 +261,9 @@ static struct sk_buff **ipv6_gro_receive(struct sk_buff **head,
/* flush if Traffic Class fields are different */
NAPI_GRO_CB(p)->flush |= !!(first_word & htonl(0x0FF00000));
NAPI_GRO_CB(p)->flush |= flush;
+
+ /* Clear flush_id, there's really no concept of ID in IPv6. */
+ NAPI_GRO_CB(p)->flush_id = 0;
}
NAPI_GRO_CB(skb)->flush |= flush;
--
2.1.0.rc2.206.gedb03e5
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH net-next 1/3] ipv6: Clear flush_id to make GRO work
2014-09-09 18:23 ` [PATCH net-next 1/3] ipv6: Clear flush_id to make GRO work Tom Herbert
@ 2014-09-09 18:38 ` Eric Dumazet
2014-09-09 18:52 ` Tom Herbert
0 siblings, 1 reply; 9+ messages in thread
From: Eric Dumazet @ 2014-09-09 18:38 UTC (permalink / raw)
To: Tom Herbert; +Cc: davem, netdev
On Tue, 2014-09-09 at 11:23 -0700, Tom Herbert wrote:
> In TCP gro we check flush_id which is derived from the IP identifier.
> In IPv4 gro path the flush_id is set with the expectation that every
> matched packet increments IP identifier. In IPv6, the flush_id is
> never set and thus is uinitialized. What's worse is that in IPv6
> over IPv4 encapsulation, the IP identifier is taken from the outer
> header which is currently not incremented on every packet for Linux
> stack, so GRO in this case never matches packets (identifier is
> not increasing).
>
> This patch clears flush_id for every time for a matched packet in
> IPv6 gro_receive. We need to do this each time to overwrite the
> setting that would be done in IPv4 gro_receive per the outer
> header in IPv6 over Ipv4 encapsulation.
>
> Signed-off-by: Tom Herbert <therbert@google.com>
> ---
> net/ipv6/ip6_offload.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/net/ipv6/ip6_offload.c b/net/ipv6/ip6_offload.c
> index 5bcda33..929bbbcd 100644
> --- a/net/ipv6/ip6_offload.c
> +++ b/net/ipv6/ip6_offload.c
> @@ -261,6 +261,9 @@ static struct sk_buff **ipv6_gro_receive(struct sk_buff **head,
> /* flush if Traffic Class fields are different */
> NAPI_GRO_CB(p)->flush |= !!(first_word & htonl(0x0FF00000));
> NAPI_GRO_CB(p)->flush |= flush;
> +
> + /* Clear flush_id, there's really no concept of ID in IPv6. */
> + NAPI_GRO_CB(p)->flush_id = 0;
> }
>
> NAPI_GRO_CB(skb)->flush |= flush;
Yeah, I mentioned this problem months ago and apparently forgot to push
the fix.
http://patchwork.ozlabs.org/patch/311212/
Signed-off-by: Eric Dumazet <edumazet@google.com>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH net-next 1/3] ipv6: Clear flush_id to make GRO work
2014-09-09 18:38 ` Eric Dumazet
@ 2014-09-09 18:52 ` Tom Herbert
0 siblings, 0 replies; 9+ messages in thread
From: Tom Herbert @ 2014-09-09 18:52 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David Miller, Linux Netdev List
On Tue, Sep 9, 2014 at 11:38 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Tue, 2014-09-09 at 11:23 -0700, Tom Herbert wrote:
>> In TCP gro we check flush_id which is derived from the IP identifier.
>> In IPv4 gro path the flush_id is set with the expectation that every
>> matched packet increments IP identifier. In IPv6, the flush_id is
>> never set and thus is uinitialized. What's worse is that in IPv6
>> over IPv4 encapsulation, the IP identifier is taken from the outer
>> header which is currently not incremented on every packet for Linux
>> stack, so GRO in this case never matches packets (identifier is
>> not increasing).
>>
>> This patch clears flush_id for every time for a matched packet in
>> IPv6 gro_receive. We need to do this each time to overwrite the
>> setting that would be done in IPv4 gro_receive per the outer
>> header in IPv6 over Ipv4 encapsulation.
>>
>> Signed-off-by: Tom Herbert <therbert@google.com>
>> ---
>> net/ipv6/ip6_offload.c | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/net/ipv6/ip6_offload.c b/net/ipv6/ip6_offload.c
>> index 5bcda33..929bbbcd 100644
>> --- a/net/ipv6/ip6_offload.c
>> +++ b/net/ipv6/ip6_offload.c
>> @@ -261,6 +261,9 @@ static struct sk_buff **ipv6_gro_receive(struct sk_buff **head,
>> /* flush if Traffic Class fields are different */
>> NAPI_GRO_CB(p)->flush |= !!(first_word & htonl(0x0FF00000));
>> NAPI_GRO_CB(p)->flush |= flush;
>> +
>> + /* Clear flush_id, there's really no concept of ID in IPv6. */
>> + NAPI_GRO_CB(p)->flush_id = 0;
>> }
>>
>> NAPI_GRO_CB(skb)->flush |= flush;
>
> Yeah, I mentioned this problem months ago and apparently forgot to push
> the fix.
>
Unfortunately, probably not the last GRO issue. This whole thing is
really rather fragile and probably easy to break undetected...
> http://patchwork.ozlabs.org/patch/311212/
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
>
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 2/3] ipip: Add gro callbacks to ipip offload
2014-09-09 18:23 [PATCH net-next 0/3] net: enable GRO for IPIP and SIT Tom Herbert
2014-09-09 18:23 ` [PATCH net-next 1/3] ipv6: Clear flush_id to make GRO work Tom Herbert
@ 2014-09-09 18:23 ` Tom Herbert
2014-09-09 18:23 ` [PATCH 3/3] sit: Add gro callbacks to sit_offload Tom Herbert
2014-09-10 3:27 ` [PATCH net-next 0/3] net: enable GRO for IPIP and SIT David Miller
3 siblings, 0 replies; 9+ messages in thread
From: Tom Herbert @ 2014-09-09 18:23 UTC (permalink / raw)
To: davem, netdev
Add inet_gro_receive and inet_gro_complete to ipip_offload to
support GRO.
Signed-off-by: Tom Herbert <therbert@google.com>
---
net/ipv4/af_inet.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
index d156b3c..6d6348d 100644
--- a/net/ipv4/af_inet.c
+++ b/net/ipv4/af_inet.c
@@ -1670,6 +1670,8 @@ static const struct net_offload ipip_offload = {
.callbacks = {
.gso_send_check = inet_gso_send_check,
.gso_segment = inet_gso_segment,
+ .gro_receive = inet_gro_receive,
+ .gro_complete = inet_gro_complete,
},
};
--
2.1.0.rc2.206.gedb03e5
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH 3/3] sit: Add gro callbacks to sit_offload
2014-09-09 18:23 [PATCH net-next 0/3] net: enable GRO for IPIP and SIT Tom Herbert
2014-09-09 18:23 ` [PATCH net-next 1/3] ipv6: Clear flush_id to make GRO work Tom Herbert
2014-09-09 18:23 ` [PATCH 2/3] ipip: Add gro callbacks to ipip offload Tom Herbert
@ 2014-09-09 18:23 ` Tom Herbert
2014-09-10 3:27 ` [PATCH net-next 0/3] net: enable GRO for IPIP and SIT David Miller
3 siblings, 0 replies; 9+ messages in thread
From: Tom Herbert @ 2014-09-09 18:23 UTC (permalink / raw)
To: davem, netdev
Add ipv6_gro_receive and ipv6_gro_complete to sit_offload to
support GRO.
Signed-off-by: Tom Herbert <therbert@google.com>
---
net/ipv6/ip6_offload.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/net/ipv6/ip6_offload.c b/net/ipv6/ip6_offload.c
index 929bbbcd..9952f3f 100644
--- a/net/ipv6/ip6_offload.c
+++ b/net/ipv6/ip6_offload.c
@@ -317,6 +317,8 @@ static const struct net_offload sit_offload = {
.callbacks = {
.gso_send_check = ipv6_gso_send_check,
.gso_segment = ipv6_gso_segment,
+ .gro_receive = ipv6_gro_receive,
+ .gro_complete = ipv6_gro_complete,
},
};
--
2.1.0.rc2.206.gedb03e5
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH net-next 0/3] net: enable GRO for IPIP and SIT
2014-09-09 18:23 [PATCH net-next 0/3] net: enable GRO for IPIP and SIT Tom Herbert
` (2 preceding siblings ...)
2014-09-09 18:23 ` [PATCH 3/3] sit: Add gro callbacks to sit_offload Tom Herbert
@ 2014-09-10 3:27 ` David Miller
2014-09-10 4:04 ` Tom Herbert
3 siblings, 1 reply; 9+ messages in thread
From: David Miller @ 2014-09-10 3:27 UTC (permalink / raw)
To: therbert; +Cc: netdev
From: Tom Herbert <therbert@google.com>
Date: Tue, 9 Sep 2014 11:23:13 -0700
> This patch sets populates the IPIP and SIT offload structures with
> gro_receive and gro_complete functions. This enables use of GRO
> for these. Also, fixed a problem in IPv6 where we were not properly
> initializing flush_id.
We're going to need to sort this out, because these functions are
now static after Eric's patches to fix sparse warnings.
You're going to have to respin this with that patch you intended
to add from the previous series that exported these routines.
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH net-next 0/3] net: enable GRO for IPIP and SIT
2014-09-10 3:27 ` [PATCH net-next 0/3] net: enable GRO for IPIP and SIT David Miller
@ 2014-09-10 4:04 ` Tom Herbert
2014-09-10 4:32 ` David Miller
0 siblings, 1 reply; 9+ messages in thread
From: Tom Herbert @ 2014-09-10 4:04 UTC (permalink / raw)
To: David Miller; +Cc: Linux Netdev List
On Tue, Sep 9, 2014 at 8:27 PM, David Miller <davem@davemloft.net> wrote:
> From: Tom Herbert <therbert@google.com>
> Date: Tue, 9 Sep 2014 11:23:13 -0700
>
>> This patch sets populates the IPIP and SIT offload structures with
>> gro_receive and gro_complete functions. This enables use of GRO
>> for these. Also, fixed a problem in IPv6 where we were not properly
>> initializing flush_id.
>
> We're going to need to sort this out, because these functions are
> now static after Eric's patches to fix sparse warnings.
>
> You're going to have to respin this with that patch you intended
> to add from the previous series that exported these routines.
I might be missing something, but I believe the functions referred in
this patch series were already static.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH net-next 0/3] net: enable GRO for IPIP and SIT
2014-09-10 4:04 ` Tom Herbert
@ 2014-09-10 4:32 ` David Miller
0 siblings, 0 replies; 9+ messages in thread
From: David Miller @ 2014-09-10 4:32 UTC (permalink / raw)
To: therbert; +Cc: netdev
From: Tom Herbert <therbert@google.com>
Date: Tue, 9 Sep 2014 21:04:27 -0700
> On Tue, Sep 9, 2014 at 8:27 PM, David Miller <davem@davemloft.net> wrote:
>> From: Tom Herbert <therbert@google.com>
>> Date: Tue, 9 Sep 2014 11:23:13 -0700
>>
>>> This patch sets populates the IPIP and SIT offload structures with
>>> gro_receive and gro_complete functions. This enables use of GRO
>>> for these. Also, fixed a problem in IPv6 where we were not properly
>>> initializing flush_id.
>>
>> We're going to need to sort this out, because these functions are
>> now static after Eric's patches to fix sparse warnings.
>>
>> You're going to have to respin this with that patch you intended
>> to add from the previous series that exported these routines.
>
> I might be missing something, but I believe the functions referred in
> this patch series were already static.
Oops my bad, let me rereview this...
Yep it's fine, applied, thanks!
^ permalink raw reply [flat|nested] 9+ messages in thread