[RFC net-next 0/4] ynl: YAML netlink protocol descriptions

[RFC net-next 0/4] ynl: YAML netlink protocol descriptions

[Jakub Kicinski]

Netlink seems simple and reasonable to those who understand it.
It appears cumbersome and arcane to those who don't.

This RFC introduces machine readable netlink protocol descriptions
in YAML, in an attempt to make creation of truly generic netlink
libraries a possibility. Truly generic netlink library here means
a library which does not require changes to support a new family
or a new operation.

Each YAML spec lists attributes and operations the family supports.
The specs are fully standalone, meaning that there is no dependency
on existing uAPI headers in C. Numeric values of all attribute types,
operations, enums, and defines and listed in the spec (or unambiguous).
This property removes the need to manually translate the headers for
languages which are not compatible with C.

The expectation is that the spec can be used to either dynamically
translate between whatever types the high level language likes (see
the Python example below) or codegen a complete libarary / bindings
for a netlink family at compilation time (like popular RPC libraries
do).

Currently only genetlink is supported, but the "old netlink" should
be supportable as well (I don't need it myself).

On the kernel side the YAML spec can be used to generate:
 - the C uAPI header
 - documentation of the protocol as a ReST file
 - policy tables for input attribute validation
 - operation tables

We can also codegen parsers and dump helpers, but right now the level
of "creativity & cleverness" when it comes to netlink parsing is so
high it's quite hard to generalize it for most families without major
refactoring.

Being able to generate the header, documentation and policy tables
should balance out the extra effort of writing the YAML spec.

Here is a Python example I promised earlier:

  ynl = YnlFamily("path/to/ethtool.yaml")
  channels = ynl.channels_get({'header': {'dev_name': 'eni1np1'}})

If the call was successful "channels" will hold a standard Python dict,
e.g.:

  {'header': {'dev_index': 6, 'dev_name': 'eni1np1'},
   'combined_max': 1,
   'combined_count': 1}

for a netdevsim device with a single combined queue.

YnlFamily is an implementation of a YAML <> netlink translator (patch 3).
It takes a path to the YAML spec - hopefully one day we will make
the YAMLs themselves uAPI and distribute them like we distribute
C headers. Or get them distributed to a standard search path another
way. Until then, the YNL library needs a full path to the YAML spec and
application has to worry about the distribution of those.

The YnlFamily reads all the info it needs from the spec, resolves
the genetlink family id, and creates methods based on the spec.
channels_get is such a dynamically-generated method (i.e. grep for
channels_get in the python code shows nothing). The method can be called
passing a standard Python dict as an argument. YNL will look up each key
in the YAML spec and render the appropriate binary (netlink TLV)
representation of the value. It then talks thru a netlink socket
to the kernel, and deserilizes the response, converting the netlink
TLVs into Python types and constructing a dictionary.

Again, the YNL code is completely generic and has no knowledge specific
to ethtool. It's fairly simple an incomplete (in terms of types
for example), I wrote it this afternoon. I'm also pretty bad at Python,
but it's the only language I can type which allows the method
magic, so please don't judge :) I have a rather more complete codegen
for C, with support for notifications, kernel -> user policy/type
verification, resolving extack attr offsets into a path
of attribute names etc, etc. But that stuff needs polishing and
is less suitable for an RFC.

The ability for a high level language like Python to talk to the kernel
so easily, without ctypes, manually packing structs, copy'n'pasting
values for defines etc. excites me more than C codegen, anyway.


Patch 1 adds a bit of documentation under Documentation/, it talks
more about the schemas themselves.

Patch 2 contains the YAML schema for the YAML specs.

Patch 3 adds the YNL Python library.

Patch 4 adds a sample schema for ethtool channels and a demo script.


Jakub Kicinski (4):
  ynl: add intro docs for the concept
  ynl: add the schema for the schemas
  ynl: add a sample python library
  ynl: add a sample user for ethtool

 Documentation/index.rst                     |   1 +
 Documentation/netlink/bindings/ethtool.yaml | 115 +++++++
 Documentation/netlink/index.rst             |  13 +
 Documentation/netlink/netlink-bindings.rst  | 104 ++++++
 Documentation/netlink/schema.yaml           | 242 ++++++++++++++
 tools/net/ynl/samples/ethtool.py            |  30 ++
 tools/net/ynl/samples/ynl.py                | 342 ++++++++++++++++++++
 7 files changed, 847 insertions(+)
 create mode 100644 Documentation/netlink/bindings/ethtool.yaml
 create mode 100644 Documentation/netlink/index.rst
 create mode 100644 Documentation/netlink/netlink-bindings.rst
 create mode 100644 Documentation/netlink/schema.yaml
 create mode 100755 tools/net/ynl/samples/ethtool.py
 create mode 100644 tools/net/ynl/samples/ynl.py

-- 
2.37.1

[RFC net-next 1/4] ynl: add intro docs for the concept

[Jakub Kicinski]

Short overview of the sections. I presume most people will start
by copy'n'pasting existing schemas rather than studying the docs,
but FWIW...

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 Documentation/index.rst                    |   1 +
 Documentation/netlink/index.rst            |  13 +++
 Documentation/netlink/netlink-bindings.rst | 104 +++++++++++++++++++++
 3 files changed, 118 insertions(+)
 create mode 100644 Documentation/netlink/index.rst
 create mode 100644 Documentation/netlink/netlink-bindings.rst

diff --git a/Documentation/index.rst b/Documentation/index.rst
index 67036a05b771..130e39c18fe0 100644
--- a/Documentation/index.rst
+++ b/Documentation/index.rst
@@ -112,6 +112,7 @@ needed).
    infiniband/index
    leds/index
    netlabel/index
+   netlink/index
    networking/index
    pcmcia/index
    power/index
diff --git a/Documentation/netlink/index.rst b/Documentation/netlink/index.rst
new file mode 100644
index 000000000000..a7f063f31ff3
--- /dev/null
+++ b/Documentation/netlink/index.rst
@@ -0,0 +1,13 @@
+.. SPDX-License-Identifier: BSD-3-Clause
+
+====================
+Netlink API Handbook
+====================
+
+Netlink documentation.
+
+.. toctree::
+   :maxdepth: 2
+
+   netlink-bindings
+
diff --git a/Documentation/netlink/netlink-bindings.rst b/Documentation/netlink/netlink-bindings.rst
new file mode 100644
index 000000000000..af0c069001f3
--- /dev/null
+++ b/Documentation/netlink/netlink-bindings.rst
@@ -0,0 +1,104 @@
+.. SPDX-License-Identifier: BSD-3-Clause
+
+Netlink protocol specifications
+===============================
+
+Netlink protocol specifications are complete, machine readable descriptions of
+genetlink protocols written in YAML. The schema (in ``jsonschema``) can be found
+in the same directory as this documentation file.
+
+Schema structure
+----------------
+
+YAML schema has the following conceptual sections. Most properties in the schema
+accept (or in fact require) a ``description`` sub-property documenting the defined
+object.
+
+Globals
+~~~~~~~
+
+There is a handful of global attributes such as the family name, version of
+the protocol, and additional C headers (used only for uAPI and C-compatible
+codegen).
+
+Global level also contains a handful of customization properties, like
+``attr-cnt-suffix`` which allow accommodating quirks of existing families.
+Those properties should not be used in new families.
+
+Attribute Spaces
+~~~~~~~~~~~~~~~~
+
+First of the main two sections is ``attribute-spaces``. This property contains
+information about netlink attributes of the family. All families have at least
+one attribute space, most have multiple. ``attribute-spaces`` is an array/list,
+with each entry describing a single space. The ``name`` of the space is not used
+in uAPI/C codegen, it's internal to the spec itself, used by operations and nested
+attributes to refer to a space.
+
+Each attribute space has properties used to render uAPI header enums. ``name-prefix``
+is prepended to the name of each attribute, allowing the attribute names to be shorter
+compared to the enum names in the uAPI.
+Optionally attribute space may contain ``enum-name`` if the uAPI header's enum should
+have a name. Most netlink uAPIs do not name attribute enums, the attribute names are
+heavily prefixed, which is sufficient.
+
+Most importantly each attribute space contains a list of attributes under the ``attributes``
+property. The properties of an attribute should look fairly familiar to anyone who ever
+wrote netlink code (``name``, ``type``, optional validation constraints like ``len`` and
+reference to the internal space for nests).
+
+Note that attribute spaces do not themselves nest, nested attributes refer to their internal
+space via a ``nested-attributes`` property, so the YAML spec does not resemble the format
+of the netlink messages directly.
+
+YAML spec may also contain fractional spaces - spaces which contain a ``subspace-of``
+property. Such spaces describe a section of a full space, allowing narrowing down which
+attributes are allowed in a nest or refining the validation criteria. Fractional spaces
+can only be used in nests. They are not rendered to the uAPI in any fashion.
+
+Operations and notifications
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+This section describes messages passed between the kernel and the user space.
+There are three types of entries in this section - operations, notifications
+and events.
+
+Notifications and events both refer to the asynchronous messages sent by the kernel
+to members of a multicast group. The difference between the two is that a notification
+shares its contents with a GET operation (the name of the GET operation is specified
+in the ``notify`` property). This arrangement is commonly used for notifications about
+objects where the notification carries the full object definition.
+
+Events are more focused and carry only a subset of information rather than full
+object state (a made up example would be a link state change event with just
+the interface name and the new link state).
+Events are considered less idiomatic for netlink and notifications
+should be preferred. After all, if the information in an event is sufficiently
+complete to be useful, it should also be useful enough to have a corresponding
+GET command.
+
+Operations describe the most common request - response communication. User
+sends a request and kernel replies. Each operation may contain any combination
+of the two modes familiar to netlink users - ``do`` and ``dump``.
+``do`` and ``dump`` in turn contain a combination of ``request`` and ``response``
+properties. If no explicit message with attributes is passed in a given
+direction (e.g. a ``dump`` which doesn't not accept filter, or a ``do``
+of a SET operation to which the kernel responds with just the netlink error code)
+``request`` or ``response`` section can be skipped. ``request`` and ``response``
+sections list the attributes allowed in a message. The list contains only
+the names of attributes from a space referred to by the ``attribute-space``
+property.
+
+An astute reader will notice that there are two ways of defining sub-spaces.
+A full fractional space with a ``subspace-of`` property and a de facto subspace
+created by list attributes for an operation. This is only for convenience.
+The abilities to refine the selection of attributes and change their definition
+afforded by the fractional space result in much more verbose YAML, and the full
+definition of a space (i.e. containing all attributes) is always required to render
+the uAPI header, anyway. So the per-operation attribute selection is a form of
+a shorthand.
+
+Multicast groups
+~~~~~~~~~~~~~~~~
+
+This section lists the multicast groups of the family, not much to be said.
-- 
2.37.1

Re: [RFC net-next 1/4] ynl: add intro docs for the concept

[Edward Cree]

On 11/08/2022 03:23, Jakub Kicinski wrote:
> Short overview of the sections. I presume most people will start
> by copy'n'pasting existing schemas rather than studying the docs,
> but FWIW...
> 
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> ---
[...]
> +Operations describe the most common request - response communication. User
> +sends a request and kernel replies. Each operation may contain any combination
> +of the two modes familiar to netlink users - ``do`` and ``dump``.
> +``do`` and ``dump`` in turn contain a combination of ``request`` and ``response``
> +properties. If no explicit message with attributes is passed in a given
> +direction (e.g. a ``dump`` which doesn't not accept filter, or a ``do``

Double negative.  I think you just meant "doesn't accept filter" here?
-ed

Re: [RFC net-next 1/4] ynl: add intro docs for the concept

[Jakub Kicinski]

On Thu, 11 Aug 2022 21:17:37 +0100 Edward Cree wrote:
> > +direction (e.g. a ``dump`` which doesn't not accept filter, or a ``do``  
> 
> Double negative.  I think you just meant "doesn't accept filter" here?

Yup, thanks!

Re: [RFC net-next 1/4] ynl: add intro docs for the concept

[Johannes Berg]

On Wed, 2022-08-10 at 19:23 -0700, Jakub Kicinski wrote:
> 
> +Note that attribute spaces do not themselves nest, nested attributes refer to their internal
> +space via a ``nested-attributes`` property, so the YAML spec does not resemble the format
> +of the netlink messages directly.

I find this a bit ... confusing.

I think reading the other patch I know what you mean, but if I think of
this I think more of the policy declarations than the message itself,
and there we do refer to another policy?

Maybe reword a bit and say

   Note that attribute spaces do not themselves nest, nested attributes
   refer to their internal space via a ``nested-attributes`` property
   (the name of another or the same attribute space).

or something?

johannes

Re: [RFC net-next 1/4] ynl: add intro docs for the concept

[Jakub Kicinski]

On Mon, 15 Aug 2022 22:09:29 +0200 Johannes Berg wrote:
> On Wed, 2022-08-10 at 19:23 -0700, Jakub Kicinski wrote:
> > 
> > +Note that attribute spaces do not themselves nest, nested attributes refer to their internal
> > +space via a ``nested-attributes`` property, so the YAML spec does not resemble the format
> > +of the netlink messages directly.  
> 
> I find this a bit ... confusing.
> 
> I think reading the other patch I know what you mean, but if I think of
> this I think more of the policy declarations than the message itself,
> and there we do refer to another policy?
> 
> Maybe reword a bit and say
> 
>    Note that attribute spaces do not themselves nest, nested attributes
>    refer to their internal space via a ``nested-attributes`` property
>    (the name of another or the same attribute space).
> 
> or something?

I think I put the cart before the horse in this looong sentence. How
about:

  Note that the YAML spec is "flattened" and is not meant to visually
  resemble the format of the netlink messages (unlike certain ad-hoc documentation
  formats seen in kernel comments). In the YAML spec subordinate attribute sets
  are not defined inline as a nest, but defined in a separate attribute set
  referred to with a ``nested-attributes`` property of the container.

Re: [RFC net-next 1/4] ynl: add intro docs for the concept

[Johannes Berg]

On Mon, 2022-08-15 at 17:32 -0700, Jakub Kicinski wrote:
> On Mon, 15 Aug 2022 22:09:29 +0200 Johannes Berg wrote:
> > On Wed, 2022-08-10 at 19:23 -0700, Jakub Kicinski wrote:
> > > 
> > > +Note that attribute spaces do not themselves nest, nested attributes refer to their internal
> > > +space via a ``nested-attributes`` property, so the YAML spec does not resemble the format
> > > +of the netlink messages directly.  
> > 
> > I find this a bit ... confusing.
> > 
> > I think reading the other patch I know what you mean, but if I think of
> > this I think more of the policy declarations than the message itself,
> > and there we do refer to another policy?
> > 
> > Maybe reword a bit and say
> > 
> >    Note that attribute spaces do not themselves nest, nested attributes
> >    refer to their internal space via a ``nested-attributes`` property
> >    (the name of another or the same attribute space).
> > 
> > or something?
> 
> I think I put the cart before the horse in this looong sentence. How
> about:
> 
>   Note that the YAML spec is "flattened" and is not meant to visually
>   resemble the format of the netlink messages (unlike certain ad-hoc documentation
>   formats seen in kernel comments). In the YAML spec subordinate attribute sets
>   are not defined inline as a nest, but defined in a separate attribute set
>   referred to with a ``nested-attributes`` property of the container.
> 
Yeah, that makes sense.

Like I said, I was already thinking of the policy structures (and the
policy advertisement to userspace) which is exactly the same way, so I
didn't see this as much different - but of course it _is_ different from
the message itself.

johannes

[RFC net-next 2/4] ynl: add the schema for the schemas

[Jakub Kicinski]

A schema in jsonschema format which should be familiar
to dt-bindings writers. It looks kinda hard to read, TBH,
I'm not sure how to make it better.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 Documentation/netlink/schema.yaml | 242 ++++++++++++++++++++++++++++++
 1 file changed, 242 insertions(+)
 create mode 100644 Documentation/netlink/schema.yaml

diff --git a/Documentation/netlink/schema.yaml b/Documentation/netlink/schema.yaml
new file mode 100644
index 000000000000..1290aa4794ba
--- /dev/null
+++ b/Documentation/netlink/schema.yaml
@@ -0,0 +1,242 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: "http://kernel.org/schemas/netlink/schema.yaml#"
+$schema: "http://kernel.org/meta-schemas/core.yaml#"
+
+title: Protocol
+description: Specification of a genetlink protocol
+type: object
+required: [ name, description, attribute-spaces, operations ]
+additionalProperties: False
+properties:
+  name:
+    description: Name of the genetlink family
+    type: string
+  description:
+    description: Description of the family
+    type: string
+  version:
+    description: Version of the family as defined by genetlink.
+    type: integer
+  attr-cnt-suffix:
+    description: Suffix for last member of attribute enum, default is "MAX".
+    type: string
+  headers:
+    description: C headers defining the protocol
+    type: object
+    additionalProperties: False
+    properties:
+      uapi:
+        description: Path under include/uapi where protocol definition is placed
+        type: string
+      kernel:
+        description: Additional headers on which the protocol definition depends (kernel side)
+        anyOf: &str-or-arrstr
+          -
+            type: array
+            items:
+              type: string
+          -
+            type: string
+      user:
+        description: Additional headers on which the protocol definition depends (user side)
+        anyOf: *str-or-arrstr
+  constants:
+    description: Enums and defines of the protocol
+    type: array
+    items:
+      type: object
+      required: [ type, name ]
+      additionalProperties: False
+      properties:
+        name:
+          type: string
+        type:
+          enum: [ enum, flags ]
+        value-prefix:
+          description: For enum the prefix of the values, optional.
+          type: string
+        value-start:
+          description: For enum the literal initializer for the first value.
+          oneOf: [ { type: string }, { type: integer }]
+        values:
+          description: For enum array of values
+          type: array
+          items:
+            type: string
+
+  attribute-spaces:
+    description: Definition of attribute spaces for this family.
+    type: array
+    items:
+      description: Definition of a single attribute space.
+      type: object
+      required: [ name, attributes ]
+      additionalProperties: False
+      properties:
+        name:
+          description: |
+            Name used when referring to this space in other definitions, not used outside of YAML.
+          type: string
+        # Strictly speaking 'name-prefix' and 'subspace-of' should be mutually exclusive.
+        name-prefix:
+          description: Prefix for the C enum name of the attributes.
+          type: string
+        name-enum:
+          description: Name for the enum type of the attribute.
+          type: string
+        description:
+          description: Documentation of the space.
+          type: string
+        subspace-of:
+          description: |
+            Name of another space which this is a logical part of. Sub-spaces can be used to define
+            a limitted group of attributes which are used in a nest.
+          type: string
+        attributes:
+          description: List of attributes in the space.
+          type: array
+          items:
+            type: object
+            required: [ name, type ]
+            additionalProperties: False
+            properties:
+              name:
+                type: string
+              type: &attr-type
+                enum: [ unused, flag, binary, u8, u16, u32, u64, s32, s64,
+                        nul-string, multi-attr, nest, array-nest, nest-type-value ]
+              description:
+                description: Documentation of the attribute.
+                type: string
+              type-value:
+                description: Name of the value extracted from the type of a nest-type-value attribute.
+                type: array
+                items:
+                  type: string
+              len:
+                oneOf: [ { type: string }, { type: integer }]
+              sub-type: *attr-type
+              nested-attributes:
+                description: Name of the space (sub-space) used inside the attribute.
+                type: string
+              enum:
+                description: Name of the enum used for the atttribute.
+                type: string
+              flags-mask:
+                description: Name of the flags constant on which to base mask (unsigned scalar types only).
+                type: string
+  operations:
+    description: Operations supported by the protocol.
+    type: object
+    required: [ name-prefix, list ]
+    additionalProperties: False
+    properties:
+      name-prefix:
+        description: |
+          Prefix for the C enum name of the command. The name is formed by concatenating
+          the prefix with the upper case name of the command, with dashes replaced by underscores.
+        type: string
+      name-enum:
+        description: Name for the enum type with commands.
+        type: string
+      async-prefix:
+        description: Same as name-prefix but used to render notifications and events to separate enum.
+        type: string
+      async-enum:
+        description: Name for the enum type with notifications/events.
+        type: string
+      list:
+        description: List of commands
+        type: array
+        items:
+          type: object
+          additionalProperties: False
+          required: [ name, description ]
+          properties:
+            name:
+              description: Name of the operation, also defining its C enum value in uAPI.
+              type: string
+            description:
+              description: Documentation for the command.
+              type: string
+            value:
+              description: Value for the enum in the uAPI.
+              type: integer
+            attribute-space:
+              description: |
+                Attribute space from which attributes directly in the requests and replies
+                to this command are defined.
+              type: string
+            flags: &cmd_flags
+              description: Command flags.
+              type: array
+              items:
+                enum: [ admin-perm ]
+            dont-validate:
+              description: Kernel attribute validation flags.
+              type: array
+              items:
+                enum: [ strict, dump ]
+            do: &subop-type
+              description: Main command handler.
+              type: object
+              additionalProperties: False
+              properties:
+                request: &subop-attr-list
+                  description: Definition of the request message for a given command.
+                  type: object
+                  additionalProperties: False
+                  properties:
+                    attributes:
+                      description: |
+                        Names of attributes from the attribute-space (not full attribute
+                        definitions, just names).
+                      type: array
+                      items:
+                        type: string
+                reply: *subop-attr-list
+            dump: *subop-type
+            notify:
+              description: Name of the command sharing the reply type with this notification.
+              type: string
+            event:
+              description: Explicit list of the attributes for the notification.
+              type: array
+              items:
+                type: string
+            mcgrp:
+              description: Name of the multicast group generating given notification.
+              type: string
+  mcast-groups:
+    description: List of multicast groups.
+    type: object
+    required: [ name-prefix, list ]
+    additionalProperties: False
+    properties:
+      name-prefix:
+        description: Name prefix for the define associated with the group.
+        type: string
+      name-suffix:
+        description: |
+          Name suffix for the define associated with the group. Multicast group defines seem to be unique
+          in having a name-prefix as well as suffix.
+        type: string
+      list:
+        description: List of groups.
+        type: array
+        items:
+          type: object
+          required: [ name ]
+          additionalProperties: False
+          properties:
+            name:
+              description: |
+                The name for the group, used to form the define and the value of the define, unless value
+                is specified separately.
+              type: string
+            value:
+              description: String value for the define and group name.
+              type: string
+            flags: *cmd_flags
-- 
2.37.1

Re: [RFC net-next 2/4] ynl: add the schema for the schemas

[Johannes Berg]

On Wed, 2022-08-10 at 19:23 -0700, Jakub Kicinski wrote:
> 
> +        subspace-of:
> +          description: |
> +            Name of another space which this is a logical part of. Sub-spaces can be used to define
> +            a limitted group of attributes which are used in a nest.

limited

johannes

Re: [RFC net-next 2/4] ynl: add the schema for the schemas

[Johannes Berg]

On Wed, 2022-08-10 at 19:23 -0700, Jakub Kicinski wrote:
> 
> +        attributes:
> +          description: List of attributes in the space.
> +          type: array
> +          items:
> +            type: object
> +            required: [ name, type ]
> +            additionalProperties: False
> +            properties:
> +              name:
> +                type: string
> +              type: &attr-type
> +                enum: [ unused, flag, binary, u8, u16, u32, u64, s32, s64,
> +                        nul-string, multi-attr, nest, array-nest, nest-type-value ]

nest-type-value?

> +              description:
> +                description: Documentation of the attribute.
> +                type: string
> +              type-value:
> +                description: Name of the value extracted from the type of a nest-type-value attribute.
> +                type: array
> +                items:
> +                  type: string
> +              len:
> +                oneOf: [ { type: string }, { type: integer }]
> +              sub-type: *attr-type
> +              nested-attributes:
> +                description: Name of the space (sub-space) used inside the attribute.
> +                type: string

Maybe expand that description a bit, it's not really accurate for
"array-nest"?

> +              enum:
> +                description: Name of the enum used for the atttribute.

typo - attribute

Do you mean the "name of the enumeration" or the "name of the
enumeration constant"? (per C99 concepts) I'm a bit confused? I guess
you mean the "name of the enumeration constant" though I agree most
people probably don't know the names from C99 (I had to look them up too
for the sake of being precise here ...)

johannes

Re: [RFC net-next 2/4] ynl: add the schema for the schemas

[Jakub Kicinski]

On Mon, 15 Aug 2022 22:09:11 +0200 Johannes Berg wrote:
> On Wed, 2022-08-10 at 19:23 -0700, Jakub Kicinski wrote:
> > 
> > +        attributes:
> > +          description: List of attributes in the space.
> > +          type: array
> > +          items:
> > +            type: object
> > +            required: [ name, type ]
> > +            additionalProperties: False
> > +            properties:
> > +              name:
> > +                type: string
> > +              type: &attr-type
> > +                enum: [ unused, flag, binary, u8, u16, u32, u64, s32, s64,
> > +                        nul-string, multi-attr, nest, array-nest, nest-type-value ]  
> 
> nest-type-value?

It's the incredibly inventive nesting format used in genetlink policy
dumps where the type of the sub-attr(s there are actually two levels)
carry a value (index of the policy and attribute) rather than denoting
a type :S :S :S

I really need to document the types, I know...

> > +              description:
> > +                description: Documentation of the attribute.
> > +                type: string
> > +              type-value:
> > +                description: Name of the value extracted from the type of a nest-type-value attribute.
> > +                type: array
> > +                items:
> > +                  type: string
> > +              len:
> > +                oneOf: [ { type: string }, { type: integer }]
> > +              sub-type: *attr-type
> > +              nested-attributes:
> > +                description: Name of the space (sub-space) used inside the attribute.
> > +                type: string  
> 
> Maybe expand that description a bit, it's not really accurate for
> "array-nest"?

Slightly guessing but I think I know what you mean -> the value of the
array is a nest with index as the type and then inside that is the
entry of the array with its attributes <- and that's where the space is
applied, not at the first nest level?

Right, I should probably put that in the docs rather than the schema,
array-nests are expected to strip one layer of nesting and put the
value taken from the type (:D) into an @idx member of the struct
representing the values of the array. Or at least that's what I do in
the C codegen.

Not that any of these beautiful, precious formats should be encouraged
going forward. multi-attr all the way!

> > +              enum:
> > +                description: Name of the enum used for the atttribute.  
> 
> typo - attribute

Thanks!

> Do you mean the "name of the enumeration" or the "name of the
> enumeration constant"? (per C99 concepts) I'm a bit confused? I guess
> you mean the "name of the enumeration constant" though I agree most
> people probably don't know the names from C99 (I had to look them up too
> for the sake of being precise here ...)

I meant the type. I think. When u32 carries values of an enum.
Enumeration constant for the attribute type is constructed from
it's name and the prefix/suffix kludge.

Re: [RFC net-next 2/4] ynl: add the schema for the schemas

[Johannes Berg]

On Mon, 2022-08-15 at 17:47 -0700, Jakub Kicinski wrote:
> On Mon, 15 Aug 2022 22:09:11 +0200 Johannes Berg wrote:
> > On Wed, 2022-08-10 at 19:23 -0700, Jakub Kicinski wrote:
> > > 
> > > +        attributes:
> > > +          description: List of attributes in the space.
> > > +          type: array
> > > +          items:
> > > +            type: object
> > > +            required: [ name, type ]
> > > +            additionalProperties: False
> > > +            properties:
> > > +              name:
> > > +                type: string
> > > +              type: &attr-type
> > > +                enum: [ unused, flag, binary, u8, u16, u32, u64, s32, s64,
> > > +                        nul-string, multi-attr, nest, array-nest, nest-type-value ]  
> > 
> > nest-type-value?
> 
> It's the incredibly inventive nesting format used in genetlink policy
> dumps where the type of the sub-attr(s there are actually two levels)
> carry a value (index of the policy and attribute) rather than denoting
> a type :S :S :S

Hmm, OK, in the policy dump (not specific to genetlink, btw, can be used
for any policy, but is only generically hooked up for genetlink), we
have

[policy_idx] = {
  [attr_idx] = {
    [NL_POLICY_TYPE_ATTR_...] = ...
  }
}

Is that what you mean?

I guess I never really thought about this format much from a description
POV, no need to have a policy since you simply iterate (for_each_attr)
when reading it, and don't really need to care about the attribute
index, at least.

For future reference, how would you suggest to have done this instead?


> > > +              description:
> > > +                description: Documentation of the attribute.
> > > +                type: string
> > > +              type-value:
> > > +                description: Name of the value extracted from the type of a nest-type-value attribute.
> > > +                type: array
> > > +                items:
> > > +                  type: string
> > > +              len:
> > > +                oneOf: [ { type: string }, { type: integer }]
> > > +              sub-type: *attr-type
> > > +              nested-attributes:
> > > +                description: Name of the space (sub-space) used inside the attribute.
> > > +                type: string  
> > 
> > Maybe expand that description a bit, it's not really accurate for
> > "array-nest"?
> 
> Slightly guessing but I think I know what you mean -> the value of the
> array is a nest with index as the type and then inside that is the
> entry of the array with its attributes <- and that's where the space is
> applied, not at the first nest level?

Right.

> Right, I should probably put that in the docs rather than the schema,
> array-nests are expected to strip one layer of nesting and put the
> value taken from the type (:D) into an @idx member of the struct
> representing the values of the array. Or at least that's what I do in
> the C codegen.

Well mostly you're not supposed to care about the 'value'/'type', I
guess?

> Not that any of these beautiful, precious formats should be encouraged
> going forward. multi-attr all the way!

multi-attr?

> > Do you mean the "name of the enumeration" or the "name of the
> > enumeration constant"? (per C99 concepts) I'm a bit confused? I guess
> > you mean the "name of the enumeration constant" though I agree most
> > people probably don't know the names from C99 (I had to look them up too
> > for the sake of being precise here ...)
> 
> I meant the type. I think. When u32 carries values of an enum.
> Enumeration constant for the attribute type is constructed from
> it's name and the prefix/suffix kludge.
> 
Indeed, I confused myself too ...

johannes

Re: [RFC net-next 2/4] ynl: add the schema for the schemas

[Jakub Kicinski]

On Tue, 16 Aug 2022 09:21:27 +0200 Johannes Berg wrote:
> On Mon, 2022-08-15 at 17:47 -0700, Jakub Kicinski wrote:
> > On Mon, 15 Aug 2022 22:09:11 +0200 Johannes Berg wrote:  
> > It's the incredibly inventive nesting format used in genetlink policy
> > dumps where the type of the sub-attr(s there are actually two levels)
> > carry a value (index of the policy and attribute) rather than denoting
> > a type :S :S :S  
> 
> Hmm, OK, in the policy dump (not specific to genetlink, btw, can be used
> for any policy, but is only generically hooked up for genetlink), we
> have
> 
> [policy_idx] = {
>   [attr_idx] = {
>     [NL_POLICY_TYPE_ATTR_...] = ...
>   }
> }
> 
> Is that what you mean?

Yes.

> I guess I never really thought about this format much from a description
> POV, no need to have a policy since you simply iterate (for_each_attr)
> when reading it, and don't really need to care about the attribute
> index, at least.
> 
> For future reference, how would you suggest to have done this instead?

My guess was that some of the wrapping was for ease of canceling here
(cancel is used both on skip and on error). What I think we should push
for is multi-attr, so the same attribute happens multiple times.

[msg]
 [ATTR1]
 [ATTR2] // elem 1
   [SubATTR1]
   [SubATTR2]
 [ATTR2] // elem 2
   [SubATTR1]
   [SubATTR2]
 [ATTR2] // elem 3
   [SubATTR1]
   [SubATTR2]
 [ATTR3]
 [ATTR4]

Instead of wrapping into an array and then elements.

As Michal pointed out a number of times - the wrapping ends up limiting 
the size of the array to U16_MAX, and I have a suspicion that most of
wrapping is done because we tend to parse into a pointer array, which
makes multi-attr a little tricky. But we shouldn't let one parsing
technique in a relatively uncommon language like C dictate the format :)

> > Slightly guessing but I think I know what you mean -> the value of the
> > array is a nest with index as the type and then inside that is the
> > entry of the array with its attributes <- and that's where the space is
> > applied, not at the first nest level?  
> 
> Right.
> 
> > Right, I should probably put that in the docs rather than the schema,
> > array-nests are expected to strip one layer of nesting and put the
> > value taken from the type (:D) into an @idx member of the struct
> > representing the values of the array. Or at least that's what I do in
> > the C codegen.  
> 
> Well mostly you're not supposed to care about the 'value'/'type', I
> guess?

Fair, I wasn't sure if it's ever used, but I figured if I have to parse
it out why not save the values. Same for array-next indexes.

I'm leaning heavily towards defining a subset of the YAML spec as 
"the way to do things in new family" which will allow only one form 
of arrays.

Re: [RFC net-next 2/4] ynl: add the schema for the schemas

[Johannes Berg]

On Tue, 2022-08-16 at 08:53 -0700, Jakub Kicinski wrote:
> 
> My guess was that some of the wrapping was for ease of canceling here
> (cancel is used both on skip and on error). 
> 

Not sure I'd say that, but can't say I really remember why I did it this
way.

> What I think we should push
> for is multi-attr, so the same attribute happens multiple times.
> 
> [msg]
>  [ATTR1]
>  [ATTR2] // elem 1
>    [SubATTR1]
>    [SubATTR2]
>  [ATTR2] // elem 2
>    [SubATTR1]
>    [SubATTR2]
>  [ATTR2] // elem 3
>    [SubATTR1]
>    [SubATTR2]
>  [ATTR3]
>  [ATTR4]
> 
> Instead of wrapping into an array and then elements.

Hmm, ok, I guess that works.

> 
> As Michal pointed out a number of times - the wrapping ends up limiting 
> the size of the array to U16_MAX,

True.

> and I have a suspicion that most of
> wrapping is done because we tend to parse into a pointer array, which
> makes multi-attr a little tricky. But we shouldn't let one parsing
> technique in a relatively uncommon language like C dictate the format :)

:-)

To be fair, for cases where today we use nla_for_each_nested() we could
also invent an "nlmsg_for_each_attr_of_type()" macro:

#define nlmsg_for_each_attr_of_type(type, pos, nlh, hdrlen, rem) \
	nlmsg_for_each_attr(pos, nlh, hdrlen, rem)               \
		if (pos->nla_type == type)

and then that's basically all you need?

In the policy we'd declare it as a normal nested (not array), and I
think that's it because today if you give the same attribute type twice,
the last one wins in the normal parsing anyway (IIRC)...

> I'm leaning heavily towards defining a subset of the YAML spec as 
> "the way to do things in new family" which will allow only one form 
> of arrays.

Fair enough.

johannes

Re: [RFC net-next 2/4] ynl: add the schema for the schemas

[Rob Herring]

On Wed, Aug 10, 2022 at 07:23:02PM -0700, Jakub Kicinski wrote:
> A schema in jsonschema format which should be familiar
> to dt-bindings writers. It looks kinda hard to read, TBH,
> I'm not sure how to make it better.

This got my attention in the Plumbers agenda though I missed the talk. 
It's nice to see another jsonschema user in the kernel. I hope you make 
jsonschema a dependency for everyone before I do. :) Hopefully we don't 
hit any comflict in required version of jsonschema as I've needed both a 
minimum version for features as well as been broken by new versions.

I would avoid calling all this 'YAML netlink' as YAML is just the file 
format you are using. We started with calling things YAML, but I nudge 
folks away from that to 'DT schema'. Also, probably not an issue here, 
but be aware that YAML is much slower to parse than JSON. 

> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> ---
>  Documentation/netlink/schema.yaml | 242 ++++++++++++++++++++++++++++++
>  1 file changed, 242 insertions(+)
>  create mode 100644 Documentation/netlink/schema.yaml
> 
> diff --git a/Documentation/netlink/schema.yaml b/Documentation/netlink/schema.yaml
> new file mode 100644
> index 000000000000..1290aa4794ba
> --- /dev/null
> +++ b/Documentation/netlink/schema.yaml
> @@ -0,0 +1,242 @@
> +# SPDX-License-Identifier: GPL-2.0
> +%YAML 1.2
> +---
> +$id: "http://kernel.org/schemas/netlink/schema.yaml#"
> +$schema: "http://kernel.org/meta-schemas/core.yaml#"

In case there's ever another one: meta-schemas/netlink/core.yaml

Or something similar.

> +
> +title: Protocol
> +description: Specification of a genetlink protocol
> +type: object
> +required: [ name, description, attribute-spaces, operations ]
> +additionalProperties: False
> +properties:
> +  name:
> +    description: Name of the genetlink family
> +    type: string
> +  description:

It's better if your schema vocabulary is disjoint from jsonschema 
vocabulary. From what I've seen, it's fairly common to get the 
indentation off and jsonschema behavior is to ignore unknown keywords. 
If the vocabularies are disjoint, you can write a meta-schema that only 
allows jsonschema schema vocabulary at the right levels. Probably less 
of an issue here as you don't have 1000s of schemas.

> +    description: Description of the family
> +    type: string
> +  version:
> +    description: Version of the family as defined by genetlink.
> +    type: integer

Do you have the need to define the int size? We did our own keyword for 
this, but since then I've looked at several other projects that have 
used something like 'format: uint32'. There was some chatter about 
trying to standardize this, but I haven't checked in a while.

> +  attr-cnt-suffix:
> +    description: Suffix for last member of attribute enum, default is "MAX".
> +    type: string
> +  headers:
> +    description: C headers defining the protocol
> +    type: object
> +    additionalProperties: False
> +    properties:
> +      uapi:
> +        description: Path under include/uapi where protocol definition is placed
> +        type: string
> +      kernel:
> +        description: Additional headers on which the protocol definition depends (kernel side)
> +        anyOf: &str-or-arrstr
> +          -
> +            type: array
> +            items:
> +              type: string
> +          -
> +            type: string
> +      user:
> +        description: Additional headers on which the protocol definition depends (user side)
> +        anyOf: *str-or-arrstr

For DT, we stick to a JSON compatible subset of YAML, so no anchors. The 
jsonschema way to do this is using '$defs' (or 'definitions' before the 
spec standardized it) and '$ref'.

> +  constants:
> +    description: Enums and defines of the protocol
> +    type: array
> +    items:
> +      type: object
> +      required: [ type, name ]
> +      additionalProperties: False
> +      properties:
> +        name:
> +          type: string
> +        type:
> +          enum: [ enum, flags ]
> +        value-prefix:
> +          description: For enum the prefix of the values, optional.
> +          type: string
> +        value-start:
> +          description: For enum the literal initializer for the first value.
> +          oneOf: [ { type: string }, { type: integer }]

I think you can do just 'type: [ string, integer ]'.

Rob

Re: [RFC net-next 2/4] ynl: add the schema for the schemas

[Jakub Kicinski]

On Mon, 26 Sep 2022 11:10:56 -0500 Rob Herring wrote:
> On Wed, Aug 10, 2022 at 07:23:02PM -0700, Jakub Kicinski wrote:
> > A schema in jsonschema format which should be familiar
> > to dt-bindings writers. It looks kinda hard to read, TBH,
> > I'm not sure how to make it better.  
> 
> This got my attention in the Plumbers agenda though I missed the talk. 
> It's nice to see another jsonschema user in the kernel. I hope you make 
> jsonschema a dependency for everyone before I do. :) Hopefully we don't 
> hit any comflict in required version of jsonschema as I've needed both a 
> minimum version for features as well as been broken by new versions.

I'm a complete noob on this, my jsonschema is really crude.
But thanks to this it's unlikely to depend on any particular version? :)

> I would avoid calling all this 'YAML netlink' as YAML is just the file 
> format you are using. We started with calling things YAML, but I nudge 
> folks away from that to 'DT schema'.

Good point, I'll try to stick to Netlink schema as well.

> Also, probably not an issue here, but be aware that YAML is much
> slower to parse than JSON. 

Fingers crossed. Worst case we can convert formats later.

> > Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> > ---
> >  Documentation/netlink/schema.yaml | 242 ++++++++++++++++++++++++++++++
> >  1 file changed, 242 insertions(+)
> >  create mode 100644 Documentation/netlink/schema.yaml
> > 
> > diff --git a/Documentation/netlink/schema.yaml b/Documentation/netlink/schema.yaml
> > new file mode 100644
> > index 000000000000..1290aa4794ba
> > --- /dev/null
> > +++ b/Documentation/netlink/schema.yaml
> > @@ -0,0 +1,242 @@
> > +# SPDX-License-Identifier: GPL-2.0
> > +%YAML 1.2
> > +---
> > +$id: "http://kernel.org/schemas/netlink/schema.yaml#"
> > +$schema: "http://kernel.org/meta-schemas/core.yaml#"  
> 
> In case there's ever another one: meta-schemas/netlink/core.yaml
> 
> Or something similar.

Ack!

> > +
> > +title: Protocol
> > +description: Specification of a genetlink protocol
> > +type: object
> > +required: [ name, description, attribute-spaces, operations ]
> > +additionalProperties: False
> > +properties:
> > +  name:
> > +    description: Name of the genetlink family
> > +    type: string
> > +  description:  
> 
> It's better if your schema vocabulary is disjoint from jsonschema 
> vocabulary. From what I've seen, it's fairly common to get the 
> indentation off and jsonschema behavior is to ignore unknown keywords. 
> If the vocabularies are disjoint, you can write a meta-schema that only 
> allows jsonschema schema vocabulary at the right levels. Probably less 
> of an issue here as you don't have 1000s of schemas.

Ack, let me s/decription/doc/
 
> > +    description: Description of the family
> > +    type: string
> > +  version:
> > +    description: Version of the family as defined by genetlink.
> > +    type: integer  
> 
> Do you have the need to define the int size? We did our own keyword for 
> this, but since then I've looked at several other projects that have 
> used something like 'format: uint32'. There was some chatter about 
> trying to standardize this, but I haven't checked in a while.

It's 8 bits in theory (struct genlmsghdr::version), in practice it's
never used, and pretty much ignored. The jsonschema I have on Fedora
does not know about uint8.

> > +  attr-cnt-suffix:
> > +    description: Suffix for last member of attribute enum, default is "MAX".
> > +    type: string
> > +  headers:
> > +    description: C headers defining the protocol
> > +    type: object
> > +    additionalProperties: False
> > +    properties:
> > +      uapi:
> > +        description: Path under include/uapi where protocol definition is placed
> > +        type: string
> > +      kernel:
> > +        description: Additional headers on which the protocol definition depends (kernel side)
> > +        anyOf: &str-or-arrstr
> > +          -
> > +            type: array
> > +            items:
> > +              type: string
> > +          -
> > +            type: string
> > +      user:
> > +        description: Additional headers on which the protocol definition depends (user side)
> > +        anyOf: *str-or-arrstr  
> 
> For DT, we stick to a JSON compatible subset of YAML, so no anchors. The 
> jsonschema way to do this is using '$defs' (or 'definitions' before the 
> spec standardized it) and '$ref'.

I need to read up on this. Is it possible to extend a type?
We really need a way to define a narrow set of properties for "going
forward" while the old families have extra quirks. I couldn't find any
jsonschema docs on how the inherit and extend.

> > +  constants:
> > +    description: Enums and defines of the protocol
> > +    type: array
> > +    items:
> > +      type: object
> > +      required: [ type, name ]
> > +      additionalProperties: False
> > +      properties:
> > +        name:
> > +          type: string
> > +        type:
> > +          enum: [ enum, flags ]
> > +        value-prefix:
> > +          description: For enum the prefix of the values, optional.
> > +          type: string
> > +        value-start:
> > +          description: For enum the literal initializer for the first value.
> > +          oneOf: [ { type: string }, { type: integer }]  
> 
> I think you can do just 'type: [ string, integer ]'.

Works, thanks!

Re: [RFC net-next 2/4] ynl: add the schema for the schemas

[Rob Herring]

On Tue, Sep 27, 2022 at 4:56 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Mon, 26 Sep 2022 11:10:56 -0500 Rob Herring wrote:
> > On Wed, Aug 10, 2022 at 07:23:02PM -0700, Jakub Kicinski wrote:
> > > A schema in jsonschema format which should be familiar
> > > to dt-bindings writers. It looks kinda hard to read, TBH,
> > > I'm not sure how to make it better.

[...]

> > > +    description: Description of the family
> > > +    type: string
> > > +  version:
> > > +    description: Version of the family as defined by genetlink.
> > > +    type: integer
> >
> > Do you have the need to define the int size? We did our own keyword for
> > this, but since then I've looked at several other projects that have
> > used something like 'format: uint32'. There was some chatter about
> > trying to standardize this, but I haven't checked in a while.
>
> It's 8 bits in theory (struct genlmsghdr::version), in practice it's
> never used, and pretty much ignored. The jsonschema I have on Fedora
> does not know about uint8.

It wouldn't. It's some users of jsonschema that have added their own
thing. With python-jsonschema, you can add your own FormatChecker
class to handle custom 'format' entries.


> > > +  attr-cnt-suffix:
> > > +    description: Suffix for last member of attribute enum, default is "MAX".
> > > +    type: string
> > > +  headers:
> > > +    description: C headers defining the protocol
> > > +    type: object
> > > +    additionalProperties: False
> > > +    properties:
> > > +      uapi:
> > > +        description: Path under include/uapi where protocol definition is placed
> > > +        type: string
> > > +      kernel:
> > > +        description: Additional headers on which the protocol definition depends (kernel side)
> > > +        anyOf: &str-or-arrstr
> > > +          -
> > > +            type: array
> > > +            items:
> > > +              type: string
> > > +          -
> > > +            type: string
> > > +      user:
> > > +        description: Additional headers on which the protocol definition depends (user side)
> > > +        anyOf: *str-or-arrstr
> >
> > For DT, we stick to a JSON compatible subset of YAML, so no anchors. The
> > jsonschema way to do this is using '$defs' (or 'definitions' before the
> > spec standardized it) and '$ref'.
>
> I need to read up on this. Is it possible to extend a type?
> We really need a way to define a narrow set of properties for "going
> forward" while the old families have extra quirks. I couldn't find any
> jsonschema docs on how the inherit and extend.

You can add constraints, but you can't override what you inherit.

You do that with a $ref (and unevaluatedProperties if adding
properties) to the base schema and then add more schema constraints.
For example, we define something as an array with a $ref and then add
the bounds to it. That can also be done by 2 schemas applied
independently. However, if it's constraining properties present in an
object, then that can't be independent (that's where
unevaluatedProperties comes into play).

Rob

[RFC net-next 3/4] ynl: add a sample python library

[Jakub Kicinski]

A very short and very incomplete generic python library.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 tools/net/ynl/samples/ynl.py | 342 +++++++++++++++++++++++++++++++++++
 1 file changed, 342 insertions(+)
 create mode 100644 tools/net/ynl/samples/ynl.py

diff --git a/tools/net/ynl/samples/ynl.py b/tools/net/ynl/samples/ynl.py
new file mode 100644
index 000000000000..59c178e063f1
--- /dev/null
+++ b/tools/net/ynl/samples/ynl.py
@@ -0,0 +1,342 @@
+# SPDX-License-Identifier: BSD-3-Clause
+
+import functools
+import jsonschema
+import random
+import socket
+import struct
+import yaml
+
+#
+# Generic Netlink code which should really be in some library, but I can't quickly find one.
+#
+
+
+class Netlink:
+    SOL_NETLINK = 270
+
+    NETLINK_CAP_ACK = 10
+
+    NLMSG_ERROR = 2
+    NLMSG_DONE = 3
+
+    NLM_F_REQUEST = 1
+    NLM_F_ACK = 4
+    NLM_F_ROOT = 0x100
+    NLM_F_MATCH = 0x200
+    NLM_F_APPEND = 0x800
+
+    NLM_F_DUMP = NLM_F_ROOT | NLM_F_MATCH
+
+    NLA_F_NESTED = 0x8000
+    NLA_F_NET_BYTEORDER = 0x4000
+
+    NLA_TYPE_MASK = NLA_F_NESTED | NLA_F_NET_BYTEORDER
+
+    # Genetlink defines
+    NETLINK_GENERIC = 16
+
+    GENL_ID_CTRL = 0x10
+
+    # nlctrl
+    CTRL_CMD_GETFAMILY = 3
+
+    CTRL_ATTR_FAMILY_ID = 1
+    CTRL_ATTR_FAMILY_NAME = 2
+
+
+class NlAttr:
+    def __init__(self, raw, offset):
+        self._len, self._type = struct.unpack("HH", raw[offset:offset + 4])
+        self.type = self._type & ~Netlink.NLA_TYPE_MASK
+        self.payload_len = self._len
+        self.full_len = (self.payload_len + 3) & ~3
+        self.raw = raw[offset + 4:offset + self.payload_len]
+
+    def as_u16(self):
+        return struct.unpack("H", self.raw)[0]
+
+    def as_u32(self):
+        return struct.unpack("I", self.raw)[0]
+
+    def as_strz(self):
+        return self.raw.decode('ascii')[:-1]
+
+    def __repr__(self):
+        return f"[type:{self.type} len:{self._len}] {self.raw}"
+
+
+class NlAttrs:
+    def __init__(self, msg):
+        self.attrs = []
+
+        offset = 0
+        while offset < len(msg):
+            attr = NlAttr(msg, offset)
+            offset += attr.full_len
+            self.attrs.append(attr)
+
+    def __iter__(self):
+        yield from self.attrs
+
+
+class NlMsg:
+    def __init__(self, msg, offset):
+        self.hdr = msg[offset:offset + 16]
+
+        self.nl_len, self.nl_type, self.nl_flags, self.nl_seq, self.nl_portid = \
+            struct.unpack("IHHII", self.hdr)
+
+        self.raw = msg[offset + 16:offset + self.nl_len]
+
+        self.error = 0
+        self.done = 0
+
+        if self.nl_type == Netlink.NLMSG_ERROR:
+            self.error = struct.unpack("i", self.raw[0:4])[0]
+            self.done = 1
+        elif self.nl_type == Netlink.NLMSG_DONE:
+            self.done = 1
+
+    def __repr__(self):
+        msg = f"nl_len = {self.nl_len} nl_flags = 0x{self.nl_flags:x} nl_type = {self.nl_type}\n"
+        if self.error:
+            msg += '\terror: ' + str(self.error)
+        return msg
+
+
+class NlMsgs:
+    def __init__(self, data):
+        self.msgs = []
+
+        offset = 0
+        while offset < len(data):
+            msg = NlMsg(data, offset)
+            offset += msg.nl_len
+            self.msgs.append(msg)
+
+    def __iter__(self):
+        yield from self.msgs
+
+
+genl_family_name_to_id = None
+
+
+def _genl_msg(nl_type, nl_flags, genl_cmd, genl_version):
+    # we prepend length in _genl_msg_finalize()
+    nlmsg = struct.pack("HHII", nl_type, nl_flags, random.randint(1, 1024), 0)
+    genlmsg = struct.pack("bbH", genl_cmd, genl_version, 0)
+    return nlmsg + genlmsg
+
+
+def _genl_msg_finalize(msg):
+    return struct.pack("I", len(msg) + 4) + msg
+
+
+def _genl_load_families():
+    with socket.socket(socket.AF_NETLINK, socket.SOCK_RAW, Netlink.NETLINK_GENERIC) as sock:
+        sock.setsockopt(Netlink.SOL_NETLINK, Netlink.NETLINK_CAP_ACK, 1)
+
+        msg = _genl_msg(Netlink.GENL_ID_CTRL, Netlink.NLM_F_REQUEST | Netlink.NLM_F_ACK | Netlink.NLM_F_DUMP,
+                        Netlink.CTRL_CMD_GETFAMILY, 1)
+        msg = _genl_msg_finalize(msg)
+
+        sock.send(msg, 0)
+
+        global genl_family_name_to_id
+        genl_family_name_to_id = dict()
+
+        while True:
+            reply = sock.recv(128 * 1024)
+            nms = NlMsgs(reply)
+            for nl_msg in nms:
+                if nl_msg.error:
+                    print("Netlink error:", nl_msg.error)
+                    return
+                if nl_msg.done:
+                    return
+
+                gm = GenlMsg(nl_msg)
+                fam_id = None
+                fam_name = None
+                for attr in gm.raw_attrs:
+                    if attr.type == Netlink.CTRL_ATTR_FAMILY_ID:
+                        fam_id = attr.as_u16()
+                    elif attr.type == Netlink.CTRL_ATTR_FAMILY_NAME:
+                        fam_name = attr.as_strz()
+                if fam_id is not None and fam_name is not None:
+                    genl_family_name_to_id[fam_name] = fam_id
+
+
+class GenlMsg:
+    def __init__(self, nl_msg):
+        self.nl = nl_msg
+
+        self.hdr = nl_msg.raw[0:4]
+        self.raw = nl_msg.raw[4:]
+
+        self.genl_cmd, self.genl_version, _ = struct.unpack("bbH", self.hdr)
+
+        self.raw_attrs = NlAttrs(self.raw)
+
+    def __repr__(self):
+        msg = repr(self.nl)
+        msg += f"\tgenl_cmd = {self.genl_cmd} genl_ver = {self.genl_version}\n"
+        for a in self.raw_attrs:
+            msg += '\t\t' + repr(a) + '\n'
+        return msg
+
+
+class GenlFamily:
+    def __init__(self, family_name):
+        self.family_name = family_name
+
+        global genl_family_name_to_id
+        if genl_family_name_to_id is None:
+            _genl_load_families()
+
+        self.family_id = genl_family_name_to_id[family_name]
+
+#
+# YNL implementation details.
+#
+
+
+class YnlAttrSpace:
+    def __init__(self, family, yaml):
+        self.yaml = yaml
+
+        self.attrs = dict()
+        self.name = self.yaml['name']
+        self.name_prefix = self.yaml['name-prefix']
+        self.subspace_of = self.yaml['subspace-of'] if 'subspace-of' in self.yaml else None
+
+        val = 0
+        max_val = 0
+        for elem in self.yaml['attributes']:
+            if 'val' in elem:
+                val = elem['val']
+            else:
+                elem['val'] = val
+            if val > max_val:
+                max_val = val
+            val += 1
+
+            self.attrs[elem['name']] = elem
+
+        self.attr_list = [None] * (max_val + 1)
+        for elem in self.yaml['attributes']:
+            self.attr_list[elem['val']] = elem
+
+    def __getitem__(self, key):
+        return self.attrs[key]
+
+    def __contains__(self, key):
+        return key in self.yaml
+
+    def __iter__(self):
+        yield from self.attrs
+
+    def items(self):
+        return self.attrs.items()
+
+
+class YnlFamily:
+    def __init__(self, def_path, schema=None):
+        with open(def_path, "r") as stream:
+            self.yaml = yaml.safe_load(stream)
+
+        if schema:
+            with open(os.path.dirname(os.path.dirname(file_name)) + '/schema.yaml', "r") as stream:
+                schema = yaml.safe_load(stream)
+
+            jsonschema.validate(self.yaml, schema)
+
+        self.sock = socket.socket(socket.AF_NETLINK, socket.SOCK_RAW, Netlink.NETLINK_GENERIC)
+        self.sock.setsockopt(Netlink.SOL_NETLINK, Netlink.NETLINK_CAP_ACK, 1)
+
+        self._ops = dict()
+        self._spaces = dict()
+
+        for elem in self.yaml['attribute-spaces']:
+            self._spaces[elem['name']] = YnlAttrSpace(self, elem)
+
+        async_separation = 'async-prefix' in self.yaml['operations']
+        val = 0
+        for elem in self.yaml['operations']['list']:
+            if not (async_separation and ('notify' in elem or 'event' in elem)):
+                if 'val' in elem:
+                    val = elem['val']
+                else:
+                    elem['val'] = val
+                val += 1
+
+            self._ops[elem['name']] = elem
+
+            bound_f = functools.partial(self._op, elem['name'])
+            setattr(self, elem['name'], bound_f)
+
+        self.family = GenlFamily(self.yaml['name'])
+
+    def _add_attr(self, space, name, value):
+        attr = self._spaces[space][name]
+        nl_type = attr['val']
+        if attr["type"] == 'nest':
+            nl_type |= Netlink.NLA_F_NESTED
+            attr_payload = b''
+            for subname, subvalue in value.items():
+                attr_payload += self._add_attr(attr['nested-attributes'], subname, subvalue)
+        elif attr["type"] == 'u32':
+            attr_payload = struct.pack("I", int(value))
+        elif attr["type"] == 'nul-string':
+            attr_payload = str(value).encode('ascii') + b'\x00'
+        else:
+            raise Exception(f'Unknown type at {space} {name} {value} {attr["type"]}')
+
+        pad = b'\x00' * ((4 - len(attr_payload) % 4) % 4)
+        return struct.pack('HH', len(attr_payload) + 4, nl_type) + attr_payload + pad
+
+    def _decode(self, attrs, space):
+        attr_space = self._spaces[space]
+        rsp = dict()
+        for attr in attrs:
+            attr_spec = attr_space.attr_list[attr.type]
+            if attr_spec["type"] == 'nest':
+                subdict = self._decode(NlAttrs(attr.raw), attr_spec['nested-attributes'])
+                rsp[attr_spec['name']] = subdict
+            elif attr_spec['type'] == 'u32':
+                rsp[attr_spec['name']] = attr.as_u32()
+            elif attr_spec["type"] == 'nul-string':
+                rsp[attr_spec['name']] = attr.as_strz()
+            else:
+                raise Exception(f'Unknown {attr.type} {attr_spec["name"]} {attr_spec["type"]}')
+        return rsp
+
+    def _op(self, method, vals):
+        op = self._ops[method]
+
+        msg = _genl_msg(self.family.family_id, Netlink.NLM_F_REQUEST | Netlink.NLM_F_ACK,
+                        op['val'], 1)
+        for name, value in vals.items():
+            msg += self._add_attr(op['attribute-space'], name, value)
+        msg = _genl_msg_finalize(msg)
+
+        self.sock.send(msg, 0)
+
+        done = False
+        rsp = None
+        while not done:
+            reply = self.sock.recv(128 * 1024)
+            nms = NlMsgs(reply)
+            for nl_msg in nms:
+                if nl_msg.error:
+                    print("Netlink error:", nl_msg.error)
+                    return
+                if nl_msg.done:
+                    done = True
+                    break
+
+                gm = GenlMsg(nl_msg)
+                rsp = self._decode(gm.raw_attrs, op['attribute-space'])
+
+        return rsp
-- 
2.37.1

Re: [RFC net-next 3/4] ynl: add a sample python library

[Benjamin Poirier]

On 2022-08-10 19:23 -0700, Jakub Kicinski wrote:
> A very short and very incomplete generic python library.
> 
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> ---
>  tools/net/ynl/samples/ynl.py | 342 +++++++++++++++++++++++++++++++++++
>  1 file changed, 342 insertions(+)
>  create mode 100644 tools/net/ynl/samples/ynl.py
> 
> diff --git a/tools/net/ynl/samples/ynl.py b/tools/net/ynl/samples/ynl.py
> new file mode 100644
> index 000000000000..59c178e063f1
> --- /dev/null
> +++ b/tools/net/ynl/samples/ynl.py
> @@ -0,0 +1,342 @@
[...]
> +class YnlFamily:
> +    def __init__(self, def_path, schema=None):
> +        with open(def_path, "r") as stream:
> +            self.yaml = yaml.safe_load(stream)
> +
> +        if schema:
> +            with open(os.path.dirname(os.path.dirname(file_name)) + '/schema.yaml', "r") as stream:
> +                schema = yaml.safe_load(stream)
> +
> +            jsonschema.validate(self.yaml, schema)
> +

The schema validation part was not working. I got it going with the
following changes. It then flags some problems in ethtool.yaml.

diff --git a/tools/net/ynl/samples/ethtool.py b/tools/net/ynl/samples/ethtool.py
index 63c8e29f8e5d..4c5a4629748d 100755
--- a/tools/net/ynl/samples/ethtool.py
+++ b/tools/net/ynl/samples/ethtool.py
@@ -14,7 +14,7 @@ def main():
     parser.add_argument('--ifindex', dest='ifindex', type=str)
     args = parser.parse_args()
 
-    ynl = YnlFamily(args.spec)
+    ynl = YnlFamily(args.spec, args.schema)
 
     if args.dev_name:
         channels = ynl.channels_get({'header': {'dev_name': args.dev_name}})
diff --git a/tools/net/ynl/samples/ynl.py b/tools/net/ynl/samples/ynl.py
index 59c178e063f1..35c894b0ec19 100644
--- a/tools/net/ynl/samples/ynl.py
+++ b/tools/net/ynl/samples/ynl.py
@@ -247,7 +247,7 @@ class YnlFamily:
             self.yaml = yaml.safe_load(stream)
 
         if schema:
-            with open(os.path.dirname(os.path.dirname(file_name)) + '/schema.yaml', "r") as stream:
+            with open(schema, "r") as stream:
                 schema = yaml.safe_load(stream)
 
             jsonschema.validate(self.yaml, schema)

Re: [RFC net-next 3/4] ynl: add a sample python library

[Jakub Kicinski]

On Thu, 11 Aug 2022 14:48:49 +0900 Benjamin Poirier wrote:
> The schema validation part was not working. I got it going with the
> following changes. It then flags some problems in ethtool.yaml.
> 
> diff --git a/tools/net/ynl/samples/ethtool.py b/tools/net/ynl/samples/ethtool.py
> index 63c8e29f8e5d..4c5a4629748d 100755
> --- a/tools/net/ynl/samples/ethtool.py
> +++ b/tools/net/ynl/samples/ethtool.py
> @@ -14,7 +14,7 @@ def main():
>      parser.add_argument('--ifindex', dest='ifindex', type=str)
>      args = parser.parse_args()
>  
> -    ynl = YnlFamily(args.spec)
> +    ynl = YnlFamily(args.spec, args.schema)
>  
>      if args.dev_name:
>          channels = ynl.channels_get({'header': {'dev_name': args.dev_name}})
> diff --git a/tools/net/ynl/samples/ynl.py b/tools/net/ynl/samples/ynl.py
> index 59c178e063f1..35c894b0ec19 100644
> --- a/tools/net/ynl/samples/ynl.py
> +++ b/tools/net/ynl/samples/ynl.py
> @@ -247,7 +247,7 @@ class YnlFamily:
>              self.yaml = yaml.safe_load(stream)
>  
>          if schema:
> -            with open(os.path.dirname(os.path.dirname(file_name)) + '/schema.yaml', "r") as stream:
> +            with open(schema, "r") as stream:
>                  schema = yaml.safe_load(stream)
>  
>              jsonschema.validate(self.yaml, schema)

Hah, thanks! Looks like I also changed my mind between 'val' and 'value'
for the explicit value of the enum item. I'll correct those.

Re: [RFC net-next 3/4] ynl: add a sample python library

[Stephen Hemminger]

Looks interesting, you might want to consider running your code
through some of the existing Python checkers such as flake8 and pylint.
If you want this to be generally available in repos, best to follow the language conventions

For example flake8 noticed:
 $ flake8 --max-line-length=120 ./tools/net/ynl/samples/ynl.py 
./tools/net/ynl/samples/ynl.py:251:55: F821 undefined name 'file_name'

And pylint has more warnings
tools/net/ynl/samples/ynl.py:1:0: C0114: Missing module docstring (missing-module-docstring)
tools/net/ynl/samples/ynl.py:4:0: E0401: Unable to import 'jsonschema' (import-error)
tools/net/ynl/samples/ynl.py:16:0: C0115: Missing class docstring (missing-class-docstring)
tools/net/ynl/samples/ynl.py:16:0: R0903: Too few public methods (0/2) (too-few-public-methods)
tools/net/ynl/samples/ynl.py:49:0: C0115: Missing class docstring (missing-class-docstring)
tools/net/ynl/samples/ynl.py:57:4: C0116: Missing function or method docstring (missing-function-docstring)
tools/net/ynl/samples/ynl.py:60:4: C0116: Missing function or method docstring (missing-function-docstring)
tools/net/ynl/samples/ynl.py:63:4: C0116: Missing function or method docstring (missing-function-docstring)
tools/net/ynl/samples/ynl.py:70:0: C0115: Missing class docstring (missing-class-docstring)
tools/net/ynl/samples/ynl.py:70:0: R0903: Too few public methods (1/2) (too-few-public-methods)
tools/net/ynl/samples/ynl.py:84:0: C0115: Missing class docstring (missing-class-docstring)
tools/net/ynl/samples/ynl.py:84:0: R0902: Too many instance attributes (9/7) (too-many-instance-attributes)
tools/net/ynl/samples/ynl.py:84:0: R0903: Too few public methods (1/2) (too-few-public-methods)
tools/net/ynl/samples/ynl.py:109:0: C0115: Missing class docstring (missing-class-docstring)
tools/net/ynl/samples/ynl.py:109:0: R0903: Too few public methods (1/2) (too-few-public-methods)
tools/net/ynl/samples/ynl.py:123:0: C0103: Constant name "genl_family_name_to_id" doesn't conform to UPPER_CASE naming style (invalid-name)
tools/net/ynl/samples/ynl.py:147:8: W0603: Using the global statement (global-statement)
tools/net/ynl/samples/ynl.py:147:8: C0103: Constant name "genl_family_name_to_id" doesn't conform to UPPER_CASE naming style (invalid-name)
tools/net/ynl/samples/ynl.py:148:33: R1735: Consider using {} instead of dict() (use-dict-literal)
tools/net/ynl/samples/ynl.py:160:16: C0103: Variable name "gm" doesn't conform to snake_case naming style (invalid-name)
tools/net/ynl/samples/ynl.py:174:8: C0103: Attribute name "nl" doesn't conform to snake_case naming style (invalid-name)
tools/net/ynl/samples/ynl.py:172:0: C0115: Missing class docstring (missing-class-docstring)
tools/net/ynl/samples/ynl.py:186:12: C0103: Variable name "a" doesn't conform to snake_case naming style (invalid-name)
tools/net/ynl/samples/ynl.py:172:0: R0903: Too few public methods (1/2) (too-few-public-methods)
tools/net/ynl/samples/ynl.py:191:0: C0115: Missing class docstring (missing-class-docstring)
tools/net/ynl/samples/ynl.py:195:8: W0602: Using global for 'genl_family_name_to_id' but no assignment is done (global-variable-not-assigned)
tools/net/ynl/samples/ynl.py:195:8: C0103: Constant name "genl_family_name_to_id" doesn't conform to UPPER_CASE naming style (invalid-name)
tools/net/ynl/samples/ynl.py:191:0: R0903: Too few public methods (0/2) (too-few-public-methods)
tools/net/ynl/samples/ynl.py:206:0: C0115: Missing class docstring (missing-class-docstring)
tools/net/ynl/samples/ynl.py:207:31: W0621: Redefining name 'yaml' from outer scope (line 8) (redefined-outer-name)
tools/net/ynl/samples/ynl.py:210:21: R1735: Consider using {} instead of dict() (use-dict-literal)
tools/net/ynl/samples/ynl.py:207:23: W0613: Unused argument 'family' (unused-argument)
tools/net/ynl/samples/ynl.py:241:4: C0116: Missing function or method docstring (missing-function-docstring)
tools/net/ynl/samples/ynl.py:245:0: C0115: Missing class docstring (missing-class-docstring)
tools/net/ynl/samples/ynl.py:247:13: W1514: Using open without explicitly specifying an encoding (unspecified-encoding)
tools/net/ynl/samples/ynl.py:251:17: W1514: Using open without explicitly specifying an encoding (unspecified-encoding)
tools/net/ynl/samples/ynl.py:251:54: E0602: Undefined variable 'file_name' (undefined-variable)
tools/net/ynl/samples/ynl.py:259:20: R1735: Consider using {} instead of dict() (use-dict-literal)
tools/net/ynl/samples/ynl.py:260:23: R1735: Consider using {} instead of dict() (use-dict-literal)
tools/net/ynl/samples/ynl.py:302:14: R1735: Consider using {} instead of dict() (use-dict-literal)
tools/net/ynl/samples/ynl.py:317:8: C0103: Variable name "op" doesn't conform to snake_case naming style (invalid-name)
tools/net/ynl/samples/ynl.py:340:16: C0103: Variable name "gm" doesn't conform to snake_case naming style (invalid-name)
tools/net/ynl/samples/ynl.py:316:4: R1710: Either all return statements in a function should return an expression, or none of them should. (inconsistent-return-statements)
tools/net/ynl/samples/ynl.py:245:0: R0903: Too few public methods (0/2) (too-few-public-methods)
tools/net/ynl/samples/ynl.py:5:0: C0411: standard import "import random" should be placed before "import jsonschema" (wrong-import-order)
tools/net/ynl/samples/ynl.py:6:0: C0411: standard import "import socket" should be placed before "import jsonschema" (wrong-import-order)
tools/net/ynl/samples/ynl.py:7:0: C0411: standard import "import struct" should be placed before "import jsonschema" (wrong-import-order)
tools/net/ynl/samples/ynl.py:9:0: C0411: standard import "import os" should be placed before "import jsonschema" (wrong-import-order)

------------------------------------------------------------------
Your code has been rated at 7.64/10 (previous run: 7.64/10, +0.00)

Re: [RFC net-next 3/4] ynl: add a sample python library

[Jakub Kicinski]

On Thu, 11 Aug 2022 13:09:06 -0700 Stephen Hemminger wrote:
> Looks interesting, you might want to consider running your code
> through some of the existing Python checkers such as flake8 and pylint.
> If you want this to be generally available in repos, best to follow the language conventions
> 
> For example flake8 noticed:
>  $ flake8 --max-line-length=120 ./tools/net/ynl/samples/ynl.py 
> ./tools/net/ynl/samples/ynl.py:251:55: F821 undefined name 'file_name'

Thanks! I'll make sure to check flake8 (pylint is too noisy for me :()

Re: [RFC net-next 3/4] ynl: add a sample python library

[Johannes Berg]

On Fri, 2022-08-12 at 15:53 -0700, Jakub Kicinski wrote:
> On Thu, 11 Aug 2022 13:09:06 -0700 Stephen Hemminger wrote:
> > Looks interesting, you might want to consider running your code
> > through some of the existing Python checkers such as flake8 and pylint.
> > If you want this to be generally available in repos, best to follow the language conventions
> > 
> > For example flake8 noticed:
> >  $ flake8 --max-line-length=120 ./tools/net/ynl/samples/ynl.py 
> > ./tools/net/ynl/samples/ynl.py:251:55: F821 undefined name 'file_name'
> 
> Thanks! I'll make sure to check flake8 (pylint is too noisy for me :()

FWIW, I've come to really believe in also adding type annotations (and
checking them with mypy, of course, I even use --strict), which has
helped even my smaller projects a lot. YMMV, but it might be something
to look into.

johannes

Re: [RFC net-next 3/4] ynl: add a sample python library

[Stephen Hemminger]

On Wed, 10 Aug 2022 19:23:03 -0700
Jakub Kicinski <kuba@kernel.org> wrote:

> A very short and very incomplete generic python library.
> 
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>

It would be great if python had standard module for netlink.
Then your code could just (re)use that.
Something like mnl but for python.

Re: [RFC net-next 3/4] ynl: add a sample python library

[Edward Cree]

On 12/08/2022 02:04, Stephen Hemminger wrote:
> On Wed, 10 Aug 2022 19:23:03 -0700
> Jakub Kicinski <kuba@kernel.org> wrote:
> 
>> A very short and very incomplete generic python library.
>>
>> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> 
> It would be great if python had standard module for netlink.
> Then your code could just (re)use that.
> Something like mnl but for python.

There's pyroute2, that seemed alright when I used it for something
 a few years back, and I think it has the pieces you need.
https://pyroute2.org/

-ed

Re: [RFC net-next 3/4] ynl: add a sample python library

[Jakub Kicinski]

On Fri, 12 Aug 2022 16:42:53 +0100 Edward Cree wrote:
> > It would be great if python had standard module for netlink.
> > Then your code could just (re)use that.
> > Something like mnl but for python.  
> 
> There's pyroute2, that seemed alright when I used it for something
>  a few years back, and I think it has the pieces you need.
> https://pyroute2.org/

I saw that and assumed that its true to its name and only supports
RTNL :( I'll reach out to Peter the maintainer for his thoughts. 

This patch was meant as a sample, I kept trying to finish up the C
codegen for a week and it still didn't feel presentable enough for 
the RFC. In practice I'd rather leave writing the language specific 
libs to the experts.

RE: [RFC net-next 3/4] ynl: add a sample python library

[Keller, Jacob E]

> -----Original Message-----
> From: Jakub Kicinski <kuba@kernel.org>
> Sent: Friday, August 12, 2022 4:08 PM
> To: Edward Cree <ecree.xilinx@gmail.com>
> Cc: Stephen Hemminger <stephen@networkplumber.org>;
> netdev@vger.kernel.org; davem@davemloft.net; edumazet@google.com;
> pabeni@redhat.com; sdf@google.com; Keller, Jacob E
> <jacob.e.keller@intel.com>; vadfed@fb.com; johannes@sipsolutions.net;
> jiri@resnulli.us; dsahern@kernel.org; fw@strlen.de; linux-doc@vger.kernel.org
> Subject: Re: [RFC net-next 3/4] ynl: add a sample python library
> 
> On Fri, 12 Aug 2022 16:42:53 +0100 Edward Cree wrote:
> > > It would be great if python had standard module for netlink.
> > > Then your code could just (re)use that.
> > > Something like mnl but for python.
> >
> > There's pyroute2, that seemed alright when I used it for something
> >  a few years back, and I think it has the pieces you need.
> > https://pyroute2.org/
> 
> I saw that and assumed that its true to its name and only supports
> RTNL :( I'll reach out to Peter the maintainer for his thoughts.
> 
> This patch was meant as a sample, I kept trying to finish up the C
> codegen for a week and it still didn't feel presentable enough for
> the RFC. In practice I'd rather leave writing the language specific
> libs to the experts.

It has some generic netlink support, but the way it parses messages was a bit confusing to me. It could benefit from the automatic generation of attribute parsing though!

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

[Jakub Kicinski]

A sample schema describing ethtool channels and a script showing
that it works. The script is called like this:

 ./tools/net/ynl/samples/ethtool.py \
    --spec Documentation/netlink/bindings/ethtool.yaml \
    --device eth0

I have schemas for genetlink, FOU and the proposed DPLL subsystem,
to validate that the concept has wide applicability, but ethtool
feels like the best demo of the 4.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 Documentation/netlink/bindings/ethtool.yaml | 115 ++++++++++++++++++++
 tools/net/ynl/samples/ethtool.py            |  30 +++++
 2 files changed, 145 insertions(+)
 create mode 100644 Documentation/netlink/bindings/ethtool.yaml
 create mode 100755 tools/net/ynl/samples/ethtool.py

diff --git a/Documentation/netlink/bindings/ethtool.yaml b/Documentation/netlink/bindings/ethtool.yaml
new file mode 100644
index 000000000000..b4540d60b4b3
--- /dev/null
+++ b/Documentation/netlink/bindings/ethtool.yaml
@@ -0,0 +1,115 @@
+# SPDX-License-Identifier: BSD-3-Clause
+
+name: ethtool
+
+description: |
+  Ethernet device configuration interface.
+
+attr-cnt-suffix: CNT
+
+attribute-spaces:
+  -
+    name: header
+    name-prefix: ETHTOOL_A_HEADER_
+    attributes:
+      -
+        name: dev_index
+        val: 1
+        type: u32
+      -
+        name: dev_name
+        type: nul-string
+        len: ALTIFNAMSIZ - 1
+      -
+        name: flags
+        type: u32
+  -
+    name: channels
+    name-prefix: ETHTOOL_A_CHANNELS_
+    attributes:
+      -
+        name: header
+        val: 1
+        type: nest
+        nested-attributes: header
+      -
+        name: rx_max
+        type: u32
+      -
+        name: tx_max
+        type: u32
+      -
+        name: other_max
+        type: u32
+      -
+        name: combined_max
+        type: u32
+      -
+        name: rx_count
+        type: u32
+      -
+        name: tx_count
+        type: u32
+      -
+        name: other_count
+        type: u32
+      -
+        name: combined_count
+        type: u32
+
+headers:
+  user: linux/if.h
+  uapi: linux/ethtool_netlink.h
+
+operations:
+  name-prefix: ETHTOOL_MSG_
+  async-prefix: ETHTOOL_MSG_
+  list:
+    -
+      name: channels_get
+      val: 17
+      description: Get current and max supported number of channels.
+      attribute-space: channels
+      do:
+        request:
+          attributes:
+            - header
+        reply: &channel_reply
+          attributes:
+            - header
+            - rx_max
+            - tx_max
+            - other_max
+            - combined_max
+            - rx_count
+            - tx_count
+            - other_count
+            - combined_count
+      dump:
+        reply: *channel_reply
+
+    -
+      name: channels_ntf
+      description: Notification for device changing its number of channels.
+      notify: channels_get
+      mcgrp: monitor
+
+    -
+      name: channels_set
+      description: Set number of channels.
+      attribute-space: channels
+      do:
+        request:
+          attributes:
+            - header
+            - rx_count
+            - tx_count
+            - other_count
+            - combined_count
+
+mcast-groups:
+  name-prefix: ETHTOOL_MCGRP_
+  name-suffix: _NAME
+  list:
+    -
+      name: monitor
diff --git a/tools/net/ynl/samples/ethtool.py b/tools/net/ynl/samples/ethtool.py
new file mode 100755
index 000000000000..63c8e29f8e5d
--- /dev/null
+++ b/tools/net/ynl/samples/ethtool.py
@@ -0,0 +1,30 @@
+#!/usr/bin/env python
+# SPDX-License-Identifier: BSD-3-Clause
+
+import argparse
+
+from ynl import YnlFamily
+
+
+def main():
+    parser = argparse.ArgumentParser(description='YNL ethtool sample')
+    parser.add_argument('--spec', dest='spec', type=str, required=True)
+    parser.add_argument('--schema', dest='schema', type=str)
+    parser.add_argument('--device', dest='dev_name', type=str)
+    parser.add_argument('--ifindex', dest='ifindex', type=str)
+    args = parser.parse_args()
+
+    ynl = YnlFamily(args.spec)
+
+    if args.dev_name:
+        channels = ynl.channels_get({'header': {'dev_name': args.dev_name}})
+    elif args.ifindex:
+        channels = ynl.channels_get({'header': {'dev_index': args.ifindex}})
+    else:
+        return
+    print('Netlink responded with:')
+    print(channels)
+
+
+if __name__ == "__main__":
+    main()
-- 
2.37.1

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

[sdf]

On 08/10, Jakub Kicinski wrote:
> A sample schema describing ethtool channels and a script showing
> that it works. The script is called like this:

>   ./tools/net/ynl/samples/ethtool.py \
>      --spec Documentation/netlink/bindings/ethtool.yaml \
>      --device eth0

> I have schemas for genetlink, FOU and the proposed DPLL subsystem,
> to validate that the concept has wide applicability, but ethtool
> feels like the best demo of the 4.

Looks super promising! I'm going to comment mostly here because it's easier
to talk about the sample schema definition.

> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> ---
>   Documentation/netlink/bindings/ethtool.yaml | 115 ++++++++++++++++++++
>   tools/net/ynl/samples/ethtool.py            |  30 +++++
>   2 files changed, 145 insertions(+)
>   create mode 100644 Documentation/netlink/bindings/ethtool.yaml
>   create mode 100755 tools/net/ynl/samples/ethtool.py

> diff --git a/Documentation/netlink/bindings/ethtool.yaml  
> b/Documentation/netlink/bindings/ethtool.yaml
> new file mode 100644
> index 000000000000..b4540d60b4b3
> --- /dev/null
> +++ b/Documentation/netlink/bindings/ethtool.yaml
> @@ -0,0 +1,115 @@
> +# SPDX-License-Identifier: BSD-3-Clause
> +
> +name: ethtool
> +
> +description: |
> +  Ethernet device configuration interface.
> +

[..]

> +attr-cnt-suffix: CNT

Is it a hack to make the generated header fit into existing
implementation? Should we #define ETHTOOL_XXX_CNT ETHTOOL_XXX in
the implementation instead? (or s/ETHTOOL_XXX_CNT/ETHTOOL_XXX/ the
source itself?)

> +attribute-spaces:

Are you open to bike shedding? :-) I like how ethtool_netlink.h calls
these 'message types'.

> +  -
> +    name: header
> +    name-prefix: ETHTOOL_A_HEADER_

Any issue with name-prefix+name-suffix being non-greppable? Have you tried
something like this instead:

- name: ETHTOOL_A_HEADER # this is fake, for ynl reference only
   attributes:
     - name: ETHTOOL_A_HEADER_DEV_INDEX
       val:
       type:
     - name ETHTOOL_A_HEADER_DEV_NAME
       ..

It seems a bit easier to map the spec into what it's going to produce.
For example, it took me a while to translate 'channels_get' below into
ETHTOOL_MSG_CHANNELS_GET.

Or is it too much ETHTOOL_A_HEADER_?

> +    attributes:
> +      -
> +        name: dev_index
> +        val: 1
> +        type: u32
> +      -
> +        name: dev_name
> +        type: nul-string

[..]

> +        len: ALTIFNAMSIZ - 1

Not sure how strict you want to be here. ALTIFNAMSIZ is defined
somewhere else it seems? (IOW, do we want to have implicit dependencies
on external/uapi headers?)

> +      -
> +        name: flags
> +        type: u32
> +  -
> +    name: channels
> +    name-prefix: ETHTOOL_A_CHANNELS_
> +    attributes:
> +      -
> +        name: header
> +        val: 1
> +        type: nest
> +        nested-attributes: header
> +      -
> +        name: rx_max
> +        type: u32
> +      -
> +        name: tx_max
> +        type: u32
> +      -
> +        name: other_max
> +        type: u32
> +      -
> +        name: combined_max
> +        type: u32
> +      -
> +        name: rx_count
> +        type: u32
> +      -
> +        name: tx_count
> +        type: u32
> +      -
> +        name: other_count
> +        type: u32
> +      -
> +        name: combined_count
> +        type: u32
> +
> +headers:
> +  user: linux/if.h
> +  uapi: linux/ethtool_netlink.h
> +
> +operations:
> +  name-prefix: ETHTOOL_MSG_
> +  async-prefix: ETHTOOL_MSG_
> +  list:
> +    -
> +      name: channels_get
> +      val: 17
> +      description: Get current and max supported number of channels.
> +      attribute-space: channels
> +      do:
> +        request:
> +          attributes:
> +            - header
> +        reply: &channel_reply
> +          attributes:
> +            - header
> +            - rx_max
> +            - tx_max
> +            - other_max
> +            - combined_max
> +            - rx_count
> +            - tx_count
> +            - other_count
> +            - combined_count
> +      dump:
> +        reply: *channel_reply
> +
> +    -
> +      name: channels_ntf
> +      description: Notification for device changing its number of  
> channels.
> +      notify: channels_get
> +      mcgrp: monitor
> +
> +    -
> +      name: channels_set
> +      description: Set number of channels.
> +      attribute-space: channels
> +      do:
> +        request:
> +          attributes:

[..]

> +            - header
> +            - rx_count
> +            - tx_count
> +            - other_count
> +            - combined_count

My netlink is super rusty, might be worth mentioning in the spec: these
are possible attributes, but are all of them required?

You also mention the validation part in the cover letter, do you plan
add some of these policy properties to the spec or everything is
there already? (I'm assuming we care about the types which we have above and
optional/required attribute indication?)

> +
> +mcast-groups:
> +  name-prefix: ETHTOOL_MCGRP_
> +  name-suffix: _NAME
> +  list:
> +    -
> +      name: monitor
> diff --git a/tools/net/ynl/samples/ethtool.py  
> b/tools/net/ynl/samples/ethtool.py
> new file mode 100755
> index 000000000000..63c8e29f8e5d
> --- /dev/null
> +++ b/tools/net/ynl/samples/ethtool.py
> @@ -0,0 +1,30 @@
> +#!/usr/bin/env python
> +# SPDX-License-Identifier: BSD-3-Clause
> +
> +import argparse
> +
> +from ynl import YnlFamily
> +
> +
> +def main():
> +    parser = argparse.ArgumentParser(description='YNL ethtool sample')
> +    parser.add_argument('--spec', dest='spec', type=str, required=True)
> +    parser.add_argument('--schema', dest='schema', type=str)
> +    parser.add_argument('--device', dest='dev_name', type=str)
> +    parser.add_argument('--ifindex', dest='ifindex', type=str)
> +    args = parser.parse_args()
> +
> +    ynl = YnlFamily(args.spec)
> +
> +    if args.dev_name:
> +        channels = ynl.channels_get({'header': {'dev_name':  
> args.dev_name}})
> +    elif args.ifindex:
> +        channels = ynl.channels_get({'header': {'dev_index':  
> args.ifindex}})
> +    else:
> +        return
> +    print('Netlink responded with:')
> +    print(channels)
> +
> +
> +if __name__ == "__main__":
> +    main()
> --
> 2.37.1

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

[Jakub Kicinski]

On Thu, 11 Aug 2022 09:18:03 -0700 sdf@google.com wrote:
> > +attr-cnt-suffix: CNT  
> 
> Is it a hack to make the generated header fit into existing
> implementation?

Yup.

> Should we #define ETHTOOL_XXX_CNT ETHTOOL_XXX in
> the implementation instead? (or s/ETHTOOL_XXX_CNT/ETHTOOL_XXX/ the
> source itself?)

We could, I guess. To be clear this controls the count, IOW:

enum {
	PREFIX_A_BLA_ATTR = 1,
	PREFIX_A_ANOTHER_ATTR,
	PREFIX_A_AND_ONEMORE,
	__PFREIX_A_CNT, // <--- This thing
};
#define PREFIX_A_MAX (__PFREIX_A_CNT - 1)

It's not used in the generated code, only if we codegen the uAPI,
AFAIR. So we'd need a way to tell the generator of the uAPI about
the situation, anyway. I could be misremembering.

> > +attribute-spaces:  
> 
> Are you open to bike shedding? :-)

I can't make promises that I'll change things but I'm curious 
to hear it!

> I like how ethtool_netlink.h calls these 'message types'.

It calls operation types message types, not attr spaces.
I used ops because that's what genetlink calls things.

> > +  -
> > +    name: header
> > +    name-prefix: ETHTOOL_A_HEADER_  
> 
> Any issue with name-prefix+name-suffix being non-greppable? Have you tried
> something like this instead:
> 
> - name: ETHTOOL_A_HEADER # this is fake, for ynl reference only
>    attributes:
>      - name: ETHTOOL_A_HEADER_DEV_INDEX
>        val:
>        type:
>      - name ETHTOOL_A_HEADER_DEV_NAME
>        ..
> 
> It seems a bit easier to map the spec into what it's going to produce.
> For example, it took me a while to translate 'channels_get' below into
> ETHTOOL_MSG_CHANNELS_GET.
> 
> Or is it too much ETHTOOL_A_HEADER_?

Dunno, that'd mean that the Python method is called
ETHTOOL_MSG_CHANNELS_GET rather than just channels_get.
I don't want to force all languages to use the C naming.
The C naming just leads to silly copy'n'paste issues like
f329a0ebeab.

> > +        len: ALTIFNAMSIZ - 1  
> 
> Not sure how strict you want to be here. ALTIFNAMSIZ is defined
> somewhere else it seems? (IOW, do we want to have implicit dependencies
> on external/uapi headers?)

Good catch, I'm aware. I was planning to add a "header constants" 
section or some such. A section in "headers" which defines the 
constants which C code will get from the headers.

For Python it does not matter, as we don't have to size arrays.
I was wondering if it will matter for other languages, like Rust?

> > +            - header
> > +            - rx_count
> > +            - tx_count
> > +            - other_count
> > +            - combined_count  
> 
> My netlink is super rusty, might be worth mentioning in the spec: these
> are possible attributes, but are all of them required?

Right, will do, nothing is required, or rather requirements are kinda
hard to express and checked by the code in the kernel.

> You also mention the validation part in the cover letter, do you plan
> add some of these policy properties to the spec or everything is
> there already? (I'm assuming we care about the types which we have above and
> optional/required attribute indication?)

Yeah, my initial plan was to encode requirement in the policy but its
not trivial. So I left it as future extension. Besides things which are
required today may not be tomorrow, so its a bit of a strange thing.

Regarding policy properties I'm intending to support all of the stuff
that the kernel policies recognize... but somehow most families I
converted don't have validation (only mask and length :S).

Actually for DPLL I have a nice validation trick. You can define an
enum:

constants:
  -
    type: flags
    name: genl_get_flags
    value-prefix: DPLL_FLAG_
    values: [ sources, outputs, status ]

Then for an attribute you link it:

      -
        name: flags
        type: u32
        flags-mask: genl_get_flags

And that will auto an enum:

enum dpll_genl_get_flags {
	DPLL_FLAG_SOURCES = 1,
	DPLL_FLAG_OUTPUTS = 2,
	DPLL_FLAG_STATUS = 4,
};

And a policy with a mask:

	[DPLLA_FLAGS] = NLA_POLICY_MASK(NLA_U32, 0x7),

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

[Stanislav Fomichev]

On Thu, Aug 11, 2022 at 12:35 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Thu, 11 Aug 2022 09:18:03 -0700 sdf@google.com wrote:
> > > +attr-cnt-suffix: CNT
> >
> > Is it a hack to make the generated header fit into existing
> > implementation?
>
> Yup.
>
> > Should we #define ETHTOOL_XXX_CNT ETHTOOL_XXX in
> > the implementation instead? (or s/ETHTOOL_XXX_CNT/ETHTOOL_XXX/ the
> > source itself?)
>
> We could, I guess. To be clear this controls the count, IOW:
>
> enum {
>         PREFIX_A_BLA_ATTR = 1,
>         PREFIX_A_ANOTHER_ATTR,
>         PREFIX_A_AND_ONEMORE,
>         __PFREIX_A_CNT, // <--- This thing
> };
> #define PREFIX_A_MAX (__PFREIX_A_CNT - 1)
>
> It's not used in the generated code, only if we codegen the uAPI,
> AFAIR. So we'd need a way to tell the generator of the uAPI about
> the situation, anyway. I could be misremembering.

My worry is that we'll have more hacks like these and it's hard, as a
spec reader/writer, to figure out that they exist..
So I was wondering if it's "easier" (from the spec reader/writer pov)
to have some c-header-fixup: section where we can have plain
c-preprocessor hacks like these (where we need to redefine something
to match the old behavior).

> > > +attribute-spaces:
> >
> > Are you open to bike shedding? :-)
>
> I can't make promises that I'll change things but I'm curious
> to hear it!
>
> > I like how ethtool_netlink.h calls these 'message types'.
>
> It calls operation types message types, not attr spaces.
> I used ops because that's what genetlink calls things.

Coming from stubby/grpc, I was expecting to see words like
message/field/struct. The question is what's more confusing: sticking
with netlink naming or trying to map grpc/thrift concepts on top of
what we have. (I'm assuming more people know about grpc/thrift than
netlink)

messages: # or maybe 'attribute-sets' ?
  - name: channels
    ...

operations:
  - name: channel_get
    message: channels
    do:
      request:
        fields:
        - header
        - rx_max

Or maybe all we really need is a section in the doc called 'Netlink
for gRPC/Thrift users' where we map these concepts:
- attribute-spaces (attribute-sets?) -> messages
- attributes -> fields

> > > +  -
> > > +    name: header
> > > +    name-prefix: ETHTOOL_A_HEADER_
> >
> > Any issue with name-prefix+name-suffix being non-greppable? Have you tried
> > something like this instead:
> >
> > - name: ETHTOOL_A_HEADER # this is fake, for ynl reference only
> >    attributes:
> >      - name: ETHTOOL_A_HEADER_DEV_INDEX
> >        val:
> >        type:
> >      - name ETHTOOL_A_HEADER_DEV_NAME
> >        ..
> >
> > It seems a bit easier to map the spec into what it's going to produce.
> > For example, it took me a while to translate 'channels_get' below into
> > ETHTOOL_MSG_CHANNELS_GET.
> >
> > Or is it too much ETHTOOL_A_HEADER_?
>
> Dunno, that'd mean that the Python method is called
> ETHTOOL_MSG_CHANNELS_GET rather than just channels_get.
> I don't want to force all languages to use the C naming.
> The C naming just leads to silly copy'n'paste issues like
> f329a0ebeab.

Can we have 'name:' and 'long-name:' or 'c-name:' or 'full-name' ?

- name: header
   attributes:
    - name: dev_index
      full-name: ETHTOOL_A_HEADER_DEV_INDEX
      val:
      type:

Suppose I'm rewriting my c application from uapi to some generated (in
the future) python-like channels_get() method. If I can grep for
ETHTOOL_MSG_CHANNELS_GET, that would save me a bunch of time figuring
out what the new canonical wrapper is.

Also, maybe, at some point we'll have:
- name: dev_index
  c-name: ETHTOOL_A_HEADER_DEV_INDEX
  java-name: headerDevIndex

:-D

> > > +        len: ALTIFNAMSIZ - 1
> >
> > Not sure how strict you want to be here. ALTIFNAMSIZ is defined
> > somewhere else it seems? (IOW, do we want to have implicit dependencies
> > on external/uapi headers?)
>
> Good catch, I'm aware. I was planning to add a "header constants"
> section or some such. A section in "headers" which defines the
> constants which C code will get from the headers.

Define as in 're-define' or define as in 'you need to include some
other header for this to work'?

const:
  - name: ALTIFNAMSIZ
    val: 128

which then does

#ifndef
#define ALTIFNAMSIZ 128
#else
static_assert(ALTIFNAMSIZ == 128)
#endif

?

or:

external-const:
  - name: ALTIFNAMSIZ
    header: include/uapi/linux/if.h

which then might generate the following:

#include <include/uapi/linux/if.h>
#ifndef ALTIFNAMSIZ
#error "include/uapi/linux/if.h does not define ALTIFNAMSIZ"
#endif

?

> For Python it does not matter, as we don't have to size arrays.

Hm, I was expecting the situation to be the opposite :-) Because if
you really have to know this len in python, how do you resolve
ALTIFNAMSIZ?

The simplest thing to do might be to require these headers to be
hermetic (i.e., redefine all consts the spec cares about internally)?

> I was wondering if it will matter for other languages, like Rust?
>
> > > +            - header
> > > +            - rx_count
> > > +            - tx_count
> > > +            - other_count
> > > +            - combined_count
> >
> > My netlink is super rusty, might be worth mentioning in the spec: these
> > are possible attributes, but are all of them required?
>
> Right, will do, nothing is required, or rather requirements are kinda
> hard to express and checked by the code in the kernel.
>
> > You also mention the validation part in the cover letter, do you plan
> > add some of these policy properties to the spec or everything is
> > there already? (I'm assuming we care about the types which we have above and
> > optional/required attribute indication?)
>
> Yeah, my initial plan was to encode requirement in the policy but its
> not trivial. So I left it as future extension. Besides things which are
> required today may not be tomorrow, so its a bit of a strange thing.

That's the hardest part, but it should improve the observability, so
I'm looking forward :-)
As you say, it is probably hard to declaratively define these
requirements at this point for everything, but maybe some parts
(majority?) are doable.

> Regarding policy properties I'm intending to support all of the stuff
> that the kernel policies recognize... but somehow most families I
> converted don't have validation (only mask and length :S).
>
> Actually for DPLL I have a nice validation trick. You can define an
> enum:
>
> constants:
>   -
>     type: flags
>     name: genl_get_flags
>     value-prefix: DPLL_FLAG_
>     values: [ sources, outputs, status ]
>
> Then for an attribute you link it:
>
>       -
>         name: flags
>         type: u32
>         flags-mask: genl_get_flags
>
> And that will auto an enum:
>
> enum dpll_genl_get_flags {
>         DPLL_FLAG_SOURCES = 1,
>         DPLL_FLAG_OUTPUTS = 2,
>         DPLL_FLAG_STATUS = 4,
> };
>
> And a policy with a mask:
>
>         [DPLLA_FLAGS] = NLA_POLICY_MASK(NLA_U32, 0x7),

Yeah, that looks nice!

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

[Jakub Kicinski]

On Thu, 11 Aug 2022 15:55:44 -0700 Stanislav Fomichev wrote:
> > We could, I guess. To be clear this controls the count, IOW:
> >
> > enum {
> >         PREFIX_A_BLA_ATTR = 1,
> >         PREFIX_A_ANOTHER_ATTR,
> >         PREFIX_A_AND_ONEMORE,
> >         __PFREIX_A_CNT, // <--- This thing
> > };
> > #define PREFIX_A_MAX (__PFREIX_A_CNT - 1)
> >
> > It's not used in the generated code, only if we codegen the uAPI,
> > AFAIR. So we'd need a way to tell the generator of the uAPI about
> > the situation, anyway. I could be misremembering.  
> 
> My worry is that we'll have more hacks like these and it's hard, as a
> spec reader/writer, to figure out that they exist..
> So I was wondering if it's "easier" (from the spec reader/writer pov)
> to have some c-header-fixup: section where we can have plain
> c-preprocessor hacks like these (where we need to redefine something
> to match the old behavior).

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.

> Coming from stubby/grpc, I was expecting to see words like
> message/field/struct. The question is what's more confusing: sticking
> with netlink naming or trying to map grpc/thrift concepts on top of
> what we have. (I'm assuming more people know about grpc/thrift than
> netlink)
> 
> messages: # or maybe 'attribute-sets' ?
>   - name: channels
>     ...

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"?

> operations:
>   - name: channel_get
>     message: channels
>     do:
>       request:
>         fields:
>         - header
>         - rx_max
> 
> Or maybe all we really need is a section in the doc called 'Netlink
> for gRPC/Thrift users' where we map these concepts:
> - attribute-spaces (attribute-sets?) -> messages
> - attributes -> fields

Excellent idea!

> > Dunno, that'd mean that the Python method is called
> > ETHTOOL_MSG_CHANNELS_GET rather than just channels_get.
> > I don't want to force all languages to use the C naming.
> > The C naming just leads to silly copy'n'paste issues like
> > f329a0ebeab.  
> 
> Can we have 'name:' and 'long-name:' or 'c-name:' or 'full-name' ?
> 
> - name: header
>    attributes:
>     - name: dev_index
>       full-name: ETHTOOL_A_HEADER_DEV_INDEX
>       val:
>       type:
> 
> Suppose I'm rewriting my c application from uapi to some generated (in
> the future) python-like channels_get() method. If I can grep for
> ETHTOOL_MSG_CHANNELS_GET, that would save me a bunch of time figuring
> out what the new canonical wrapper is.
> 
> Also, maybe, at some point we'll have:
> - name: dev_index
>   c-name: ETHTOOL_A_HEADER_DEV_INDEX
>   java-name: headerDevIndex

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..

> > Good catch, I'm aware. I was planning to add a "header constants"
> > section or some such. A section in "headers" which defines the
> > constants which C code will get from the headers.  
> 
> Define as in 're-define' or define as in 'you need to include some
> other header for this to work'?
> 
> const:
>   - name: ALTIFNAMSIZ
>     val: 128

This one. In most cases the constant is defined in the same uAPI header
as the proto so we're good. But there's IFNAMSIZ and friends which are
shared.

> which then does
> 
> #ifndef
> #define ALTIFNAMSIZ 128
> #else
> static_assert(ALTIFNAMSIZ == 128)
> #endif
> 
> ?
> 
> or:
> 
> external-const:
>   - name: ALTIFNAMSIZ
>     header: include/uapi/linux/if.h
> 
> which then might generate the following:
> 
> #include <include/uapi/linux/if.h>
> #ifndef ALTIFNAMSIZ
> #error "include/uapi/linux/if.h does not define ALTIFNAMSIZ"
> #endif
> 
> > For Python it does not matter, as we don't have to size arrays.  
> 
> Hm, I was expecting the situation to be the opposite :-) Because if
> you really have to know this len in python, how do you resolve
> ALTIFNAMSIZ?

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.

In C the struct has a char bla[FIXED_SIZE] so if we get an oversized
string we're pooped, that's my point, dunno what other practical use
the string sizing has.

> The simplest thing to do might be to require these headers to be
> hermetic (i.e., redefine all consts the spec cares about internally)?

That's what I'm thinking if they are actually needed. But it only C
cares we can just slap the right includes and not worry. Dunno if other
languages are similarly string-challenged. 

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

[Stanislav Fomichev]

On Thu, Aug 11, 2022 at 4:31 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Thu, 11 Aug 2022 15:55:44 -0700 Stanislav Fomichev wrote:
> > > We could, I guess. To be clear this controls the count, IOW:
> > >
> > > enum {
> > >         PREFIX_A_BLA_ATTR = 1,
> > >         PREFIX_A_ANOTHER_ATTR,
> > >         PREFIX_A_AND_ONEMORE,
> > >         __PFREIX_A_CNT, // <--- This thing
> > > };
> > > #define PREFIX_A_MAX (__PFREIX_A_CNT - 1)
> > >
> > > It's not used in the generated code, only if we codegen the uAPI,
> > > AFAIR. So we'd need a way to tell the generator of the uAPI about
> > > the situation, anyway. I could be misremembering.
> >
> > My worry is that we'll have more hacks like these and it's hard, as a
> > spec reader/writer, to figure out that they exist..
> > So I was wondering if it's "easier" (from the spec reader/writer pov)
> > to have some c-header-fixup: section where we can have plain
> > c-preprocessor hacks like these (where we need to redefine something
> > to match the old behavior).
>
> 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 :-)

> > Coming from stubby/grpc, I was expecting to see words like
> > message/field/struct. The question is what's more confusing: sticking
> > with netlink naming or trying to map grpc/thrift concepts on top of
> > what we have. (I'm assuming more people know about grpc/thrift than
> > netlink)
> >
> > messages: # or maybe 'attribute-sets' ?
> >   - name: channels
> >     ...
>
> 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 :-)

> > operations:
> >   - name: channel_get
> >     message: channels
> >     do:
> >       request:
> >         fields:
> >         - header
> >         - rx_max
> >
> > Or maybe all we really need is a section in the doc called 'Netlink
> > for gRPC/Thrift users' where we map these concepts:
> > - attribute-spaces (attribute-sets?) -> messages
> > - attributes -> fields
>
> Excellent idea!
>
> > > Dunno, that'd mean that the Python method is called
> > > ETHTOOL_MSG_CHANNELS_GET rather than just channels_get.
> > > I don't want to force all languages to use the C naming.
> > > The C naming just leads to silly copy'n'paste issues like
> > > f329a0ebeab.
> >
> > Can we have 'name:' and 'long-name:' or 'c-name:' or 'full-name' ?
> >
> > - name: header
> >    attributes:
> >     - name: dev_index
> >       full-name: ETHTOOL_A_HEADER_DEV_INDEX
> >       val:
> >       type:
> >
> > Suppose I'm rewriting my c application from uapi to some generated (in
> > the future) python-like channels_get() method. If I can grep for
> > ETHTOOL_MSG_CHANNELS_GET, that would save me a bunch of time figuring
> > out what the new canonical wrapper is.
> >
> > Also, maybe, at some point we'll have:
> > - name: dev_index
> >   c-name: ETHTOOL_A_HEADER_DEV_INDEX
> >   java-name: headerDevIndex
>
> 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.. 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 :-)

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.


> > > Good catch, I'm aware. I was planning to add a "header constants"
> > > section or some such. A section in "headers" which defines the
> > > constants which C code will get from the headers.
> >
> > Define as in 're-define' or define as in 'you need to include some
> > other header for this to work'?
> >
> > const:
> >   - name: ALTIFNAMSIZ
> >     val: 128
>
> This one. In most cases the constant is defined in the same uAPI header
> as the proto so we're good. But there's IFNAMSIZ and friends which are
> shared.
>
> > which then does
> >
> > #ifndef
> > #define ALTIFNAMSIZ 128
> > #else
> > static_assert(ALTIFNAMSIZ == 128)
> > #endif
> >
> > ?
> >
> > or:
> >
> > external-const:
> >   - name: ALTIFNAMSIZ
> >     header: include/uapi/linux/if.h
> >
> > which then might generate the following:
> >
> > #include <include/uapi/linux/if.h>
> > #ifndef ALTIFNAMSIZ
> > #error "include/uapi/linux/if.h does not define ALTIFNAMSIZ"
> > #endif
> >
> > > For Python it does not matter, as we don't have to size arrays.
> >
> > Hm, I was expecting the situation to be the opposite :-) Because if
> > you really have to know this len in python, how do you resolve
> > ALTIFNAMSIZ?
>
> 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..

> In C the struct has a char bla[FIXED_SIZE] so if we get an oversized
> string we're pooped, that's my point, dunno what other practical use
> the string sizing has.
>
> > The simplest thing to do might be to require these headers to be
> > hermetic (i.e., redefine all consts the spec cares about internally)?
>
> That's what I'm thinking if they are actually needed. But it only C
> cares we can just slap the right includes and not worry. Dunno if other
> languages are similarly string-challenged.

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

[Jakub Kicinski]

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)

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

[Ido Schimmel]

On Wed, Aug 10, 2022 at 07:23:04PM -0700, Jakub Kicinski wrote:
> @@ -0,0 +1,115 @@
> +# SPDX-License-Identifier: BSD-3-Clause
> +
> +name: ethtool
> +
> +description: |
> +  Ethernet device configuration interface.
> +
> +attr-cnt-suffix: CNT
> +
> +attribute-spaces:
> +  -
> +    name: header
> +    name-prefix: ETHTOOL_A_HEADER_
> +    attributes:
> +      -
> +        name: dev_index
> +        val: 1
> +        type: u32
> +      -
> +        name: dev_name
> +        type: nul-string
> +        len: ALTIFNAMSIZ - 1
> +      -
> +        name: flags
> +        type: u32
> +  -
> +    name: channels
> +    name-prefix: ETHTOOL_A_CHANNELS_
> +    attributes:
> +      -
> +        name: header
> +        val: 1
> +        type: nest
> +        nested-attributes: header
> +      -
> +        name: rx_max
> +        type: u32
> +      -
> +        name: tx_max
> +        type: u32
> +      -
> +        name: other_max
> +        type: u32
> +      -
> +        name: combined_max
> +        type: u32
> +      -
> +        name: rx_count
> +        type: u32
> +      -
> +        name: tx_count
> +        type: u32
> +      -
> +        name: other_count
> +        type: u32
> +      -
> +        name: combined_count
> +        type: u32

Another interesting use case for the schema can be automatic generation
of syzkaller descriptions. These are the corresponding descriptions for
syzkaller:

https://github.com/google/syzkaller/blob/master/sys/linux/socket_netlink_generic_ethtool.txt#L125

Last I checked, these descriptions had to be written by hand, which is
why they are generally out of date, leading to sub-optimal fuzzing. If
schemas are sent along with the kernel code and syzkaller/syzbot
automatically derives descriptions from them, then we should be able to
get meaningful fuzzing as soon as a feature lands in net-next.

> +
> +headers:
> +  user: linux/if.h
> +  uapi: linux/ethtool_netlink.h
> +
> +operations:
> +  name-prefix: ETHTOOL_MSG_
> +  async-prefix: ETHTOOL_MSG_
> +  list:
> +    -
> +      name: channels_get
> +      val: 17
> +      description: Get current and max supported number of channels.
> +      attribute-space: channels
> +      do:
> +        request:
> +          attributes:
> +            - header
> +        reply: &channel_reply
> +          attributes:
> +            - header
> +            - rx_max
> +            - tx_max
> +            - other_max
> +            - combined_max
> +            - rx_count
> +            - tx_count
> +            - other_count
> +            - combined_count
> +      dump:
> +        reply: *channel_reply
> +
> +    -
> +      name: channels_ntf
> +      description: Notification for device changing its number of channels.
> +      notify: channels_get
> +      mcgrp: monitor
> +
> +    -
> +      name: channels_set
> +      description: Set number of channels.
> +      attribute-space: channels
> +      do:
> +        request:
> +          attributes:
> +            - header
> +            - rx_count
> +            - tx_count
> +            - other_count
> +            - combined_count
> +
> +mcast-groups:
> +  name-prefix: ETHTOOL_MCGRP_
> +  name-suffix: _NAME
> +  list:
> +    -
> +      name: monitor

Re: [RFC net-next 0/4] ynl: YAML netlink protocol descriptions

[Stephen Hemminger]

On Wed, 10 Aug 2022 19:23:00 -0700
Jakub Kicinski <kuba@kernel.org> wrote:

> Netlink seems simple and reasonable to those who understand it.
> It appears cumbersome and arcane to those who don't.
> 
> This RFC introduces machine readable netlink protocol descriptions
> in YAML, in an attempt to make creation of truly generic netlink
> libraries a possibility. Truly generic netlink library here means
> a library which does not require changes to support a new family
> or a new operation.
> 
> Each YAML spec lists attributes and operations the family supports.
> The specs are fully standalone, meaning that there is no dependency
> on existing uAPI headers in C. Numeric values of all attribute types,
> operations, enums, and defines and listed in the spec (or unambiguous).
> This property removes the need to manually translate the headers for
> languages which are not compatible with C.
> 
> The expectation is that the spec can be used to either dynamically
> translate between whatever types the high level language likes (see
> the Python example below) or codegen a complete libarary / bindings
> for a netlink family at compilation time (like popular RPC libraries
> do).
> 
> Currently only genetlink is supported, but the "old netlink" should
> be supportable as well (I don't need it myself).
> 
> On the kernel side the YAML spec can be used to generate:
>  - the C uAPI header
>  - documentation of the protocol as a ReST file
>  - policy tables for input attribute validation
>  - operation tables
> 
> We can also codegen parsers and dump helpers, but right now the level
> of "creativity & cleverness" when it comes to netlink parsing is so
> high it's quite hard to generalize it for most families without major
> refactoring.
> 
> Being able to generate the header, documentation and policy tables
> should balance out the extra effort of writing the YAML spec.
> 
> Here is a Python example I promised earlier:
> 
>   ynl = YnlFamily("path/to/ethtool.yaml")
>   channels = ynl.channels_get({'header': {'dev_name': 'eni1np1'}})
> 
> If the call was successful "channels" will hold a standard Python dict,
> e.g.:
> 
>   {'header': {'dev_index': 6, 'dev_name': 'eni1np1'},
>    'combined_max': 1,
>    'combined_count': 1}
> 
> for a netdevsim device with a single combined queue.
> 
> YnlFamily is an implementation of a YAML <> netlink translator (patch 3).
> It takes a path to the YAML spec - hopefully one day we will make
> the YAMLs themselves uAPI and distribute them like we distribute
> C headers. Or get them distributed to a standard search path another
> way. Until then, the YNL library needs a full path to the YAML spec and
> application has to worry about the distribution of those.
> 
> The YnlFamily reads all the info it needs from the spec, resolves
> the genetlink family id, and creates methods based on the spec.
> channels_get is such a dynamically-generated method (i.e. grep for
> channels_get in the python code shows nothing). The method can be called
> passing a standard Python dict as an argument. YNL will look up each key
> in the YAML spec and render the appropriate binary (netlink TLV)
> representation of the value. It then talks thru a netlink socket
> to the kernel, and deserilizes the response, converting the netlink
> TLVs into Python types and constructing a dictionary.
> 
> Again, the YNL code is completely generic and has no knowledge specific
> to ethtool. It's fairly simple an incomplete (in terms of types
> for example), I wrote it this afternoon. I'm also pretty bad at Python,
> but it's the only language I can type which allows the method
> magic, so please don't judge :) I have a rather more complete codegen
> for C, with support for notifications, kernel -> user policy/type
> verification, resolving extack attr offsets into a path
> of attribute names etc, etc. But that stuff needs polishing and
> is less suitable for an RFC.
> 
> The ability for a high level language like Python to talk to the kernel
> so easily, without ctypes, manually packing structs, copy'n'pasting
> values for defines etc. excites me more than C codegen, anyway.
> 
> 
> Patch 1 adds a bit of documentation under Documentation/, it talks
> more about the schemas themselves.
> 
> Patch 2 contains the YAML schema for the YAML specs.
> 
> Patch 3 adds the YNL Python library.
> 
> Patch 4 adds a sample schema for ethtool channels and a demo script.
> 
> 
> Jakub Kicinski (4):
>   ynl: add intro docs for the concept
>   ynl: add the schema for the schemas
>   ynl: add a sample python library
>   ynl: add a sample user for ethtool
> 
>  Documentation/index.rst                     |   1 +
>  Documentation/netlink/bindings/ethtool.yaml | 115 +++++++
>  Documentation/netlink/index.rst             |  13 +
>  Documentation/netlink/netlink-bindings.rst  | 104 ++++++
>  Documentation/netlink/schema.yaml           | 242 ++++++++++++++
>  tools/net/ynl/samples/ethtool.py            |  30 ++
>  tools/net/ynl/samples/ynl.py                | 342 ++++++++++++++++++++
>  7 files changed, 847 insertions(+)
>  create mode 100644 Documentation/netlink/bindings/ethtool.yaml
>  create mode 100644 Documentation/netlink/index.rst
>  create mode 100644 Documentation/netlink/netlink-bindings.rst
>  create mode 100644 Documentation/netlink/schema.yaml
>  create mode 100755 tools/net/ynl/samples/ethtool.py
>  create mode 100644 tools/net/ynl/samples/ynl.py
> 

Would rather this be part of iproute2 rather than requiring it
to be maintained separately and part of the kernel tree.

Re: [RFC net-next 0/4] ynl: YAML netlink protocol descriptions

[Jakub Kicinski]

On Wed, 10 Aug 2022 21:15:34 -0700 Stephen Hemminger wrote:
> Would rather this be part of iproute2 rather than requiring it
> to be maintained separately and part of the kernel tree.

I don't understand what you're trying to say. What is "this", 
what is "separate" from what?

Did I fall victim of the "if the cover letter is too long nobody
actually reads it" problem? Or am I simply too tired to parse?

iproute2 is welcome to use the protocol descriptions like any other
user space, but I'm intending to codegen kernel code based on the YAML:

>> On the kernel side the YAML spec can be used to generate:
>>  - the C uAPI header
>>  - documentation of the protocol as a ReST file
>>  - policy tables for input attribute validation
>>  - operation tables

So how can it not be in the kernel tree?

Re: [RFC net-next 0/4] ynl: YAML netlink protocol descriptions

[Stephen Hemminger]

On Wed, 10 Aug 2022 21:47:01 -0700
Jakub Kicinski <kuba@kernel.org> wrote:

> On Wed, 10 Aug 2022 21:15:34 -0700 Stephen Hemminger wrote:
> > Would rather this be part of iproute2 rather than requiring it
> > to be maintained separately and part of the kernel tree.  
> 
> I don't understand what you're trying to say. What is "this", 
> what is "separate" from what?

I am saying that ynl could live as a standalone project or as
part of the iproute2 tools collection.

> 
> Did I fall victim of the "if the cover letter is too long nobody
> actually reads it" problem? Or am I simply too tired to parse?
> 
> iproute2 is welcome to use the protocol descriptions like any other
> user space, but I'm intending to codegen kernel code based on the YAML:

Ok, that makes sense then. I was hoping that user configuration
of network devices could be done with YAML. But probably that is
best left networkd, netplan, and others.


> >> On the kernel side the YAML spec can be used to generate:
> >>  - the C uAPI header
> >>  - documentation of the protocol as a ReST file
> >>  - policy tables for input attribute validation
> >>  - operation tables  
> 
> So how can it not be in the kernel tree?

As code generator then sure.

Re: [RFC net-next 0/4] ynl: YAML netlink protocol descriptions

[Jakub Kicinski]

Randomly adding Michal to CC since I just realized I forgot
to CC him on the series.

On Thu, 11 Aug 2022 08:01:52 -0700 Stephen Hemminger wrote:
> > On Wed, 10 Aug 2022 21:15:34 -0700 Stephen Hemminger wrote:  
> > > Would rather this be part of iproute2 rather than requiring it
> > > to be maintained separately and part of the kernel tree.    
> > 
> > I don't understand what you're trying to say. What is "this", 
> > what is "separate" from what?  
> 
> I am saying that ynl could live as a standalone project or as
> part of the iproute2 tools collection.

It's a bit of a strange beast, because the YNL C library ends up being
relatively small:

 tools/net/ynl/lib/ynl.c                  | 528 +++++++++++++++++++++++++
 tools/net/ynl/lib/ynl.h                  | 112 ++++++

The logic is mostly in the codegen:

 gen.py                                   | 1601 +++++++++++++++++++++++++

but that part we need for kernel C code as well.

The generated code is largish:

 tools/net/ynl/generated/dpll-user.c      | 371 ++++++++++++++++++
 tools/net/ynl/generated/dpll-user.h      | 204 ++++++++++
 tools/net/ynl/generated/ethtool-user.c   | 367 ++++++++++++++++++
 tools/net/ynl/generated/ethtool-user.h   | 190 +++++++++
 tools/net/ynl/generated/fou-user.c       | 322 ++++++++++++++++
 tools/net/ynl/generated/fou-user.h       | 287 ++++++++++++++
 tools/net/ynl/generated/genetlink-user.c | 635 +++++++++++++++++++++++++++++++
 tools/net/ynl/generated/genetlink-user.h | 201 ++++++++++

but we don't have to commit it, it can be created on the fly 
(for instance when a selftest wants to make use of YNL).

Then again it would feel a lot cleaner for the user space library
to be a separate project. I've been putting off thinking about the
distribution until I'm done coding, TBH. Dunno.

Re: [RFC net-next 0/4] ynl: YAML netlink protocol descriptions

[sdf]

On 08/11, Jakub Kicinski wrote:
> Randomly adding Michal to CC since I just realized I forgot
> to CC him on the series.

> On Thu, 11 Aug 2022 08:01:52 -0700 Stephen Hemminger wrote:
> > > On Wed, 10 Aug 2022 21:15:34 -0700 Stephen Hemminger wrote:
> > > > Would rather this be part of iproute2 rather than requiring it
> > > > to be maintained separately and part of the kernel tree.
> > >
> > > I don't understand what you're trying to say. What is "this",
> > > what is "separate" from what?
> >
> > I am saying that ynl could live as a standalone project or as
> > part of the iproute2 tools collection.

> It's a bit of a strange beast, because the YNL C library ends up being
> relatively small:

>   tools/net/ynl/lib/ynl.c                  | 528 +++++++++++++++++++++++++
>   tools/net/ynl/lib/ynl.h                  | 112 ++++++

> The logic is mostly in the codegen:

>   gen.py                                   | 1601 +++++++++++++++++++++++++

> but that part we need for kernel C code as well.

> The generated code is largish:

>   tools/net/ynl/generated/dpll-user.c      | 371 ++++++++++++++++++
>   tools/net/ynl/generated/dpll-user.h      | 204 ++++++++++
>   tools/net/ynl/generated/ethtool-user.c   | 367 ++++++++++++++++++
>   tools/net/ynl/generated/ethtool-user.h   | 190 +++++++++
>   tools/net/ynl/generated/fou-user.c       | 322 ++++++++++++++++
>   tools/net/ynl/generated/fou-user.h       | 287 ++++++++++++++
>   tools/net/ynl/generated/genetlink-user.c | 635  
> +++++++++++++++++++++++++++++++
>   tools/net/ynl/generated/genetlink-user.h | 201 ++++++++++

> but we don't have to commit it, it can be created on the fly
> (for instance when a selftest wants to make use of YNL).

> Then again it would feel a lot cleaner for the user space library
> to be a separate project. I've been putting off thinking about the
> distribution until I'm done coding, TBH. Dunno.

my 2c:

Putting it into iproute2 will make it carry a 'networking' badge on it
meaning no other subsystem would look into it.

I'd rather make netlink more generic (s/netlink/kernelink/?) and remove
CONFIG_NET dependency to address https://lwn.net/Articles/897202/

Re: [RFC net-next 0/4] ynl: YAML netlink protocol descriptions

[Jakub Kicinski]

On Thu, 11 Aug 2022 09:28:45 -0700 sdf@google.com wrote:
> Putting it into iproute2 will make it carry a 'networking' badge on it
> meaning no other subsystem would look into it.

Good point.

> I'd rather make netlink more generic (s/netlink/kernelink/?) and remove
> CONFIG_NET dependency to address https://lwn.net/Articles/897202/

I think people just use that as an excuse, TBH.

Re: [RFC net-next 0/4] ynl: YAML netlink protocol descriptions

[Florian Fainelli]

On 8/10/22 19:23, Jakub Kicinski wrote:
> Netlink seems simple and reasonable to those who understand it.
> It appears cumbersome and arcane to those who don't.
> 
> This RFC introduces machine readable netlink protocol descriptions
> in YAML, in an attempt to make creation of truly generic netlink
> libraries a possibility. Truly generic netlink library here means
> a library which does not require changes to support a new family
> or a new operation.
> 
> Each YAML spec lists attributes and operations the family supports.
> The specs are fully standalone, meaning that there is no dependency
> on existing uAPI headers in C. Numeric values of all attribute types,
> operations, enums, and defines and listed in the spec (or unambiguous).
> This property removes the need to manually translate the headers for
> languages which are not compatible with C.
> 
> The expectation is that the spec can be used to either dynamically
> translate between whatever types the high level language likes (see
> the Python example below) or codegen a complete libarary / bindings
> for a netlink family at compilation time (like popular RPC libraries
> do).
> 
> Currently only genetlink is supported, but the "old netlink" should
> be supportable as well (I don't need it myself).
> 
> On the kernel side the YAML spec can be used to generate:
>   - the C uAPI header
>   - documentation of the protocol as a ReST file
>   - policy tables for input attribute validation
>   - operation tables
> 
> We can also codegen parsers and dump helpers, but right now the level
> of "creativity & cleverness" when it comes to netlink parsing is so
> high it's quite hard to generalize it for most families without major
> refactoring.
> 
> Being able to generate the header, documentation and policy tables
> should balance out the extra effort of writing the YAML spec.
> 
> Here is a Python example I promised earlier:
> 
>    ynl = YnlFamily("path/to/ethtool.yaml")
>    channels = ynl.channels_get({'header': {'dev_name': 'eni1np1'}})
> 
> If the call was successful "channels" will hold a standard Python dict,
> e.g.:
> 
>    {'header': {'dev_index': 6, 'dev_name': 'eni1np1'},
>     'combined_max': 1,
>     'combined_count': 1}
> 
> for a netdevsim device with a single combined queue.
> 
> YnlFamily is an implementation of a YAML <> netlink translator (patch 3).
> It takes a path to the YAML spec - hopefully one day we will make
> the YAMLs themselves uAPI and distribute them like we distribute
> C headers. Or get them distributed to a standard search path another
> way. Until then, the YNL library needs a full path to the YAML spec and
> application has to worry about the distribution of those.
> 
> The YnlFamily reads all the info it needs from the spec, resolves
> the genetlink family id, and creates methods based on the spec.
> channels_get is such a dynamically-generated method (i.e. grep for
> channels_get in the python code shows nothing). The method can be called
> passing a standard Python dict as an argument. YNL will look up each key
> in the YAML spec and render the appropriate binary (netlink TLV)
> representation of the value. It then talks thru a netlink socket
> to the kernel, and deserilizes the response, converting the netlink
> TLVs into Python types and constructing a dictionary.
> 
> Again, the YNL code is completely generic and has no knowledge specific
> to ethtool. It's fairly simple an incomplete (in terms of types
> for example), I wrote it this afternoon. I'm also pretty bad at Python,
> but it's the only language I can type which allows the method
> magic, so please don't judge :) I have a rather more complete codegen
> for C, with support for notifications, kernel -> user policy/type
> verification, resolving extack attr offsets into a path
> of attribute names etc, etc. But that stuff needs polishing and
> is less suitable for an RFC.
> 
> The ability for a high level language like Python to talk to the kernel
> so easily, without ctypes, manually packing structs, copy'n'pasting
> values for defines etc. excites me more than C codegen, anyway.

This is really cool BTW, and it makes a lot of sense to me that we are 
moving that way, especially with Rust knocking at the door. I will try 
to do a more thorough review, than "cool, I like it".
-- 
Florian

Re: [RFC net-next 0/4] ynl: YAML netlink protocol descriptions

[Jakub Kicinski]

On Fri, 12 Aug 2022 10:00:52 -0700 Florian Fainelli wrote:
> > The ability for a high level language like Python to talk to the kernel
> > so easily, without ctypes, manually packing structs, copy'n'pasting
> > values for defines etc. excites me more than C codegen, anyway.  
> 
> This is really cool BTW, and it makes a lot of sense to me that we are 
> moving that way, especially with Rust knocking at the door. I will try 
> to do a more thorough review, than "cool, I like it".

Thanks! Feel free to ping me for the latest version whenever you want
to take a look, because the code will hopefully be under very active
development for a few more weeks..

Re: [RFC net-next 0/4] ynl: YAML netlink protocol descriptions

[Jakub Kicinski]

On Wed, 10 Aug 2022 19:23:00 -0700 Jakub Kicinski wrote:
> Netlink seems simple and reasonable to those who understand it.
> It appears cumbersome and arcane to those who don't.

FWIW I put the work in progress code on GH hoping this will be more
engaging to user space devs who are a pretty important here:

https://github.com/kuba-moo/ynl

If anyone is interested in helping out, it'd be most welcome.