Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20220213150234.31602-1-thomas.liu@ucloud.cn>
In-Reply-To: <20220213150234.31602-1-thomas.liu@ucloud.cn>
From:   Willem de Bruijn <willemdebruijn.kernel@gmail.com>
Date:   Sun, 13 Feb 2022 20:28:45 -0500
Message-ID: <CA+FuTSdODATw3hSAMv9aZUmJNM8ZE-YP58pr17bO9rGJUgfegw@mail.gmail.com>
Subject: Re: [PATCH] gso: do not skip outer ip header in case of ipip and net_failover
To:     Tao Liu <thomas.liu@ucloud.cn>
Cc:     davem@davemloft.net, yoshfuji@linux-ipv6.org, dsahern@kernel.org,
        kuba@kernel.org, edumazet@google.com, sridhar.samudrala@intel.com,
        netdev@vger.kernel.org, linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

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.