On Tue, 2022-08-16 at 08:53 -0700, Jakub Kicinski wrote:

My guess was that some of the wrapping was for ease of canceling here
(cancel is used both on skip and on error). 

Not sure I'd say that, but can't say I really remember why I did it this
way.

What I think we should push
for is multi-attr, so the same attribute happens multiple times.

[msg]
 [ATTR1]
 [ATTR2] // elem 1
   [SubATTR1]
   [SubATTR2]
 [ATTR2] // elem 2
   [SubATTR1]
   [SubATTR2]
 [ATTR2] // elem 3
   [SubATTR1]
   [SubATTR2]
 [ATTR3]
 [ATTR4]

Instead of wrapping into an array and then elements.

Hmm, ok, I guess that works.

As Michal pointed out a number of times - the wrapping ends up limiting 
the size of the array to U16_MAX,

True.

and I have a suspicion that most of
wrapping is done because we tend to parse into a pointer array, which
makes multi-attr a little tricky. But we shouldn't let one parsing
technique in a relatively uncommon language like C dictate the format :)

:-)

To be fair, for cases where today we use nla_for_each_nested() we could
also invent an "nlmsg_for_each_attr_of_type()" macro:

#define nlmsg_for_each_attr_of_type(type, pos, nlh, hdrlen, rem) \
	nlmsg_for_each_attr(pos, nlh, hdrlen, rem)               \
		if (pos->nla_type == type)

and then that's basically all you need?

In the policy we'd declare it as a normal nested (not array), and I
think that's it because today if you give the same attribute type twice,
the last one wins in the normal parsing anyway (IIRC)...

I'm leaning heavily towards defining a subset of the YAML spec as 
"the way to do things in new family" which will allow only one form 
of arrays.

Fair enough.

johannes