On Tue, 16 Aug 2022 09:21:27 +0200 Johannes Berg wrote:

On Mon, 2022-08-15 at 17:47 -0700, Jakub Kicinski wrote:

quoted

On Mon, 15 Aug 2022 22:09:11 +0200 Johannes Berg wrote:  
It's the incredibly inventive nesting format used in genetlink policy
dumps where the type of the sub-attr(s there are actually two levels)
carry a value (index of the policy and attribute) rather than denoting
a type :S :S :S  

Hmm, OK, in the policy dump (not specific to genetlink, btw, can be used
for any policy, but is only generically hooked up for genetlink), we
have

[policy_idx] = {
  [attr_idx] = {
    [NL_POLICY_TYPE_ATTR_...] = ...
  }
}

Is that what you mean?

Yes.

I guess I never really thought about this format much from a description
POV, no need to have a policy since you simply iterate (for_each_attr)
when reading it, and don't really need to care about the attribute
index, at least.

For future reference, how would you suggest to have done this instead?

My guess was that some of the wrapping was for ease of canceling here
(cancel is used both on skip and on error). 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.

As Michal pointed out a number of times - the wrapping ends up limiting 
the size of the array to U16_MAX, 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 :)

quoted

Slightly guessing but I think I know what you mean -> the value of the
array is a nest with index as the type and then inside that is the
entry of the array with its attributes <- and that's where the space is
applied, not at the first nest level?  

Right.

quoted

Right, I should probably put that in the docs rather than the schema,
array-nests are expected to strip one layer of nesting and put the
value taken from the type (:D) into an @idx member of the struct
representing the values of the array. Or at least that's what I do in
the C codegen.  

Well mostly you're not supposed to care about the 'value'/'type', I
guess?

Fair, I wasn't sure if it's ever used, but I figured if I have to parse
it out why not save the values. Same for array-next indexes.

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.