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: <CAAywjhTV-N5fASNy708sPWn24isyeROqCezeS6qannotJk97hQ@mail.gmail.com>
Date: Sun, 22 Jun 2025 15:33:34 -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>, 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 Wed, Jun 18, 2025 at 6:28 PM Jakub Kicinski <kuba@...nel.org> wrote:
>
> 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
+1
>
> 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
Will remove backticks.
>
> > +             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}'
+1

Will update
>
> > +/**
> > + * 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.
Will move
>
> > @@ -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?
Will update

> --
> pw-bot: cr