| Message ID | 20220213150234.31602-1-thomas.liu@ucloud.cn (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | gso: do not skip outer ip header in case of ipip and net_failover | expand |
On Sun, Feb 13, 2022 at 10:10 AM Tao Liu <thomas.liu@ucloud.cn> wrote: > > We encouter a tcp drop issue in our cloud environment. Packet GROed in host > forwards to a VM virtio_net nic with net_failover enabled. VM acts as a > IPVS LB with ipip encapsulation. The full path like: > host gro -> vm virtio_net rx -> net_failover rx -> ipvs fullnat > -> ipip encap -> net_failover tx -> virtio_net tx > > When net_failover transmits a ipip pkt (gso_type = 0x0103), there is no gso > performed because it supports TSO and GSO_IPXIP4. But network_header has > been pointing to inner ip header. If the packet is configured correctly, and net_failover advertises that it can handle TSO packets with IPIP encap, then still virtio_net should not advertise it and software GSO be applied on its dev_queue_xmit call. This is assuming that the packet not only has SKB_GSO_IPXIP4 correctly set, but also tunneling fields like skb->encapsulated and skb_inner_network_header. > --- > net/ipv4/af_inet.c | 10 +++++++++- > 1 file changed, 9 insertions(+), 1 deletion(-) > > diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c > index 9c465ba..f8b3f8a 100644 > --- a/net/ipv4/af_inet.c > +++ b/net/ipv4/af_inet.c > @@ -1425,10 +1425,18 @@ struct sk_buff *inet_gso_segment(struct sk_buff *skb, > static struct sk_buff *ipip_gso_segment(struct sk_buff *skb, > netdev_features_t features) > { > + struct sk_buff *segs; > + int nhoff; > + > if (!(skb_shinfo(skb)->gso_type & SKB_GSO_IPXIP4)) > return ERR_PTR(-EINVAL); > > - return inet_gso_segment(skb, features); > + nhoff = skb_network_header(skb) - skb_mac_header(skb); > + segs = inet_gso_segment(skb, features); > + if (!segs) > + skb->network_header = skb_mac_header(skb) + nhoff - skb->head; > + > + return segs; > } If this would be needed for IPIP, then the same would be needed for SIT, etc. Is the skb_network_header 1. correctly pointing to the outer header of the TSO packet before the call to inet_gso_segment 2. incorrectly pointing to the inner header of the (still) TSO packet after the call to inet_gso_segment inet_gso_segment already does the same operation: save nhoff, pull network header, call callbacks.gso_segment (which can be ipip_gso_segment->inet_gso_segment), then place the network header back at nhoff.
Sorry for bothering, just repost it. > 2022年2月14日 09:28，Willem de Bruijn <willemdebruijn.kernel@gmail.com> 写道： > > On Sun, Feb 13, 2022 at 10:10 AM Tao Liu <thomas.liu@ucloud.cn> wrote: >> >> We encouter a tcp drop issue in our cloud environment. Packet GROed in host >> forwards to a VM virtio_net nic with net_failover enabled. VM acts as a >> IPVS LB with ipip encapsulation. The full path like: >> host gro -> vm virtio_net rx -> net_failover rx -> ipvs fullnat >> -> ipip encap -> net_failover tx -> virtio_net tx >> >> When net_failover transmits a ipip pkt (gso_type = 0x0103), there is no gso >> performed because it supports TSO and GSO_IPXIP4. But network_header has >> been pointing to inner ip header. > > If the packet is configured correctly, and net_failover advertises > that it can handle TSO packets with IPIP encap, then still virtio_net > should not advertise it and software GSO be applied on its > dev_queue_xmit call. > > This is assuming that the packet not only has SKB_GSO_IPXIP4 correctly > set, but also tunneling fields like skb->encapsulated and > skb_inner_network_header. Thanks very much for your comment! Yes, the packet is correct. Another thing i have not pointed directly is that the pkt has SKB_GSO_DODGY. net_failover do not advertises GSO_ROBUST but virtio_net do. >> --- >> net/ipv4/af_inet.c | 10 +++++++++- >> 1 file changed, 9 insertions(+), 1 deletion(-) >> >> diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c >> index 9c465ba..f8b3f8a 100644 >> --- a/net/ipv4/af_inet.c >> +++ b/net/ipv4/af_inet.c >> @@ -1425,10 +1425,18 @@ struct sk_buff *inet_gso_segment(struct sk_buff *skb, >> static struct sk_buff *ipip_gso_segment(struct sk_buff *skb, >> netdev_features_t features) >> { >> + struct sk_buff *segs; >> + int nhoff; >> + >> if (!(skb_shinfo(skb)->gso_type & SKB_GSO_IPXIP4)) >> return ERR_PTR(-EINVAL); >> >> - return inet_gso_segment(skb, features); >> + nhoff = skb_network_header(skb) - skb_mac_header(skb); >> + segs = inet_gso_segment(skb, features); >> + if (!segs) >> + skb->network_header = skb_mac_header(skb) + nhoff - skb->head; >> + >> + return segs; >> } > > If this would be needed for IPIP, then the same would be needed for SIT, etc. > > Is the skb_network_header > > 1. correctly pointing to the outer header of the TSO packet before the > call to inet_gso_segment > 2. incorrectly pointing to the inner header of the (still) TSO packet > after the call to inet_gso_segment > > inet_gso_segment already does the same operation: save nhoff, pull > network header, call callbacks.gso_segment (which can be > ipip_gso_segment->inet_gso_segment), then place the network header > back at nhoff. > values print in skb_mac_gso_segment() before callbacks.gso_segment: ipip: vlan_depth=0 mac_len=0 skb->network_header=206 net_failover: vlan_depth=14 mac_len=14 skb->network_header=186 virtio_net: vlan_depth=34 mac_len=34 skb->network_header=206 agree to add sit/ip4ip6/ip6ip6, and patch can be simplified as: diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c index 9c465ba..72fde28 100644 --- a/net/ipv4/af_inet.c +++ b/net/ipv4/af_inet.c @@ -1376,8 +1376,11 @@ struct sk_buff *inet_gso_segment(struct sk_buff *skb, } ops = rcu_dereference(inet_offloads[proto]); - if (likely(ops && ops->callbacks.gso_segment)) + if (likely(ops && ops->callbacks.gso_segment)) { segs = ops->callbacks.gso_segment(skb, features); + if (!segs) + skb->network_header = skb_mac_header(skb) + nhoff - skb->head; + } if (IS_ERR_OR_NULL(segs)) goto out; diff --git a/net/ipv6/ip6_offload.c b/net/ipv6/ip6_offload.c index b29e9ba..5f577e2 100644 --- a/net/ipv6/ip6_offload.c +++ b/net/ipv6/ip6_offload.c @@ -114,6 +114,8 @@ static struct sk_buff *ipv6_gso_segment(struct sk_buff *skb, if (likely(ops && ops->callbacks.gso_segment)) { skb_reset_transport_header(skb); segs = ops->callbacks.gso_segment(skb, features); + if (!segs) + skb->network_header = skb_mac_header(skb) + nhoff - skb->head; } if (IS_ERR_OR_NULL(segs))
On Sun, Feb 13, 2022 at 11:03 PM Tao Liu <thomas.liu@ucloud.cn> wrote: > > Sorry for bothering, just repost it. > > > 2022年2月14日 09:28，Willem de Bruijn <willemdebruijn.kernel@gmail.com> 写道： > > > > On Sun, Feb 13, 2022 at 10:10 AM Tao Liu <thomas.liu@ucloud.cn> wrote: > >> > >> We encouter a tcp drop issue in our cloud environment. Packet GROed in host > >> forwards to a VM virtio_net nic with net_failover enabled. VM acts as a > >> IPVS LB with ipip encapsulation. The full path like: > >> host gro -> vm virtio_net rx -> net_failover rx -> ipvs fullnat > >> -> ipip encap -> net_failover tx -> virtio_net tx > >> > >> When net_failover transmits a ipip pkt (gso_type = 0x0103), there is no gso > >> performed because it supports TSO and GSO_IPXIP4. But network_header has > >> been pointing to inner ip header. > > > > If the packet is configured correctly, and net_failover advertises > > that it can handle TSO packets with IPIP encap, then still virtio_net > > should not advertise it and software GSO be applied on its > > dev_queue_xmit call. > > > > This is assuming that the packet not only has SKB_GSO_IPXIP4 correctly > > set, but also tunneling fields like skb->encapsulated and > > skb_inner_network_header. > Thanks very much for your comment! > > Yes, the packet is correct. Another thing i have not pointed directly is > that the pkt has SKB_GSO_DODGY. net_failover do not advertises GSO_ROBUST > but virtio_net do. If net_failover does not advertise NETIF_F_GSO_ROBUST, then tcp_gso_segment will pass a packet with SKB_GSO_DODGY to the software gso stack, not taking the branch if (skb_gso_ok(skb, features | NETIF_F_GSO_ROBUST)) { > >> --- > >> net/ipv4/af_inet.c | 10 +++++++++- > >> 1 file changed, 9 insertions(+), 1 deletion(-) > >> > >> diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c > >> index 9c465ba..f8b3f8a 100644 > >> --- a/net/ipv4/af_inet.c > >> +++ b/net/ipv4/af_inet.c > >> @@ -1425,10 +1425,18 @@ struct sk_buff *inet_gso_segment(struct sk_buff *skb, > >> static struct sk_buff *ipip_gso_segment(struct sk_buff *skb, > >> netdev_features_t features) > >> { > >> + struct sk_buff *segs; > >> + int nhoff; > >> + > >> if (!(skb_shinfo(skb)->gso_type & SKB_GSO_IPXIP4)) > >> return ERR_PTR(-EINVAL); > >> > >> - return inet_gso_segment(skb, features); > >> + nhoff = skb_network_header(skb) - skb_mac_header(skb); > >> + segs = inet_gso_segment(skb, features); > >> + if (!segs) > >> + skb->network_header = skb_mac_header(skb) + nhoff - skb->head; > >> + > >> + return segs; > >> } > > > > If this would be needed for IPIP, then the same would be needed for SIT, etc. > > > > Is the skb_network_header > > > > 1. correctly pointing to the outer header of the TSO packet before the > > call to inet_gso_segment > > 2. incorrectly pointing to the inner header of the (still) TSO packet > > after the call to inet_gso_segment > > > > inet_gso_segment already does the same operation: save nhoff, pull > > network header, call callbacks.gso_segment (which can be > > ipip_gso_segment->inet_gso_segment), then place the network header > > back at nhoff. > > > values print in skb_mac_gso_segment() before callbacks.gso_segment: > ipip: vlan_depth=0 mac_len=0 skb->network_header=206 > net_failover: vlan_depth=14 mac_len=14 skb->network_header=186 > virtio_net: vlan_depth=34 mac_len=34 skb->network_header=206 > > agree to add sit/ip4ip6/ip6ip6, and patch can be simplified as: If IPIP GSO was so broken, I think we would have found it long before. As said, inet_gso_segment should already do the right thing for ipip: it will be called twice.
On Mon, Feb 14, 2022 at 8:38 PM Tao Liu <thomas.liu@ucloud.cn> wrote: > > Sorry to resend it. > > 2022年2月14日 12:27，Willem de Bruijn <willemdebruijn.kernel@gmail.com> 写道： > > On Sun, Feb 13, 2022 at 11:03 PM Tao Liu <thomas.liu@ucloud.cn> wrote: > > > Sorry for bothering, just repost it. > > 2022年2月14日 09:28，Willem de Bruijn <willemdebruijn.kernel@gmail.com> 写道： > > On Sun, Feb 13, 2022 at 10:10 AM Tao Liu <thomas.liu@ucloud.cn> wrote: > > > We encouter a tcp drop issue in our cloud environment. Packet GROed in host > forwards to a VM virtio_net nic with net_failover enabled. VM acts as a > IPVS LB with ipip encapsulation. The full path like: > host gro -> vm virtio_net rx -> net_failover rx -> ipvs fullnat > -> ipip encap -> net_failover tx -> virtio_net tx > > When net_failover transmits a ipip pkt (gso_type = 0x0103), there is no gso > performed because it supports TSO and GSO_IPXIP4. But network_header has > been pointing to inner ip header. > > > If the packet is configured correctly, and net_failover advertises > that it can handle TSO packets with IPIP encap, then still virtio_net > should not advertise it and software GSO be applied on its > dev_queue_xmit call. > > This is assuming that the packet not only has SKB_GSO_IPXIP4 correctly > set, but also tunneling fields like skb->encapsulated and > skb_inner_network_header. > > Thanks very much for your comment! > > Yes, the packet is correct. Another thing i have not pointed directly is > that the pkt has SKB_GSO_DODGY. net_failover do not advertises GSO_ROBUST > but virtio_net do. > > > If net_failover does not advertise NETIF_F_GSO_ROBUST, then > tcp_gso_segment will pass a packet with SKB_GSO_DODGY to the > software gso stack, not taking the branch > > if (skb_gso_ok(skb, features | NETIF_F_GSO_ROBUST)) { > > As i tested, packet with SKB_GSO_DODGY hits this branch. packet's gso_type=0x0103, which > means SKB_GSO_TCPV4, SKB_GSO_DODGY and SKB_GSO_IPXIP4. net_failover matches > the condition. > > Consequently, tcp_gso_segment returns NULL, there is no software gso did here. And > network_header points to inner iph. > > Software gso is did by virtio_net which not advertises NETIF_F_GSO_IPXIP4. It skips the outer > iph, and keeps it unchanged. > > --- > net/ipv4/af_inet.c | 10 +++++++++- > 1 file changed, 9 insertions(+), 1 deletion(-) > > diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c > index 9c465ba..f8b3f8a 100644 > --- a/net/ipv4/af_inet.c > +++ b/net/ipv4/af_inet.c > @@ -1425,10 +1425,18 @@ struct sk_buff *inet_gso_segment(struct sk_buff *skb, > static struct sk_buff *ipip_gso_segment(struct sk_buff *skb, > netdev_features_t features) > { > + struct sk_buff *segs; > + int nhoff; > + > if (!(skb_shinfo(skb)->gso_type & SKB_GSO_IPXIP4)) > return ERR_PTR(-EINVAL); > > - return inet_gso_segment(skb, features); > + nhoff = skb_network_header(skb) - skb_mac_header(skb); > + segs = inet_gso_segment(skb, features); > + if (!segs) > + skb->network_header = skb_mac_header(skb) + nhoff - skb->head; > + > + return segs; > } > > > If this would be needed for IPIP, then the same would be needed for SIT, etc. > > Is the skb_network_header > > 1. correctly pointing to the outer header of the TSO packet before the > call to inet_gso_segment > 2. incorrectly pointing to the inner header of the (still) TSO packet > after the call to inet_gso_segment > > inet_gso_segment already does the same operation: save nhoff, pull > network header, call callbacks.gso_segment (which can be > ipip_gso_segment->inet_gso_segment), then place the network header > back at nhoff. > > values print in skb_mac_gso_segment() before callbacks.gso_segment: > ipip: vlan_depth=0 mac_len=0 skb->network_header=206 > net_failover: vlan_depth=14 mac_len=14 skb->network_header=186 > virtio_net: vlan_depth=34 mac_len=34 skb->network_header=206 > > agree to add sit/ip4ip6/ip6ip6, and patch can be simplified as: > > > If IPIP GSO was so broken, I think we would have found it long before. > > As said, inet_gso_segment should already do the right thing for ipip: > it will be called twice. > > > SKB_GSO_DODGY flag and net_failover conduct this issue. local traffic just works fine. Got it. That is an uncommon combination. SKB_GSO_DODGY is set from external virtio_net, which does not support tunnels. But a path with an added tunnel might cause this combination. And inet_gso_segment resets the network header, both times, before calling callbacks.gso_segment() skb_reset_network_header(skb); nhoff = skb_network_header(skb) - skb_mac_header(skb); [...] if (likely(ops && ops->callbacks.gso_segment)) segs = ops->callbacks.gso_segment(skb, features); And resets that after for each skb in segs. skb = segs; do { [...] skb->network_header = (u8 *)iph - skb->head; But does not do this if segs == NULL. The packet has to be restored before it is passed to the device. I think we have to handle this case correctly in inet_gso_segment, instead of patching it up in all the various tunnel devices. The same holds for ipv6_gso_segment.
On Tue, Feb 15, 2022 at 7:01 AM Willem de Bruijn <willemdebruijn.kernel@gmail.com> wrote: > > On Mon, Feb 14, 2022 at 8:38 PM Tao Liu <thomas.liu@ucloud.cn> wrote: > > > > Sorry to resend it. > > > > 2022年2月14日 12:27，Willem de Bruijn <willemdebruijn.kernel@gmail.com> 写道： > > > > On Sun, Feb 13, 2022 at 11:03 PM Tao Liu <thomas.liu@ucloud.cn> wrote: > > > > > > Sorry for bothering, just repost it. > > > > 2022年2月14日 09:28，Willem de Bruijn <willemdebruijn.kernel@gmail.com> 写道： > > > > On Sun, Feb 13, 2022 at 10:10 AM Tao Liu <thomas.liu@ucloud.cn> wrote: > > > > > > We encouter a tcp drop issue in our cloud environment. Packet GROed in host > > forwards to a VM virtio_net nic with net_failover enabled. VM acts as a > > IPVS LB with ipip encapsulation. The full path like: > > host gro -> vm virtio_net rx -> net_failover rx -> ipvs fullnat > > -> ipip encap -> net_failover tx -> virtio_net tx > > > > When net_failover transmits a ipip pkt (gso_type = 0x0103), there is no gso > > performed because it supports TSO and GSO_IPXIP4. But network_header has > > been pointing to inner ip header. > > > > > > If the packet is configured correctly, and net_failover advertises > > that it can handle TSO packets with IPIP encap, then still virtio_net > > should not advertise it and software GSO be applied on its > > dev_queue_xmit call. > > > > This is assuming that the packet not only has SKB_GSO_IPXIP4 correctly > > set, but also tunneling fields like skb->encapsulated and > > skb_inner_network_header. > > > > Thanks very much for your comment! > > > > Yes, the packet is correct. Another thing i have not pointed directly is > > that the pkt has SKB_GSO_DODGY. net_failover do not advertises GSO_ROBUST > > but virtio_net do. > > > > > > If net_failover does not advertise NETIF_F_GSO_ROBUST, then > > tcp_gso_segment will pass a packet with SKB_GSO_DODGY to the > > software gso stack, not taking the branch > > > > if (skb_gso_ok(skb, features | NETIF_F_GSO_ROBUST)) { > > > > As i tested, packet with SKB_GSO_DODGY hits this branch. packet's gso_type=0x0103, which > > means SKB_GSO_TCPV4, SKB_GSO_DODGY and SKB_GSO_IPXIP4. net_failover matches > > the condition. > > > > Consequently, tcp_gso_segment returns NULL, there is no software gso did here. And > > network_header points to inner iph. > > > > Software gso is did by virtio_net which not advertises NETIF_F_GSO_IPXIP4. It skips the outer > > iph, and keeps it unchanged. > > > > --- > > net/ipv4/af_inet.c | 10 +++++++++- > > 1 file changed, 9 insertions(+), 1 deletion(-) > > > > diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c > > index 9c465ba..f8b3f8a 100644 > > --- a/net/ipv4/af_inet.c > > +++ b/net/ipv4/af_inet.c > > @@ -1425,10 +1425,18 @@ struct sk_buff *inet_gso_segment(struct sk_buff *skb, > > static struct sk_buff *ipip_gso_segment(struct sk_buff *skb, > > netdev_features_t features) > > { > > + struct sk_buff *segs; > > + int nhoff; > > + > > if (!(skb_shinfo(skb)->gso_type & SKB_GSO_IPXIP4)) > > return ERR_PTR(-EINVAL); > > > > - return inet_gso_segment(skb, features); > > + nhoff = skb_network_header(skb) - skb_mac_header(skb); > > + segs = inet_gso_segment(skb, features); > > + if (!segs) > > + skb->network_header = skb_mac_header(skb) + nhoff - skb->head; > > + > > + return segs; > > } > > > > > > If this would be needed for IPIP, then the same would be needed for SIT, etc. > > > > Is the skb_network_header > > > > 1. correctly pointing to the outer header of the TSO packet before the > > call to inet_gso_segment > > 2. incorrectly pointing to the inner header of the (still) TSO packet > > after the call to inet_gso_segment > > > > inet_gso_segment already does the same operation: save nhoff, pull > > network header, call callbacks.gso_segment (which can be > > ipip_gso_segment->inet_gso_segment), then place the network header > > back at nhoff. > > > > values print in skb_mac_gso_segment() before callbacks.gso_segment: > > ipip: vlan_depth=0 mac_len=0 skb->network_header=206 > > net_failover: vlan_depth=14 mac_len=14 skb->network_header=186 > > virtio_net: vlan_depth=34 mac_len=34 skb->network_header=206 > > > > agree to add sit/ip4ip6/ip6ip6, and patch can be simplified as: > > > > > > If IPIP GSO was so broken, I think we would have found it long before. > > > > As said, inet_gso_segment should already do the right thing for ipip: > > it will be called twice. > > > > > > SKB_GSO_DODGY flag and net_failover conduct this issue. local traffic just works fine. > > Got it. That is an uncommon combination. SKB_GSO_DODGY is set from > external virtio_net, which does not support tunnels. But a path with > an added tunnel might cause this combination. > > And inet_gso_segment resets the network header, both times, before > calling callbacks.gso_segment() > > skb_reset_network_header(skb); > nhoff = skb_network_header(skb) - skb_mac_header(skb); > > [...] > > if (likely(ops && ops->callbacks.gso_segment)) > segs = ops->callbacks.gso_segment(skb, features); > > And resets that after for each skb in segs. > > skb = segs; > do { > [...] > skb->network_header = (u8 *)iph - skb->head; > > But does not do this if segs == NULL. > > The packet has to be restored before it is passed to the device. I > think we have to handle this case correctly in inet_gso_segment, > instead of patching it up in all the various tunnel devices. > > The same holds for ipv6_gso_segment. Back in the days, GRO was modified so that we passed a context (nhoff) in called functions, instead of changing skb offsets. The concept of outer/inner header only works with 1 encap. Perhaps it is time to do the same in GSO, to allow arbitrary levels of encapsulation. Then we no longer mess with these limited 'network_header/inner_network_header' fields in the skb. Stuffing state in the skb has been a mistake I think.
> > Got it. That is an uncommon combination. SKB_GSO_DODGY is set from > > external virtio_net, which does not support tunnels. But a path with > > an added tunnel might cause this combination. > > > > And inet_gso_segment resets the network header, both times, before > > calling callbacks.gso_segment() > > > > skb_reset_network_header(skb); > > nhoff = skb_network_header(skb) - skb_mac_header(skb); > > > > [...] > > > > if (likely(ops && ops->callbacks.gso_segment)) > > segs = ops->callbacks.gso_segment(skb, features); > > > > And resets that after for each skb in segs. > > > > skb = segs; > > do { > > [...] > > skb->network_header = (u8 *)iph - skb->head; > > > > But does not do this if segs == NULL. > > > > The packet has to be restored before it is passed to the device. I > > think we have to handle this case correctly in inet_gso_segment, > > instead of patching it up in all the various tunnel devices. > > > > The same holds for ipv6_gso_segment. > > Back in the days, GRO was modified so that we passed a context (nhoff) > in called functions, > instead of changing skb offsets. The concept of outer/inner header > only works with 1 encap. > > Perhaps it is time to do the same in GSO, to allow arbitrary levels of > encapsulation. > Then we no longer mess with these limited > 'network_header/inner_network_header' fields > in the skb. > > Stuffing state in the skb has been a mistake I think. If we could unwind those skb inner_* fields (and reclaim the skbuff space!) that would be fantastic. Immediately for this bug: perhaps it can be fixed by resetting the network_header on the gso skb if segs == NULL. As the offset is stored on the stack.
On Tue, Feb 15, 2022 at 7:46 AM Willem de Bruijn <willemdebruijn.kernel@gmail.com> wrote: > > Stuffing state in the skb has been a mistake I think. > > If we could unwind those skb inner_* fields (and reclaim the skbuff > space!) that would be fantastic. Not sure we can easily remove the space, many networking drivers need them, we probably do not want to dissect packets in their ndo_start_xmit() > > Immediately for this bug: perhaps it can be fixed by resetting the > network_header on the gso skb if segs == NULL. As the offset is stored > on the stack. It seems correct. Any other fields we need to take care of ?
diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c index 9c465ba..f8b3f8a 100644 --- a/net/ipv4/af_inet.c +++ b/net/ipv4/af_inet.c @@ -1425,10 +1425,18 @@ struct sk_buff *inet_gso_segment(struct sk_buff *skb, static struct sk_buff *ipip_gso_segment(struct sk_buff *skb, netdev_features_t features) { + struct sk_buff *segs; + int nhoff; + if (!(skb_shinfo(skb)->gso_type & SKB_GSO_IPXIP4)) return ERR_PTR(-EINVAL); - return inet_gso_segment(skb, features); + nhoff = skb_network_header(skb) - skb_mac_header(skb); + segs = inet_gso_segment(skb, features); + if (!segs) + skb->network_header = skb_mac_header(skb) + nhoff - skb->head; + + return segs; } struct sk_buff *inet_gro_receive(struct list_head *head, struct sk_buff *skb)
We encouter a tcp drop issue in our cloud environment. Packet GROed in host forwards to a VM virtio_net nic with net_failover enabled. VM acts as a IPVS LB with ipip encapsulation. The full path like: host gro -> vm virtio_net rx -> net_failover rx -> ipvs fullnat -> ipip encap -> net_failover tx -> virtio_net tx When net_failover transmits a ipip pkt (gso_type = 0x0103), there is no gso performed because it supports TSO and GSO_IPXIP4. But network_header has been pointing to inner ip header. Call Trace: tcp4_gso_segment ------> return NULL inet_gso_segment ------> inner iph, network_header points to ipip_gso_segment inet_gso_segment ------> outer iph skb_mac_gso_segment Afterwards virtio_net transmits the pkt, only inner ip header is modified. And the outer one just keeps untouched. The pkt will be dropped in remote host. So we need to reset network header if there is no gso performed in net_failover. Call Trace: inet_gso_segment ------> inner iph, outer iph is skipped skb_mac_gso_segment __skb_gso_segment validate_xmit_skb validate_xmit_skb_list sch_direct_xmit __qdisc_run __dev_queue_xmit ------> virtio_net dev_hard_start_xmit __dev_queue_xmit ------> net_failover ip_finish_output2 ip_output iptunnel_xmit ip_tunnel_xmit ipip_tunnel_xmit ------> ipip dev_hard_start_xmit __dev_queue_xmit ip_finish_output2 ip_output ip_forward ip_rcv __netif_receive_skb_one_core netif_receive_skb_internal napi_gro_receive receive_buf virtnet_poll net_rx_action Fixes: cb32f511a70b ("ipip: add GSO/TSO support") Fixes: cfc80d9a1163 ("net: Introduce net_failover driver") Signed-off-by: Tao Liu <thomas.liu@ucloud.cn> --- net/ipv4/af_inet.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)