linux-kernel - Re: [PATCH net] virtio-net: enable all napis before scheduling refill work

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CACGkMEuF0rNYcSSUCdAgsW2Xfen9NGZHNxXpkO2Mt0a4zQJDqQ@mail.gmail.com>
Date: Wed, 10 Dec 2025 13:45:19 +0800
From: Jason Wang <jasowang@...hat.com>
To: Bui Quang Minh <minhquangbui99@...il.com>
Cc: netdev@...r.kernel.org, "Michael S. Tsirkin" <mst@...hat.com>, 
	Xuan Zhuo <xuanzhuo@...ux.alibaba.com>, Eugenio Pérez <eperezma@...hat.com>, 
	Andrew Lunn <andrew+netdev@...n.ch>, "David S. Miller" <davem@...emloft.net>, 
	Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, 
	Alexei Starovoitov <ast@...nel.org>, Daniel Borkmann <daniel@...earbox.net>, 
	Jesper Dangaard Brouer <hawk@...nel.org>, John Fastabend <john.fastabend@...il.com>, 
	Stanislav Fomichev <sdf@...ichev.me>, virtualization@...ts.linux.dev, 
	linux-kernel@...r.kernel.org, bpf@...r.kernel.org
Subject: Re: [PATCH net] virtio-net: enable all napis before scheduling refill work

On Tue, Dec 9, 2025 at 11:23 PM Bui Quang Minh <minhquangbui99@...il.com> wrote:
>
> On 12/9/25 11:30, Jason Wang wrote:
> > On Mon, Dec 8, 2025 at 11:35 PM Bui Quang Minh <minhquangbui99@...il.com> wrote:
> >> Calling napi_disable() on an already disabled napi can cause the
> >> deadlock. In commit 4bc12818b363 ("virtio-net: disable delayed refill
> >> when pausing rx"), to avoid the deadlock, when pausing the RX in
> >> virtnet_rx_pause[_all](), we disable and cancel the delayed refill work.
> >> However, in the virtnet_rx_resume_all(), we enable the delayed refill
> >> work too early before enabling all the receive queue napis.
> >>
> >> The deadlock can be reproduced by running
> >> selftests/drivers/net/hw/xsk_reconfig.py with multiqueue virtio-net
> >> device and inserting a cond_resched() inside the for loop in
> >> virtnet_rx_resume_all() to increase the success rate. Because the worker
> >> processing the delayed refilled work runs on the same CPU as
> >> virtnet_rx_resume_all(), a reschedule is needed to cause the deadlock.
> >> In real scenario, the contention on netdev_lock can cause the
> >> reschedule.
> >>
> >> This fixes the deadlock by ensuring all receive queue's napis are
> >> enabled before we enable the delayed refill work in
> >> virtnet_rx_resume_all() and virtnet_open().
> >>
> >> Fixes: 4bc12818b363 ("virtio-net: disable delayed refill when pausing rx")
> >> Reported-by: Paolo Abeni <pabeni@...hat.com>
> >> Closes: https://netdev-ctrl.bots.linux.dev/logs/vmksft/drv-hw-dbg/results/400961/3-xdp-py/stderr
> >> Signed-off-by: Bui Quang Minh <minhquangbui99@...il.com>
> >> ---
> >>   drivers/net/virtio_net.c | 59 +++++++++++++++++++---------------------
> >>   1 file changed, 28 insertions(+), 31 deletions(-)
> >>
> >> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> >> index 8e04adb57f52..f2b1ea65767d 100644
> >> --- a/drivers/net/virtio_net.c
> >> +++ b/drivers/net/virtio_net.c
> >> @@ -2858,6 +2858,20 @@ static bool try_fill_recv(struct virtnet_info *vi, struct receive_queue *rq,
> >>          return err != -ENOMEM;
> >>   }
> >>
> >> +static void virtnet_rx_refill_all(struct virtnet_info *vi)
> >> +{
> >> +       bool schedule_refill = false;
> >> +       int i;
> >> +
> >> +       enable_delayed_refill(vi);
> > This seems to be still racy?
> >
> > For example, in virtnet_open() we had:
> >
> > static int virtnet_open(struct net_device *dev)
> > {
> >          struct virtnet_info *vi = netdev_priv(dev);
> >          int i, err;
> >
> >          for (i = 0; i < vi->max_queue_pairs; i++) {
> >                  err = virtnet_enable_queue_pair(vi, i);
> >                  if (err < 0)
> >                          goto err_enable_qp;
> >          }
> >
> >          virtnet_rx_refill_all(vi);
> >
> > So NAPI and refill work is enabled in this case, so the refill work
> > could be scheduled and run at the same time?
>
> Yes, that's what we expect. We must ensure that refill work is scheduled
> only when all NAPIs are enabled. The deadlock happens when refill work
> is scheduled but there are still disabled RX NAPIs.

Just to make sure we are on the same page, I meant, after refill work
is enabled, rq0 is NAPI is enabled, in this case the refill work could
be triggered by the rq0's NAPI so we may end up in the refill work
that it tries to disable rq1's NAPI while holding the netdev lock.

Thanks

>
> Thanks,
> Quang Minh.
>