| Message ID | 153670437243.12756.693381878569982309.stgit@alrua-kau |
|---|---|
| State | Superseded, archived |
| Delegated to: | David Miller |
| Headers | show |
| Series | [net] gso_segment: Reset skb->mac_len after modifying network header | expand |
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>
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
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
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);
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(+)