[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20250603072308.GW1484967@horms.kernel.org>
Date: Tue, 3 Jun 2025 08:23:08 +0100
From: Simon Horman <horms@...nel.org>
To: Ronak Doshi <ronak.doshi@...adcom.com>
Cc: netdev@...r.kernel.org, Guolin Yang <guolin.yang@...adcom.com>,
Broadcom internal kernel review list <bcm-kernel-feedback-list@...adcom.com>,
Andrew Lunn <andrew+netdev@...n.ch>,
"David S. Miller" <davem@...emloft.net>,
Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH net v4] vmxnet3: correctly report gso type for UDP tunnels
On Fri, May 30, 2025 at 03:27:00PM +0000, Ronak Doshi wrote:
> Commit 3d010c8031e3 ("udp: do not accept non-tunnel GSO skbs landing
> in a tunnel") added checks in linux stack to not accept non-tunnel
> GRO packets landing in a tunnel. This exposed an issue in vmxnet3
> which was not correctly reporting GRO packets for tunnel packets.
>
> This patch fixes this issue by setting correct GSO type for the
> tunnel packets.
>
> Currently, vmxnet3 does not support reporting inner fields for LRO
> tunnel packets. The issue is not seen for egress drivers that do not
> use skb inner fields. The workaround is to enable tnl-segmentation
> offload on the egress interfaces if the driver supports it. This
> problem pre-exists this patch fix and can be addressed as a separate
> future patch.
>
> Fixes: dacce2be3312 ("vmxnet3: add geneve and vxlan tunnel offload support")
> Signed-off-by: Ronak Doshi <ronak.doshi@...adcom.com>
> Acked-by: Guolin Yang <guolin.yang@...adcom.com>
>
> Changes v1-->v2:
> Do not set encapsulation bit as inner fields are not updated
> Changes v2-->v3:
> Update the commit message explaining the next steps to address
> segmentation issues that pre-exists this patch fix.
> Changes v3->v4:
> Update the commit message to clarify the workaround.
> ---
> drivers/net/vmxnet3/vmxnet3_drv.c | 26 ++++++++++++++++++++++++++
> 1 file changed, 26 insertions(+)
>
> diff --git a/drivers/net/vmxnet3/vmxnet3_drv.c b/drivers/net/vmxnet3/vmxnet3_drv.c
> index c676979c7ab9..287b7c20c0d6 100644
> --- a/drivers/net/vmxnet3/vmxnet3_drv.c
> +++ b/drivers/net/vmxnet3/vmxnet3_drv.c
> @@ -1568,6 +1568,30 @@ vmxnet3_get_hdr_len(struct vmxnet3_adapter *adapter, struct sk_buff *skb,
> return (hlen + (hdr.tcp->doff << 2));
> }
>
> +static void
> +vmxnet3_lro_tunnel(struct sk_buff *skb, __be16 ip_proto)
> +{
> + struct udphdr *uh = NULL;
> +
> + if (ip_proto == htons(ETH_P_IP)) {
> + struct iphdr *iph = (struct iphdr *)skb->data;
> +
> + if (iph->protocol == IPPROTO_UDP)
> + uh = (struct udphdr *)(iph + 1);
> + } else {
> + struct ipv6hdr *iph = (struct ipv6hdr *)skb->data;
> +
> + if (iph->nexthdr == IPPROTO_UDP)
> + uh = (struct udphdr *)(iph + 1);
> + }
Hi Ronak,
Possibly a naive question, but does skb->data always contain an iphdr
or ipv6hdr? Or perhaps more to the point, is it safe to assume IPv6
is ip_proto is not ETH_P_IP?
> + if (uh) {
> + if (uh->check)
> + skb_shinfo(skb)->gso_type |= SKB_GSO_UDP_TUNNEL_CSUM;
> + else
> + skb_shinfo(skb)->gso_type |= SKB_GSO_UDP_TUNNEL;
> + }
> +}
> +
> static int
> vmxnet3_rq_rx_complete(struct vmxnet3_rx_queue *rq,
> struct vmxnet3_adapter *adapter, int quota)
> @@ -1881,6 +1905,8 @@ vmxnet3_rq_rx_complete(struct vmxnet3_rx_queue *rq,
> if (segCnt != 0 && mss != 0) {
> skb_shinfo(skb)->gso_type = rcd->v4 ?
> SKB_GSO_TCPV4 : SKB_GSO_TCPV6;
> + if (encap_lro)
> + vmxnet3_lro_tunnel(skb, skb->protocol);
> skb_shinfo(skb)->gso_size = mss;
> skb_shinfo(skb)->gso_segs = segCnt;
> } else if ((segCnt != 0 || skb->len > mtu) && !encap_lro) {
> --
> 2.45.2
>
>
Powered by blists - more mailing lists