Message-ID: <7fff6b2f-f17e-4179-8507-397b76ea24bb@intel.com>
Date: Wed, 10 Sep 2025 09:58:37 -0700
From: Jacob Keller <jacob.e.keller@...el.com>
To: Asbjørn Sloth Tønnesen <ast@...erby.net>, "Jason A.
 Donenfeld" <Jason@...c4.com>, "David S. Miller" <davem@...emloft.net>, "Eric
 Dumazet" <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>, Paolo Abeni
	<pabeni@...hat.com>
CC: Donald Hunter <donald.hunter@...il.com>, Simon Horman <horms@...nel.org>,
	Andrew Lunn <andrew+netdev@...n.ch>, <wireguard@...ts.zx2c4.com>,
	<netdev@...r.kernel.org>, <linux-kernel@...r.kernel.org>, Johannes Berg
	<johannes.berg@...el.com>
Subject: Re: [PATCH net-next 06/11] tools: ynl-gen: don't validate nested
 array attribute types



On 9/6/2025 8:10 AM, Asbjørn Sloth Tønnesen wrote:
> CC: Johannes
> 
> On 9/6/25 12:24 AM, Jacob Keller wrote:
>> On 9/4/2025 3:01 PM, Asbjørn Sloth Tønnesen wrote:
>>> In nested arrays don't require that the intermediate
>>> attribute type should be a valid attribute type, it
>>> might just be an index or simple 0, it is often not
>>> even used.
>>>
>>> See include/net/netlink.h about NLA_NESTED_ARRAY:
>>>> The difference to NLA_NESTED is the structure:
>>>> NLA_NESTED has the nested attributes directly inside
>>>> while an array has the nested attributes at another
>>>> level down and the attribute types directly in the
>>>> nesting don't matter.
>>>
>>
>> To me, it would seem like it makes more sense to define these (even if
>> thats defined per family?) than to just say they aren't defined at all?
>>
>> Hm.
> 
> I considered adding some of that metadata too, as I am actually removing
> it for wireguard (in comment form, but still).
> 
> In include/uapi/linux/wireguard.h in the comment block at the top, it is
> very clear that wireguard only used type 0 for all the nested array
> entries, however the truth is that it doesn't care. It therefore doesn't
> matter if the generated -user.* keeps track of the index in .idx, or that
> cli.py decodes a JSON array and sends it with indexes, it's not needed,
> but it still works.
> 
> In practice I don't think we will break any clients if we enforced it, and
> validated that wireguard only accepts type 0 entries, in it's nested arrays.
> 
> For the other families, I don't know how well defined it is, Johannes have
> stated that nl80211 doesn't care which types are used, but I have no idea
> how consistent clients have abused that statement to send random data,
> or do they all just send zeros?
> 

Changing it at this point could be a significant backwards compat break,
as some clients might somehow send data that wasn't zero-initialized,
and checking would break them. At this point I guess it makes sense to
leave it as is... It would increase code cost and complexity for no gain.

> This would make a lot more sense if 'array-nest' hadn't been renamed to
> 'indexed-array' in ynl, because it feels wrong to add 'unindexed: true' now.
> We could also call it 'all-zero-indexed: true'.
> 
> In cli.py this gives some extra issues, as seen in [1], the nested arrays
> are outputted as '[{0: {..}}, {0: {..}}, ..]', but on input has the format
> '[{..},{..}, ..]' because it has to be JSON-compatible on input.
> 
> If we had an attribute like 'all-zero-indexed' then cli.py, could also output
> '[{..},{..}, ..]'.
> 

This part would be cool. If we know the index is "uninteresting",
eliding it so that the input and output formats match is good.

> [1] https://lore.kernel.org/netdev/20250904220255.1006675-3-ast@fiberby.net/



Download attachment "OpenPGP_signature.asc" of type "application/pgp-signature" (237 bytes)

Powered by blists - more mailing lists