netdev - Re: [net-next PATCH v7 01/10] netdev-genl: spec: Extend netdev netlink spec in YAML for queue

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: <20231114234801.0faee5db@kernel.org>
Date: Tue, 14 Nov 2023 23:48:01 -0500
From: Jakub Kicinski <kuba@...nel.org>
To: Amritha Nambiar <amritha.nambiar@...el.com>
Cc: netdev@...r.kernel.org, pabeni@...hat.com, sridhar.samudrala@...el.com
Subject: Re: [net-next PATCH v7 01/10] netdev-genl: spec: Extend netdev
 netlink spec in YAML for queue

On Mon, 13 Nov 2023 16:29:37 -0800 Amritha Nambiar wrote:
> Add support in netlink spec(netdev.yaml) for queue information.
> Add code generated from the spec.
> 
> Note: The "queue-type" attribute takes values 0 and 1 for rx
> and tx queue type respectively.


> index 14511b13f305..e7bf6007d77f 100644
> --- a/Documentation/netlink/specs/netdev.yaml
> +++ b/Documentation/netlink/specs/netdev.yaml
> @@ -55,6 +55,10 @@ definitions:
>          name: hash
>          doc:
>            Device is capable of exposing receive packet hash via bpf_xdp_metadata_rx_hash().
> +  -
> +    name: queue-type
> +    type: enum
> +    entries: [ rx, tx ]
>  
>  attribute-sets:
>    -
> @@ -87,6 +91,31 @@ attribute-sets:
>          type: u64
>          enum: xdp-rx-metadata
>  
> +  -
> +    name: queue
> +    attributes:
> +      -
> +        name: queue-id

Hm. I guess it looks okay in the Python / JSON but the C defines
will say NETDEV_QUEUE_QUEUE_ID or some such. Should we drop the word
queue from all attrs in the queue set?

Sorry, not sure how I missed this earlier. Some extra nits below while
I'm requesting changes...

> +        doc: Queue index for most queue types are indexed like a C array, with

s/ for/;/ ?

> +             indexes starting at 0 and ending at queue count - 1. Queue indexes
> +             are scoped to an interface and queue type.
> +        type: u32
> +      -
> +        name: ifindex
> +        doc: ifindex of the netdevice to which the queue belongs.
> +        type: u32
> +        checks:
> +          min: 1
> +      -
> +        name: queue-type
> +        doc: queue type as rx, tx

Add: ". Each queue type defines a separate ID space."

> +        type: u32
> +        enum: queue-type
> +      -
> +        name: napi-id
> +        doc: ID of the NAPI instance which services this queue.
> +        type: u32
> +
>  operations:
>    list:
>      -
> @@ -120,6 +149,29 @@ operations:
>        doc: Notification about device configuration being changed.
>        notify: dev-get
>        mcgrp: mgmt
> +    -
> +      name: queue-get
> +      doc: Get queue information from the kernel.
> +           Only configured queues will be reported (as opposed to all available
> +           queues).

maybe add "hardware", so "all available hardware queues)" ?
That may help the reader connect the dots