From: "Michael S. Tsirkin" <mst@redhat.com>
To: Jon Kohler <jon@nutanix.com>
Cc: "Paolo Abeni" <pabeni@redhat.com>,
"Jason Wang" <jasowang@redhat.com>,
"Xuan Zhuo" <xuanzhuo@linux.alibaba.com>,
"Eugenio Pérez" <eperezma@redhat.com>,
"virtualization@lists.linux.dev" <virtualization@lists.linux.dev>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"netdev@vger.kernel.org" <netdev@vger.kernel.org>
Subject: Re: [PATCH net-next] virtio-net: avoid unnecessary checksum calculation on guest RX
Date: Tue, 25 Nov 2025 19:03:34 -0500 [thread overview]
Message-ID: <20251125190207-mutt-send-email-mst@kernel.org> (raw)
In-Reply-To: <3ED1B031-7C20-45F9-AB47-8FCDB68B448E@nutanix.com>
On Tue, Nov 25, 2025 at 08:00:55PM +0000, Jon Kohler wrote:
>
>
> > On Nov 25, 2025, at 12:57 PM, Paolo Abeni <pabeni@redhat.com> wrote:
> >
> > CC netdev
>
> Thats odd, I used git send-email --to-cmd='./scripts/get_maintainer.pl,
> but it looks like in MAINTAINERS, that only would have hit
> VIRTIO CORE AND NET DRIVERS, which does not include netdev@
>
> Should that have ?
> L: netdev@vger.kernel.org <mailto:netdev@vger.kernel.org>
>
> Said another way, should all changes to include/linux/virtio_net.h
> be cc’d to netdev DL?
>
> I suspect the answer is yes, I’ll send a patch for that in the
> interest of not having this issue again :)
I think yes. But only virtio net not core. I guess we should
split net from core then.
> >
> > On 11/25/25 6:51 PM, 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.
> >>
> >> 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>
> >> ---
> >> include/linux/virtio_net.h | 2 +-
> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
> >> index b673c31569f3..570c6dd1666d 100644
> >> --- a/include/linux/virtio_net.h
> >> +++ b/include/linux/virtio_net.h
> >> @@ -394,7 +394,7 @@ 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,
> >> + return virtio_net_hdr_from_skb(skb, hdr, little_endian, true,
> >> vlan_hlen);
> >>
> >> /* Tunnel support not negotiated but skb ask for it. */
> >
> > 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.
>
> Ah! Good eye, I’ll see what trouble I can get into and send a v2
> >
> > I think you need to add another argument to
> > virtio_net_hdr_tnl_from_skb(), or possibly implement a separate helper
> > to take care of csum offload - the symmetric of
> > virtio_net_handle_csum_offload().
> >
> > Also you need to CC netdev, otherwise the patch will not be processed by
> > patchwork.
> >
> > /P
>
> No problems on cc’ing netdev, just didn’t realize this one header didn’t
> auto cc the list. Will keep an eye on that, and happy to send a patch to
> MAINTAINERS file for discussion.
next prev parent reply other threads:[~2025-11-26 0:03 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-11-25 17:51 [PATCH net-next] virtio-net: avoid unnecessary checksum calculation on guest RX Jon Kohler
2025-11-25 17:57 ` Paolo Abeni
2025-11-25 17:59 ` Paolo Abeni
2025-11-25 20:00 ` Jon Kohler
2025-11-26 0:03 ` Michael S. Tsirkin [this message]
2025-11-26 11:16 ` Paolo Abeni
2025-11-26 12:30 ` Michael S. Tsirkin
2025-11-26 19:00 ` Jon Kohler
2025-11-26 18:58 ` Jon Kohler
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20251125190207-mutt-send-email-mst@kernel.org \
--to=mst@redhat.com \
--cc=eperezma@redhat.com \
--cc=jasowang@redhat.com \
--cc=jon@nutanix.com \
--cc=linux-kernel@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
--cc=virtualization@lists.linux.dev \
--cc=xuanzhuo@linux.alibaba.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.