Re: [PATCH net-next v2 0/2] tools/net/ynl: enable json configuration

[Jakub Kicinski <kuba@kernel.org>]

On Fri, 28 Jul 2023 11:24:51 +0100 Maryam Tahhan wrote:
> On 28/07/2023 01:37, Jakub Kicinski wrote:
> > On Thu, 27 Jul 2023 08:03:29 -0400 Maryam Tahhan wrote:  
> >> Use a json configuration file to pass parameters to ynl to allow
> >> for operations on multiple specs in one go. Additionally, check
> >> this new configuration against a schema to validate it in the cli
> >> module before parsing it and passing info to the ynl module.  
> > Interesting. Is this related to Donald's comments about subscribing
> > to notifications from multiple families?
> >
> > Can you share some info about your use case?  
> 
> 
> Yes it's related. We are working towards using YNL as a netlink agent or 
> part of a netlink agent that's driven by YAML specs. We are
> 
> trying to enable existing Kubernetes CNIs to integrate with DPUs via an 
> OPI [1] API without having to change these existing CNIs. In several
> 
> cases these CNIs program the Kernel as both the control plane and the 
> fallback dataplane (for packets the DPU accelerator doesn't know what
> to do with). And so being able to monitor netlink state and reflect it 
> to the DPU accelerator (and vice versa) via an OPI API would be 
> extremely useful.
> 
> 
> We think the YAML part gives us a solid model that showcases the breadth 
> of what these CNIs program (via netlink) as well as a base for the grpc 
> protobufs that the OPI API would like to define/use.

So agent on the host is listening to netlink and sending to DPU gRPC
requests? From what you're describing it sounds like you'd mostly want
to pass the notifications. The multi-command thing is to let the DPU
also make requests if it needs to do/know something specific?

> >> Example configs would be:
> >>
> >> {
> >>      "yaml-specs-path": "/<path-to>/linux/Documentation/netlink/specs",
> >>      "spec-args": {
> >>          "ethtool.yaml": {
> >>              "do": "rings-get",
> >>              "json-params": {
> >>                  "header": {
> >>                      "dev-name": "eno1"
> >>                  }
> >>              }
> >>          },
> >>         "netdev.yaml": {
> >>              "do": "dev-get",
> >>              "json-params": {
> >>              "ifindex": 3
> >>              }
> >>          }
> >>      }
> >> }  
> > Why is the JSON preferable to writing a script to the same effect?
> > It'd actually be shorter and more flexible.
> > Maybe we should focus on packaging YNL as a python lib?  
> 
> I guess you can write a script. The reasons I picked JSON were mainly:
> 
> -  Simplicity and Readability for both developers and non-developers/users.
> 
> - With the JSON Schema Validation I could very quickly validate the 
> incoming configuration without too much logic in cli.py.
> 
> - I thought of it as a stepping stone towards an agent configuration 
> file if YNL evolves to provide or be part of a netlink agent (driven by 
> yaml specs)...

Those are very valid. My worry is that:
 - it's not a great fit for asynchronous stuff like notifications
   (at least a simple version built directly from cli.py)
 - we'd end up needing some flow control and/or transfer of values
   at some point, and it will evolve into a full blown DSL

> >> OR
> >>
> >> {
> >>      "yaml-specs-path": "/<path-to>/linux/Documentation/netlink/specs",
> >>      "spec-args": {
> >>          "ethtool.yaml": {
> >>              "subscribe": "monitor",
> >>              "sleep": 10
> >>          },
> >>          "netdev.yaml": {
> >>              "subscribe": "mgmt",
> >>              "sleep": 5
> >>          }
> >>      }
> >> }  
> > Could you also share the outputs the examples would produce?
> >  
> Right now the output is simple, an example would be for the first config 
> in the email:
> 
> [ linux]# ./tools/net/ynl/cli.py --config ./tools/net/ynl/multi-do.json
> ###############  ethtool.yaml  ###############
> 
> {'header': {'dev-index': 3, 'dev-name': 'eno1'},
>   'rx': 512,
>   'rx-max': 8192,
>   'rx-push': 0,
>   'tx': 512,
>   'tx-max': 8192,
>   'tx-push': 0}
> ###############  netdev.yaml  ###############
> 
> {'ifindex': 3, 'xdp-features': {'xsk-zerocopy', 'redirect', 'basic'}}

My concern was that this will not be optimal for the receiver to parse.
Because the answer is not valid JSON. We'd need something like:

[
 { "cmd-id": "some-identifier?".
   "response": { ... }
 },
 { "cmd-id": "identifier-of-second-command".
   "response": { ... }
 }
]

> Or for the second config in the email (note: I just toggled the tx ring 
> descriptors on one of my NICs to trigger an ethtool notification):
> 
> [root@nfvsdn-06 linux]# ./tools/net/ynl/cli.py --config 
> ./tools/net/ynl/multi-ntf.json
> ###############  ethtool.yaml  ###############
> 
> [{'msg': {'header': {'dev-index': 3, 'dev-name': 'eno1'},
>            'rx': 512,
>            'rx-max': 8192,
>            'rx-push': 0,
>            'tx': 8192,
>            'tx-max': 8192,
>            'tx-push': 0},
>    'name': 'rings-ntf'}]
> ###############  netdev.yaml  ###############
> 
> []
> 
> At the moment (even with these changes) YNL subscribes-sleeps-checks for 
> notification for each family sequentially...
> I will be looking into enabling an agent like behaviour: subscribe to 
> notifications from multiple families and monitor (babysteps)....
> 
> [1] https://opiproject.org/

Modulo the nits it sounds fairly reasonable. Main question is how much
of that we put in the kernel tree, and how much lives elsewhere :S
If we have a dependency on gRPC at some point, for example, that may
be too much for kernel tools/