All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Michael S. Tsirkin" <mst@redhat.com>
To: Guikun Yang <yangguikun19@qq.com>
Cc: Jason Wang <jasowang@redhat.com>,
	Eugenio Perez <eperezma@redhat.com>,
	kvm@vger.kernel.org, virtualization@lists.linux.dev,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] vhost/net: initialize the whole negotiated RX vnet header
Date: Sat, 24 Oct 2026 02:45:39 -0400	[thread overview]
Message-ID: <20261024024028-mutt-send-email-mst@kernel.org> (raw)
In-Reply-To: <tencent_E3543FCAC26722EA3D15F87B69C4C769D905@qq.com>

On Sat, Oct 24, 2026 at 10:11:01AM +0800, Guikun Yang wrote:
> On Fri, Oct 23, 2026 at 08:13:22AM -0400, Michael S. Tsirkin wrote:
> > Which backend did you use? Sounds like you did not set
> > TUN_VNET_TNL_SIZE or did not set TUNSETOFFLOAD?
> > That's the issue then.
> 
> The backend is a tap device attached via the normal tun path:
> TUNSETIFF (IFF_TAP|IFF_NO_PI, no IFF_VNET_HDR),

Ah, no IFF_VNET_HDR *with* vhost. That's a weird combination.

> then
> VHOST_NET_SET_BACKEND on the RX vq.
> 
> I re-ran the repro with TUNSETOFFLOAD declared before attaching the
> backend, listing every offload bit the interface understands:
> 
> 	TUN_F_CSUM | TUN_F_TSO4 | TUN_F_TSO6 |
> 	TUN_F_USO4 | TUN_F_USO6 |
> 	TUN_F_UDP_TUNNEL_GSO | TUN_F_UDP_TUNNEL_GSO_CSUM
> 
> TUNSETOFFLOAD succeeds (0 is returned, so tun accepted all of them,
> including the UDP tunnel GSO bits).  The result is unchanged:
> 
> 	features[0] = 0x108000000, features[1] = 0xa   (tunnel bits on)
> 	VHOST_SET_FEATURES_ARRAY ok
> 	TUNSETOFFLOAD ok
> 	--- RX buffer first 32 bytes (initially all 0xAA) ---
> 	hdr: 00 00 00 00 00 00 00 00 00 00 01 00
> 	     aa aa aa aa aa aa aa aa aa aa aa aa     <-- offsets 12..23
> 	     33 33 00 00 ...
> 
> Offsets 12..23 still contain the stale 0xAA fill.
> 
> The reason is that vhost_net_set_features() computes vhost_hlen purely
> from the feature bits passed via VHOST_SET_FEATURES(_ARRAY):
> 
> 	hdr_len = ... sizeof(struct virtio_net_hdr_v1_hash_tunnel) when
> 	           HOST/GUEST_UDP_TUNNEL_GSO is set ...
> 	if (test_bit(VHOST_NET_F_VIRTIO_NET_HDR))
> 		vhost_hlen = hdr_len;
> 
> TUNSETOFFLOAD only updates tun->set_features and the tap device's
> wanted_features; it does not feed back into the vhost feature bits or
> into vhost_hlen.  handle_rx() then writes vhost_hlen bytes of the
> virtio-net header into the guest buffer while handle_tx() / the GSO
> path never touches those 12 bytes for a header vhost itself built.
> 
> I could not find TUN_VNET_TNL_SIZE anywhere in 7.3.0-rc7 or in

Apologies. TUN_VNET_TNL_SIZE is an internal macro that header size
is set it. TUNSETVNETHDRSZ is what I meant.

> linux-next (drivers/net/tun.c, drivers/vhost/net.c, include/uapi/linux/
> if_tun.h, include/uapi/linux/vhost.h) — is that an interface from a
> tree/series I'm missing?  If it is meant to gate vhost_hlen I'd be
> happy to re-test with it.
> 
> Meanwhile the stale 12 bytes are observable with a plain tun backend
> that has declared full tunnel offload support, so I believe the patch
> (Zero-initialize the largest header layout and copy vhost_hlen bytes)
> is still needed.  Happy to re-test under whatever backend setup you
> had in mind.
> 
> Best,
> Guikun Yang


I have in mind simply disabling this combo: vhost + !IFF_VNET_HDR)

I'll research a bit whether anything anywhere could reasonably
use it.

Can you reproduce any problem *with* IFF_VNET_HDR?

-- 
MST


  reply	other threads:[~2026-10-24  6:45 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-23 11:37 [PATCH] vhost/net: initialize the whole negotiated RX vnet header Guikun Yang
2026-10-23 11:44 ` sashiko-bot
2026-10-23 12:13 ` Michael S. Tsirkin
2026-10-24  2:11   ` Guikun Yang
2026-10-24  6:45     ` Michael S. Tsirkin [this message]
2026-10-24  7:08   ` Guikun Yang
     [not found] ` <20261024101101.71420-vhost-yangguikun19@qq.com>
     [not found]   ` <20261024150815.36216-vhost-yangguikun19@qq.com>
2026-10-24  7:33     ` Guikun Yang

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=20261024024028-mutt-send-email-mst@kernel.org \
    --to=mst@redhat.com \
    --cc=eperezma@redhat.com \
    --cc=jasowang@redhat.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=virtualization@lists.linux.dev \
    --cc=yangguikun19@qq.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.