Thread (55 messages) 55 messages, 6 authors, 2025-09-11

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

From: Jacob Keller <jacob.e.keller@intel.com>
Date: 2025-09-10 16:58:48
Also in: lkml


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:
quoted
On 9/4/2025 3:01 PM, Asbjørn Sloth Tønnesen wrote:
quoted
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:
quoted
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/ (local)
  

Attachments

Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help