Messages in this thread Patch in this message | | | Subject | Re: [PATCH] gso: do not skip outer ip header in case of ipip and net_failover | From | Tao Liu <> | Date | Mon, 14 Feb 2022 12:03:38 +0800 |
| |
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))
| |