netdev - Re: [PATCH net-next v4 3/4] Extend napi threaded polling to allow kthread based busy polling

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: <CAAywjhQvC2gzCyJSzQaiw68UdL0nK++8BgVgh5=vywF4fcrytQ@mail.gmail.com>
Date: Thu, 27 Mar 2025 09:39:21 -0700
From: Samiullah Khawaja <skhawaja@...gle.com>
To: Joe Damato <jdamato@...tly.com>, Samiullah Khawaja <skhawaja@...gle.com>, 
	Jakub Kicinski <kuba@...nel.org>, "David S . Miller" <davem@...emloft.net>, 
	Eric Dumazet <edumazet@...gle.com>, Paolo Abeni <pabeni@...hat.com>, almasrymina@...gle.com, 
	willemb@...gle.com, mkarsten@...terloo.ca, netdev@...r.kernel.org
Subject: Re: [PATCH net-next v4 3/4] Extend napi threaded polling to allow
 kthread based busy polling

On Fri, Mar 21, 2025 at 10:39 AM Joe Damato <jdamato@...tly.com> wrote:
>
> On Fri, Mar 21, 2025 at 02:15:20AM +0000, Samiullah Khawaja wrote:
> > Add a new state to napi state enum:
> >
> > - STATE_THREADED_BUSY_POLL
> >   Threaded busy poll is enabled/running for this napi.
> >
> > Following changes are introduced in the napi scheduling and state logic:
> >
> > - When threaded busy poll is enabled through sysfs it also enables
> >   NAPI_STATE_THREADED so a kthread is created per napi. It also sets
> >   NAPI_STATE_THREADED_BUSY_POLL bit on each napi to indicate that we are
> >   supposed to busy poll for each napi.
> >
> > - When napi is scheduled with STATE_SCHED_THREADED and associated
> >   kthread is woken up, the kthread owns the context. If
> >   NAPI_STATE_THREADED_BUSY_POLL and NAPI_SCHED_THREADED both are set
> >   then it means that we can busy poll.
> >
> > - To keep busy polling and to avoid scheduling of the interrupts, the
> >   napi_complete_done returns false when both SCHED_THREADED and
> >   THREADED_BUSY_POLL flags are set. Also napi_complete_done returns
> >   early to avoid the STATE_SCHED_THREADED being unset.
> >
> > - If at any point STATE_THREADED_BUSY_POLL is unset, the
> >   napi_complete_done will run and unset the SCHED_THREADED bit also.
> >   This will make the associated kthread go to sleep as per existing
> >   logic.
> >
> > Signed-off-by: Samiullah Khawaja <skhawaja@...gle.com>
> > ---
> >  Documentation/ABI/testing/sysfs-class-net     |  3 +-
> >  Documentation/netlink/specs/netdev.yaml       | 12 ++-
> >  Documentation/networking/napi.rst             | 67 ++++++++++++-
> >  .../net/ethernet/atheros/atl1c/atl1c_main.c   |  2 +-
> >  drivers/net/ethernet/mellanox/mlxsw/pci.c     |  2 +-
> >  drivers/net/ethernet/renesas/ravb_main.c      |  2 +-
> >  drivers/net/wireless/ath/ath10k/snoc.c        |  2 +-
> >  include/linux/netdevice.h                     | 20 +++-
> >  include/uapi/linux/netdev.h                   |  6 ++
> >  net/core/dev.c                                | 93 ++++++++++++++++---
> >  net/core/net-sysfs.c                          |  2 +-
> >  net/core/netdev-genl-gen.c                    |  2 +-
> >  net/core/netdev-genl.c                        |  2 +-
> >  tools/include/uapi/linux/netdev.h             |  6 ++
> >  14 files changed, 188 insertions(+), 33 deletions(-)
>
> I think this should be split into two patches which would ease
> review and bisection:
>
>   - First patch: introduce enum netdev_napi_threaded and
>     NETDEV_NAPI_THREADED_ENABLE and the associated driver changes.
>
>   - Second patch: introduce NETDEV_NAPI_THREADED_BUSY_POLL_ENABLE
Agreed. I will split this up and send again.
>
> I'll have to take a closer look at all the changes here after I've
> read the cover letter and have reproduced the results, but one issue
> stands out:
>
> > diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> > index 3c244fd9ae6d..b990cbe76f86 100644
> > --- a/include/linux/netdevice.h
> > +++ b/include/linux/netdevice.h
>
> [...]
>
> > @@ -2432,7 +2442,7 @@ struct net_device {
> >       struct sfp_bus          *sfp_bus;
> >       struct lock_class_key   *qdisc_tx_busylock;
> >       bool                    proto_down;
> > -     bool                    threaded;
> > +     u8                      threaded;
>
> Doesn't
>
> Documentation/networking/net_cachelines/net_device.rst
>
> Also need to be updated if you are changing the width of this field
> from bool to u8?