Date:   Thu, 19 Jan 2023 17:02:05 -0600
From:   Rob Herring <robh@...nel.org>
To:     Jakub Kicinski <kuba@...nel.org>
Cc:     davem@...emloft.net, netdev@...r.kernel.org, edumazet@...gle.com,
        pabeni@...hat.com, johannes@...solutions.net,
        stephen@...workplumber.org, ecree.xilinx@...il.com, sdf@...gle.com,
        f.fainelli@...il.com, fw@...len.de, linux-doc@...r.kernel.org,
        razor@...ckwall.org, nicolas.dichtel@...nd.com
Subject: Re: [PATCH net-next v3 2/8] netlink: add schemas for YAML specs

On Thu, Jan 19, 2023 at 3:49 PM Jakub Kicinski <kuba@...nel.org> wrote:
>
> On Thu, 19 Jan 2023 08:07:31 -0600 Rob Herring wrote:
> > On Wed, Jan 18, 2023 at 6:36 PM Jakub Kicinski <kuba@...nel.org> wrote:
> > >
> > > Add schemas for Netlink spec files. As described in the docs
> > > we have 4 "protocols" or compatibility levels, and each one
> > > comes with its own schema, but the more general / legacy
> > > schemas are superset of more modern ones: genetlink is
> > > the smallest followed by genetlink-c and genetlink-legacy.
> > > There is no schema for raw netlink, yet, I haven't found the time..
> > >
> > > I don't know enough jsonschema to do inheritance or something
> > > but the repetition is not too bad. I hope.
> >
> > Generally you put common schemas under '$defs' and the then reference
> > them with '$ref'.
> >
> > $defs:
> >   some-prop-type:
> >     type: integer
> >     minimum: 0
> >
> > properties:
> >   foo:
> >     $ref: '#/$defs/some-prop-type'
> >   bar:
> >     $ref: '#/$defs/some-prop-type'
>
> Thanks! Is it possible to move the common definitions to a separate
> file? I tried to create a file called defs.yaml and change the ref to:
>
>   $ref: "defs.yaml#/$defs/len-or-define"

Yes, but...

> But:
>
>   File "/usr/lib/python3.11/site-packages/jsonschema/validators.py", line 257, in iter_errors
>     for error in errors:
>   File "/usr/lib/python3.11/site-packages/jsonschema/_validators.py", line 294, in ref
>     scope, resolved = validator.resolver.resolve(ref)
>                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>   File "/usr/lib/python3.11/site-packages/jsonschema/validators.py", line 856, in resolve
>     return url, self._remote_cache(url)
>                 ^^^^^^^^^^^^^^^^^^^^^^^
>   File "/usr/lib/python3.11/site-packages/jsonschema/validators.py", line 870, in resolve_from_url
>     raise exceptions.RefResolutionError(exc)
> jsonschema.exceptions.RefResolutionError: Expecting value: line 1 column 1 (char 0)

You either need the ref at the URL or a custom resolver to convert
http path to a file path. Also, the default resolver doesn't handle
YAML either. It's not much code to do and the DT tools do this. The
hardest part was just getting the path right. For a ref with just a
filename and no path, the URL will be the same path as the $id in the
referring schema.

> > If you have objects with common sets of properties, you can do the
> > same thing, but then you need 'unevaluatedProperties' if you want to
> > define a base set of properties and add to them. We do that frequently
> > in DT schemas. Unlike typical inheritance, you can't override the
> > 'base' schema. It's an AND operation.
>
> This is hard to comprehend :o Most of the time I seem to need only the
> ability to add a custom "description" to the object, so for example:

By object, schemas here which are 'type: object'. Those all have a
list of properties. If several schemas for 'objects' have the same set
of properties like 'name' and 'type' for example, then you could
define a schema for that and then add more properties for each
specific object. I didn't really look close enough whether that makes
sense here.

> $defs:
>   len-or-define:
>     oneOf:
>       -
>         type: string
>         pattern: ^[0-9A-Za-z_-]*( - 1)?$
>       -
>         type: integer
>         minimum: 0
>
> Then:
>
>        min-len:
>          description: Min length for a binary attribute.
>          $ref: '#/$defs/len-or-define'
>
> And that seems to work. Should I be using unevaluatedProperties somehow
> as well here?

No, it looks fine. 'unevaluatedProperties' wouldn't make sense here as
'type' is a string or integer.

>
> > > +          description: |
> > > +            Name used when referring to this space in other definitions, not used outside of YAML.
> > > +          type: string
> > > +        # Strictly speaking 'name-prefix' and 'subset-of' should be mutually exclusive.
> >
> > If one is required:
> >
> > oneOf:
> >   - required: [ name-prefix ]
> >   - required: [ subset-of ]
> >
> > Or if both are optional:
> >
> > dependencies:
> >   name-prefix:
> >     not:
> >       required: [ subset-of ]
> >   subset-of:
> >     not:
> >       required: [ name-prefix ]
>
> Nice, let me try this.
>
> > > +                  min-len:
> > > +                    description: Min length for a binary attribute.
> > > +                    oneOf:
> > > +                      - type: string
> > > +                        pattern: ^[0-9A-Za-z_-]*( - 1)?$
> > > +                      - type: integer
> >
> > How can a length be a string?
>
> For readability in C I wanted to allow using a define for the length.
> Then the name of the define goes here, and the value can be fetched
> from the "definitions" section of the spec.

Ah, makes sense.

Don't you need to drop the '-' in the regex then? Also, the '*' should
be a '+' for 1 or more instead of 0 or more.

> > Anyways, this is something you could pull out into a $defs entry and
> > reference. It will also work without the oneOf because 'pattern' will
> > just be ignored for an integer. That's one gotcha with json-schema. If
> > a keyword doesn't apply to the instance, it is silently ignored. (That
> > includes unknown keywords such as ones with typos. Fun!). 'oneOf' will
> > give you pretty crappy error messages, so it's good to avoid when
> > possible.
>
> Oh, interesting. Changed to:
>
> $defs:
>   len-or-define:
>     type: [ string, integer ]
>     pattern: ^[0-9A-Za-z_-]*( - 1)?$
>     minimum: 0

Powered by blists - more mailing lists