Re: [RFC net-next 4/4] ynl: add a sample user for ethtool

[Jakub Kicinski <kuba@kernel.org>]

On Fri, 12 Aug 2022 09:26:13 -0700 Stanislav Fomichev wrote:
> On Thu, Aug 11, 2022 at 4:31 PM Jakub Kicinski <kuba@kernel.org> wrote:
> > Let me think about it some more. My main motivation is people writing
> > new families, I haven't sent too much time worrying about the existing
> > ones with all their quirks. It's entirely possible that the uAPI quirks
> > can just go and we won't generate uAPI for existing families as it
> > doesn't buy us anything.  
> 
> Ack. Although, we have to have some existing examples for people to
> write new families. So you might still have to convert something :-)

We do seem to have at least a couple on the horizon for 6.1.

Another thought I had yesterday was that maybe we want to have two
schemas - one "full" with all the historical baggage. And one "blessed"
for new families which narrows the options?

> > Still not convinced about messages, as it makes me think that every
> > "space" is then a definition of a message rather than just container
> > for field definitions with independent ID spaces.
> >
> > Attribute-sets sounds good, happy to rename.
> >
> > Another thought I just had was to call it something like "data-types"
> > or "field-types" or "type-spaces". To indicate the split into "data"
> > and "actions"/"operations"?  
> 
> I like attribute-set better than attribute-space :-)

OK!

> > Herm, looking at my commits where I started going with the C codegen
> > (which I haven't posted here) is converting the values to the same
> > format as keys (i.e. YAML/JSON style with dashes). So the codegen does:
> >
> >         c_name = attr['name']
> >         if c_name in c_keywords:
> >                 c_name += '_'
> >         c_name = c_name.replace('-', '_')
> >
> > So the name would be "dev-index", C will make that dev_index, Java will
> > make that devIndex (or whatever) etc.
> >
> > I really don't want people to have to prefix the names because that's
> > creating more work. We can slap a /* header.dev_index */ comment in
> > the generated uAPI, for the grep? Dunno..  
> 
> Yeah, dunno as well, not sure how much of the per-language knowledge
> you should bake into the tool itself..

By "the tool" you mean the codegen? I assumed that's gotta be pretty
language specific. TBH I expected everyone will write their own codegen
or support library. Like I don't think Rust people will want to look at
my nasty Python C generator :S

I'd prefer to keep as much C-ness out of the spec as possible but
some may have to leak in because of the need to gen the uAPI.

> I think my confusion mostly
> comes from the fact that 'name' is mixed together with 'name-prefix'
> and one is 'low_caps' while the other one is 'ALL_CAPS'. Too much
> magic :-)

Fair point, at the very least they should be consistent, I'll fix.

> Thinking out loud: maybe these transformations should all go via some
> extra/separate yaml (or separate sections)? Like:
> 
> fixup-attribute-sets:
>   - name: header
>     canonical-c-name: "ETHTOOL_A_HEADER_{name.upper()}"
>   - name: channels
>     canonical-c-name: "ETHTOOL_A_CHANNELS_{name.upper()}"
> 
> fixup-operations:
>   canonical-c-name: "ETHTOOL_MSG_{name.upper()}"
> 
>   # in case the "generic" catch-all above doesn't work
>   - name: channgels_get
>      canonical-c-name: "ETHTOOL_MSG_CHANNELS_GET"
> 
> We can call it "compatibility" yaml and put all sorts of weird stuff in it.
> New users hopefully don't need to care about it and don't need to
> write any of the -prefix/-suffix stuff.

Let me chew on this for a little bit.

> > Why does Python need to know the length of the string tho?
> > On receive if kernel gives you a longer name - great, no problem.
> > On send the kernel will tell you so also meh.  
> 
> I was thinking that you wanted to do some client size validation as
> well? As in sizeof(rx_buf) > ALTIFNAMSIZ -> panic? But I agree that
> there is really no value in that, the kernel will do the validation
> anyway..

Yup, I don't see the point, the ext_ack handling must be solid anyway,
so the kernel error is as good as local panic for cases which should
"never happen" (famous last words, maybe, we'll see)