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

[Joe Damato <jdamato@fastly.com>]

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@google.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

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?