Merge branch 'tools-ynl-more-docs-and-basic-ethtool-support'

Jakub Kicinski says:

====================
tools: ynl: more docs and basic ethtool support

I got discouraged from supporting ethtool in specs, because
generating the user space C code seems a little tricky.
The messages are ID'ed in a "directional" way (to and from
kernel are separate ID "spaces"). There is value, however,
in having the spec and being able to for example use it
in Python.

After paying off some technical debt - add a partial
ethtool spec. Partial because the header for ethtool is almost
a 1000 LoC, so converting in one sitting is tough. But adding
new commands should be trivial now.

Last but not least I add more docs, I realized that I've been
sending a similar "instructions" email to people working on
new families. It's now intro-specs.rst.
====================

Link: https://lore.kernel.org/r/20230131023354.1732677-1-kuba@kernel.orgSigned-off-by: Jakub Kicinski <kuba@kernel.org>

Documentation/netlink/genetlink-c.yaml

@@ -218,9 +218,7 @@ properties:
           to a single enum.
           "directional" has the messages sent to the kernel and from the kernel
           enumerated separately.
-          "notify-split" has the notifications and request-response types in
-          different enums.
-        enum: [ unified, directional, notify-split ]
+        enum: [ unified ]
       name-prefix:
         description: |
           Prefix for the C enum name of the command. The name is formed by concatenating

Documentation/netlink/genetlink-legacy.yaml

@@ -241,9 +241,7 @@ properties:
           to a single enum.
           "directional" has the messages sent to the kernel and from the kernel
           enumerated separately.
-          "notify-split" has the notifications and request-response types in
-          different enums.
-        enum: [ unified, directional, notify-split ]
+        enum: [ unified, directional ] # Trim
       name-prefix:
         description: |
           Prefix for the C enum name of the command. The name is formed by concatenating
@@ -307,6 +305,13 @@ properties:
                       type: array
                       items:
                         type: string
+                    # Start genetlink-legacy
+                    value:
+                      description: |
+                        ID of this message if value for request and response differ,
+                        i.e. requests and responses have different message enums.
+                      $ref: '#/$defs/uint'
+                    # End genetlink-legacy
                 reply: *subop-attr-list
                 pre:
                   description: Hook for a function to run before the main callback (pre_doit or start).

Documentation/netlink/genetlink.yaml

@@ -188,9 +188,7 @@ properties:
           to a single enum.
           "directional" has the messages sent to the kernel and from the kernel
           enumerated separately.
-          "notify-split" has the notifications and request-response types in
-          different enums.
-        enum: [ unified, directional, notify-split ]
+        enum: [ unified ]
       name-prefix:
         description: |
           Prefix for the C enum name of the command. The name is formed by concatenating

Documentation/netlink/specs/ethtool.yaml

+name: ethtool
+
+protocol: genetlink-legacy
+
+doc: Partial family for Ethtool Netlink.
+
+attribute-sets:
+  -
+    name: header
+    attributes:
+      -
+        name: dev-index
+        type: u32
+        value: 1
+      -
+        name: dev-name
+        type: string
+      -
+        name: flags
+        type: u32
+
+  -
+    name: bitset-bit
+    attributes:
+      -
+        name: index
+        type: u32
+        value: 1
+      -
+        name: name
+        type: string
+      -
+        name: value
+        type: flag
+  -
+    name: bitset-bits
+    attributes:
+      -
+        name: bit
+        type: nest
+        nested-attributes: bitset-bit
+        value: 1
+  -
+    name: bitset
+    attributes:
+      -
+        name: nomask
+        type: flag
+        value: 1
+      -
+        name: size
+        type: u32
+      -
+        name: bits
+        type: nest
+        nested-attributes: bitset-bits
+
+  -
+    name: string
+    attributes:
+      -
+        name: index
+        type: u32
+        value: 1
+      -
+        name: value
+        type: string
+  -
+    name: strings
+    attributes:
+      -
+        name: string
+        type: nest
+        value: 1
+        multi-attr: true
+        nested-attributes: string
+  -
+    name: stringset
+    attributes:
+      -
+        name: id
+        type: u32
+        value: 1
+      -
+        name: count
+        type: u32
+      -
+        name: strings
+        type: nest
+        multi-attr: true
+        nested-attributes: strings
+  -
+    name: stringsets
+    attributes:
+      -
+        name: stringset
+        type: nest
+        multi-attr: true
+        value: 1
+        nested-attributes: stringset
+  -
+    name: strset
+    attributes:
+      -
+        name: header
+        value: 1
+        type: nest
+        nested-attributes: header
+      -
+        name: stringsets
+        type: nest
+        nested-attributes: stringsets
+      -
+        name: counts-only
+        type: flag
+
+  -
+    name: privflags
+    attributes:
+      -
+        name: header
+        value: 1
+        type: nest
+        nested-attributes: header
+      -
+        name: flags
+        type: nest
+        nested-attributes: bitset
+
+  -
+    name: rings
+    attributes:
+      -
+        name: header
+        value: 1
+        type: nest
+        nested-attributes: header
+      -
+        name: rx-max
+        type: u32
+      -
+        name: rx-mini-max
+        type: u32
+      -
+        name: rx-jumbo-max
+        type: u32
+      -
+        name: tx-max
+        type: u32
+      -
+        name: rx
+        type: u32
+      -
+        name: rx-mini
+        type: u32
+      -
+        name: rx-jumbo
+        type: u32
+      -
+        name: tx
+        type: u32
+      -
+        name: rx-buf-len
+        type: u32
+      -
+        name: tcp-data-split
+        type: u8
+      -
+        name: cqe-size
+        type: u32
+      -
+        name: tx-push
+        type: u8
+
+  -
+    name: mm-stat
+    attributes:
+      -
+        name: pad
+        value: 1
+        type: pad
+      -
+        name: reassembly-errors
+        type: u64
+      -
+        name: smd-errors
+        type: u64
+      -
+        name: reassembly-ok
+        type: u64
+      -
+        name: rx-frag-count
+        type: u64
+      -
+        name: tx-frag-count
+        type: u64
+      -
+        name: hold-count
+        type: u64
+  -
+    name: mm
+    attributes:
+      -
+        name: header
+        value: 1
+        type: nest
+        nested-attributes: header
+      -
+        name: pmac-enabled
+        type: u8
+      -
+        name: tx-enabled
+        type: u8
+      -
+        name: tx-active
+        type: u8
+      -
+        name: tx-min-frag-size
+        type: u32
+      -
+        name: tx-min-frag-size
+        type: u32
+      -
+        name: verify-enabled
+        type: u8
+      -
+        name: verify-status
+        type: u8
+      -
+        name: verify-time
+        type: u32
+      -
+        name: max-verify-time
+        type: u32
+      -
+        name: stats
+        type: nest
+        nested-attributes: mm-stat
+
+operations:
+  enum-model: directional
+  list:
+    -
+      name: strset-get
+      doc: Get string set from the kernel.
+
+      attribute-set: strset
+
+      do: &strset-get-op
+        request:
+          value: 1
+          attributes:
+            - header
+            - stringsets
+            - counts-only
+        reply:
+          value: 1
+          attributes:
+            - header
+            - stringsets
+      dump: *strset-get-op
+
+    # TODO: fill in the requests in between
+
+    -
+      name: privflags-get
+      doc: Get device private flags.
+
+      attribute-set: privflags
+
+      do: &privflag-get-op
+        request:
+          value: 13
+          attributes:
+            - header
+        reply:
+          value: 14
+          attributes:
+            - header
+            - flags
+      dump: *privflag-get-op
+    -
+      name: privflags-set
+      doc: Set device private flags.
+
+      attribute-set: privflags
+
+      do:
+        request:
+          attributes:
+            - header
+            - flags
+    -
+      name: privflags-ntf
+      doc: Notification for change in device private flags.
+      notify: privflags-get
+
+    -
+      name: rings-get
+      doc: Get ring params.
+
+      attribute-set: rings
+
+      do: &ring-get-op
+        request:
+          attributes:
+            - header
+        reply:
+          attributes:
+            - header
+            - rx-max
+            - rx-mini-max
+            - rx-jumbo-max
+            - tx-max
+            - rx
+            - rx-mini
+            - rx-jumbo
+            - tx
+            - rx-buf-len
+            - tcp-data-split
+            - cqe-size
+            - tx-push
+      dump: *ring-get-op
+    -
+      name: rings-set
+      doc: Set ring params.
+
+      attribute-set: rings
+
+      do:
+        request:
+          attributes:
+            - header
+            - rx
+            - rx-mini
+            - rx-jumbo
+            - tx
+            - rx-buf-len
+            - tcp-data-split
+            - cqe-size
+            - tx-push
+    -
+      name: rings-ntf
+      doc: Notification for change in ring params.
+      notify: rings-get
+
+    # TODO: fill in the requests in between
+
+    -
+      name: mm-get
+      doc: Get MAC Merge configuration and state
+
+      attribute-set: mm
+
+      do: &mm-get-op
+        request:
+          value: 42
+          attributes:
+            - header
+        reply:
+          value: 42
+          attributes:
+            - header
+            - pmac-enabled
+            - tx-enabled
+            - tx-active
+            - tx-min-frag-size
+            - rx-min-frag-size
+            - verify-enabled
+            - verify-time
+            - max-verify-time
+            - stats
+      dump: *mm-get-op
+    -
+      name: mm-set
+      doc: Set MAC Merge configuration
+
+      attribute-set: mm
+
+      do:
+        request:
+          attributes:
+            - header
+            - verify-enabled
+            - verify-time
+            - tx-enabled
+            - pmac-enabled
+            - tx-min-frag-size
+    -
+      name: mm-ntf
+      doc: Notification for change in MAC Merge configuration.
+      notify: mm-get

Documentation/userspace-api/netlink/genetlink-legacy.rst

@@ -74,6 +74,88 @@ type. Inside the attr-index nest are the policy attributes. Modern
 Netlink families should have instead defined this as a flat structure,
 the nesting serves no good purpose here.
 
+Operations
+==========
+
+Enum (message ID) model
+-----------------------
+
+unified
+~~~~~~~
+
+Modern families use the ``unified`` message ID model, which uses
+a single enumeration for all messages within family. Requests and
+responses share the same message ID. Notifications have separate
+IDs from the same space. For example given the following list
+of operations:
+
+.. code-block:: yaml
+
+  -
+    name: a
+    value: 1
+    do: ...
+  -
+    name: b
+    do: ...
+  -
+    name: c
+    value: 4
+    notify: a
+  -
+    name: d
+    do: ...
+
+Requests and responses for operation ``a`` will have the ID of 1,
+the requests and responses of ``b`` - 2 (since there is no explicit
+``value`` it's previous operation ``+ 1``). Notification ``c`` will
+use the ID of 4, operation ``d`` 5 etc.
+
+directional
+~~~~~~~~~~~
+
+The ``directional`` model splits the ID assignment by the direction of
+the message. Messages from and to the kernel can't be confused with
+each other so this conserves the ID space (at the cost of making
+the programming more cumbersome).
+
+In this case ``value`` attribute should be specified in the ``request``
+``reply`` sections of the operations (if an operation has both ``do``
+and ``dump`` the IDs are shared, ``value`` should be set in ``do``).
+For notifications the ``value`` is provided at the op level but it
+only allocates a ``reply`` (i.e. a "from-kernel" ID). Let's look
+at an example:
+
+.. code-block:: yaml
+
+  -
+    name: a
+    do:
+      request:
+        value: 2
+        attributes: ...
+      reply:
+        value: 1
+        attributes: ...
+  -
+    name: b
+    notify: a
+  -
+    name: c
+    notify: a
+    value: 7
+  -
+    name: d
+    do: ...
+
+In this case ``a`` will use 2 when sending the message to the kernel
+and expects message with ID 1 in response. Notification ``b`` allocates
+a "from-kernel" ID which is 2. ``c`` allocates "from-kernel" ID of 7.
+If operation ``d`` does not set ``values`` explicitly in the spec
+it will be allocated 3 for the request (``a`` is the previous operation
+with a request section and the value of 2) and 8 for response (``c`` is
+the previous operation in the "from-kernel" direction).
+
 Other quirks (todo)
 ===================
 

Documentation/userspace-api/netlink/index.rst

@@ -10,6 +10,7 @@ Netlink documentation for users.
    :maxdepth: 2
 
    intro
+   intro-specs
    specs
    c-code-gen
    genetlink-legacy

Documentation/userspace-api/netlink/intro-specs.rst

+.. SPDX-License-Identifier: BSD-3-Clause
+
+=====================================
+Using Netlink protocol specifications
+=====================================
+
+This document is a quick starting guide for using Netlink protocol
+specifications. For more detailed description of the specs see :doc:`specs`.
+
+Simple CLI
+==========
+
+Kernel comes with a simple CLI tool which should be useful when
+developing Netlink related code. The tool is implemented in Python
+and can use a YAML specification to issue Netlink requests
+to the kernel. Only Generic Netlink is supported.
+
+The tool is located at ``tools/net/ynl/cli.py``. It accepts
+a handul of arguments, the most important ones are:
+
+ - ``--spec`` - point to the spec file
+ - ``--do $name`` / ``--dump $name`` - issue request ``$name``
+ - ``--json $attrs`` - provide attributes for the request
+ - ``--subscribe $group`` - receive notifications from ``$group``
+
+YAML specs can be found under ``Documentation/netlink/specs/``.
+
+Example use::
+
+  $ ./tools/net/ynl/cli.py --spec Documentation/netlink/specs/ethtool.yaml \
+        --do rings-get \
+	--json '{"header":{"dev-index": 18}}'
+  {'header': {'dev-index': 18, 'dev-name': 'eni1np1'},
+   'rx': 0,
+   'rx-jumbo': 0,
+   'rx-jumbo-max': 4096,
+   'rx-max': 4096,
+   'rx-mini': 0,
+   'rx-mini-max': 4096,
+   'tx': 0,
+   'tx-max': 4096,
+   'tx-push': 0}
+
+The input arguments are parsed as JSON, while the output is only
+Python-pretty-printed. This is because some Netlink types can't
+be expressed as JSON directly. If such attributes are needed in
+the input some hacking of the script will be necessary.
+
+The spec and Netlink internals are factored out as a standalone
+library - it should be easy to write Python tools / tests reusing
+code from ``cli.py``.
+
+Generating kernel code
+======================
+
+``tools/net/ynl/ynl-regen.sh`` scans the kernel tree in search of
+auto-generated files which need to be updated. Using this tool is the easiest
+way to generate / update auto-generated code.
+
+By default code is re-generated only if spec is newer than the source,
+to force regeneration use ``-f``.
+
+``ynl-regen.sh`` searches for ``YNL-GEN`` in the contents of files
+(note that it only scans files in the git index, that is only files
+tracked by git!) For instance the ``fou_nl.c`` kernel source contains::
+
+  /*	Documentation/netlink/specs/fou.yaml */
+  /* YNL-GEN kernel source */
+
+``ynl-regen.sh`` will find this marker and replace the file with
+kernel source based on fou.yaml.
+
+The simplest way to generate a new file based on a spec is to add
+the two marker lines like above to a file, add that file to git,
+and run the regeneration tool. Grep the tree for ``YNL-GEN``
+to see other examples.
+
+The code generation itself is performed by ``tools/net/ynl/ynl-gen-c.py``
+but it takes a few arguments so calling it directly for each file
+quickly becomes tedious.

Documentation/userspace-api/netlink/specs.rst

@@ -21,6 +21,9 @@ Internally kernel uses the YAML specs to generate:
 
 YAML specifications can be found under ``Documentation/netlink/specs/``
 
+This document describes details of the schema.
+See :doc:`intro-specs` for a practical starting guide.
+
 Compatibility levels
 ====================
 

tools/net/ynl/samples/cli.py → tools/net/ynl/cli.py

-#!/usr/bin/env python
+#!/usr/bin/env python3
 # SPDX-License-Identifier: BSD-3-Clause
 
 import argparse
@@ -6,13 +6,14 @@ import json
 import pprint
 import time
 
-from ynl import YnlFamily
+from lib import YnlFamily
 
 
 def main():
     parser = argparse.ArgumentParser(description='YNL CLI sample')
     parser.add_argument('--spec', dest='spec', type=str, required=True)
     parser.add_argument('--schema', dest='schema', type=str)
+    parser.add_argument('--no-schema', action='store_true')
     parser.add_argument('--json', dest='json_text', type=str)
     parser.add_argument('--do', dest='do', type=str)
     parser.add_argument('--dump', dest='dump', type=str)
@@ -20,6 +21,9 @@ def main():
     parser.add_argument('--subscribe', dest='ntf', type=str)
     args = parser.parse_args()
 
+    if args.no_schema:
+        args.schema = ''
+
     attrs = {}
     if args.json_text:
         attrs = json.loads(args.json_text)
@@ -32,10 +36,11 @@ def main():
     if args.sleep:
         time.sleep(args.sleep)
 
-    if args.do or args.dump:
-        method = getattr(ynl, args.do if args.do else args.dump)
-
-        reply = method(attrs, dump=bool(args.dump))
+    if args.do:
+        reply = ynl.do(args.do, attrs)
+        pprint.PrettyPrinter().pprint(reply)
+    if args.dump:
+        reply = ynl.dump(args.dump, attrs)
         pprint.PrettyPrinter().pprint(reply)
 
     if args.ntf:

tools/net/ynl/lib/__init__.py

+# SPDX-License-Identifier: BSD-3-Clause
+
+from .nlspec import SpecAttr, SpecAttrSet, SpecFamily, SpecOperation
+from .ynl import YnlFamily
+
+__all__ = ["SpecAttr", "SpecAttrSet", "SpecFamily", "SpecOperation",
+           "YnlFamily"]