* Re: [PATCH v3] hwrng: virtio: clamp device-reported used.len at copy_data()
From: Michael Bommarito @ 2026-06-14 16:14 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Herbert Xu, Olivia Mackall, linux-crypto, Jason Wang, Kees Cook,
Christian Borntraeger, virtualization, linux-kernel, Dan Williams,
Ingo Molnar, H. Peter Anvin, torvalds, alan, tglx
In-Reply-To: <20260611064040-mutt-send-email-mst@kernel.org>
On Thu, Jun 11, 2026 at 6:43 AM Michael S. Tsirkin <mst@redhat.com> wrote:
> AKA defence is depth programming)
> Alright we can drop this. No biggie.
Sorry for the delay. I'll ship a v4 without the nospec
Thanks,
Mike
^ permalink raw reply
* Re: [PATCH net-next 1/3] net: busy-poll: introduce sk_tx_busy_loop()
From: Menglong Dong @ 2026-06-14 10:12 UTC (permalink / raw)
To: menglong8.dong, Jakub Kicinski
Cc: jasowang, mst, xuanzhuo, eperezma, andrew+netdev, davem, edumazet,
pabeni, magnus.karlsson, maciej.fijalkowski, sdf, horms, ast,
daniel, hawk, john.fastabend, bjorn, kerneljasonxing, netdev,
virtualization, linux-kernel, bpf
In-Reply-To: <20260613112113.55d9313f@kernel.org>
On 2026/6/14 02:21, Jakub Kicinski wrote:
> On Thu, 11 Jun 2026 15:12:40 +0800 menglong8.dong@gmail.com wrote:
> > For now, we use sk_busy_loop() for both rx and tx path. The sk_busy_loop()
> > will call napi_busy_loop() for the specified napi_id. However, some
> > nic drivers have tx napi, such as virtio-net. In this case, sk_busy_loop()
> > doesn't work, as it can only schedule the NAPI for the rx queue.
> >
> > Therefore, introduce sk_tx_busy_loop() for the nic drivers that support tx
> > napi, which will schedule the tx napi if available.
>
> First, I thought the only difference with Tx NAPI is that it can't be
> busy polled. So if you want to poll an instance don't register it as
> a Tx one instead of adding all this "tx polling" stuff in the core?
I see. Register the tx NAPI with netif_napi_add_config() allow us
busy poll it. But we still have two NAPI instance: rx NAPI and tx NAPI.
sk_busy_loop() can only busy poll on one of them.
Before AF_XDP, we don't have the need to send packet via tx NAPI, which
means that we don't need to busy poll it.
I analyst some nic drivers on the implement of AF_XDP. Some of them
will check xsk tx ring of current queue and send the data in it in the
rx NAPI, such as mlx5. Some of them will allocate a extra "rxtx" NAPI
for the AF_XDP zero-copy queue, which will poll both the data receiving
and sending.
In the case about, they will do the data sending and receiving for the
AF_XDP in a single NAPI instance.
However, some driver receiving the data in rx NAPI and send data in
tx NAPI for AF_XDP. In this case, we can't use sk_busy_loop() for both
rx path and tx path, as we need to wake different NAPI instance.
>
> Second, can this problem happen for any other NIC or is it purely
> an artifact of virtio's delayed Tx completion handling?
According to my analysis, only virtio-net and ICSSG driver have
split NAPI for AF_XDP. I don't have a ICSSG nic, but the codex tell
me that it does have the same problem.
I'm not sure if it is a good idea to introduce the sk_tx_busy_loop().
Maybe we can modify the driver instead by using the same NAPI
for both data sending and receiving, just like others do. The
advantage of introduce sk_tx_busy_loop() is that we can split the
data sending and receiving, which maybe more efficient.
>
> Third, this series does not apply.
Ah, I'll rebase this series if a V2 is acceptable.
Thanks!
Menglong Dong
>
>
^ permalink raw reply
* Re: [PATCH net-next] virtio-net: support xsk wake up
From: Menglong Dong @ 2026-06-14 2:21 UTC (permalink / raw)
To: Menglong Dong, Jakub Kicinski
Cc: xuanzhuo, mst, jasowang, eperezma, andrew+netdev, davem, edumazet,
pabeni, netdev, virtualization, linux-kernel
In-Reply-To: <20260613144438.767f8069@kernel.org>
On 2026/6/14 05:44, Jakub Kicinski wrote:
> On Wed, 10 Jun 2026 16:16:48 +0800 Menglong Dong wrote:
> > + /* If both rq->vq and fill ring are empty, and then the user submit
> > + * all the chunks to the fill ring and check the wake up flag
> > + * after xsk_buff_alloc_batch() and before xsk_set_rx_need_wakeup(),
> > + * we will lose the chance to wake up the rx napi, so we have to
> > + * set the need_wakeup flag here.
> > + */
>
> TBH all the comments you're adding are harder to understand than the
> code itself ;( Please try to phrase them better or just remove them.
Ah, sorry about that. The race condition here is a little hard to describe
for me. After the discussion in the V2:
https://lore.kernel.org/netdev/rHZz5_ylT4WggoZ-Ic2Q4w@linux.dev/
the race condition seems not likely to happen. So I'll remove this
part in V3.
Thanks!
Menglong Dong
>
>
^ permalink raw reply
* [PATCH] drm/vblank: Don't arm vblank timer with invalid frame duration
From: Roman Ilin @ 2026-06-13 22:44 UTC (permalink / raw)
To: Thomas Zimmermann, Maarten Lankhorst, Maxime Ripard, David Airlie,
Simona Vetter
Cc: Louis Chauvet, Javier Martinez Canillas, Dmitry Osipenko,
dri-devel, virtualization, linux-kernel, Roman Ilin
When a CRTC's display mode carries a too small pixel clock,
drm_calc_timestamping_constants() computes a frame duration that
exceeds INT_MAX. drm_vblank_crtc.framedur_ns becomes negative.
drm_crtc_vblank_start_timer() then arms the vblank hrtimer with this
interval, after which vblank events are no longer delivered. Pending
page flips never complete and the display appears frozen.
This could be triggered on virtio-gpu guests that have dynamic resolution
enabled: when the SPICE agent or the X server resizes the output, it
submits a mode whose pixel clock is off by a factor of 1000, e.g.:
clock = 406 kHz, htotal = 3152, vtotal = 2148
framedur_ns = 3152 * 2148 * 1000000 / 406 = 16675852216 ns (~16.7 s)
16675852216 does not fit into an int and wraps to roughly -504000000.
ns_to_ktime() then yields a negative interval and the timer stops working.
Found by bisection, which pointed at commit a036f5fceedb ("drm/virtgpu:
Use vblank timer"). That commit merely made virtio-gpu use the vblank
timer and thereby exposed the pre-existing problem in the timer setup
added by commit 74afeb812850 ("drm/vblank: Add vblank timer").
Reject a non-positive frame duration in drm_crtc_vblank_start_timer() and
return an error. enable_vblank then fails and the driver falls back to
sending the vblank event immediately, as it did before the vblank timer
was introduced. Valid modes are unaffected, and the timer self-heals on
the next mode that has a sane clock.
Fixes: 74afeb812850 ("drm/vblank: Add vblank timer")
Cc: stable@vger.kernel.org
Signed-off-by: Roman Ilin <me@romanilin.is>
---
Notes:
Based on v7.1-rc7. Tested on 6.19 and 7.1-rc7.
Open questions:
This relies on the int overflow producing a negative value. The deeper
issue is that drm_calc_timestamping_constants() truncates framedur_ns to
int. Would you prefer to widen framedur_ns to s64, or to bound the
interval here (e.g. reject framedur_ns above one second) so that any
bogus interval is rejected regardless of sign?
Should virtio-gpu additionally sanitize the user-supplied clock in its
atomic_check (similar to vmwgfx for the clock==0 case) so the
vblank-timer throttling is preserved for these resizes, instead of
falling back to immediate events?
drivers/gpu/drm/drm_vblank.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
index f78bf37f1..557cd0bc8 100644
--- a/drivers/gpu/drm/drm_vblank.c
+++ b/drivers/gpu/drm/drm_vblank.c
@@ -2235,7 +2235,13 @@ int drm_crtc_vblank_start_timer(struct drm_crtc *crtc)
drm_calc_timestamping_constants(crtc, &crtc->mode);
+ /*
+ * Return an error so the driver falls back to sending vblank events
+ * when a small mode clock yields a frame duration exceeding INT_MAX.
+ */
+ if (vblank->framedur_ns <= 0)
+ return -EINVAL;
+
spin_lock_irqsave(&vtimer->interval_lock, flags);
vtimer->interval = ns_to_ktime(vblank->framedur_ns);
spin_unlock_irqrestore(&vtimer->interval_lock, flags);
--
2.54.0
^ permalink raw reply related
* Re: [PATCH net-next] virtio-net: support xsk wake up
From: Jakub Kicinski @ 2026-06-13 21:46 UTC (permalink / raw)
To: Eugenio Perez Martin
Cc: Menglong Dong, xuanzhuo, mst, jasowang, andrew+netdev, davem,
edumazet, pabeni, netdev, virtualization, linux-kernel
In-Reply-To: <CAJaqyWeYiruNosJsMTh2jQ=XCEcPg7956aqeRRpDSyynfpjNZA@mail.gmail.com>
On Wed, 10 Jun 2026 10:27:28 +0200 Eugenio Perez Martin wrote:
> And the From and Signed-off-by emails don't match, which I'm not sure is valid.
It's clearly the same person. Please focus on the code, not trivial
process issues.
Quoting documentation:
Reviewer guidance
-----------------
[...]
Reviewers are highly encouraged to do more in-depth review of submissions
and not focus exclusively on process issues, trivial or subjective
matters like code formatting, tags etc.
See: https://www.kernel.org/doc/html/next/process/maintainer-netdev.html#reviewer-guidance
^ permalink raw reply
* Re: [PATCH net-next] virtio-net: support xsk wake up
From: Jakub Kicinski @ 2026-06-13 21:44 UTC (permalink / raw)
To: Menglong Dong
Cc: xuanzhuo, mst, jasowang, eperezma, andrew+netdev, davem, edumazet,
pabeni, netdev, virtualization, linux-kernel
In-Reply-To: <20260610081648.2205711-1-dongml2@chinatelecom.cn>
On Wed, 10 Jun 2026 16:16:48 +0800 Menglong Dong wrote:
> + /* If both rq->vq and fill ring are empty, and then the user submit
> + * all the chunks to the fill ring and check the wake up flag
> + * after xsk_buff_alloc_batch() and before xsk_set_rx_need_wakeup(),
> + * we will lose the chance to wake up the rx napi, so we have to
> + * set the need_wakeup flag here.
> + */
TBH all the comments you're adding are harder to understand than the
code itself ;( Please try to phrase them better or just remove them.
^ permalink raw reply
* Re: [PATCH net v3] virtio-net: fix len check in receive_big()
From: Xiang Mei @ 2026-06-13 20:15 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: jasowang, xuanzhuo, eperezma, andrew+netdev, davem, edumazet,
kuba, pabeni, netdev, virtualization, linux-kernel,
minhquangbui99, bestswngs
In-Reply-To: <20260611014519-mutt-send-email-mst@kernel.org>
On Wed, Jun 10, 2026 at 10:56 PM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Wed, Jun 10, 2026 at 07:46:16PM -0700, Xiang Mei wrote:
> > receive_big() bounds the device-announced length by
> > (big_packets_num_skbfrags + 1) * PAGE_SIZE. That is still too loose:
> > add_recvbuf_big() sets sg[1] to start at offset
> > sizeof(struct padded_vnet_hdr) into the first page, so the chain
> > actually carries hdr_len + (PAGE_SIZE - sizeof(padded_vnet_hdr)) +
> > big_packets_num_skbfrags * PAGE_SIZE bytes -- 20 bytes less than the
> > check allows for the common hdr_len == 12 case.
> >
> > A malicious virtio backend can announce a len in that gap. page_to_skb()
> > then walks one frag past the page chain, storing a NULL page->private
> > into skb_shinfo()->frags[MAX_SKB_FRAGS], which is both an out-of-bounds
> > write past the static frag array and a NULL frag handed up the rx path.
> >
> > Bound len by the size add_recvbuf_big() actually advertised.
> >
> > Fixes: 0c716703965f ("virtio-net: fix received length check in big packets")
> > Reported-by: Weiming Shi <bestswngs@gmail.com>
> > Signed-off-by: Xiang Mei <xmei5@asu.edu>
> > Reviewed-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
>
> Thanks for the patch! Something small to improve:
>
> > ---
> > v3: revoke 2/2 and add Xuan Zhuo's Reviewed-by tag
> >
> > drivers/net/virtio_net.c | 8 +++++---
> > 1 file changed, 5 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > index f4adcfee7a80..afe73eda1491 100644
> > --- a/drivers/net/virtio_net.c
> > +++ b/drivers/net/virtio_net.c
> > @@ -1999,15 +1999,17 @@ static struct sk_buff *receive_big(struct net_device *dev,
> > struct virtnet_rq_stats *stats)
> > {
> > struct page *page = buf;
> > + unsigned long max_len;
>
> Assignment can happen here?
>
> > struct sk_buff *skb;
> >
> > /* Make sure that len does not exceed the size allocated in
> > * add_recvbuf_big.
> > */
> > - if (unlikely(len > (vi->big_packets_num_skbfrags + 1) * PAGE_SIZE)) {
> > + max_len = vi->hdr_len + (PAGE_SIZE - sizeof(struct padded_vnet_hdr)) +
> > + vi->big_packets_num_skbfrags * PAGE_SIZE;
>
> Took me a while to figure out what is going on, but I finally
> understand:
>
>
> Reducing
> (vi->big_packets_num_skbfrags + 1) * PAGE_SIZE
>
> (what we allocated)
>
> by sizeof(struct padded_vnet_hdr) - vi->hdr_len
>
>
> right?
>
> So clearer as:
>
>
> unsigned long max_len = (vi->big_packets_num_skbfrags + 1) * PAGE_SIZE -
> sizeof(struct padded_vnet_hdr) + vi->hdr_len;
>
Right, that's the same value. Yours reads better!
I'll fold this into the next respin. One thing I'd like to settle
first: David suggested storing this in a vi field computed once at the
probe (it's a per-device constant) and just comparing len against it
on the datapath, instead of re-deriving it in receive_big() each time.
I'll wait for his take on that and send a single v4 that covers both.
Xiang
>
>
>
> > + if (unlikely(len > max_len)) {
> > pr_debug("%s: rx error: len %u exceeds allocated size %lu\n",
> > - dev->name, len,
> > - (vi->big_packets_num_skbfrags + 1) * PAGE_SIZE);
> > + dev->name, len, max_len);
> > goto err;
> > }
> >
> > --
> > 2.43.0
>
^ permalink raw reply
* Re: [PATCH net] virtio-net: fix len check in receive_big()
From: Xiang Mei @ 2026-06-13 19:58 UTC (permalink / raw)
To: David Laight
Cc: mst, jasowang, xuanzhuo, eperezma, andrew+netdev, davem, edumazet,
kuba, pabeni, netdev, virtualization, linux-kernel,
minhquangbui99, bestswngs
In-Reply-To: <20260611104836.469610b0@pumpkin>
On Thu, Jun 11, 2026 at 2:48 AM David Laight
<david.laight.linux@gmail.com> wrote:
>
> On Wed, 10 Jun 2026 15:16:06 -0700
> Xiang Mei <xmei5@asu.edu> wrote:
>
> > receive_big() bounds the device-announced length by
> > (big_packets_num_skbfrags + 1) * PAGE_SIZE. That is still too loose:
> > add_recvbuf_big() sets sg[1] to start at offset
> > sizeof(struct padded_vnet_hdr) into the first page, so the chain
> > actually carries hdr_len + (PAGE_SIZE - sizeof(padded_vnet_hdr)) +
> > big_packets_num_skbfrags * PAGE_SIZE bytes -- 20 bytes less than the
> > check allows for the common hdr_len == 12 case.
> >
> > A malicious virtio backend can announce a len in that gap. page_to_skb()
> > then walks one frag past the page chain, storing a NULL page->private
> > into skb_shinfo()->frags[MAX_SKB_FRAGS], which is both an out-of-bounds
> > write past the static frag array and a NULL frag handed up the rx path.
> >
> > Bound len by the size add_recvbuf_big() actually advertised.
> >
> > Fixes: 0c716703965f ("virtio-net: fix received length check in big packets")
> > Reported-by: Weiming Shi <bestswngs@gmail.com>
> > Signed-off-by: Xiang Mei <xmei5@asu.edu>
> > ---
> > drivers/net/virtio_net.c | 8 +++++---
> > 1 file changed, 5 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > index f4adcfee7a80..afe73eda1491 100644
> > --- a/drivers/net/virtio_net.c
> > +++ b/drivers/net/virtio_net.c
> > @@ -1999,15 +1999,17 @@ static struct sk_buff *receive_big(struct net_device *dev,
> > struct virtnet_rq_stats *stats)
> > {
> > struct page *page = buf;
> > + unsigned long max_len;
> > struct sk_buff *skb;
> >
> > /* Make sure that len does not exceed the size allocated in
> > * add_recvbuf_big.
> > */
> > - if (unlikely(len > (vi->big_packets_num_skbfrags + 1) * PAGE_SIZE)) {
> > + max_len = vi->hdr_len + (PAGE_SIZE - sizeof(struct padded_vnet_hdr)) +
> > + vi->big_packets_num_skbfrags * PAGE_SIZE;
>
> That looks like a constant (for the vi).
Thanks, agreed it's a per-device constant, so the expression never
changes after setup.
> Probably worth saving rather than recalculating all the time.
>
I thought about caching it in a vi->big_packets_max_len field computed
once after virtnet_set_big_packets(). My only concern is that the
saving is marginal here: receive_big() is only reached when
mergeable_rx_bufs is off (the "else if (vi->big_packets)" branch). Any
backend that negotiates VIRTIO_NET_F_MRG_RXBUF takes the
receive_mergeable() path for all RX, so receive_big() is never hit
there at all. And the bound has exactly one consumer.
So it's trading a 4-byte field (probably free in existing padding)
against one cache-hit load plus a shift, and two adds, on a path
that's effectively cold on modern setups. I mainly work on security
issues, so I would like to listen to your idea.
Xiang
> -- David
>
> > + if (unlikely(len > max_len)) {
> > pr_debug("%s: rx error: len %u exceeds allocated size %lu\n",
> > - dev->name, len,
> > - (vi->big_packets_num_skbfrags + 1) * PAGE_SIZE);
> > + dev->name, len, max_len);
> > goto err;
> > }
> >
>
^ permalink raw reply
* Re: [PATCH net-next 1/3] net: busy-poll: introduce sk_tx_busy_loop()
From: Jakub Kicinski @ 2026-06-13 18:21 UTC (permalink / raw)
To: menglong8.dong
Cc: jasowang, mst, xuanzhuo, eperezma, andrew+netdev, davem, edumazet,
pabeni, magnus.karlsson, maciej.fijalkowski, sdf, horms, ast,
daniel, hawk, john.fastabend, bjorn, kerneljasonxing, netdev,
virtualization, linux-kernel, bpf
In-Reply-To: <20260611071242.2485058-2-dongml2@chinatelecom.cn>
On Thu, 11 Jun 2026 15:12:40 +0800 menglong8.dong@gmail.com wrote:
> For now, we use sk_busy_loop() for both rx and tx path. The sk_busy_loop()
> will call napi_busy_loop() for the specified napi_id. However, some
> nic drivers have tx napi, such as virtio-net. In this case, sk_busy_loop()
> doesn't work, as it can only schedule the NAPI for the rx queue.
>
> Therefore, introduce sk_tx_busy_loop() for the nic drivers that support tx
> napi, which will schedule the tx napi if available.
First, I thought the only difference with Tx NAPI is that it can't be
busy polled. So if you want to poll an instance don't register it as
a Tx one instead of adding all this "tx polling" stuff in the core?
Second, can this problem happen for any other NIC or is it purely
an artifact of virtio's delayed Tx completion handling?
Third, this series does not apply.
^ permalink raw reply
* Re: [PATCH net-next v3 0/4] vsock: consolidate acceptq accounting into core helpers
From: patchwork-bot+netdevbpf @ 2026-06-13 17:50 UTC (permalink / raw)
To: Raf Dickson
Cc: netdev, virtualization, pabeni, sgarzare, stefanha, bryan-bt.tan,
vishnu.dasa, bcm-kernel-feedback-list, bobbyeshleman, leonardi,
horms, edumazet, kuba
In-Reply-To: <20260612045216.105796-1-rafdog35@gmail.com>
Hello:
This series was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:
On Fri, 12 Jun 2026 04:52:12 +0000 you wrote:
> These patches follow up on commit c05fa14db43e
> ("vsock/vmci: fix sk_ack_backlog leak on failed handshake")
> by consolidating sk_acceptq_added() and sk_acceptq_removed() into
> the core vsock helpers so transports cannot forget them.
>
> Changes since v2:
> - Add vsock_pending_to_accept() helper for the vmci pending->accept
> transition, avoiding a double sk_acceptq_added() (Stefano Garzarella)
> - Split into 4 patches for bisectability (Stefano Garzarella)
> - Fold sk_acceptq_added() into vsock_add_pending() as a separate patch
>
> [...]
Here is the summary with links:
- [net-next,v3,1/4] vsock: introduce vsock_pending_to_accept() helper
https://git.kernel.org/netdev/net-next/c/77eee189397d
- [net-next,v3,2/4] vsock: fold sk_acceptq_added() into vsock_add_pending()
https://git.kernel.org/netdev/net-next/c/a6fd2cfdcdf5
- [net-next,v3,3/4] vsock: fold sk_acceptq_added() into vsock_enqueue_accept()
https://git.kernel.org/netdev/net-next/c/6f6f9b65a991
- [net-next,v3,4/4] vsock: fold sk_acceptq_removed() into vsock_remove_pending()
https://git.kernel.org/netdev/net-next/c/27fc25bb82e6
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply
* Re: [PATCH net-next v3 4/4] vsock: fold sk_acceptq_removed() into vsock_remove_pending()
From: Jakub Kicinski @ 2026-06-13 17:40 UTC (permalink / raw)
To: Stefano Garzarella
Cc: Raf Dickson, netdev, virtualization, pabeni, stefanha,
bryan-bt.tan, vishnu.dasa, bcm-kernel-feedback-list,
bobbyeshleman, leonardi, horms, edumazet
In-Reply-To: <aivjf4TZU4Q_s20y@sgarzare-redhat>
On Fri, 12 Jun 2026 12:48:14 +0200 Stefano Garzarella wrote:
> >@@ -773,7 +774,6 @@ static void vsock_pending_work(struct work_struct *work)
> > if (vsock_is_pending(sk)) {
> > vsock_remove_pending(listener, sk);
> >
> ^^
> There is an extra blank line that we can now remove here.
>
> BTW, the code LGTM:
Since the merge window is upon us - also updated when applying.
^ permalink raw reply
* Re: [PATCH net-next v2] vsock/vmci: use sk_acceptq_is_full() helper
From: patchwork-bot+netdevbpf @ 2026-06-13 17:40 UTC (permalink / raw)
To: Raf Dickson
Cc: netdev, virtualization, pabeni, sgarzare, stefanha, bryan-bt.tan,
vishnu.dasa, bcm-kernel-feedback-list, leonardi, horms, edumazet,
kuba
In-Reply-To: <20260612045842.122207-1-rafdog35@gmail.com>
Hello:
This patch was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:
On Fri, 12 Jun 2026 04:58:42 +0000 you wrote:
> Replace the open-coded backlog check with sk_acceptq_is_full().
> The helper uses > instead of >=, which is the correct comparison
> per commit 64a146513f8f ("[NET]: Revert incorrect accept queue
> backlog changes."), and adds READ_ONCE() for proper memory ordering.
>
> Suggested-by: Stefano Garzarella <sgarzare@redhat.com>
> Signed-off-by: Raf Dickson <rafdog35@gmail.com>
>
> [...]
Here is the summary with links:
- [net-next,v2] vsock/vmci: use sk_acceptq_is_full() helper
https://git.kernel.org/netdev/net-next/c/4ff2e84ff1b3
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply
* Re: [PATCH net-next v2] vsock/vmci: use sk_acceptq_is_full() helper
From: Jakub Kicinski @ 2026-06-13 17:37 UTC (permalink / raw)
To: Stefano Garzarella
Cc: Raf Dickson, netdev, virtualization, pabeni, stefanha,
bryan-bt.tan, vishnu.dasa, bcm-kernel-feedback-list, leonardi,
horms, edumazet
In-Reply-To: <aivKma8mRjTXV0BM@sgarzare-redhat>
On Fri, 12 Jun 2026 11:03:24 +0200 Stefano Garzarella wrote:
> nit: title should be updated since now this is not just vmci
> (e.g. vsock: use sk_acceptq_is_full() helper in all transports)
>
> Not sure if it can be fixed while applying by netdev maintainers.
Updated and applied, thanks!
^ permalink raw reply
* Re: [PATCH] vduse: Fix error around jumping over a __cleanup() variable
From: Nathan Chancellor @ 2026-06-13 16:26 UTC (permalink / raw)
To: David Laight
Cc: Michael S. Tsirkin, Jason Wang, Xuan Zhuo, Eugenio Pérez,
virtualization, linux-kernel, llvm
In-Reply-To: <20260611110346.2b9388a1@pumpkin>
On Thu, Jun 11, 2026 at 11:03:46AM +0100, David Laight wrote:
> On Wed, 10 Jun 2026 12:16:49 -0700
> Nathan Chancellor <nathan@kernel.org> wrote:
>
> > When building with clang, there is an error in vduse_vq_kick() from
> > attempting to jump over a variable declared with the cleanup attribute
> > using goto:
> .
> > Jumping over a variable declared with the cleanup attribute does not
> > prevent the cleanup function from running, it would just result in the
> > variable being passed uninitialized to the cleanup function .clang
> > errors instead of generating the invalid code, unlike GCC.
>
> Does the same apply to variables allocated inside switch statements?
> I'm sure I've seen one that wasn't inside an extra block.
Yes:
https://lore.kernel.org/20251002233627.GA3978676@ax162/
--
Cheers,
Nathan
^ permalink raw reply
* Re: [PATCH net-next v2 1/2] virtio_net: xsk: fix race in rx wake up
From: Menglong Dong @ 2026-06-13 12:26 UTC (permalink / raw)
To: menglong8.dong, xuanzhuo, eperezma, Bui Quang Minh
Cc: mst, jasowang, andrew+netdev, davem, edumazet, kuba, pabeni,
kerneljasonxing, netdev, virtualization, linux-kernel
In-Reply-To: <41eefa1d-99bf-450d-988e-7dec67c6b61e@gmail.com>
On 2026/6/12 00:24, Bui Quang Minh wrote:
> On 6/11/26 09:56, menglong8.dong@gmail.com wrote:
> > From: Menglong Dong <dongml2@chinatelecom.cn>
> >
> > During packet receiving in virtio-net, the rq can be empty, which means
> > "rq->vq->num_free == virtqueue_get_vring_size(rq->vq)", in
> > virtnet_add_recvbuf_xsk(), if we are using xsk. Meanwhile, the fill ring
> > can be empty too, which means we can't allocate anything from
> > xsk_buff_alloc_batch(). Then, we will set the XDP_RING_NEED_WAKEUP flag.
> >
> > However, if the user clean all the data in rx ring and fill the
> > "fill ring" and check the XDP_RING_NEED_WAKEUP flag after
> > xsk_buff_alloc_batch() and before xsk_set_rx_need_wakeup(), then the rx
> > napi will never be scheduled: the rx ring is empty, which means we will
> > never receive a packet to trigger the further recv fill. The rx ring is
> > empty now, so the user will not check the flag too.
> >
> > Fix this by set the XDP_RING_NEED_WAKEUP flag before
> > xsk_buff_alloc_batch() if both rq->vq and fill ring are empty.
> >
> > Meanwhile, set the XDP_RING_NEED_WAKEUP flag if we have any free entry in
> > rq->vq.
> >
> > Fixes: e3f8800aa243 ("virtio-net: xsk: Support wakeup on RX side")
> > Signed-off-by: Menglong Dong <dongml2@chinatelecom.cn>
> > ---
> > drivers/net/virtio_net.c | 25 ++++++++++++++++++++++---
> > 1 file changed, 22 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > index f4adcfee7a80..4b5b3fa62008 100644
> > --- a/drivers/net/virtio_net.c
> > +++ b/drivers/net/virtio_net.c
> > @@ -1323,16 +1323,27 @@ static int virtnet_add_recvbuf_xsk(struct virtnet_info *vi, struct receive_queue
> > struct xsk_buff_pool *pool, gfp_t gfp)
> > {
> > struct xdp_buff **xsk_buffs;
> > + bool need_wakeup;
> > dma_addr_t addr;
> > int err = 0;
> > u32 len, i;
> > int num;
> >
> > + need_wakeup = xsk_uses_need_wakeup(pool);
> > xsk_buffs = rq->xsk_buffs;
> >
> > + /* If both rq->vq and fill ring are empty, and then the user submit
> > + * all the chunks to the fill ring and check the wake up flag
> > + * after xsk_buff_alloc_batch() and before xsk_set_rx_need_wakeup(),
> > + * we will lose the chance to wake up the rx napi, so we have to
> > + * set the need_wakeup flag here.
> > + */
> > + if (need_wakeup && virtqueue_get_vring_size(rq->vq) == rq->vq->num_free)
> > + xsk_set_rx_need_wakeup(pool);
>
Hi, Bui Quang. Thanks for your reply. I spent some time learning
what you said.
> I think when polling the receive queue, the userspace program needs to
> check the XDP_RING_NEED_WAKEUP flag if it does not see any packets. The
> flag check is quite lightweight in my opinion. Here are some examples I find
>
> -
> https://github.com/xdp-project/xdp-tools/blob/e9469501622aa22a7e452a671000bec8685edcde/lib/util/xdpsock.c#L1206
You are right, I'm over concerned about this point. My origin
concern is that we can't wake up from the poll syscall in this case:
The chunk of the umem is 2000. In the beginning, the xsk->fill_ring
is filled with 2000 chunk, and then the user fall asleep and don't
do anything.
Kernel: the 2000th packet is received
Kernel: xsk_buff_alloc_batch return 0(xsk->fill_ring is empty and xsk->rx_ring is full)
User: handle the xsk->rx_ring
User: fill the xsk->fill_ring with 2000 chunks
User: check the wake up flag
User: no need_wakeup flag, fall asleep with poll() syscall
Kernel: call xsk_set_rx_need_wakeup()
Kernel: virio-net rx ringbuf is empty, we can't receive any packet further
Kernel: to call virtnet_add_recvbuf_xsk(), we are dead
But then, I found that we can still be wake up with the 2000th
packet from the poll syscall, which means that the case that
the NAPI and the user can't both be waked up doesn't exist.
> -
> https://github.com/xdp-project/bpf-examples/blob/43e565901c4287efa863edca7f0e6cd6e35ed896/AF_XDP-forwarding/xsk_fwd.c#L540
>
> Furthermore, the XDP_RING_NEED_WAKEUP flag related functions does not
> provide any memory orderings. So even with your patch, I'm worried that
> this case is possible
>
> kernel userspace
>
> xsk_buff_alloc_batch -> failed
> submit fill
> ring
> flag !=
> XDP_RING_NEED_WAKEUP
> // reordering due to lack of memory orderings
> xsk_set_rx_need_wakeup
>
> I'm not expert here, so correct me if I'm wrong. I think the wake up
> flag is designed with no orderings so we cannot rely on it to reason and
> skip further checks.
>
> > +
> > num = xsk_buff_alloc_batch(pool, xsk_buffs, rq->vq->num_free);
[....]
> > +
>
> Why do we need to set XDP_RING_NEED_WAKEUP even when
> xsk_buff_alloc_batch succeeds?
Ah, don't mind here. I just thought that if xsk_buff_alloc_batch()
didn't allocate enough chunks as we need, we can wake up
the NAPI as soon as possible, in case that the virtio-net ringbuf
is full and cause packet dropping :)
Anyway, I'll remove the first patch, and send the second patch
only in the V3.
Thanks!
Menglong Dong
>
> > return num;
> >
> > err:
>
> Thanks,
> Quang Minh.
>
>
>
>
^ permalink raw reply
* [PATCH net v2 2/2] vsock/virtio: restore msg_iter on transmission failure
From: Octavian Purdila @ 2026-06-13 0:09 UTC (permalink / raw)
To: netdev
Cc: Alexander Viro, Andrew Morton, Arseniy Krasnov, David S. Miller,
Eric Dumazet, Eugenio Pérez, Jakub Kicinski, Jason Wang, kvm,
linux-block, linux-fsdevel, linux-kernel, Michael S. Tsirkin,
Paolo Abeni, Simon Horman, Stefan Hajnoczi, Stefano Garzarella,
virtualization, Xuan Zhuo, Octavian Purdila,
syzbot+28e5f3d207b14bae122a
In-Reply-To: <20260613000953.467473-1-tavip@google.com>
When transmission fails in virtio_transport_send_pkt_info, the msg_iter
might have been partially advanced. If we don't restore it, the next
attempt to send data will use an incorrect iterator state, leading to
desync and warnings like "send_pkt() returns 0, but X expected".
Specifically, this can happen in the following scenario, triggered by
the syzkaller repro:
1. A write-only VMA (PROT_WRITE only) is partially populated by a
prior TUN write that failed with -EIO but still faulted in some
pages).
2. A vsock sendmmsg call with MSG_ZEROCOPY requests transmission of a
buffer from this VMA.
3. The first packet (64KB) is sent successfully because the pages are
populated.
4. The second packet allocation fails because GUP fast pins the first page
but GUP slow fails on the next unpopulated page due to PROT_WRITE-only
permissions.
5. The iterator is advanced by the partially successful GUP (68KB total
advanced: 64KB from first packet + 4KB from second), but the send loop
breaks and only reports 64KB sent. This creates a 4KB desync.
6. The next retry starts with a non-zero iov_offset, disabling zerocopy
and falling back to copy mode.
7. In copy mode, the transmission succeeds for the next packets but
exhausts the iterator early because of the desync.
8. The final retry sees an empty iterator but zerocopy is re-enabled
(offset resets). It attempts to send the remaining bytes with zerocopy
but pins 0 pages, creating an empty packet.
9. The transport sends the empty packet, triggering the warning because
the returned bytes (header only) do not match the expected payload size.
10. The loop continues to spin, allocating ubuf_info each time, eventually
exhausting sysctl_optmem_max and returning -ENOMEM to userspace.
Restore msg_iter to its original state before the packet allocation
and transmission attempt if they fail.
Fixes: e0718bd82e27 ("vsock: enable setting SO_ZEROCOPY")
Reported-by: syzbot+28e5f3d207b14bae122a@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=28e5f3d207b14bae122a
Assisted-by: gemini:gemini-3.1-pro
Signed-off-by: Octavian Purdila <tavip@google.com>
---
net/vmw_vsock/virtio_transport_common.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
index b10666937c490..2baa5a6ebd750 100644
--- a/net/vmw_vsock/virtio_transport_common.c
+++ b/net/vmw_vsock/virtio_transport_common.c
@@ -295,6 +295,7 @@ static int virtio_transport_send_pkt_info(struct vsock_sock *vsk,
u32 max_skb_len = VIRTIO_VSOCK_MAX_PKT_BUF_SIZE;
u32 src_cid, src_port, dst_cid, dst_port;
const struct virtio_transport *t_ops;
+ struct iov_iter_state msg_iter_state;
struct virtio_vsock_sock *vvs;
struct ubuf_info *uarg = NULL;
u32 pkt_len = info->pkt_len;
@@ -368,8 +369,17 @@ static int virtio_transport_send_pkt_info(struct vsock_sock *vsk,
struct sk_buff *skb;
size_t skb_len;
+ /* Save iterator state in case allocation or transmission fails
+ * so we can restore it and retry.
+ */
+ if (info->msg)
+ iov_iter_save_state(&info->msg->msg_iter, &msg_iter_state);
+
skb_len = min(max_skb_len, rest_len);
+ /* Note: virtio_transport_alloc_skb() can advance info->msg->msg_iter
+ * even if it fails (e.g. partial GUP success).
+ */
skb = virtio_transport_alloc_skb(info, skb_len, can_zcopy,
uarg,
src_cid, src_port,
@@ -399,6 +409,9 @@ static int virtio_transport_send_pkt_info(struct vsock_sock *vsk,
break;
} while (rest_len);
+ if (info->msg && ret < 0)
+ iov_iter_restore(&info->msg->msg_iter, &msg_iter_state);
+
virtio_transport_put_credit(vvs, rest_len);
/* msg_zerocopy_realloc() initializes the ubuf_info refcnt to 1.
--
2.54.0.1136.gdb2ca164c4-goog
^ permalink raw reply related
* [PATCH net v2 1/2] iov_iter: export iov_iter_restore
From: Octavian Purdila @ 2026-06-13 0:09 UTC (permalink / raw)
To: netdev
Cc: Alexander Viro, Andrew Morton, Arseniy Krasnov, David S. Miller,
Eric Dumazet, Eugenio Pérez, Jakub Kicinski, Jason Wang, kvm,
linux-block, linux-fsdevel, linux-kernel, Michael S. Tsirkin,
Paolo Abeni, Simon Horman, Stefan Hajnoczi, Stefano Garzarella,
virtualization, Xuan Zhuo, Octavian Purdila
In-Reply-To: <20260613000953.467473-1-tavip@google.com>
Export iov_iter_restore so that it can be used by modules.
This is needed by the virtio vsock transport (which can be built as a
module) to restore the msg_iter state when transmission fails.
Signed-off-by: Octavian Purdila <tavip@google.com>
---
lib/iov_iter.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/lib/iov_iter.c b/lib/iov_iter.c
index 243662af1af73..067e745f9ef53 100644
--- a/lib/iov_iter.c
+++ b/lib/iov_iter.c
@@ -1491,6 +1491,7 @@ void iov_iter_restore(struct iov_iter *i, struct iov_iter_state *state)
i->__iov -= state->nr_segs - i->nr_segs;
i->nr_segs = state->nr_segs;
}
+EXPORT_SYMBOL(iov_iter_restore);
/*
* Extract a list of contiguous pages from an ITER_FOLIOQ iterator. This does
--
2.54.0.1136.gdb2ca164c4-goog
^ permalink raw reply related
* [PATCH net v2 0/2] vsock/virtio: fix msg_iter desync on transmission failure
From: Octavian Purdila @ 2026-06-13 0:09 UTC (permalink / raw)
To: netdev
Cc: Alexander Viro, Andrew Morton, Arseniy Krasnov, David S. Miller,
Eric Dumazet, Eugenio Pérez, Jakub Kicinski, Jason Wang, kvm,
linux-block, linux-fsdevel, linux-kernel, Michael S. Tsirkin,
Paolo Abeni, Simon Horman, Stefan Hajnoczi, Stefano Garzarella,
virtualization, Xuan Zhuo, Octavian Purdila
This series fixes a msg_iter desync issue in the virtio vsock transport
that can lead to warnings and eventual -ENOMEM under specific failure
scenarios (e.g. partial GUP failure during MSG_ZEROCOPY transmission).
To fix this, we need to restore the msg_iter state on transmission failure.
However, since virtio vsock transport can be built as a module, we first
need to export iov_iter_restore.
Patch 1 exports iov_iter_restore.
Patch 2 implements the msg_iter restoration in virtio vsock.
Changes in v2:
- Use iov_iter_savestate()/iov_iter_restore() (Stefano)
- Use a single restore point (Stefano)
- Reverse xmas tree (Stefano)
- Added comments in the code (Stefano)
v1: https://lore.kernel.org/all/20260609004809.1285028-1-tavip@google.com/
Octavian Purdila (2):
iov_iter: export iov_iter_restore
vsock/virtio: restore msg_iter on transmission failure
lib/iov_iter.c | 1 +
net/vmw_vsock/virtio_transport_common.c | 13 +++++++++++++
2 files changed, 14 insertions(+)
--
2.54.0.1136.gdb2ca164c4-goog
^ permalink raw reply
* Re: [PATCH v2 net] virtio_net: do not allow tunnel csum offload for non GSO packets
From: patchwork-bot+netdevbpf @ 2026-06-12 23:50 UTC (permalink / raw)
To: Paolo Abeni
Cc: netdev, mst, jasowang, xuanzhuo, eperezma, andrew+netdev, davem,
edumazet, kuba, virtualization, g.goller, f.ebner
In-Reply-To: <6c3b6c47fb05c100f384630dc48f3975cf37b67a.1781195144.git.pabeni@redhat.com>
Hello:
This patch was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:
On Thu, 11 Jun 2026 18:36:48 +0200 you wrote:
> Fiona reports broken connectivity for virtio net setup using UDP tunnel
> inside the guest and NIC with not UDP tunnel TSO support in the host.
>
> Currently the virtio_net driver exposes csum offload for UDP-tunneled,
> TCP non GSO packets. Such packet reach the host as CSUM_PARTIAL ones
> with the 'encapsulation' flag cleared, as the virtio specification do
> not support this specific kind of offload.
>
> [...]
Here is the summary with links:
- [v2,net] virtio_net: do not allow tunnel csum offload for non GSO packets
https://git.kernel.org/netdev/net/c/86c51f0f2313
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply
* [PATCH net-next 2/2] selftests/vsock: skip vng setsid workaround on >= 1.41
From: Bobby Eshleman @ 2026-06-12 19:08 UTC (permalink / raw)
To: Stefano Garzarella, Shuah Khan
Cc: virtualization, netdev, linux-kselftest, linux-kernel,
Bobby Eshleman
In-Reply-To: <20260612-vsock-test-update-v1-0-7d7eeed3ac8f@meta.com>
From: Bobby Eshleman <bobbyeshleman@meta.com>
virtme-ng 1.41 ships the upstream fix for the SIGTTOU hang
(https://github.com/arighi/virtme-ng/pull/453), so the setsid wrapper in
vng_dry_run() is no longer needed there. Gate the workaround on the vng
version: setsid is used for vng < 1.41, and vng is invoked directly on
>= 1.41.
Signed-off-by: Bobby Eshleman <bobbyeshleman@meta.com>
---
tools/testing/selftests/vsock/vmtest.sh | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/vsock/vmtest.sh b/tools/testing/selftests/vsock/vmtest.sh
index ee69ac9dd3dc..310dfc2a39ad 100755
--- a/tools/testing/selftests/vsock/vmtest.sh
+++ b/tools/testing/selftests/vsock/vmtest.sh
@@ -445,8 +445,14 @@ vng_dry_run() {
# stopped with SIGTTOU and hangs until kselftest's timer expires.
# setsid works around this by launching vng in a new session that has
# no controlling terminal, so tcsetattr() succeeds.
+ #
+ # Fixed in 1.41 (https://github.com/arighi/virtme-ng/pull/453).
- setsid -w vng --run "$@" --dry-run &>/dev/null
+ if version_lt "$(vng --version | awk '{print $2}')" "1.41"; then
+ setsid -w vng --run "$@" --dry-run &>/dev/null
+ else
+ vng --run "$@" --dry-run &>/dev/null
+ fi
}
vm_start() {
--
2.53.0-Meta
^ permalink raw reply related
* [PATCH net-next 1/2] selftests/vsock: accept vng 1.33 or >= 1.36
From: Bobby Eshleman @ 2026-06-12 19:08 UTC (permalink / raw)
To: Stefano Garzarella, Shuah Khan
Cc: virtualization, netdev, linux-kselftest, linux-kernel,
Bobby Eshleman
In-Reply-To: <20260612-vsock-test-update-v1-0-7d7eeed3ac8f@meta.com>
From: Bobby Eshleman <bobbyeshleman@meta.com>
The current vng version check uses a discrete allowlist of "1.33",
"1.36", and "1.37", which forces a script update on every new release
even though all post-1.36 releases work.
Replace the discrete list with: "1.33", or any version >= 1.36. 1.34
and 1.35 are skipped because they were not tested. Add a version_lt()
helper that compares MAJOR.MINOR numerically, so the check reads as a
straightforward version comparison.
Signed-off-by: Bobby Eshleman <bobbyeshleman@meta.com>
---
tools/testing/selftests/vsock/vmtest.sh | 39 +++++++++++++++++++--------------
1 file changed, 23 insertions(+), 16 deletions(-)
diff --git a/tools/testing/selftests/vsock/vmtest.sh b/tools/testing/selftests/vsock/vmtest.sh
index d97913a6bdc7..ee69ac9dd3dc 100755
--- a/tools/testing/selftests/vsock/vmtest.sh
+++ b/tools/testing/selftests/vsock/vmtest.sh
@@ -330,27 +330,34 @@ check_netns() {
return 0
}
+# Compare MAJOR.MINOR versions numerically. Returns 0 (true) if $1 < $2.
+version_lt() {
+ local -a a=(${1//./ })
+ local -a b=(${2//./ })
+
+ if [[ "${a[0]}" -lt "${b[0]}" ]]; then
+ return 0
+ elif [[ "${a[0]}" -gt "${b[0]}" ]]; then
+ return 1
+ elif [[ "${a[1]}" -lt "${b[1]}" ]]; then
+ return 0
+ fi
+
+ return 1
+}
+
check_vng() {
- local tested_versions
local version
- local ok
- tested_versions=("1.33" "1.36" "1.37")
- version="$(vng --version)"
+ version="$(vng --version | awk '{print $2}')"
- ok=0
- for tv in "${tested_versions[@]}"; do
- if [[ "${version}" == *"${tv}"* ]]; then
- ok=1
- break
- fi
- done
-
- if [[ ! "${ok}" -eq 1 ]]; then
- printf "warning: vng version '%s' has not been tested and may " "${version}" >&2
- printf "not function properly.\n\tThe following versions have been tested: " >&2
- echo "${tested_versions[@]}" >&2
+ # Supported: 1.33, or any version >= 1.36. 1.34 and 1.35 are untested.
+ if [[ "${version}" == "1.33" ]] || ! version_lt "${version}" "1.36"; then
+ return
fi
+
+ printf "warning: vng version '%s' has not been tested and may " "${version}" >&2
+ printf "not function properly.\n\tSupported: 1.33 or >= 1.36\n" >&2
}
check_socat() {
--
2.53.0-Meta
^ permalink raw reply related
* [PATCH net-next 0/2] selftests/vsock: improve vng version and quirk handling
From: Bobby Eshleman @ 2026-06-12 19:08 UTC (permalink / raw)
To: Stefano Garzarella, Shuah Khan
Cc: virtualization, netdev, linux-kselftest, linux-kernel,
Bobby Eshleman
As vng has continued updating, there have been two things in our
selftests that have been affected. One is that newer versions always
emit the vng version warning, and two is that we have a workaround that
is not needed in newer versions.
This series just updates the version handling to allow all newer
versions without warning and version-gates the workaround to only those
versions that don't have the commit that fixed the root cause.
Additionally, we add function for comparing major.minor versions which
is used in both patches.
Signed-off-by: Bobby Eshleman <bobbyeshleman@meta.com>
---
Bobby Eshleman (2):
selftests/vsock: accept vng 1.33 or >= 1.36
selftests/vsock: skip vng setsid workaround on >= 1.41
tools/testing/selftests/vsock/vmtest.sh | 47 +++++++++++++++++++++------------
1 file changed, 30 insertions(+), 17 deletions(-)
---
base-commit: dfcc2ff12925d99e858eaf539eaa4aaaf81fe2a6
change-id: 20260612-vsock-test-update-fcae9ffced52
Best regards,
--
Bobby Eshleman <bobbyeshleman@meta.com>
^ permalink raw reply
* Re: [PATCH v4 0/7] nvdimm: virtio_pmem: fix request lifetime and converge broken queue failures
From: Alison Schofield @ 2026-06-12 18:42 UTC (permalink / raw)
To: Li Chen
Cc: Pankaj Gupta, Dan Williams, Vishal Verma, Dave Jiang, Ira Weiny,
virtualization, nvdimm, linux-kernel
In-Reply-To: <20260609120726.1714780-1-me@linux.beauty>
On Tue, Jun 09, 2026 at 08:07:14PM +0800, Li Chen wrote:
> Hi,
Hi Li Chen,
In case you missed it, a Sashiko AI review of this set has posted
feedback. Please take a look.
https://sashiko.dev/#/patchset/20260609120726.1714780-1-me%40linux.beauty
-- Alison
>
> The nvdimm flush helper currently converts any non-zero provider flush
> callback error to -EIO. That hides useful errno values from providers. For
> example, virtio-pmem may fail child flush bio allocation with -ENOMEM, but
> that is currently reported as -EIO by nvdimm_flush().
>
> The raw failure seen in the local mkfs sanity test was:
>
> wipefs: /dev/pmem0: cannot flush modified buffers: Input/output error
> mkfs.ext4: Input/output error while writing out and closing file system
> nd_region region0: dbg: nvdimm_flush rc=-5
>
> The first two patches keep provider flush errors intact and make the
> virtio-pmem child flush bio allocation use GFP_NOIO. async_pmem_flush() can
> allocate a child flush bio from filesystem flush and writeback paths;
> GFP_NOIO is a better fit than GFP_ATOMIC there because the allocation may
> sleep but must not recurse into filesystem I/O reclaim. With these changes,
> the same mkfs sanity test reached mkfs_rc=0, mount_rc=0, and umount_rc=0.
>
> The rest of the series addresses virtio-pmem request lifetime and broken
> virtqueue handling. The virtio-pmem flush path uses a virtqueue cookie/token
> to carry a per-request context through completion. Under broken virtqueue /
> notify failure conditions, the submitter can return and free the request
> object while the host/backend may still complete the published request. The
> IRQ completion handler then dereferences freed memory when waking waiters,
> which is reported by KASAN as a slab-use-after-free and may manifest as lock
> corruption (e.g. "BUG: spinlock already unlocked") without KASAN.
>
> In addition, the flush path has two wait sites: one for virtqueue descriptor
> availability (-ENOSPC from virtqueue_add_sgs()) and one for request
> completion. If the virtqueue becomes broken, forward progress is no longer
> guaranteed and these waiters may sleep indefinitely unless the driver
> converges the failure and wakes all wait sites.
>
> This series addresses these issues:
>
> 1/7 nvdimm: preserve flush callback errors
> Return provider flush callback errors directly from nvdimm_flush().
>
> 2/7 nvdimm: virtio_pmem: use GFP_NOIO for child flush bio
> Use GFP_NOIO for the child flush bio allocation.
>
> 3/7 nvdimm: virtio_pmem: always wake -ENOSPC waiters
> Wake one -ENOSPC waiter for each reclaimed used buffer, decoupled from
> token completion.
>
> 4/7 nvdimm: virtio_pmem: use READ_ONCE()/WRITE_ONCE() for wait flags
> Use READ_ONCE()/WRITE_ONCE() for the wait_event() flags (done and
> wq_buf_avail).
>
> 5/7 nvdimm: virtio_pmem: refcount requests for token lifetime
> Refcount request objects so the token lifetime spans the window where it is
> reachable through the virtqueue until completion/drain drops the virtqueue
> reference.
>
> 6/7 nvdimm: virtio_pmem: converge broken virtqueue to -EIO
> Track a device-level broken state to converge broken/notify failures to -EIO:
> wake all waiters and drain/detach outstanding requests to complete them with
> an error, and fail-fast new requests.
>
> 7/7 nvdimm: virtio_pmem: drain requests in freeze
> Drain outstanding requests in freeze() before tearing down virtqueues so
> waiters do not sleep indefinitely.
>
> Testing was done on QEMU x86_64 with a virtio-pmem device exported as
> /dev/pmem0. This v4 series applies to v7.1-rc7, builds with
> CONFIG_VIRTIO_PMEM=m, passes checkpatch, and passed the local repro checks
> with a local-only virtqueue_kick() fault injection. I also checked that it
> applies cleanly to next/master at 6e845bcb78c9 ("Add linux-next specific
> files for 20260605").
>
> Thanks,
> Li Chen
>
> Changelog:
> v3->v4:
> - Rebased the series onto v7.1-rc7 so it applies cleanly to Linux 7.1-rc7.
> - Update the allocation site in 6/7 from kmalloc(sizeof(*req_data),
> GFP_KERNEL) to kmalloc_obj(*req_data) to match current nvdimm code.
> - Add 1/7 to preserve provider flush callback errors in nvdimm_flush().
> - Include the GFP_NOIO child flush bio allocation fix as 2/7.
> - Renumber the old request lifetime and broken virtqueue fixes after the two
> new flush error patches.
> v2->v3:
> - Split patch 1 as suggested by Pankaj Gupta: keep the waiter wakeup
> ordering change in 1/5 and move READ_ONCE()/WRITE_ONCE() updates to
> 2/5 (no functional change intended).
> - Add log report to commit msg
> - Fold the export fix into 4/5 to keep the series bisectable when
> CONFIG_VIRTIO_PMEM=m.
> v1->v2: add the export patch to fix compile issue.
>
> Links:
> v3: https://lore.kernel.org/all/20260226025712.2236279-1-me@linux.beauty/#t
> v2: https://lore.kernel.org/all/20251225042915.334117-1-me@linux.beauty/
> v1: https://www.spinics.net/lists/kernel/msg5974818.html
>
> Li Chen (7):
> nvdimm: preserve flush callback errors
> nvdimm: virtio_pmem: use GFP_NOIO for child flush bio
> nvdimm: virtio_pmem: always wake -ENOSPC waiters
> nvdimm: virtio_pmem: use READ_ONCE()/WRITE_ONCE() for wait flags
> nvdimm: virtio_pmem: refcount requests for token lifetime
> nvdimm: virtio_pmem: converge broken virtqueue to -EIO
> nvdimm: virtio_pmem: drain requests in freeze
>
> drivers/nvdimm/nd_virtio.c | 139 +++++++++++++++++++++++++++++------
> drivers/nvdimm/region_devs.c | 6 +-
> drivers/nvdimm/virtio_pmem.c | 14 ++++
> drivers/nvdimm/virtio_pmem.h | 6 ++
> 4 files changed, 139 insertions(+), 26 deletions(-)
> --
> 2.52.0
^ permalink raw reply
* [PATCH v4 2/2] vduse: Add suspend
From: Eugenio Pérez @ 2026-06-12 18:14 UTC (permalink / raw)
To: Michael S . Tsirkin
Cc: Maxime Coquelin, linux-kernel, Yongji Xie, Jason Wang,
virtualization, Cindy Lu, Stefano Garzarella, Xuan Zhuo,
Eugenio Pérez, Laurent Vivier
In-Reply-To: <20260612181457.622955-1-eperezma@redhat.com>
Implement suspend operation for vduse devices, so vhost-vdpa will offer
that backend feature and userspace can effectively suspend the device.
This is a must before get virtqueue indexes (base) for live migration,
since the device could modify them after userland gets them.
This patch does not implement resume, so VMM resets the whole device
to recover from a live migration failure. Resume optimization can be
implemented on top of these patches, as other vDPA devices have done in
the past.
Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
---
v4:
* Add preparatory patch to not flush the kick and irq works under rwsem
(MST).
* Fix jump over a semaphore guard (Nathan Chancellor).
* Fix take the device semaphore in the vq spinlock context (MST).
* Add suspend guard at vq_signal_irqfd so the device will not send an
IRQ after suspend.
v3:
* Expand the patch message with information about resume operation.
v2:
* Take the rwsem only before the actual kick, not in vduse_vdpa_kick_vq.
This assures that we're not in a critical section.
---
drivers/vdpa/vdpa_user/vduse_dev.c | 101 +++++++++++++++++++++++++----
include/uapi/linux/vduse.h | 4 ++
2 files changed, 94 insertions(+), 11 deletions(-)
diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c b/drivers/vdpa/vdpa_user/vduse_dev.c
index 0f15575df394..80dc37ed7e13 100644
--- a/drivers/vdpa/vdpa_user/vduse_dev.c
+++ b/drivers/vdpa/vdpa_user/vduse_dev.c
@@ -54,7 +54,8 @@
#define IRQ_UNBOUND -1
/* Supported VDUSE features */
-static const uint64_t vduse_features = BIT_U64(VDUSE_F_QUEUE_READY);
+static const uint64_t vduse_features = BIT_U64(VDUSE_F_QUEUE_READY) |
+ BIT_U64(VDUSE_F_SUSPEND);
/*
* VDUSE instance have not asked the vduse API version, so assume 0.
@@ -85,6 +86,7 @@ struct vduse_virtqueue {
int irq_effective_cpu;
struct cpumask irq_affinity;
struct kobject kobj;
+ struct vduse_dev *dev;
};
struct vduse_dev;
@@ -134,6 +136,7 @@ struct vduse_dev {
int minor;
bool broken;
bool connected;
+ bool suspended;
u64 api_version;
u64 device_features;
u64 driver_features;
@@ -502,6 +505,7 @@ static void vduse_dev_reset(struct vduse_dev *dev)
}
scoped_guard(rwsem_write, &dev->rwsem) {
+ dev->suspended = false;
dev->status = 0;
dev->driver_features = 0;
dev->generation++;
@@ -560,16 +564,18 @@ static int vduse_vdpa_set_vq_address(struct vdpa_device *vdpa, u16 idx,
static void vduse_vq_kick(struct vduse_virtqueue *vq)
{
- spin_lock(&vq->kick_lock);
+ guard(rwsem_read)(&vq->dev->rwsem);
+ if (vq->dev->suspended)
+ return;
+
+ guard(spinlock)(&vq->kick_lock);
if (!vq->ready)
- goto unlock;
+ return;
if (vq->kickfd)
eventfd_signal(vq->kickfd);
else
vq->kicked = true;
-unlock:
- spin_unlock(&vq->kick_lock);
}
static void vduse_vq_kick_work(struct work_struct *work)
@@ -922,6 +928,27 @@ static int vduse_vdpa_set_map(struct vdpa_device *vdpa,
return 0;
}
+static int vduse_vdpa_suspend(struct vdpa_device *vdpa)
+{
+ struct vduse_dev *dev = vdpa_to_vduse(vdpa);
+ struct vduse_dev_msg msg = { 0 };
+ int ret;
+
+ msg.req.type = VDUSE_SUSPEND;
+
+ ret = vduse_dev_msg_sync(dev, &msg);
+ if (ret == 0) {
+ scoped_guard(rwsem_write, &dev->rwsem)
+ dev->suspended = true;
+
+ cancel_work_sync(&dev->inject);
+ for (u32 i = 0; i < dev->vq_num; i++)
+ cancel_work_sync(&dev->vqs[i]->inject);
+ }
+
+ return ret;
+}
+
static void vduse_vdpa_free(struct vdpa_device *vdpa)
{
struct vduse_dev *dev = vdpa_to_vduse(vdpa);
@@ -963,6 +990,41 @@ static const struct vdpa_config_ops vduse_vdpa_config_ops = {
.free = vduse_vdpa_free,
};
+static const struct vdpa_config_ops vduse_vdpa_config_ops_with_suspend = {
+ .set_vq_address = vduse_vdpa_set_vq_address,
+ .kick_vq = vduse_vdpa_kick_vq,
+ .set_vq_cb = vduse_vdpa_set_vq_cb,
+ .set_vq_num = vduse_vdpa_set_vq_num,
+ .get_vq_size = vduse_vdpa_get_vq_size,
+ .get_vq_group = vduse_get_vq_group,
+ .set_vq_ready = vduse_vdpa_set_vq_ready,
+ .get_vq_ready = vduse_vdpa_get_vq_ready,
+ .set_vq_state = vduse_vdpa_set_vq_state,
+ .get_vq_state = vduse_vdpa_get_vq_state,
+ .get_vq_align = vduse_vdpa_get_vq_align,
+ .get_device_features = vduse_vdpa_get_device_features,
+ .set_driver_features = vduse_vdpa_set_driver_features,
+ .get_driver_features = vduse_vdpa_get_driver_features,
+ .set_config_cb = vduse_vdpa_set_config_cb,
+ .get_vq_num_max = vduse_vdpa_get_vq_num_max,
+ .get_device_id = vduse_vdpa_get_device_id,
+ .get_vendor_id = vduse_vdpa_get_vendor_id,
+ .get_status = vduse_vdpa_get_status,
+ .set_status = vduse_vdpa_set_status,
+ .get_config_size = vduse_vdpa_get_config_size,
+ .get_config = vduse_vdpa_get_config,
+ .set_config = vduse_vdpa_set_config,
+ .get_generation = vduse_vdpa_get_generation,
+ .set_vq_affinity = vduse_vdpa_set_vq_affinity,
+ .get_vq_affinity = vduse_vdpa_get_vq_affinity,
+ .reset = vduse_vdpa_reset,
+ .set_map = vduse_vdpa_set_map,
+ .set_group_asid = vduse_set_group_asid,
+ .get_vq_map = vduse_get_vq_map,
+ .suspend = vduse_vdpa_suspend,
+ .free = vduse_vdpa_free,
+};
+
static void vduse_dev_sync_single_for_device(union virtio_map token,
dma_addr_t dma_addr, size_t size,
enum dma_data_direction dir)
@@ -1174,6 +1236,10 @@ static void vduse_dev_irq_inject(struct work_struct *work)
{
struct vduse_dev *dev = container_of(work, struct vduse_dev, inject);
+ guard(rwsem_read)(&dev->rwsem);
+ if (dev->suspended)
+ return;
+
spin_lock_bh(&dev->irq_lock);
if (dev->config_cb.callback)
dev->config_cb.callback(dev->config_cb.private);
@@ -1185,6 +1251,10 @@ static void vduse_vq_irq_inject(struct work_struct *work)
struct vduse_virtqueue *vq = container_of(work,
struct vduse_virtqueue, inject);
+ guard(rwsem_read)(&vq->dev->rwsem);
+ if (vq->dev->suspended)
+ return;
+
spin_lock_bh(&vq->irq_lock);
if (vq->ready && vq->cb.callback)
vq->cb.callback(vq->cb.private);
@@ -1195,6 +1265,10 @@ static bool vduse_vq_signal_irqfd(struct vduse_virtqueue *vq)
{
bool signal = false;
+ guard(rwsem_read)(&vq->dev->rwsem);
+ if (vq->dev->suspended)
+ return false;
+
if (!vq->cb.trigger)
return false;
@@ -1214,9 +1288,9 @@ static int vduse_dev_queue_irq_work(struct vduse_dev *dev,
{
int ret = -EINVAL;
- down_read(&dev->rwsem);
- if (!(dev->status & VIRTIO_CONFIG_S_DRIVER_OK))
- goto unlock;
+ guard(rwsem_read)(&dev->rwsem);
+ if (dev->suspended || !(dev->status & VIRTIO_CONFIG_S_DRIVER_OK))
+ return ret;
ret = 0;
if (irq_effective_cpu == IRQ_UNBOUND)
@@ -1224,8 +1298,6 @@ static int vduse_dev_queue_irq_work(struct vduse_dev *dev,
else
queue_work_on(irq_effective_cpu,
vduse_irq_bound_wq, irq_work);
-unlock:
- up_read(&dev->rwsem);
return ret;
}
@@ -1979,6 +2051,7 @@ static int vduse_dev_init_vqs(struct vduse_dev *dev, u32 vq_align, u32 vq_num)
}
dev->vqs[i]->index = i;
+ dev->vqs[i]->dev = dev;
dev->vqs[i]->irq_effective_cpu = IRQ_UNBOUND;
INIT_WORK(&dev->vqs[i]->inject, vduse_vq_irq_inject);
INIT_WORK(&dev->vqs[i]->kick, vduse_vq_kick_work);
@@ -2429,12 +2502,18 @@ static struct vduse_mgmt_dev *vduse_mgmt;
static int vduse_dev_init_vdpa(struct vduse_dev *dev, const char *name)
{
struct vduse_vdpa *vdev;
+ const struct vdpa_config_ops *ops;
if (dev->vdev)
return -EEXIST;
+ if (dev->vduse_features & BIT_U64(VDUSE_F_SUSPEND))
+ ops = &vduse_vdpa_config_ops_with_suspend;
+ else
+ ops = &vduse_vdpa_config_ops;
+
vdev = vdpa_alloc_device(struct vduse_vdpa, vdpa, dev->dev,
- &vduse_vdpa_config_ops, &vduse_map_ops,
+ ops, &vduse_map_ops,
dev->ngroups, dev->nas, name, true);
if (IS_ERR(vdev))
return PTR_ERR(vdev);
diff --git a/include/uapi/linux/vduse.h b/include/uapi/linux/vduse.h
index 7324faea5df4..8c616895c511 100644
--- a/include/uapi/linux/vduse.h
+++ b/include/uapi/linux/vduse.h
@@ -17,6 +17,9 @@
/* The VDUSE instance expects a request for vq ready */
#define VDUSE_F_QUEUE_READY 0
+/* The VDUSE instance expects a request for suspend */
+#define VDUSE_F_SUSPEND 1
+
/*
* Get the version of VDUSE API that kernel supported (VDUSE_API_VERSION).
* This is used for future extension.
@@ -334,6 +337,7 @@ enum vduse_req_type {
VDUSE_UPDATE_IOTLB,
VDUSE_SET_VQ_GROUP_ASID,
VDUSE_SET_VQ_READY,
+ VDUSE_SUSPEND,
};
/**
--
2.54.0
^ permalink raw reply related
* [PATCH v4 1/2] vduse: do not take rwsem at reset work flush
From: Eugenio Pérez @ 2026-06-12 18:14 UTC (permalink / raw)
To: Michael S . Tsirkin
Cc: Maxime Coquelin, linux-kernel, Yongji Xie, Jason Wang,
virtualization, Cindy Lu, Stefano Garzarella, Xuan Zhuo,
Eugenio Pérez, Laurent Vivier
In-Reply-To: <20260612181457.622955-1-eperezma@redhat.com>
Next patches need to check suspend flag at this work item, and the
rwlock is used to protect the suspend flag update. If the work takes
the rwlock too it will produce a deadlock.
Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
---
drivers/vdpa/vdpa_user/vduse_dev.c | 63 ++++++++++++++++--------------
1 file changed, 33 insertions(+), 30 deletions(-)
diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c b/drivers/vdpa/vdpa_user/vduse_dev.c
index eff104f90cee..0f15575df394 100644
--- a/drivers/vdpa/vdpa_user/vduse_dev.c
+++ b/drivers/vdpa/vdpa_user/vduse_dev.c
@@ -501,44 +501,47 @@ static void vduse_dev_reset(struct vduse_dev *dev)
vduse_domain_reset_bounce_map(domain);
}
- down_write(&dev->rwsem);
+ scoped_guard(rwsem_write, &dev->rwsem) {
+ dev->status = 0;
+ dev->driver_features = 0;
+ dev->generation++;
+ spin_lock(&dev->irq_lock);
+ dev->config_cb.callback = NULL;
+ dev->config_cb.private = NULL;
+ spin_unlock(&dev->irq_lock);
+
+ for (i = 0; i < dev->vq_num; i++) {
+ struct vduse_virtqueue *vq = dev->vqs[i];
+
+ vq->ready = false;
+ vq->desc_addr = 0;
+ vq->driver_addr = 0;
+ vq->device_addr = 0;
+ vq->num = 0;
+ memset(&vq->state, 0, sizeof(vq->state));
+
+ spin_lock(&vq->kick_lock);
+ vq->kicked = false;
+ if (vq->kickfd)
+ eventfd_ctx_put(vq->kickfd);
+ vq->kickfd = NULL;
+ spin_unlock(&vq->kick_lock);
+
+ spin_lock(&vq->irq_lock);
+ vq->cb.callback = NULL;
+ vq->cb.private = NULL;
+ vq->cb.trigger = NULL;
+ spin_unlock(&vq->irq_lock);
+ }
+ }
- dev->status = 0;
- dev->driver_features = 0;
- dev->generation++;
- spin_lock(&dev->irq_lock);
- dev->config_cb.callback = NULL;
- dev->config_cb.private = NULL;
- spin_unlock(&dev->irq_lock);
flush_work(&dev->inject);
-
for (i = 0; i < dev->vq_num; i++) {
struct vduse_virtqueue *vq = dev->vqs[i];
- vq->ready = false;
- vq->desc_addr = 0;
- vq->driver_addr = 0;
- vq->device_addr = 0;
- vq->num = 0;
- memset(&vq->state, 0, sizeof(vq->state));
-
- spin_lock(&vq->kick_lock);
- vq->kicked = false;
- if (vq->kickfd)
- eventfd_ctx_put(vq->kickfd);
- vq->kickfd = NULL;
- spin_unlock(&vq->kick_lock);
-
- spin_lock(&vq->irq_lock);
- vq->cb.callback = NULL;
- vq->cb.private = NULL;
- vq->cb.trigger = NULL;
- spin_unlock(&vq->irq_lock);
flush_work(&vq->inject);
flush_work(&vq->kick);
}
-
- up_write(&dev->rwsem);
}
static int vduse_vdpa_set_vq_address(struct vdpa_device *vdpa, u16 idx,
--
2.54.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox