netdev - Re: [PATCH net-next v3 03/15] net: Add peer info to queue-get response

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: <b6b3cef5-195c-40cc-8c37-cebdee05a5bd@davidwei.uk>
Date: Tue, 28 Oct 2025 19:08:10 -0700
From: David Wei <dw@...idwei.uk>
To: Jakub Kicinski <kuba@...nel.org>, Daniel Borkmann <daniel@...earbox.net>
Cc: netdev@...r.kernel.org, bpf@...r.kernel.org, davem@...emloft.net,
 razor@...ckwall.org, pabeni@...hat.com, willemb@...gle.com, sdf@...ichev.me,
 john.fastabend@...il.com, martin.lau@...nel.org, jordan@...fe.io,
 maciej.fijalkowski@...el.com, magnus.karlsson@...el.com, toke@...hat.com,
 yangzhenze@...edance.com, wangdongdong.6@...edance.com
Subject: Re: [PATCH net-next v3 03/15] net: Add peer info to queue-get
 response

On 2025-10-24 16:18, Jakub Kicinski wrote:
> On Fri, 24 Oct 2025 14:59:39 +0200 Daniel Borkmann wrote:
>> On 10/24/25 4:33 AM, Jakub Kicinski wrote:
>>> On Mon, 20 Oct 2025 18:23:43 +0200 Daniel Borkmann wrote:
>>>> Add a nested peer field to the queue-get response that returns the peered
>>>> ifindex and queue id.
>>>>
>>>> Example with ynl client:
>>>>
>>>>     # ip netns exec foo ./pyynl/cli.py \
>>>>         --spec ~/netlink/specs/netdev.yaml \
>>>>         --do queue-get \
>>>>         --json '{"ifindex": 3, "id": 1, "type": "rx"}'
>>>>     {'id': 1, 'ifindex': 3, 'peer': {'id': 15, 'ifindex': 4, 'netns-id': 21}, 'type': 'rx'}
>>>
>>> I'm struggling with the roles of what is src and dst and peer :(
>>> No great suggestion off the top of my head but better terms would
>>> make this much easier to review.
>>>
>>> The example seems to be from the container side. Do we need to show peer
>>> info on the container side? Not just on the host side?
>>
>> I think up to us which side we want to show. My thinking was to allow user
>> introspection from both, but we don't have to. Right now the above example
>> was from the container side, but technically it could be either side depending
>> in which netns the phys dev would be located.
>>
>> The user knows which is which based on the ifindex passed to the queue-get
>> query: if the ifindex is from a virtual device (e.g. netkit type), then the
>> 'peer' section shows the phys dev, and vice versa, if the ifindex is from a
>> phys device (say, mlx5), then the 'peer' section shows the virtual one.
>>
>> Maybe I'll provide a better more in-depth example with both sides and above
>> explanation in the commit msg for v4..
> 
> Yes, FWIW my mental model is that "leaking" host information into the
> container is best avoided. Not a problem, but shouldn't be done without
> a clear reason.
> Typical debug scenario can be covered from the host side (container X
> is having issues with queue Y, dump all the queues, find out which one
> is bound to X/Y).

Makes sense, I didn't consider leaking host info in a container. Happy
to remove the introspection from the container side, leaving it only on
the host side when queues are dumped.

Like Daniel mentioned, I didn't add 'src/real' or 'dst/virtual' because
I believed this information is implicit to the user when querying a
netdev based on its type. Do you find this to be confusing? Happy to add
a clarifying field in the nested struct.