Re: [net-next/RFC PATCH v1 4/4] netdev-genl: Add support for exposing napi info from netdev

[Jakub Kicinski <kuba@kernel.org>]

On Thu, 01 Jun 2023 10:42:41 -0700 Amritha Nambiar wrote:
> Add support in ynl/netdev.yaml for napi related information. The
> netdev structure tracks all the napi instances and napi fields.
> The napi instances and associated queue[s] can be retrieved this way.
> 
> Refactored netdev-genl to support exposing napi<->queue[s] mapping
> that is retained in a netdev.
> 
> Signed-off-by: Amritha Nambiar <amritha.nambiar@intel.com>
> ---
>  Documentation/netlink/specs/netdev.yaml |   39 +++++
>  include/uapi/linux/netdev.h             |    4 +
>  net/core/netdev-genl.c                  |  239 ++++++++++++++++++++++++++-----
>  tools/include/uapi/linux/netdev.h       |    4 +
>  4 files changed, 247 insertions(+), 39 deletions(-)
> 
> diff --git a/Documentation/netlink/specs/netdev.yaml b/Documentation/netlink/specs/netdev.yaml
> index b99e7ffef7a1..8d0edb529563 100644
> --- a/Documentation/netlink/specs/netdev.yaml
> +++ b/Documentation/netlink/specs/netdev.yaml
> @@ -62,6 +62,44 @@ attribute-sets:
>          type: u64
>          enum: xdp-act
>          enum-as-flags: true
> +      -
> +        name: napi-info
> +        doc: napi information such as napi-id, napi queues etc.
> +        type: nest
> +        multi-attr: true

Let's make a new attr space for the napi info command.
We don't reuse much of the attributes, and as the commands 
grow stuffing all attrs into one space makes finding stuff 
harder.

> +        nested-attributes: dev-napi-info

And what's inside this nest should also be a separate attr space.


> +      -
> +        name: napi-id
> +        doc: napi id
> +        type: u32
> +      -
> +        name: rx-queues
> +        doc: list of rx queues associated with a napi
> +        type: u16

Make it u32, at the uAPI level we're tried to the width of fields, and
u16 ends up being the same size as u32 "on the wire" due to padding.

> +        multi-attr: true
> +      -
> +        name: tx-queues
> +        doc: list of tx queues associated with a napi
> +        type: u16
> +        multi-attr: true


> +  -
> +    name: dev-napi-info
> +    subset-of: dev

Yeah, this shouldn't be a subset just a full-on separate attr space.
The handshake family may be a good example to look at, it's the biggest
so far written with the new rules in mind. 

> +    attributes:
> +      -
> +        name: napi-id
> +        doc: napi id
> +        type: u32
> +      -
> +        name: rx-queues
> +        doc: list rx of queues associated with a napi
> +        type: u16
> +        multi-attr: true
> +      -
> +        name: tx-queues
> +        doc: list tx of queues associated with a napi
> +        type: u16
> +        multi-attr: true
>  
>  operations:
>    list:
> @@ -77,6 +115,7 @@ operations:
>            attributes:
>              - ifindex
>              - xdp-features
> +            - napi-info

Aaah, separate command, please. Let's not stuff all the information
into a single command like we did for rtnl.

>        dump:
>          reply: *dev-all
>      -