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

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Wed, 12 Jul 2023 13:10:45 -0700
From: "Nambiar, Amritha" <amritha.nambiar@...el.com>
To: Jakub Kicinski <kuba@...nel.org>
CC: <netdev@...r.kernel.org>, <davem@...emloft.net>,
	<sridhar.samudrala@...el.com>
Subject: Re: [net-next/RFC PATCH v1 4/4] netdev-genl: Add support for exposing
 napi info from netdev

On 6/2/2023 11:17 PM, Jakub Kicinski wrote:
> 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@...el.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.
> 

Agree. I should not have overloaded the 'dev' command to begin with.

>> +        nested-attributes: dev-napi-info
> 
> And what's inside this nest should also be a separate attr space.
> 

So, I think we could have two new commands for napi data. Would this be 
acceptable, a 'napi-queue-get' command for napi-queue specific 
information (set of TX and RX queues, IRQ number etc.), and another 
'napi-info-get' for other information,  such as PID for the napi thread, 
CPU etc.

Example:
  $ ./cli.py --spec netdev.yaml  --do napi-queue-get --json='{"ifindex": 
12}'

[{'napi-info': [{'napi-id': 600, 'rx-queues': [7], 'tx-queues': [7], 
'irq': 298},
                 {'napi-id': 599, 'rx-queues': [6], 'tx-queues': [6], 
'irq': 297},
                 {'napi-id': 598, 'rx-queues': [5], 'tx-queues': [5], 
'irq': 296},
                 {'napi-id': 597, 'rx-queues': [4], 'tx-queues': [4], 
'irq': 295},
                 {'napi-id': 596, 'rx-queues': [3], 'tx-queues': [3], 
'irq': 294},
                 {'napi-id': 595, 'rx-queues': [2], 'tx-queues': [2], 
'irq': 293},
                 {'napi-id': 594, 'rx-queues': [1], 'tx-queues': [1], 
'irq': 292},
                 {'napi-id': 593, 'rx-queues': [0], 'tx-queues': [0], 
'irq': 291}]}]
				

$ ./cli.py --spec netdev.yaml  --do napi-info-get --json='{"ifindex": 12}'

[{'napi-info': [{'napi-id': 600, 'pid': 68114},
                 {'napi-id': 599, 'pid': 68113},
                 {'napi-id': 598, 'pid': 68112},
                 {'napi-id': 597, 'pid': 68111},
                 {'napi-id': 596, 'pid': 68110},
                 {'napi-id': 595, 'pid': 68109},
                 {'napi-id': 594, 'pid': 68108},
                 {'napi-id': 593, 'pid': 68107}]}]
> 
>> +      -
>> +        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.
> 

Makes sense. Will fix.

>> +        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.
> 

Okay.

>> +    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.
> 

Okay.

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