netdev - Re: [PATCH net] net: Use NAPI_* in test

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAAywjhTkX3N5CY8+DCEu-DD_0y+Ts0SEkkVphKam1vScMRWdgA@mail.gmail.com>
Date: Thu, 11 Sep 2025 07:10:23 -0700
From: Samiullah Khawaja <skhawaja@...gle.com>
To: Jakub Kicinski <kuba@...nel.org>
Cc: "David S . Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, 
	Paolo Abeni <pabeni@...hat.com>, willemb@...gle.com, netdev@...r.kernel.org, 
	mkarsten@...terloo.ca
Subject: Re: [PATCH net] net: Use NAPI_* in test_bit when stopping napi kthread

On Thu, Sep 11, 2025 at 6:40 AM Jakub Kicinski <kuba@...nel.org> wrote:
>
> On Wed, 10 Sep 2025 20:37:16 +0000 Samiullah Khawaja wrote:
> > napi_stop_kthread waits for the NAPI_STATE_SCHED_THREADED to be unset
> > before stopping the kthread. But it uses test_bit with the
> > NAPIF_STATE_SCHED_THREADED and that might stop the kthread early before
> > the flag is unset.
> >
> > Use the NAPI_* variant of the NAPI state bits in test_bit instead.
> >
> > Tested:
> >  ./tools/testing/selftests/net/nl_netdev.py
> >  TAP version 13
> >  1..7
> >  ok 1 nl_netdev.empty_check
> >  ok 2 nl_netdev.lo_check
> >  ok 3 nl_netdev.page_pool_check
> >  ok 4 nl_netdev.napi_list_check
> >  ok 5 nl_netdev.dev_set_threaded
> >  ok 6 nl_netdev.napi_set_threaded
> >  ok 7 nl_netdev.nsim_rxq_reset_down
> >  # Totals: pass:7 fail:0 xfail:0 xpass:0 skip:0 error:0
> >
> >  ./tools/testing/selftests/drivers/net/napi_threaded.py
> >  TAP version 13
> >  1..2
> >  ok 1 napi_threaded.change_num_queues
> >  ok 2 napi_threaded.enable_dev_threaded_disable_napi_threaded
> >  # Totals: pass:2 fail:0 xfail:0 xpass:0 skip:0 error:0
> >
> > Fixes: 689883de94dd ("net: stop napi kthreads when THREADED napi is disabled")
> > Signed-off-by: Samiullah Khawaja <skhawaja@...gle.com>
>
> Is this basically addressing the bug that Martin run into?
Not really. That one was because the busy polling bit remained set
during kthread stop. Basically in this function when we unset
STATE_THREADED, we also needed to unset STATE_THREADED_BUSY_POLL.

@@ -7000,7 +7002,8 @@ static void napi_stop_kthread(struct napi_struct *napi)
                 */
                if ((val & NAPIF_STATE_SCHED_THREADED) ||
                    !(val & NAPIF_STATE_SCHED)) {
-                       new = val & (~NAPIF_STATE_THREADED);
+                       new = val & (~(NAPIF_STATE_THREADED |
+                                      NAPIF_STATE_THREADED_BUSY_POLL));

>
> > diff --git a/net/core/dev.c b/net/core/dev.c
> > index 93a25d87b86b..8d49b2198d07 100644
> > --- a/net/core/dev.c
> > +++ b/net/core/dev.c
> > @@ -6965,7 +6965,7 @@ static void napi_stop_kthread(struct napi_struct *napi)
> >        * the kthread.
> >        */
> >       while (true) {
> > -             if (!test_bit(NAPIF_STATE_SCHED_THREADED, &napi->state))
> > +             if (!test_bit(NAPI_STATE_SCHED_THREADED, &napi->state))
> >                       break;
> >
> >               msleep(20);
>