Re: [PATCH net-next 06/11] tools: ynl-gen: don't validate nested array attribute types

[Jacob Keller <jacob.e.keller@intel.com>]

[-- Attachment #1.1: Type: text/plain, Size: 2876 bytes --]



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/


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]