netdev - Re: [PATCH net-next v8] Add support to set napi threaded for individual napi

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: <20250618182803.2e367473@kernel.org>
Date: Wed, 18 Jun 2025 18:28:03 -0700
From: Jakub Kicinski <kuba@...nel.org>
To: Samiullah Khawaja <skhawaja@...gle.com>
Cc: "David S . Miller " <davem@...emloft.net>, Eric Dumazet
 <edumazet@...gle.com>, Paolo Abeni <pabeni@...hat.com>,
 almasrymina@...gle.com, willemb@...gle.com, jdamato@...tly.com,
 mkarsten@...terloo.ca, netdev@...r.kernel.org
Subject: Re: [PATCH net-next v8] Add support to set napi threaded for
 individual napi

On Mon, 16 Jun 2025 16:57:06 +0000 Samiullah Khawaja wrote:
> A net device has a threaded sysctl that can be used to enable threaded
> napi polling on all of the NAPI contexts under that device. Allow
> enabling threaded napi polling at individual napi level using netlink.
> 
> Extend the netlink operation `napi-set` and allow setting the threaded
> attribute of a NAPI. This will enable the threaded polling on a napi
> context.
> 
> Add a test in `nl_netdev.py` that verifies various cases of threaded
> napi being set at napi and at device level.
> 
> 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

> diff --git a/Documentation/netlink/specs/netdev.yaml b/Documentation/netlink/specs/netdev.yaml
> index ce4cfec82100..ec2c9d66519b 100644
> --- a/Documentation/netlink/specs/netdev.yaml
> +++ b/Documentation/netlink/specs/netdev.yaml
> @@ -283,6 +283,14 @@ attribute-sets:
>          doc: The timeout, in nanoseconds, of how long to suspend irq
>               processing, if event polling finds events
>          type: uint
> +      -
> +        name: threaded
> +        doc: Whether the napi is configured to operate in threaded polling

lower case napi here

> +             mode. If this is set to `1` then the NAPI context operates

upper case here, let's pick one form

This is technically a form of kernel documentation, fed into Sphinx
I'm a bit unclear on Sphinx and backticks, but IIUC it wants double
backticks? ``1`` ? Or none at all

> +             in threaded polling mode.
> +        type: uint
> +        checks:
> +          max: 1
>    -
>      name: xsk-info
>      attributes: []

>  Threaded NAPI is controlled by writing 0/1 to the ``threaded`` file in
> -netdev's sysfs directory.
> +netdev's sysfs directory. It can also be enabled for a specific napi using
> +netlink interface.
> +
> +For example, using the script:
> +
> +.. code-block:: bash
> +
> +  $ kernel-source/tools/net/ynl/pyynl/cli.py \
> +            --spec Documentation/netlink/specs/netdev.yaml \
> +            --do napi-set \
> +            --json='{"id": 66,
> +                     "threaded": 1}'

I wonder if it's okay now to use ynl CLI in the form that is packaged
for Fedora and RHEL ? It's much more concise, tho not sure if / when
other distros will catch up:

  $ ynl --family netdev --do napi-set --json='{"id": 66, "threaded": 1}'

> +/**
> + * napi_set_threaded - set napi threaded state
> + * @n: napi struct to set the threaded state on
> + * @threaded: whether this napi does threaded polling
> + *
> + * Return: 0 on success and negative errno on failure.
> + */
> +int napi_set_threaded(struct napi_struct *n, bool threaded);

IIRC by the kernel coding standards the kdoc if necessary should 
be on the definition.

> @@ -322,8 +326,14 @@ netdev_nl_napi_set_config(struct napi_struct *napi, struct genl_info *info)
>  {
>  	u64 irq_suspend_timeout = 0;
>  	u64 gro_flush_timeout = 0;
> +	u8 threaded = 0;
>  	u32 defer = 0;
>  
> +	if (info->attrs[NETDEV_A_NAPI_THREADED]) {
> +		threaded = nla_get_u8(info->attrs[NETDEV_A_NAPI_THREADED]);

nla_get_uint(), nla_get_u8 will not work on big endian

> +		napi_set_threaded(napi, !!threaded);

why ignore the error?
-- 
pw-bot: cr