* [Cake] [PATCH net] gso_segment: Reset skb->mac_len after modifying network header
@ 2018-09-11 22:19 Toke Høiland-Jørgensen
2018-09-13 14:11 ` Eric Dumazet
2018-09-13 14:43 ` [Cake] [PATCH net v2] " Toke Høiland-Jørgensen
0 siblings, 2 replies; 6+ messages in thread
From: Toke Høiland-Jørgensen @ 2018-09-11 22:19 UTC (permalink / raw)
To: netdev; +Cc: cake, Eric Dumazet
When splitting a GSO segment that consists of encapsulated packets, the
skb->mac_len of the segments can end up being set wrong, causing packet
drops in particular when using act_mirred and ifb interfaces in
combination with a qdisc that splits GSO packets.
This happens because at the time skb_segment() is called, network_header
will point to the inner header, throwing off the calculation in
skb_reset_mac_len(). The network_header is subsequently adjust by the
outer IP gso_segment handlers, but they don't set the mac_len.
Fix this by adding skb_reset_mac_len() calls to both the IPv4 and IPv6
gso_segment handlers, after they modify the network_header.
Signed-off-by: Toke Høiland-Jørgensen <toke@toke.dk>
---
net/ipv4/af_inet.c | 1 +
net/ipv6/ip6_offload.c | 1 +
2 files changed, 2 insertions(+)
diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
index 20fda8fb8ffd..1fbe2f815474 100644
--- a/net/ipv4/af_inet.c
+++ b/net/ipv4/af_inet.c
@@ -1377,6 +1377,7 @@ struct sk_buff *inet_gso_segment(struct sk_buff *skb,
if (encap)
skb_reset_inner_headers(skb);
skb->network_header = (u8 *)iph - skb->head;
+ skb_reset_mac_len(skb);
} while ((skb = skb->next));
out:
diff --git a/net/ipv6/ip6_offload.c b/net/ipv6/ip6_offload.c
index 37ff4805b20c..c7e495f12011 100644
--- a/net/ipv6/ip6_offload.c
+++ b/net/ipv6/ip6_offload.c
@@ -115,6 +115,7 @@ static struct sk_buff *ipv6_gso_segment(struct sk_buff *skb,
payload_len = skb->len - nhoff - sizeof(*ipv6h);
ipv6h->payload_len = htons(payload_len);
skb->network_header = (u8 *)ipv6h - skb->head;
+ skb_reset_mac_len(skb);
if (udpfrag) {
int err = ip6_find_1stfragopt(skb, &prevhdr);
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [Cake] [PATCH net] gso_segment: Reset skb->mac_len after modifying network header
2018-09-11 22:19 [Cake] [PATCH net] gso_segment: Reset skb->mac_len after modifying network header Toke Høiland-Jørgensen
@ 2018-09-13 14:11 ` Eric Dumazet
2018-09-13 14:14 ` Dave Taht
2018-09-13 14:22 ` Toke Høiland-Jørgensen
2018-09-13 14:43 ` [Cake] [PATCH net v2] " Toke Høiland-Jørgensen
1 sibling, 2 replies; 6+ messages in thread
From: Eric Dumazet @ 2018-09-13 14:11 UTC (permalink / raw)
To: Toke Høiland-Jørgensen, netdev; +Cc: cake, Eric Dumazet
On 09/11/2018 03:19 PM, Toke Høiland-Jørgensen wrote:
> When splitting a GSO segment that consists of encapsulated packets, the
> skb->mac_len of the segments can end up being set wrong, causing packet
> drops in particular when using act_mirred and ifb interfaces in
> combination with a qdisc that splits GSO packets.
>
> This happens because at the time skb_segment() is called, network_header
> will point to the inner header, throwing off the calculation in
> skb_reset_mac_len(). The network_header is subsequently adjust by the
> outer IP gso_segment handlers, but they don't set the mac_len.
>
> Fix this by adding skb_reset_mac_len() calls to both the IPv4 and IPv6
> gso_segment handlers, after they modify the network_header.
>
> Signed-off-by: Toke Høiland-Jørgensen <toke@toke.dk>
Looks good but I would have appreciated a thanks or something
after the help I gave on this problem.
Reviewed-by: Eric Dumazet <edumazet@google.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Cake] [PATCH net] gso_segment: Reset skb->mac_len after modifying network header
2018-09-13 14:11 ` Eric Dumazet
@ 2018-09-13 14:14 ` Dave Taht
2018-09-13 14:22 ` Toke Høiland-Jørgensen
1 sibling, 0 replies; 6+ messages in thread
From: Dave Taht @ 2018-09-13 14:14 UTC (permalink / raw)
To: Eric Dumazet
Cc: Toke Høiland-Jørgensen,
Linux Kernel Network Developers, Cake List, Eric Dumazet
On Thu, Sep 13, 2018 at 7:11 AM Eric Dumazet <eric.dumazet@gmail.com> wrote:
>
>
>
> On 09/11/2018 03:19 PM, Toke Høiland-Jørgensen wrote:
> > When splitting a GSO segment that consists of encapsulated packets, the
> > skb->mac_len of the segments can end up being set wrong, causing packet
> > drops in particular when using act_mirred and ifb interfaces in
> > combination with a qdisc that splits GSO packets.
> >
> > This happens because at the time skb_segment() is called, network_header
> > will point to the inner header, throwing off the calculation in
> > skb_reset_mac_len(). The network_header is subsequently adjust by the
> > outer IP gso_segment handlers, but they don't set the mac_len.
> >
> > Fix this by adding skb_reset_mac_len() calls to both the IPv4 and IPv6
> > gso_segment handlers, after they modify the network_header.
> >
> > Signed-off-by: Toke Høiland-Jørgensen <toke@toke.dk>
>
> Looks good but I would have appreciated a thanks or something
> after the help I gave on this problem.
>
> Reviewed-by: Eric Dumazet <edumazet@google.com>
Thanks, eric! (post-hoc)
Acked-by: Dave Taht <dave.taht@gmail.com>
> _______________________________________________
> Cake mailing list
> Cake@lists.bufferbloat.net
> https://lists.bufferbloat.net/listinfo/cake
--
Dave Täht
CEO, TekLibre, LLC
http://www.teklibre.com
Tel: 1-669-226-2619
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Cake] [PATCH net] gso_segment: Reset skb->mac_len after modifying network header
2018-09-13 14:11 ` Eric Dumazet
2018-09-13 14:14 ` Dave Taht
@ 2018-09-13 14:22 ` Toke Høiland-Jørgensen
1 sibling, 0 replies; 6+ messages in thread
From: Toke Høiland-Jørgensen @ 2018-09-13 14:22 UTC (permalink / raw)
To: Eric Dumazet, netdev; +Cc: cake, Eric Dumazet
On 13 September 2018 16:11:18 CEST, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>
>
>On 09/11/2018 03:19 PM, Toke Høiland-Jørgensen wrote:
>> When splitting a GSO segment that consists of encapsulated packets,
>the
>> skb->mac_len of the segments can end up being set wrong, causing
>packet
>> drops in particular when using act_mirred and ifb interfaces in
>> combination with a qdisc that splits GSO packets.
>>
>> This happens because at the time skb_segment() is called,
>network_header
>> will point to the inner header, throwing off the calculation in
>> skb_reset_mac_len(). The network_header is subsequently adjust by the
>> outer IP gso_segment handlers, but they don't set the mac_len.
>>
>> Fix this by adding skb_reset_mac_len() calls to both the IPv4 and
>IPv6
>> gso_segment handlers, after they modify the network_header.
>>
>> Signed-off-by: Toke Høiland-Jørgensen <toke@toke.dk>
>
>Looks good but I would have appreciated a thanks or something
>after the help I gave on this problem.
Yes, of course. Should have mentioned that in the commit message.
My apologies, won't happen again. And thanks! :)
-Toke
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Cake] [PATCH net v2] gso_segment: Reset skb->mac_len after modifying network header
2018-09-11 22:19 [Cake] [PATCH net] gso_segment: Reset skb->mac_len after modifying network header Toke Høiland-Jørgensen
2018-09-13 14:11 ` Eric Dumazet
@ 2018-09-13 14:43 ` Toke Høiland-Jørgensen
2018-09-13 19:10 ` David Miller
1 sibling, 1 reply; 6+ messages in thread
From: Toke Høiland-Jørgensen @ 2018-09-13 14:43 UTC (permalink / raw)
To: netdev
Cc: Toke Høiland-Jørgensen, cake, David Miller,
Eric Dumazet, Dave Taht
When splitting a GSO segment that consists of encapsulated packets, the
skb->mac_len of the segments can end up being set wrong, causing packet
drops in particular when using act_mirred and ifb interfaces in
combination with a qdisc that splits GSO packets.
This happens because at the time skb_segment() is called, network_header
will point to the inner header, throwing off the calculation in
skb_reset_mac_len(). The network_header is subsequently adjust by the
outer IP gso_segment handlers, but they don't set the mac_len.
Fix this by adding skb_reset_mac_len() calls to both the IPv4 and IPv6
gso_segment handlers, after they modify the network_header.
Many thanks to Eric Dumazet for his help in identifying the cause of
the bug.
Acked-by: Dave Taht <dave.taht@gmail.com>
Reviewed-by: Eric Dumazet <edumazet@google.com>
Signed-off-by: Toke Høiland-Jørgensen <toke@toke.dk>
---
v2:
- Properly credit Eric for his help
- Add review and ack tags
net/ipv4/af_inet.c | 1 +
net/ipv6/ip6_offload.c | 1 +
2 files changed, 2 insertions(+)
diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
index 20fda8fb8ffd..1fbe2f815474 100644
--- a/net/ipv4/af_inet.c
+++ b/net/ipv4/af_inet.c
@@ -1377,6 +1377,7 @@ struct sk_buff *inet_gso_segment(struct sk_buff *skb,
if (encap)
skb_reset_inner_headers(skb);
skb->network_header = (u8 *)iph - skb->head;
+ skb_reset_mac_len(skb);
} while ((skb = skb->next));
out:
diff --git a/net/ipv6/ip6_offload.c b/net/ipv6/ip6_offload.c
index 37ff4805b20c..c7e495f12011 100644
--- a/net/ipv6/ip6_offload.c
+++ b/net/ipv6/ip6_offload.c
@@ -115,6 +115,7 @@ static struct sk_buff *ipv6_gso_segment(struct sk_buff *skb,
payload_len = skb->len - nhoff - sizeof(*ipv6h);
ipv6h->payload_len = htons(payload_len);
skb->network_header = (u8 *)ipv6h - skb->head;
+ skb_reset_mac_len(skb);
if (udpfrag) {
int err = ip6_find_1stfragopt(skb, &prevhdr);
--
2.18.0
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [Cake] [PATCH net v2] gso_segment: Reset skb->mac_len after modifying network header
2018-09-13 14:43 ` [Cake] [PATCH net v2] " Toke Høiland-Jørgensen
@ 2018-09-13 19:10 ` David Miller
0 siblings, 0 replies; 6+ messages in thread
From: David Miller @ 2018-09-13 19:10 UTC (permalink / raw)
To: toke; +Cc: netdev, cake, edumazet, dave.taht
From: Toke Høiland-Jørgensen <toke@toke.dk>
Date: Thu, 13 Sep 2018 16:43:07 +0200
> When splitting a GSO segment that consists of encapsulated packets, the
> skb->mac_len of the segments can end up being set wrong, causing packet
> drops in particular when using act_mirred and ifb interfaces in
> combination with a qdisc that splits GSO packets.
>
> This happens because at the time skb_segment() is called, network_header
> will point to the inner header, throwing off the calculation in
> skb_reset_mac_len(). The network_header is subsequently adjust by the
> outer IP gso_segment handlers, but they don't set the mac_len.
>
> Fix this by adding skb_reset_mac_len() calls to both the IPv4 and IPv6
> gso_segment handlers, after they modify the network_header.
>
> Many thanks to Eric Dumazet for his help in identifying the cause of
> the bug.
>
> Acked-by: Dave Taht <dave.taht@gmail.com>
> Reviewed-by: Eric Dumazet <edumazet@google.com>
> Signed-off-by: Toke Høiland-Jørgensen <toke@toke.dk>
> ---
> v2:
> - Properly credit Eric for his help
> - Add review and ack tags
Applied and queued up for -stable, thanks.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-09-13 19:10 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-11 22:19 [Cake] [PATCH net] gso_segment: Reset skb->mac_len after modifying network header Toke Høiland-Jørgensen
2018-09-13 14:11 ` Eric Dumazet
2018-09-13 14:14 ` Dave Taht
2018-09-13 14:22 ` Toke Høiland-Jørgensen
2018-09-13 14:43 ` [Cake] [PATCH net v2] " Toke Høiland-Jørgensen
2018-09-13 19:10 ` David Miller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox