Message-ID: <20240304072231.6f21159e@kernel.org>
Date: Mon, 4 Mar 2024 07:22:31 -0800
From: Jakub Kicinski <kuba@...nel.org>
To: Donald Hunter <donald.hunter@...il.com>
Cc: netdev@...r.kernel.org, "David S. Miller" <davem@...emloft.net>, Eric
 Dumazet <edumazet@...gle.com>, Paolo Abeni <pabeni@...hat.com>, Jacob
 Keller <jacob.e.keller@...el.com>, Jiri Pirko <jiri@...nulli.us>, Stanislav
 Fomichev <sdf@...gle.com>, donald.hunter@...hat.com
Subject: Re: [PATCH net-next v1 3/4] tools/net/ynl: Extend array-nest for
 multi level nesting

On Sun, 3 Mar 2024 10:50:09 +0000 Donald Hunter wrote:
> On Sun, 3 Mar 2024 at 04:05, Jakub Kicinski <kuba@...nel.org> wrote:
> > On Fri,  1 Mar 2024 17:14:30 +0000 Donald Hunter wrote:  
> > > The nlctrl family uses 2 levels of array nesting for policy attributes.
> > > Add a 'nest-depth' property to genetlink-legacy and extend ynl to use
> > > it.  
> >
> > Hm, I'm 90% sure we don't need this... because nlctrl is basically what
> > the legacy level was written for, initially. The spec itself wasn't
> > sent, because the C codegen for it was quite painful. And the Python
> > CLI was an afterthought.
> >
> > Could you describe what nesting you're trying to cover here?
> > Isn't it a type-value?  
> 
> I added it for getpolicy which is indexed by policy_idx and attr_idx.
> 
> ./tools/net/ynl/cli.py \
>     --spec Documentation/netlink/specs/nlctrl.yaml \
>     --dump getpolicy --json '{"family-name": "nlctrl"}'
> [{'family-id': 16, 'op-policy': [{3: {'do': 0, 'dump': 0}}]},
>  {'family-id': 16, 'op-policy': [{0: {'dump': 1}}]},
>  {'family-id': 16,
>   'policy': [{0: [{1: {'max-value-u': 65535,
>                        'min-value-u': 0,
>                        'type': 'u16'}}]}]},
>  {'family-id': 16,
>   'policy': [{0: [{2: {'max-length': 15, 'type': 'nul-string'}}]}]},
>  {'family-id': 16,
>   'policy': [{1: [{1: {'max-value-u': 65535,
>                        'min-value-u': 0,
>                        'type': 'u16'}}]}]},
>  {'family-id': 16,
>   'policy': [{1: [{2: {'max-length': 15, 'type': 'nul-string'}}]}]},
>  {'family-id': 16,
>   'policy': [{1: [{10: {'max-value-u': 4294967295,
>                         'min-value-u': 0,
>                         'type': 'u32'}}]}]}]

Yeah.. look at the example I used for type-value :)

https://docs.kernel.org/next/userspace-api/netlink/genetlink-legacy.html#type-value

> > BTW we'll also need to deal with the C codegen situation somehow.
> > Try making it work, if it's not a simple matter of fixing up the
> > names to match the header - we can grep nlctrl out in the Makefile.  
> 
> Yeah, I forgot to check codegen but saw the failures on patchwork. I
> have fixed the names but still have a couple more things to fix.
> 
> BTW, this patchset was a step towards experimenting with removing the
> hard-coded msg decoding in the Python library. Not so much for
> genetlink families, more for the extack decoding so that I could add
> policy attr decoding. Thinking about it some more, that might be
> better done with a "core" spec that contains just extack-attrs and
> policy-attrs because they don't belong to any single family - they're
> kinda infrastructure for all families.

YAML specs describe information on how to parse data YNL doesn't have
to understand, just format correctly. The base level of netlink
processing, applicable to all families, is a different story.
I think hand-coding that is more than okay. The goal is not to express
everything in YAML but to avoid duplicated work per family, if that
makes sense.

Powered by blists - more mailing lists