netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net v3] virtio-net: avoid unnecessary checksum calculation on guest RX
@ 2025-11-25 22:27 Jon Kohler
  2025-11-26  6:28 ` Michael S. Tsirkin
  2025-11-26  6:29 ` Jason Wang
  0 siblings, 2 replies; 6+ messages in thread
From: Jon Kohler @ 2025-11-25 22:27 UTC (permalink / raw)
  To: Willem de Bruijn, Jason Wang, Andrew Lunn, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Michael S. Tsirkin,
	Xuan Zhuo, Eugenio Pérez, netdev, linux-kernel,
	virtualization
  Cc: Jon Kohler

Commit a2fb4bc4e2a6 ("net: implement virtio helpers to handle UDP
GSO tunneling.") inadvertently altered checksum offload behavior
for guests not using UDP GSO tunneling.

Before, tun_put_user called tun_vnet_hdr_from_skb, which passed
has_data_valid = true to virtio_net_hdr_from_skb.

After, tun_put_user began calling tun_vnet_hdr_tnl_from_skb instead,
which passes has_data_valid = false into both call sites.

This caused virtio hdr flags to not include VIRTIO_NET_HDR_F_DATA_VALID
for SKBs where skb->ip_summed == CHECKSUM_UNNECESSARY. As a result,
guests are forced to recalculate checksums unnecessarily.

Restore the previous behavior by ensuring has_data_valid = true is
passed in the !tnl_gso_type case, but only from tun side, as
virtio_net_hdr_tnl_from_skb() is used also by the virtio_net driver,
which in turn must not use VIRTIO_NET_HDR_F_DATA_VALID on tx.

Cc: Paolo Abeni <pabeni@redhat.com>
Fixes: a2fb4bc4e2a6 ("net: implement virtio helpers to handle UDP GSO tunneling.")
Signed-off-by: Jon Kohler <jon@nutanix.com>
---
v2-v3: Add net tag (whoops, sorry!)
v1-v2: Add arg to avoid conflict from driver (Paolo) and send to net
       instead of net-next.
 drivers/net/tun_vnet.h     | 2 +-
 drivers/net/virtio_net.c   | 3 ++-
 include/linux/virtio_net.h | 7 ++++---
 3 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/net/tun_vnet.h b/drivers/net/tun_vnet.h
index 81662328b2c7..a5f93b6c4482 100644
--- a/drivers/net/tun_vnet.h
+++ b/drivers/net/tun_vnet.h
@@ -244,7 +244,7 @@ tun_vnet_hdr_tnl_from_skb(unsigned int flags,
 
 	if (virtio_net_hdr_tnl_from_skb(skb, tnl_hdr, has_tnl_offload,
 					tun_vnet_is_little_endian(flags),
-					vlan_hlen)) {
+					vlan_hlen, true)) {
 		struct virtio_net_hdr_v1 *hdr = &tnl_hdr->hash_hdr.hdr;
 		struct skb_shared_info *sinfo = skb_shinfo(skb);
 
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index cfa006b88688..96f2d2a59003 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -3339,7 +3339,8 @@ static int xmit_skb(struct send_queue *sq, struct sk_buff *skb, bool orphan)
 		hdr = &skb_vnet_common_hdr(skb)->tnl_hdr;
 
 	if (virtio_net_hdr_tnl_from_skb(skb, hdr, vi->tx_tnl,
-					virtio_is_little_endian(vi->vdev), 0))
+					virtio_is_little_endian(vi->vdev), 0,
+					false))
 		return -EPROTO;
 
 	if (vi->mergeable_rx_bufs)
diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
index b673c31569f3..75dabb763c65 100644
--- a/include/linux/virtio_net.h
+++ b/include/linux/virtio_net.h
@@ -384,7 +384,8 @@ virtio_net_hdr_tnl_from_skb(const struct sk_buff *skb,
 			    struct virtio_net_hdr_v1_hash_tunnel *vhdr,
 			    bool tnl_hdr_negotiated,
 			    bool little_endian,
-			    int vlan_hlen)
+			    int vlan_hlen,
+			    bool has_data_valid)
 {
 	struct virtio_net_hdr *hdr = (struct virtio_net_hdr *)vhdr;
 	unsigned int inner_nh, outer_th;
@@ -394,8 +395,8 @@ virtio_net_hdr_tnl_from_skb(const struct sk_buff *skb,
 	tnl_gso_type = skb_shinfo(skb)->gso_type & (SKB_GSO_UDP_TUNNEL |
 						    SKB_GSO_UDP_TUNNEL_CSUM);
 	if (!tnl_gso_type)
-		return virtio_net_hdr_from_skb(skb, hdr, little_endian, false,
-					       vlan_hlen);
+		return virtio_net_hdr_from_skb(skb, hdr, little_endian,
+					       has_data_valid, vlan_hlen);
 
 	/* Tunnel support not negotiated but skb ask for it. */
 	if (!tnl_hdr_negotiated)
-- 
2.43.0


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

* Re: [PATCH net v3] virtio-net: avoid unnecessary checksum calculation on guest RX
  2025-11-25 22:27 [PATCH net v3] virtio-net: avoid unnecessary checksum calculation on guest RX Jon Kohler
@ 2025-11-26  6:28 ` Michael S. Tsirkin
  2025-11-26  6:29 ` Jason Wang
  1 sibling, 0 replies; 6+ messages in thread
From: Michael S. Tsirkin @ 2025-11-26  6:28 UTC (permalink / raw)
  To: Jon Kohler
  Cc: Willem de Bruijn, Jason Wang, Andrew Lunn, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Xuan Zhuo,
	Eugenio Pérez, netdev, linux-kernel, virtualization

On Tue, Nov 25, 2025 at 03:27:53PM -0700, Jon Kohler wrote:
> Commit a2fb4bc4e2a6 ("net: implement virtio helpers to handle UDP
> GSO tunneling.") inadvertently altered checksum offload behavior
> for guests not using UDP GSO tunneling.
> 
> Before, tun_put_user called tun_vnet_hdr_from_skb, which passed
> has_data_valid = true to virtio_net_hdr_from_skb.
> 
> After, tun_put_user began calling tun_vnet_hdr_tnl_from_skb instead,
> which passes has_data_valid = false into both call sites.
> 
> This caused virtio hdr flags to not include VIRTIO_NET_HDR_F_DATA_VALID
> for SKBs where skb->ip_summed == CHECKSUM_UNNECESSARY. As a result,
> guests are forced to recalculate checksums unnecessarily.
> 
> Restore the previous behavior by ensuring has_data_valid = true is
> passed in the !tnl_gso_type case, but only from tun side, as
> virtio_net_hdr_tnl_from_skb() is used also by the virtio_net driver,
> which in turn must not use VIRTIO_NET_HDR_F_DATA_VALID on tx.
> 
> Cc: Paolo Abeni <pabeni@redhat.com>
> Fixes: a2fb4bc4e2a6 ("net: implement virtio helpers to handle UDP GSO tunneling.")
> Signed-off-by: Jon Kohler <jon@nutanix.com>

Acked-by: Michael S. Tsirkin <mst@redhat.com>

> ---
> v2-v3: Add net tag (whoops, sorry!)
> v1-v2: Add arg to avoid conflict from driver (Paolo) and send to net
>        instead of net-next.
>  drivers/net/tun_vnet.h     | 2 +-
>  drivers/net/virtio_net.c   | 3 ++-
>  include/linux/virtio_net.h | 7 ++++---
>  3 files changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/net/tun_vnet.h b/drivers/net/tun_vnet.h
> index 81662328b2c7..a5f93b6c4482 100644
> --- a/drivers/net/tun_vnet.h
> +++ b/drivers/net/tun_vnet.h
> @@ -244,7 +244,7 @@ tun_vnet_hdr_tnl_from_skb(unsigned int flags,
>  
>  	if (virtio_net_hdr_tnl_from_skb(skb, tnl_hdr, has_tnl_offload,
>  					tun_vnet_is_little_endian(flags),
> -					vlan_hlen)) {
> +					vlan_hlen, true)) {
>  		struct virtio_net_hdr_v1 *hdr = &tnl_hdr->hash_hdr.hdr;
>  		struct skb_shared_info *sinfo = skb_shinfo(skb);
>  
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index cfa006b88688..96f2d2a59003 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -3339,7 +3339,8 @@ static int xmit_skb(struct send_queue *sq, struct sk_buff *skb, bool orphan)
>  		hdr = &skb_vnet_common_hdr(skb)->tnl_hdr;
>  
>  	if (virtio_net_hdr_tnl_from_skb(skb, hdr, vi->tx_tnl,
> -					virtio_is_little_endian(vi->vdev), 0))
> +					virtio_is_little_endian(vi->vdev), 0,
> +					false))
>  		return -EPROTO;
>  
>  	if (vi->mergeable_rx_bufs)
> diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
> index b673c31569f3..75dabb763c65 100644
> --- a/include/linux/virtio_net.h
> +++ b/include/linux/virtio_net.h
> @@ -384,7 +384,8 @@ virtio_net_hdr_tnl_from_skb(const struct sk_buff *skb,
>  			    struct virtio_net_hdr_v1_hash_tunnel *vhdr,
>  			    bool tnl_hdr_negotiated,
>  			    bool little_endian,
> -			    int vlan_hlen)
> +			    int vlan_hlen,
> +			    bool has_data_valid)
>  {
>  	struct virtio_net_hdr *hdr = (struct virtio_net_hdr *)vhdr;
>  	unsigned int inner_nh, outer_th;
> @@ -394,8 +395,8 @@ virtio_net_hdr_tnl_from_skb(const struct sk_buff *skb,
>  	tnl_gso_type = skb_shinfo(skb)->gso_type & (SKB_GSO_UDP_TUNNEL |
>  						    SKB_GSO_UDP_TUNNEL_CSUM);
>  	if (!tnl_gso_type)
> -		return virtio_net_hdr_from_skb(skb, hdr, little_endian, false,
> -					       vlan_hlen);
> +		return virtio_net_hdr_from_skb(skb, hdr, little_endian,
> +					       has_data_valid, vlan_hlen);
>  
>  	/* Tunnel support not negotiated but skb ask for it. */
>  	if (!tnl_hdr_negotiated)
> -- 
> 2.43.0


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

* Re: [PATCH net v3] virtio-net: avoid unnecessary checksum calculation on guest RX
  2025-11-25 22:27 [PATCH net v3] virtio-net: avoid unnecessary checksum calculation on guest RX Jon Kohler
  2025-11-26  6:28 ` Michael S. Tsirkin
@ 2025-11-26  6:29 ` Jason Wang
  2025-11-26 16:11   ` Jon Kohler
  1 sibling, 1 reply; 6+ messages in thread
From: Jason Wang @ 2025-11-26  6:29 UTC (permalink / raw)
  To: Jon Kohler
  Cc: Willem de Bruijn, Andrew Lunn, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Michael S. Tsirkin, Xuan Zhuo,
	Eugenio Pérez, netdev, linux-kernel, virtualization

On Wed, Nov 26, 2025 at 5:46 AM Jon Kohler <jon@nutanix.com> wrote:
>
> Commit a2fb4bc4e2a6 ("net: implement virtio helpers to handle UDP
> GSO tunneling.") inadvertently altered checksum offload behavior
> for guests not using UDP GSO tunneling.
>
> Before, tun_put_user called tun_vnet_hdr_from_skb, which passed
> has_data_valid = true to virtio_net_hdr_from_skb.
>
> After, tun_put_user began calling tun_vnet_hdr_tnl_from_skb instead,
> which passes has_data_valid = false into both call sites.
>
> This caused virtio hdr flags to not include VIRTIO_NET_HDR_F_DATA_VALID
> for SKBs where skb->ip_summed == CHECKSUM_UNNECESSARY. As a result,
> guests are forced to recalculate checksums unnecessarily.
>
> Restore the previous behavior by ensuring has_data_valid = true is
> passed in the !tnl_gso_type case, but only from tun side, as
> virtio_net_hdr_tnl_from_skb() is used also by the virtio_net driver,
> which in turn must not use VIRTIO_NET_HDR_F_DATA_VALID on tx.
>
> Cc: Paolo Abeni <pabeni@redhat.com>
> Fixes: a2fb4bc4e2a6 ("net: implement virtio helpers to handle UDP GSO tunneling.")
> Signed-off-by: Jon Kohler <jon@nutanix.com>
> ---

Acked-by: Jason Wang <jasowang@redhat.com>

(Should this go -stable?)

Thanks


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

* Re: [PATCH net v3] virtio-net: avoid unnecessary checksum calculation on guest RX
  2025-11-26  6:29 ` Jason Wang
@ 2025-11-26 16:11   ` Jon Kohler
  2025-11-27  1:27     ` Jason Wang
  0 siblings, 1 reply; 6+ messages in thread
From: Jon Kohler @ 2025-11-26 16:11 UTC (permalink / raw)
  To: Jason Wang
  Cc: Willem de Bruijn, Andrew Lunn, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Michael S. Tsirkin, Xuan Zhuo,
	Eugenio Pérez, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org, virtualization@lists.linux.dev



> On Nov 26, 2025, at 1:29 AM, Jason Wang <jasowang@redhat.com> wrote:
> 
> On Wed, Nov 26, 2025 at 5:46 AM Jon Kohler <jon@nutanix.com> wrote:
>> 
>> Commit a2fb4bc4e2a6 ("net: implement virtio helpers to handle UDP
>> GSO tunneling.") inadvertently altered checksum offload behavior
>> for guests not using UDP GSO tunneling.
>> 
>> Before, tun_put_user called tun_vnet_hdr_from_skb, which passed
>> has_data_valid = true to virtio_net_hdr_from_skb.
>> 
>> After, tun_put_user began calling tun_vnet_hdr_tnl_from_skb instead,
>> which passes has_data_valid = false into both call sites.
>> 
>> This caused virtio hdr flags to not include VIRTIO_NET_HDR_F_DATA_VALID
>> for SKBs where skb->ip_summed == CHECKSUM_UNNECESSARY. As a result,
>> guests are forced to recalculate checksums unnecessarily.
>> 
>> Restore the previous behavior by ensuring has_data_valid = true is
>> passed in the !tnl_gso_type case, but only from tun side, as
>> virtio_net_hdr_tnl_from_skb() is used also by the virtio_net driver,
>> which in turn must not use VIRTIO_NET_HDR_F_DATA_VALID on tx.
>> 
>> Cc: Paolo Abeni <pabeni@redhat.com>
>> Fixes: a2fb4bc4e2a6 ("net: implement virtio helpers to handle UDP GSO tunneling.")
>> Signed-off-by: Jon Kohler <jon@nutanix.com>
>> ---
> 
> Acked-by: Jason Wang <jasowang@redhat.com>
> 
> (Should this go -stable?)
> 
> Thanks

It could, sure. This made it into 6.17 branch.

Would you like me to send a separate patch with a Cc: stable
or could someone just edit the commit msg when they queue
this?

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

* Re: [PATCH net v3] virtio-net: avoid unnecessary checksum calculation on guest RX
  2025-11-26 16:11   ` Jon Kohler
@ 2025-11-27  1:27     ` Jason Wang
  2025-11-27  3:48       ` Jakub Kicinski
  0 siblings, 1 reply; 6+ messages in thread
From: Jason Wang @ 2025-11-27  1:27 UTC (permalink / raw)
  To: Jon Kohler
  Cc: Willem de Bruijn, Andrew Lunn, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Michael S. Tsirkin, Xuan Zhuo,
	Eugenio Pérez, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org, virtualization@lists.linux.dev

On Thu, Nov 27, 2025 at 12:12 AM Jon Kohler <jon@nutanix.com> wrote:
>
>
>
> > On Nov 26, 2025, at 1:29 AM, Jason Wang <jasowang@redhat.com> wrote:
> >
> > On Wed, Nov 26, 2025 at 5:46 AM Jon Kohler <jon@nutanix.com> wrote:
> >>
> >> Commit a2fb4bc4e2a6 ("net: implement virtio helpers to handle UDP
> >> GSO tunneling.") inadvertently altered checksum offload behavior
> >> for guests not using UDP GSO tunneling.
> >>
> >> Before, tun_put_user called tun_vnet_hdr_from_skb, which passed
> >> has_data_valid = true to virtio_net_hdr_from_skb.
> >>
> >> After, tun_put_user began calling tun_vnet_hdr_tnl_from_skb instead,
> >> which passes has_data_valid = false into both call sites.
> >>
> >> This caused virtio hdr flags to not include VIRTIO_NET_HDR_F_DATA_VALID
> >> for SKBs where skb->ip_summed == CHECKSUM_UNNECESSARY. As a result,
> >> guests are forced to recalculate checksums unnecessarily.
> >>
> >> Restore the previous behavior by ensuring has_data_valid = true is
> >> passed in the !tnl_gso_type case, but only from tun side, as
> >> virtio_net_hdr_tnl_from_skb() is used also by the virtio_net driver,
> >> which in turn must not use VIRTIO_NET_HDR_F_DATA_VALID on tx.
> >>
> >> Cc: Paolo Abeni <pabeni@redhat.com>
> >> Fixes: a2fb4bc4e2a6 ("net: implement virtio helpers to handle UDP GSO tunneling.")
> >> Signed-off-by: Jon Kohler <jon@nutanix.com>
> >> ---
> >
> > Acked-by: Jason Wang <jasowang@redhat.com>
> >
> > (Should this go -stable?)
> >
> > Thanks
>
> It could, sure. This made it into 6.17 branch.
>
> Would you like me to send a separate patch with a Cc: stable
> or could someone just edit the commit msg when they queue
> this?

I think a new version might be better.

Thanks


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

* Re: [PATCH net v3] virtio-net: avoid unnecessary checksum calculation on guest RX
  2025-11-27  1:27     ` Jason Wang
@ 2025-11-27  3:48       ` Jakub Kicinski
  0 siblings, 0 replies; 6+ messages in thread
From: Jakub Kicinski @ 2025-11-27  3:48 UTC (permalink / raw)
  To: Jason Wang
  Cc: Jon Kohler, Willem de Bruijn, Andrew Lunn, David S. Miller,
	Eric Dumazet, Paolo Abeni, Michael S. Tsirkin, Xuan Zhuo,
	Eugenio Pérez, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org, virtualization@lists.linux.dev

On Thu, 27 Nov 2025 09:27:16 +0800 Jason Wang wrote:
> > It could, sure. This made it into 6.17 branch.
> >
> > Would you like me to send a separate patch with a Cc: stable
> > or could someone just edit the commit msg when they queue
> > this?  
> 
> I think a new version might be better.

Added.

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

end of thread, other threads:[~2025-11-27  3:48 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-11-25 22:27 [PATCH net v3] virtio-net: avoid unnecessary checksum calculation on guest RX Jon Kohler
2025-11-26  6:28 ` Michael S. Tsirkin
2025-11-26  6:29 ` Jason Wang
2025-11-26 16:11   ` Jon Kohler
2025-11-27  1:27     ` Jason Wang
2025-11-27  3:48       ` Jakub Kicinski

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).