netdev - Re: [PATCH net-next v2 08/15] net: page_pool: add nlspec for basic access to page pools

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: <655cf5c7874bd_378cc9294f4@willemb.c.googlers.com.notmuch>
Date: Tue, 21 Nov 2023 13:24:07 -0500
From: Willem de Bruijn <willemdebruijn.kernel@...il.com>
To: Jakub Kicinski <kuba@...nel.org>, 
 davem@...emloft.net
Cc: netdev@...r.kernel.org, 
 edumazet@...gle.com, 
 pabeni@...hat.com, 
 almasrymina@...gle.com, 
 hawk@...nel.org, 
 ilias.apalodimas@...aro.org, 
 dsahern@...il.com, 
 dtatulea@...dia.com, 
 Jakub Kicinski <kuba@...nel.org>
Subject: Re: [PATCH net-next v2 08/15] net: page_pool: add nlspec for basic
 access to page pools

Jakub Kicinski wrote:
> Add a Netlink spec in YAML for getting very basic information
> about page pools.
> 
> Signed-off-by: Jakub Kicinski <kuba@...nel.org>
> ---
>  Documentation/netlink/specs/netdev.yaml | 46 +++++++++++++++++++++++++
>  1 file changed, 46 insertions(+)
> 
> diff --git a/Documentation/netlink/specs/netdev.yaml b/Documentation/netlink/specs/netdev.yaml
> index 14511b13f305..84ca3c2ab872 100644
> --- a/Documentation/netlink/specs/netdev.yaml
> +++ b/Documentation/netlink/specs/netdev.yaml
> @@ -86,6 +86,34 @@ name: netdev
>               See Documentation/networking/xdp-rx-metadata.rst for more details.
>          type: u64
>          enum: xdp-rx-metadata
> +  -
> +    name: page-pool
> +    attributes:
> +      -
> +        name: id
> +        doc: Unique ID of a Page Pool instance.
> +        type: uint
> +        checks:
> +          min: 1
> +          max: u32-max
> +      -
> +        name: ifindex
> +        doc: |
> +          ifindex of the netdev to which the pool belongs.
> +          May be reported as 0 if the page pool was allocated for a netdev
> +          which got destroyed already (page pools may outlast their netdevs
> +          because they wait for all memory to be returned).
> +        type: u32
> +        checks:
> +          min: 1
> +          max: s32-max
> +      -
> +        name: napi-id
> +        doc: Id of NAPI using this Page Pool instance.
> +        type: uint
> +        checks:
> +          min: 1
> +          max: u32-max

Do you want to introduce a separate ID for page pools? That brings some
issues regarding network namespace isolation.

As a user API, it is also possible (and intuitive?) to refer to a
page_pool by (namespacified) ifindex plus netdev_rx_queue index,
or napi_id.

In fairness, napi_id is also global, not per netns.

By iterating over "for_each_netdev(net, ..", dump already limits
output to pools in the same netns and get only reports pools that
match the netns.

So it's only a minor matter of visible numbering, and perhaps
superfluous new id.

No technical comments to this series. Looks solid to me.