Re: [PATCH net-next v1 1/2] tools/net/ynl: add support for --family and --list-families

[Jakub Kicinski <kuba@kernel.org>]

On Fri, 10 Jan 2025 14:41:44 +0000 Donald Hunter wrote:
> Add a --family option to ynl to specify the spec by family name instead
> of file path, with support for searching in-tree and system install
> location and a --list-families option to show the available families.

Neat!

>  class YnlEncoder(json.JSONEncoder):
>      def default(self, obj):
> @@ -32,7 +50,14 @@ def main():
>  
>      parser = argparse.ArgumentParser(description=description,
>                                       epilog=epilog)
> -    parser.add_argument('--spec', dest='spec', type=str, required=True)
> +    spec_group = parser.add_mutually_exclusive_group(required=True)
> +    spec_group.add_argument('--family', dest='family', type=str,
> +                            help='name of the netlink FAMILY')
> +    spec_group.add_argument('--list-families', action='store_true',
> +                            help='list all available netlink families')

Do we need to indicate that the list families lists the families for
which we found specs in the filesystem? As opposed to listing all
families currently loaded in the kernel?

Some users may be surprised if they run --list-families, see a family,
issue a request and get an exception that family is not found..

I guess OTOH we also list spec ops in --list-ops, so there's precedent.

Up to you.

> +    if args.family:
> +        spec = f"{spec_dir()}/{args.family}.yaml"
> +        if args.schema is None:

Could we only do this if spec_dir() startswith sys_schema_dir ?

We want to make sure schema is always validated during development.

> +            args.schema = ''
> +    else:
> +        spec = args.spec
> +    if not os.path.isfile(spec):
> +        raise Exception(f"Spec file {spec} does not exist")
> +
> +    ynl = YnlFamily(spec, args.schema, args.process_unknown,
>                      recv_size=args.dbg_small_recv)
>      if args.dbg_small_recv:
>          ynl.set_recv_dbg(True)