netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net v4] vmxnet3: correctly report gso type for UDP tunnels
@ 2025-05-30 15:27 Ronak Doshi
  2025-06-03  7:23 ` Simon Horman
  2025-06-03 10:10 ` patchwork-bot+netdevbpf
  0 siblings, 2 replies; 4+ messages in thread
From: Ronak Doshi @ 2025-05-30 15:27 UTC (permalink / raw)
  To: netdev
  Cc: Ronak Doshi, Guolin Yang, Broadcom internal kernel review list,
	Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, open list

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@broadcom.com>
Acked-by: Guolin Yang <guolin.yang@broadcom.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);
+	}
+	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


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH net v4] vmxnet3: correctly report gso type for UDP tunnels
  2025-05-30 15:27 [PATCH net v4] vmxnet3: correctly report gso type for UDP tunnels Ronak Doshi
@ 2025-06-03  7:23 ` Simon Horman
  2025-06-03  9:27   ` Paolo Abeni
  2025-06-03 10:10 ` patchwork-bot+netdevbpf
  1 sibling, 1 reply; 4+ messages in thread
From: Simon Horman @ 2025-06-03  7:23 UTC (permalink / raw)
  To: Ronak Doshi
  Cc: netdev, Guolin Yang, Broadcom internal kernel review list,
	Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, open list

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@broadcom.com>
> Acked-by: Guolin Yang <guolin.yang@broadcom.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
> 
> 

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH net v4] vmxnet3: correctly report gso type for UDP tunnels
  2025-06-03  7:23 ` Simon Horman
@ 2025-06-03  9:27   ` Paolo Abeni
  0 siblings, 0 replies; 4+ messages in thread
From: Paolo Abeni @ 2025-06-03  9:27 UTC (permalink / raw)
  To: Simon Horman, Ronak Doshi
  Cc: netdev, Guolin Yang, Broadcom internal kernel review list,
	Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
	open list

On 6/3/25 9:23 AM, Simon Horman wrote:
> 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@broadcom.com>
>> Acked-by: Guolin Yang <guolin.yang@broadcom.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?

I think it's safe, or at least the guest can assume that. Otherwise
there is a bug in the hypervisor cooking the descriptor, and the guest
can do little to nothing is such scenario.

/P


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH net v4] vmxnet3: correctly report gso type for UDP tunnels
  2025-05-30 15:27 [PATCH net v4] vmxnet3: correctly report gso type for UDP tunnels Ronak Doshi
  2025-06-03  7:23 ` Simon Horman
@ 2025-06-03 10:10 ` patchwork-bot+netdevbpf
  1 sibling, 0 replies; 4+ messages in thread
From: patchwork-bot+netdevbpf @ 2025-06-03 10:10 UTC (permalink / raw)
  To: Ronak Doshi
  Cc: netdev, guolin.yang, bcm-kernel-feedback-list, andrew+netdev,
	davem, edumazet, kuba, pabeni, linux-kernel

Hello:

This patch was applied to netdev/net.git (main)
by Paolo Abeni <pabeni@redhat.com>:

On Fri, 30 May 2025 15:27:00 +0000 you 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.
> 
> [...]

Here is the summary with links:
  - [net,v4] vmxnet3: correctly report gso type for UDP tunnels
    https://git.kernel.org/netdev/net/c/982d30c30eaa

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2025-06-03 10:09 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-05-30 15:27 [PATCH net v4] vmxnet3: correctly report gso type for UDP tunnels Ronak Doshi
2025-06-03  7:23 ` Simon Horman
2025-06-03  9:27   ` Paolo Abeni
2025-06-03 10:10 ` patchwork-bot+netdevbpf

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).