linux-doc.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net-next v2 00/14] tools: ynl: more docs and basic ethtool support
@ 2023-01-31  2:33 Jakub Kicinski
  2023-01-31  2:33 ` [PATCH net-next v2 01/14] tools: ynl-gen: prevent do / dump reordering Jakub Kicinski
                   ` (14 more replies)
  0 siblings, 15 replies; 16+ messages in thread
From: Jakub Kicinski @ 2023-01-31  2:33 UTC (permalink / raw)
  To: davem; +Cc: netdev, edumazet, pabeni, sdf, linux-doc, Jakub Kicinski

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.

v2:
 - spelling fixes (patch 11)
 - always use python3 (new patch 14)

Jakub Kicinski (14):
  tools: ynl-gen: prevent do / dump reordering
  tools: ynl: move the cli and netlink code around
  tools: ynl: add an object hierarchy to represent parsed spec
  tools: ynl: use the common YAML loading and validation code
  tools: ynl: add support for types needed by ethtool
  tools: ynl: support directional enum-model in CLI
  tools: ynl: support multi-attr
  tools: ynl: support pretty printing bad attribute names
  tools: ynl: use operation names from spec on the CLI
  tools: ynl: load jsonschema on demand
  netlink: specs: finish up operation enum-models
  netlink: specs: add partial specification for ethtool
  docs: netlink: add a starting guide for working with specs
  tools: net: use python3 explicitly

 Documentation/netlink/genetlink-c.yaml        |   4 +-
 Documentation/netlink/genetlink-legacy.yaml   |  11 +-
 Documentation/netlink/genetlink.yaml          |   4 +-
 Documentation/netlink/specs/ethtool.yaml      | 392 ++++++++++++++++++
 .../netlink/genetlink-legacy.rst              |  82 ++++
 Documentation/userspace-api/netlink/index.rst |   1 +
 .../userspace-api/netlink/intro-specs.rst     |  80 ++++
 Documentation/userspace-api/netlink/specs.rst |   3 +
 tools/net/ynl/{samples => }/cli.py            |  17 +-
 tools/net/ynl/lib/__init__.py                 |   7 +
 tools/net/ynl/lib/nlspec.py                   | 310 ++++++++++++++
 tools/net/ynl/{samples => lib}/ynl.py         | 192 +++++----
 tools/net/ynl/ynl-gen-c.py                    | 262 ++++++------
 13 files changed, 1109 insertions(+), 256 deletions(-)
 create mode 100644 Documentation/netlink/specs/ethtool.yaml
 create mode 100644 Documentation/userspace-api/netlink/intro-specs.rst
 rename tools/net/ynl/{samples => }/cli.py (76%)
 create mode 100644 tools/net/ynl/lib/__init__.py
 create mode 100644 tools/net/ynl/lib/nlspec.py
 rename tools/net/ynl/{samples => lib}/ynl.py (79%)

-- 
2.39.1


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH net-next v2 01/14] tools: ynl-gen: prevent do / dump reordering
  2023-01-31  2:33 [PATCH net-next v2 00/14] tools: ynl: more docs and basic ethtool support Jakub Kicinski
@ 2023-01-31  2:33 ` Jakub Kicinski
  2023-01-31  2:33 ` [PATCH net-next v2 02/14] tools: ynl: move the cli and netlink code around Jakub Kicinski
                   ` (13 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: Jakub Kicinski @ 2023-01-31  2:33 UTC (permalink / raw)
  To: davem; +Cc: netdev, edumazet, pabeni, sdf, linux-doc, Jakub Kicinski

An earlier fix tried to address generated code jumping around
one code-gen run to another. Turns out dict()s are already
ordered since Python 3.7, the problem is that we iterate over
operation modes using a set(). Sets are unordered in Python.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 tools/net/ynl/ynl-gen-c.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/net/ynl/ynl-gen-c.py b/tools/net/ynl/ynl-gen-c.py
index 1aa872e582ab..e5002d420961 100755
--- a/tools/net/ynl/ynl-gen-c.py
+++ b/tools/net/ynl/ynl-gen-c.py
@@ -933,7 +933,7 @@ import yaml
             if attr_set_name != op['attribute-set']:
                 raise Exception('For a global policy all ops must use the same set')
 
-            for op_mode in {'do', 'dump'}:
+            for op_mode in ['do', 'dump']:
                 if op_mode in op:
                     global_set.update(op[op_mode].get('request', []))
 
@@ -2244,7 +2244,7 @@ _C_KW = {
 
             for op_name, op in parsed.ops.items():
                 if parsed.kernel_policy in {'per-op', 'split'}:
-                    for op_mode in {'do', 'dump'}:
+                    for op_mode in ['do', 'dump']:
                         if op_mode in op and 'request' in op[op_mode]:
                             cw.p(f"/* {op.enum_name} - {op_mode} */")
                             ri = RenderInfo(cw, parsed, args.mode, op, op_name, op_mode)
-- 
2.39.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH net-next v2 02/14] tools: ynl: move the cli and netlink code around
  2023-01-31  2:33 [PATCH net-next v2 00/14] tools: ynl: more docs and basic ethtool support Jakub Kicinski
  2023-01-31  2:33 ` [PATCH net-next v2 01/14] tools: ynl-gen: prevent do / dump reordering Jakub Kicinski
@ 2023-01-31  2:33 ` Jakub Kicinski
  2023-01-31  2:33 ` [PATCH net-next v2 03/14] tools: ynl: add an object hierarchy to represent parsed spec Jakub Kicinski
                   ` (12 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: Jakub Kicinski @ 2023-01-31  2:33 UTC (permalink / raw)
  To: davem; +Cc: netdev, edumazet, pabeni, sdf, linux-doc, Jakub Kicinski

Move the CLI code out of samples/ and the library part
of it into tools/net/ynl/lib/. This way we can start
sharing some code with the code gen.

Initially I thought that code gen is too C-specific to
share anything but basic stuff like calculating values
for enums can easily be shared.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 tools/net/ynl/{samples => }/cli.py    | 2 +-
 tools/net/ynl/lib/__init__.py         | 5 +++++
 tools/net/ynl/{samples => lib}/ynl.py | 0
 3 files changed, 6 insertions(+), 1 deletion(-)
 rename tools/net/ynl/{samples => }/cli.py (97%)
 create mode 100644 tools/net/ynl/lib/__init__.py
 rename tools/net/ynl/{samples => lib}/ynl.py (100%)

diff --git a/tools/net/ynl/samples/cli.py b/tools/net/ynl/cli.py
similarity index 97%
rename from tools/net/ynl/samples/cli.py
rename to tools/net/ynl/cli.py
index b27159c70710..5c4eb5a68514 100755
--- a/tools/net/ynl/samples/cli.py
+++ b/tools/net/ynl/cli.py
@@ -6,7 +6,7 @@ import json
 import pprint
 import time
 
-from ynl import YnlFamily
+from lib import YnlFamily
 
 
 def main():
diff --git a/tools/net/ynl/lib/__init__.py b/tools/net/ynl/lib/__init__.py
new file mode 100644
index 000000000000..0a6102758ebe
--- /dev/null
+++ b/tools/net/ynl/lib/__init__.py
@@ -0,0 +1,5 @@
+# SPDX-License-Identifier: BSD-3-Clause
+
+from .ynl import YnlFamily
+
+__all__ = ["YnlFamily"]
diff --git a/tools/net/ynl/samples/ynl.py b/tools/net/ynl/lib/ynl.py
similarity index 100%
rename from tools/net/ynl/samples/ynl.py
rename to tools/net/ynl/lib/ynl.py
-- 
2.39.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH net-next v2 03/14] tools: ynl: add an object hierarchy to represent parsed spec
  2023-01-31  2:33 [PATCH net-next v2 00/14] tools: ynl: more docs and basic ethtool support Jakub Kicinski
  2023-01-31  2:33 ` [PATCH net-next v2 01/14] tools: ynl-gen: prevent do / dump reordering Jakub Kicinski
  2023-01-31  2:33 ` [PATCH net-next v2 02/14] tools: ynl: move the cli and netlink code around Jakub Kicinski
@ 2023-01-31  2:33 ` Jakub Kicinski
  2023-01-31  2:33 ` [PATCH net-next v2 04/14] tools: ynl: use the common YAML loading and validation code Jakub Kicinski
                   ` (11 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: Jakub Kicinski @ 2023-01-31  2:33 UTC (permalink / raw)
  To: davem; +Cc: netdev, edumazet, pabeni, sdf, linux-doc, Jakub Kicinski

There's a lot of copy and pasting going on between the "cli"
and code gen when it comes to representing the parsed spec.
Create a library which both can use.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 tools/net/ynl/lib/__init__.py |   4 +-
 tools/net/ynl/lib/nlspec.py   | 301 ++++++++++++++++++++++++++++++++++
 2 files changed, 304 insertions(+), 1 deletion(-)
 create mode 100644 tools/net/ynl/lib/nlspec.py

diff --git a/tools/net/ynl/lib/__init__.py b/tools/net/ynl/lib/__init__.py
index 0a6102758ebe..3c73f59eabab 100644
--- a/tools/net/ynl/lib/__init__.py
+++ b/tools/net/ynl/lib/__init__.py
@@ -1,5 +1,7 @@
 # SPDX-License-Identifier: BSD-3-Clause
 
+from .nlspec import SpecAttr, SpecAttrSet, SpecFamily, SpecOperation
 from .ynl import YnlFamily
 
-__all__ = ["YnlFamily"]
+__all__ = ["SpecAttr", "SpecAttrSet", "SpecFamily", "SpecOperation",
+           "YnlFamily"]
diff --git a/tools/net/ynl/lib/nlspec.py b/tools/net/ynl/lib/nlspec.py
new file mode 100644
index 000000000000..4aa3b1ad97f0
--- /dev/null
+++ b/tools/net/ynl/lib/nlspec.py
@@ -0,0 +1,301 @@
+# SPDX-License-Identifier: BSD-3-Clause
+
+import collections
+import jsonschema
+import os
+import traceback
+import yaml
+
+
+class SpecElement:
+    """Netlink spec element.
+
+    Abstract element of the Netlink spec. Implements the dictionary interface
+    for access to the raw spec. Supports iterative resolution of dependencies
+    across elements and class inheritance levels. The elements of the spec
+    may refer to each other, and although loops should be very rare, having
+    to maintain correct ordering of instantiation is painful, so the resolve()
+    method should be used to perform parts of init which require access to
+    other parts of the spec.
+
+    Attributes:
+        yaml        raw spec as loaded from the spec file
+        family      back reference to the full family
+
+        name        name of the entity as listed in the spec (optional)
+        ident_name  name which can be safely used as identifier in code (optional)
+    """
+    def __init__(self, family, yaml):
+        self.yaml = yaml
+        self.family = family
+
+        if 'name' in self.yaml:
+            self.name = self.yaml['name']
+            self.ident_name = self.name.replace('-', '_')
+
+        self._super_resolved = False
+        family.add_unresolved(self)
+
+    def __getitem__(self, key):
+        return self.yaml[key]
+
+    def __contains__(self, key):
+        return key in self.yaml
+
+    def get(self, key, default=None):
+        return self.yaml.get(key, default)
+
+    def resolve_up(self, up):
+        if not self._super_resolved:
+            up.resolve()
+            self._super_resolved = True
+
+    def resolve(self):
+        pass
+
+
+class SpecAttr(SpecElement):
+    """ Single Netlink atttribute type
+
+    Represents a single attribute type within an attr space.
+
+    Attributes:
+        value      numerical ID when serialized
+        attr_set   Attribute Set containing this attr
+    """
+    def __init__(self, family, attr_set, yaml, value):
+        super().__init__(family, yaml)
+
+        self.value = value
+        self.attr_set = attr_set
+        self.is_multi = yaml.get('multi-attr', False)
+
+
+class SpecAttrSet(SpecElement):
+    """ Netlink Attribute Set class.
+
+    Represents a ID space of attributes within Netlink.
+
+    Note that unlike other elements, which expose contents of the raw spec
+    via the dictionary interface Attribute Set exposes attributes by name.
+
+    Attributes:
+        attrs      ordered dict of all attributes (indexed by name)
+        attrs_by_val  ordered dict of all attributes (indexed by value)
+        subset_of  parent set if this is a subset, otherwise None
+    """
+    def __init__(self, family, yaml):
+        super().__init__(family, yaml)
+
+        self.subset_of = self.yaml.get('subset-of', None)
+
+        self.attrs = collections.OrderedDict()
+        self.attrs_by_val = collections.OrderedDict()
+
+        val = 0
+        for elem in self.yaml['attributes']:
+            if 'value' in elem:
+                val = elem['value']
+
+            attr = self.new_attr(elem, val)
+            self.attrs[attr.name] = attr
+            self.attrs_by_val[attr.value] = attr
+            val += 1
+
+    def new_attr(self, elem, value):
+        return SpecAttr(self.family, self, elem, value)
+
+    def __getitem__(self, key):
+        return self.attrs[key]
+
+    def __contains__(self, key):
+        return key in self.attrs
+
+    def __iter__(self):
+        yield from self.attrs
+
+    def items(self):
+        return self.attrs.items()
+
+
+class SpecOperation(SpecElement):
+    """Netlink Operation
+
+    Information about a single Netlink operation.
+
+    Attributes:
+        value       numerical ID when serialized, None if req/rsp values differ
+
+        req_value   numerical ID when serialized, user -> kernel
+        rsp_value   numerical ID when serialized, user <- kernel
+        is_call     bool, whether the operation is a call
+        is_async    bool, whether the operation is a notification
+        is_resv     bool, whether the operation does not exist (it's just a reserved ID)
+        attr_set    attribute set name
+
+        yaml        raw spec as loaded from the spec file
+    """
+    def __init__(self, family, yaml, req_value, rsp_value):
+        super().__init__(family, yaml)
+
+        self.value = req_value if req_value == rsp_value else None
+        self.req_value = req_value
+        self.rsp_value = rsp_value
+
+        self.is_call = 'do' in yaml or 'dump' in yaml
+        self.is_async = 'notify' in yaml or 'event' in yaml
+        self.is_resv = not self.is_async and not self.is_call
+
+        # Added by resolve:
+        self.attr_set = None
+        delattr(self, "attr_set")
+
+    def resolve(self):
+        self.resolve_up(super())
+
+        if 'attribute-set' in self.yaml:
+            attr_set_name = self.yaml['attribute-set']
+        elif 'notify' in self.yaml:
+            msg = self.family.msgs[self.yaml['notify']]
+            attr_set_name = msg['attribute-set']
+        elif self.is_resv:
+            attr_set_name = ''
+        else:
+            raise Exception(f"Can't resolve attribute set for op '{self.name}'")
+        if attr_set_name:
+            self.attr_set = self.family.attr_sets[attr_set_name]
+
+
+class SpecFamily(SpecElement):
+    """ Netlink Family Spec class.
+
+    Netlink family information loaded from a spec (e.g. in YAML).
+    Takes care of unfolding implicit information which can be skipped
+    in the spec itself for brevity.
+
+    The class can be used like a dictionary to access the raw spec
+    elements but that's usually a bad idea.
+
+    Attributes:
+        proto     protocol type (e.g. genetlink)
+
+        attr_sets  dict of attribute sets
+        msgs       dict of all messages (index by name)
+        msgs_by_value  dict of all messages (indexed by name)
+        ops        dict of all valid requests / responses
+    """
+    def __init__(self, spec_path, schema_path=None):
+        with open(spec_path, "r") as stream:
+            spec = yaml.safe_load(stream)
+
+        self._resolution_list = []
+
+        super().__init__(self, spec)
+
+        self.proto = self.yaml.get('protocol', 'genetlink')
+
+        if schema_path is None:
+            schema_path = os.path.dirname(os.path.dirname(spec_path)) + f'/{self.proto}.yaml'
+        if schema_path:
+            with open(schema_path, "r") as stream:
+                schema = yaml.safe_load(stream)
+
+            jsonschema.validate(self.yaml, schema)
+
+        self.attr_sets = collections.OrderedDict()
+        self.msgs = collections.OrderedDict()
+        self.req_by_value = collections.OrderedDict()
+        self.rsp_by_value = collections.OrderedDict()
+        self.ops = collections.OrderedDict()
+
+        last_exception = None
+        while len(self._resolution_list) > 0:
+            resolved = []
+            unresolved = self._resolution_list
+            self._resolution_list = []
+
+            for elem in unresolved:
+                try:
+                    elem.resolve()
+                except (KeyError, AttributeError) as e:
+                    self._resolution_list.append(elem)
+                    last_exception = e
+                    continue
+
+                resolved.append(elem)
+
+            if len(resolved) == 0:
+                traceback.print_exception(last_exception)
+                raise Exception("Could not resolve any spec element, infinite loop?")
+
+    def new_attr_set(self, elem):
+        return SpecAttrSet(self, elem)
+
+    def new_operation(self, elem, req_val, rsp_val):
+        return SpecOperation(self, elem, req_val, rsp_val)
+
+    def add_unresolved(self, elem):
+        self._resolution_list.append(elem)
+
+    def _dictify_ops_unified(self):
+        val = 0
+        for elem in self.yaml['operations']['list']:
+            if 'value' in elem:
+                val = elem['value']
+
+            op = self.new_operation(elem, val, val)
+            val += 1
+
+            self.msgs[op.name] = op
+
+    def _dictify_ops_directional(self):
+        req_val = rsp_val = 0
+        for elem in self.yaml['operations']['list']:
+            if 'notify' in elem:
+                if 'value' in elem:
+                    rsp_val = elem['value']
+                req_val_next = req_val
+                rsp_val_next = rsp_val + 1
+                req_val = None
+            elif 'do' in elem or 'dump' in elem:
+                mode = elem['do'] if 'do' in elem else elem['dump']
+
+                v = mode.get('request', {}).get('value', None)
+                if v:
+                    req_val = v
+                v = mode.get('reply', {}).get('value', None)
+                if v:
+                    rsp_val = v
+
+                rsp_inc = 1 if 'reply' in mode else 0
+                req_val_next = req_val + 1
+                rsp_val_next = rsp_val + rsp_inc
+            else:
+                raise Exception("Can't parse directional ops")
+
+            op = self.new_operation(elem, req_val, rsp_val)
+            req_val = req_val_next
+            rsp_val = rsp_val_next
+
+            self.msgs[op.name] = op
+
+    def resolve(self):
+        self.resolve_up(super())
+
+        for elem in self.yaml['attribute-sets']:
+            attr_set = self.new_attr_set(elem)
+            self.attr_sets[elem['name']] = attr_set
+
+        msg_id_model = self.yaml['operations'].get('enum-model', 'unified')
+        if msg_id_model == 'unified':
+            self._dictify_ops_unified()
+        elif msg_id_model == 'directional':
+            self._dictify_ops_directional()
+
+        for op in self.msgs.values():
+            if op.req_value is not None:
+                self.req_by_value[op.req_value] = op
+            if op.rsp_value is not None:
+                self.rsp_by_value[op.rsp_value] = op
+            if not op.is_async and 'attribute-set' in op:
+                self.ops[op.name] = op
-- 
2.39.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH net-next v2 04/14] tools: ynl: use the common YAML loading and validation code
  2023-01-31  2:33 [PATCH net-next v2 00/14] tools: ynl: more docs and basic ethtool support Jakub Kicinski
                   ` (2 preceding siblings ...)
  2023-01-31  2:33 ` [PATCH net-next v2 03/14] tools: ynl: add an object hierarchy to represent parsed spec Jakub Kicinski
@ 2023-01-31  2:33 ` Jakub Kicinski
  2023-01-31  2:33 ` [PATCH net-next v2 05/14] tools: ynl: add support for types needed by ethtool Jakub Kicinski
                   ` (10 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: Jakub Kicinski @ 2023-01-31  2:33 UTC (permalink / raw)
  To: davem; +Cc: netdev, edumazet, pabeni, sdf, linux-doc, Jakub Kicinski

Adapt the common object hierarchy in code gen and CLI.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 tools/net/ynl/lib/ynl.py   | 118 ++++-------------
 tools/net/ynl/ynl-gen-c.py | 256 +++++++++++++++++--------------------
 2 files changed, 142 insertions(+), 232 deletions(-)

diff --git a/tools/net/ynl/lib/ynl.py b/tools/net/ynl/lib/ynl.py
index b71523d71d46..0ceb627ba686 100644
--- a/tools/net/ynl/lib/ynl.py
+++ b/tools/net/ynl/lib/ynl.py
@@ -1,13 +1,14 @@
 # SPDX-License-Identifier: BSD-3-Clause
 
 import functools
-import jsonschema
 import os
 import random
 import socket
 import struct
 import yaml
 
+from .nlspec import SpecFamily
+
 #
 # Generic Netlink code which should really be in some library, but I can't quickly find one.
 #
@@ -158,8 +159,8 @@ import yaml
                 # We don't have the ability to parse nests yet, so only do global
                 if 'miss-type' in self.extack and 'miss-nest' not in self.extack:
                     miss_type = self.extack['miss-type']
-                    if len(attr_space.attr_list) > miss_type:
-                        spec = attr_space.attr_list[miss_type]
+                    if miss_type in attr_space.attrs_by_val:
+                        spec = attr_space.attrs_by_val[miss_type]
                         desc = spec['name']
                         if 'doc' in spec:
                             desc += f" ({spec['doc']})"
@@ -289,100 +290,31 @@ genl_family_name_to_id = None
 #
 
 
-class YnlAttrSpace:
-    def __init__(self, family, yaml):
-        self.yaml = yaml
-
-        self.attrs = dict()
-        self.name = self.yaml['name']
-        self.subspace_of = self.yaml['subset-of'] if 'subspace-of' in self.yaml else None
-
-        val = 0
-        max_val = 0
-        for elem in self.yaml['attributes']:
-            if 'value' in elem:
-                val = elem['value']
-            else:
-                elem['value'] = 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['value']] = 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:
+class YnlFamily(SpecFamily):
     def __init__(self, def_path, schema=None):
-        self.include_raw = False
+        super().__init__(def_path, schema)
 
-        with open(def_path, "r") as stream:
-            self.yaml = yaml.safe_load(stream)
-
-        if schema:
-            with open(schema, "r") as stream:
-                schema = yaml.safe_load(stream)
-
-            jsonschema.validate(self.yaml, schema)
+        self.include_raw = False
 
         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.sock.setsockopt(Netlink.SOL_NETLINK, Netlink.NETLINK_EXT_ACK, 1)
 
-        self._ops = dict()
-        self._spaces = dict()
         self._types = dict()
 
-        for elem in self.yaml['attribute-sets']:
-            self._spaces[elem['name']] = YnlAttrSpace(self, elem)
-
         for elem in self.yaml['definitions']:
             self._types[elem['name']] = elem
 
-        async_separation = 'async-prefix' in self.yaml['operations']
         self.async_msg_ids = set()
         self.async_msg_queue = []
-        val = 0
-        max_val = 0
-        for elem in self.yaml['operations']['list']:
-            if not (async_separation and ('notify' in elem or 'event' in elem)):
-                if 'value' in elem:
-                    val = elem['value']
-                else:
-                    elem['value'] = val
-                val += 1
-                max_val = max(val, max_val)
-
-            if 'notify' in elem or 'event' in elem:
-                self.async_msg_ids.add(elem['value'])
-
-            self._ops[elem['name']] = elem
-
-            op_name = elem['name'].replace('-', '_')
 
-            bound_f = functools.partial(self._op, elem['name'])
-            setattr(self, op_name, bound_f)
+        for msg in self.msgs.values():
+            if msg.is_async:
+                self.async_msg_ids.add(msg.value)
 
-        self._op_array = [None] * max_val
-        for _, op in self._ops.items():
-            self._op_array[op['value']] = op
-            if 'notify' in op:
-                op['attribute-set'] = self._ops[op['notify']]['attribute-set']
+        for op_name, op in self.ops.items():
+            bound_f = functools.partial(self._op, op_name)
+            setattr(self, op.ident_name, bound_f)
 
         self.family = GenlFamily(self.yaml['name'])
 
@@ -395,8 +327,8 @@ genl_family_name_to_id = None
                              self.family.genl_family['mcast'][mcast_name])
 
     def _add_attr(self, space, name, value):
-        attr = self._spaces[space][name]
-        nl_type = attr['value']
+        attr = self.attr_sets[space][name]
+        nl_type = attr.value
         if attr["type"] == 'nest':
             nl_type |= Netlink.NLA_F_NESTED
             attr_payload = b''
@@ -430,10 +362,10 @@ genl_family_name_to_id = None
         rsp[attr_spec['name']] = value
 
     def _decode(self, attrs, space):
-        attr_space = self._spaces[space]
+        attr_space = self.attr_sets[space]
         rsp = dict()
         for attr in attrs:
-            attr_spec = attr_space.attr_list[attr.type]
+            attr_spec = attr_space.attrs_by_val[attr.type]
             if attr_spec["type"] == 'nest':
                 subdict = self._decode(NlAttrs(attr.raw), attr_spec['nested-attributes'])
                 rsp[attr_spec['name']] = subdict
@@ -457,9 +389,9 @@ genl_family_name_to_id = None
         if self.include_raw:
             msg['nlmsg'] = nl_msg
             msg['genlmsg'] = genl_msg
-        op = self._op_array[genl_msg.genl_cmd]
+        op = self.msgs_by_value[genl_msg.genl_cmd]
         msg['name'] = op['name']
-        msg['msg'] = self._decode(genl_msg.raw_attrs, op['attribute-set'])
+        msg['msg'] = self._decode(genl_msg.raw_attrs, op.attr_set.name)
         self.async_msg_queue.append(msg)
 
     def check_ntf(self):
@@ -487,16 +419,16 @@ genl_family_name_to_id = None
                 self.handle_ntf(nl_msg, gm)
 
     def _op(self, method, vals, dump=False):
-        op = self._ops[method]
+        op = self.ops[method]
 
         nl_flags = Netlink.NLM_F_REQUEST | Netlink.NLM_F_ACK
         if dump:
             nl_flags |= Netlink.NLM_F_DUMP
 
         req_seq = random.randint(1024, 65535)
-        msg = _genl_msg(self.family.family_id, nl_flags, op['value'], 1, req_seq)
+        msg = _genl_msg(self.family.family_id, nl_flags, op.value, 1, req_seq)
         for name, value in vals.items():
-            msg += self._add_attr(op['attribute-set'], name, value)
+            msg += self._add_attr(op.attr_set.name, name, value)
         msg = _genl_msg_finalize(msg)
 
         self.sock.send(msg, 0)
@@ -505,7 +437,7 @@ genl_family_name_to_id = None
         rsp = []
         while not done:
             reply = self.sock.recv(128 * 1024)
-            nms = NlMsgs(reply, attr_space=self._spaces[op['attribute-set']])
+            nms = NlMsgs(reply, attr_space=op.attr_set)
             for nl_msg in nms:
                 if nl_msg.error:
                     print("Netlink error:", os.strerror(-nl_msg.error))
@@ -517,7 +449,7 @@ genl_family_name_to_id = None
 
                 gm = GenlMsg(nl_msg)
                 # Check if this is a reply to our request
-                if nl_msg.nl_seq != req_seq or gm.genl_cmd != op['value']:
+                if nl_msg.nl_seq != req_seq or gm.genl_cmd != op.value:
                     if gm.genl_cmd in self.async_msg_ids:
                         self.handle_ntf(nl_msg, gm)
                         continue
@@ -525,7 +457,7 @@ genl_family_name_to_id = None
                         print('Unexpected message: ' + repr(gm))
                         continue
 
-                rsp.append(self._decode(gm.raw_attrs, op['attribute-set']))
+                rsp.append(self._decode(gm.raw_attrs, op.attr_set.name))
 
         if not rsp:
             return None
diff --git a/tools/net/ynl/ynl-gen-c.py b/tools/net/ynl/ynl-gen-c.py
index e5002d420961..dc14da634e8e 100755
--- a/tools/net/ynl/ynl-gen-c.py
+++ b/tools/net/ynl/ynl-gen-c.py
@@ -2,10 +2,11 @@
 
 import argparse
 import collections
-import jsonschema
 import os
 import yaml
 
+from lib import SpecFamily, SpecAttrSet, SpecAttr, SpecOperation
+
 
 def c_upper(name):
     return name.upper().replace('-', '_')
@@ -28,12 +29,12 @@ import yaml
                    "ynl_cb_array, NLMSG_MIN_TYPE)"
 
 
-class Type:
-    def __init__(self, family, attr_set, attr):
-        self.family = family
+class Type(SpecAttr):
+    def __init__(self, family, attr_set, attr, value):
+        super().__init__(family, attr_set, attr, value)
+
         self.attr = attr
-        self.value = attr['value']
-        self.name = c_lower(attr['name'])
+        self.attr_set = attr_set
         self.type = attr['type']
         self.checks = attr.get('checks', {})
 
@@ -46,17 +47,17 @@ import yaml
             else:
                 self.nested_render_name = f"{family.name}_{c_lower(self.nested_attrs)}"
 
-        self.enum_name = f"{attr_set.name_prefix}{self.name}"
-        self.enum_name = c_upper(self.enum_name)
         self.c_name = c_lower(self.name)
         if self.c_name in _C_KW:
             self.c_name += '_'
 
-    def __getitem__(self, key):
-        return self.attr[key]
+        # Added by resolve():
+        self.enum_name = None
+        delattr(self, "enum_name")
 
-    def __contains__(self, key):
-        return key in self.attr
+    def resolve(self):
+        self.enum_name = f"{self.attr_set.name_prefix}{self.name}"
+        self.enum_name = c_upper(self.enum_name)
 
     def is_multi_val(self):
         return None
@@ -214,24 +215,34 @@ import yaml
 
 
 class TypeScalar(Type):
-    def __init__(self, family, attr_set, attr):
-        super().__init__(family, attr_set, attr)
+    def __init__(self, family, attr_set, attr, value):
+        super().__init__(family, attr_set, attr, value)
+
+        self.byte_order_comment = ''
+        if 'byte-order' in attr:
+            self.byte_order_comment = f" /* {attr['byte-order']} */"
+
+        # Added by resolve():
+        self.is_bitfield = None
+        delattr(self, "is_bitfield")
+        self.type_name = None
+        delattr(self, "type_name")
+
+    def resolve(self):
+        self.resolve_up(super())
 
-        self.is_bitfield = False
-        if 'enum' in self.attr:
-            self.is_bitfield = family.consts[self.attr['enum']]['type'] == 'flags'
         if 'enum-as-flags' in self.attr and self.attr['enum-as-flags']:
             self.is_bitfield = True
+        elif 'enum' in self.attr:
+            self.is_bitfield = self.family.consts[self.attr['enum']]['type'] == 'flags'
+        else:
+            self.is_bitfield = False
 
         if 'enum' in self.attr and not self.is_bitfield:
-            self.type_name = f"enum {family.name}_{c_lower(self.attr['enum'])}"
+            self.type_name = f"enum {self.family.name}_{c_lower(self.attr['enum'])}"
         else:
             self.type_name = '__' + self.type
 
-        self.byte_order_comment = ''
-        if 'byte-order' in attr:
-            self.byte_order_comment = f" /* {attr['byte-order']} */"
-
     def _mnl_type(self):
         t = self.type
         # mnl does not have a helper for signed types
@@ -648,14 +659,11 @@ import yaml
         return mask
 
 
-class AttrSet:
+class AttrSet(SpecAttrSet):
     def __init__(self, family, yaml):
-        self.yaml = yaml
+        super().__init__(family, yaml)
 
-        self.attrs = dict()
-        self.name = self.yaml['name']
-        if 'subset-of' not in yaml:
-            self.subset_of = None
+        if self.subset_of is None:
             if 'name-prefix' in yaml:
                 pfx = yaml['name-prefix']
             elif self.name == family.name:
@@ -665,83 +673,68 @@ import yaml
             self.name_prefix = c_upper(pfx)
             self.max_name = c_upper(self.yaml.get('attr-max-name', f"{self.name_prefix}max"))
         else:
-            self.subset_of = self.yaml['subset-of']
             self.name_prefix = family.attr_sets[self.subset_of].name_prefix
             self.max_name = family.attr_sets[self.subset_of].max_name
 
+        # Added by resolve:
+        self.c_name = None
+        delattr(self, "c_name")
+
+    def resolve(self):
         self.c_name = c_lower(self.name)
         if self.c_name in _C_KW:
             self.c_name += '_'
-        if self.c_name == family.c_name:
+        if self.c_name == self.family.c_name:
             self.c_name = ''
 
-        val = 0
-        for elem in self.yaml['attributes']:
-            if 'value' in elem:
-                val = elem['value']
-            else:
-                elem['value'] = val
-            val += 1
-
-            if 'multi-attr' in elem and elem['multi-attr']:
-                attr = TypeMultiAttr(family, self, elem)
-            elif elem['type'] in scalars:
-                attr = TypeScalar(family, self, elem)
-            elif elem['type'] == 'unused':
-                attr = TypeUnused(family, self, elem)
-            elif elem['type'] == 'pad':
-                attr = TypePad(family, self, elem)
-            elif elem['type'] == 'flag':
-                attr = TypeFlag(family, self, elem)
-            elif elem['type'] == 'string':
-                attr = TypeString(family, self, elem)
-            elif elem['type'] == 'binary':
-                attr = TypeBinary(family, self, elem)
-            elif elem['type'] == 'nest':
-                attr = TypeNest(family, self, elem)
-            elif elem['type'] == 'array-nest':
-                attr = TypeArrayNest(family, self, elem)
-            elif elem['type'] == 'nest-type-value':
-                attr = TypeNestTypeValue(family, self, elem)
-            else:
-                raise Exception(f"No typed class for type {elem['type']}")
-
-            self.attrs[elem['name']] = attr
-
-    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 new_attr(self, elem, value):
+        if 'multi-attr' in elem and elem['multi-attr']:
+            return TypeMultiAttr(self.family, self, elem, value)
+        elif elem['type'] in scalars:
+            return TypeScalar(self.family, self, elem, value)
+        elif elem['type'] == 'unused':
+            return TypeUnused(self.family, self, elem, value)
+        elif elem['type'] == 'pad':
+            return TypePad(self.family, self, elem, value)
+        elif elem['type'] == 'flag':
+            return TypeFlag(self.family, self, elem, value)
+        elif elem['type'] == 'string':
+            return TypeString(self.family, self, elem, value)
+        elif elem['type'] == 'binary':
+            return TypeBinary(self.family, self, elem, value)
+        elif elem['type'] == 'nest':
+            return TypeNest(self.family, self, elem, value)
+        elif elem['type'] == 'array-nest':
+            return TypeArrayNest(self.family, self, elem, value)
+        elif elem['type'] == 'nest-type-value':
+            return TypeNestTypeValue(self.family, self, elem, value)
+        else:
+            raise Exception(f"No typed class for type {elem['type']}")
 
-    def items(self):
-        return self.attrs.items()
 
+class Operation(SpecOperation):
+    def __init__(self, family, yaml, req_value, rsp_value):
+        super().__init__(family, yaml, req_value, rsp_value)
 
-class Operation:
-    def __init__(self, family, yaml, value):
-        self.yaml = yaml
-        self.value = value
+        if req_value != rsp_value:
+            raise Exception("Directional messages not supported by codegen")
 
-        self.name = self.yaml['name']
         self.render_name = family.name + '_' + c_lower(self.name)
-        self.is_async = 'notify' in yaml or 'event' in yaml
-        if not self.is_async:
-            self.enum_name = family.op_prefix + c_upper(self.name)
-        else:
-            self.enum_name = family.async_op_prefix + c_upper(self.name)
 
         self.dual_policy = ('do' in yaml and 'request' in yaml['do']) and \
                          ('dump' in yaml and 'request' in yaml['dump'])
 
-    def __getitem__(self, key):
-        return self.yaml[key]
+        # Added by resolve:
+        self.enum_name = None
+        delattr(self, "enum_name")
 
-    def __contains__(self, key):
-        return key in self.yaml
+    def resolve(self):
+        self.resolve_up(super())
+
+        if not self.is_async:
+            self.enum_name = self.family.op_prefix + c_upper(self.name)
+        else:
+            self.enum_name = self.family.async_op_prefix + c_upper(self.name)
 
     def add_notification(self, op):
         if 'notify' not in self.yaml:
@@ -751,21 +744,23 @@ import yaml
         self.yaml['notify']['cmds'].append(op)
 
 
-class Family:
+class Family(SpecFamily):
     def __init__(self, file_name):
-        with open(file_name, "r") as stream:
-            self.yaml = yaml.safe_load(stream)
-
-        self.proto = self.yaml.get('protocol', 'genetlink')
-
-        with open(os.path.dirname(os.path.dirname(file_name)) +
-                  f'/{self.proto}.yaml', "r") as stream:
-            schema = yaml.safe_load(stream)
-
-        jsonschema.validate(self.yaml, schema)
-
-        if self.yaml.get('protocol', 'genetlink') not in {'genetlink', 'genetlink-c', 'genetlink-legacy'}:
-            raise Exception("Codegen only supported for genetlink")
+        # Added by resolve:
+        self.c_name = None
+        delattr(self, "c_name")
+        self.op_prefix = None
+        delattr(self, "op_prefix")
+        self.async_op_prefix = None
+        delattr(self, "async_op_prefix")
+        self.mcgrps = None
+        delattr(self, "mcgrps")
+        self.consts = None
+        delattr(self, "consts")
+        self.hooks = None
+        delattr(self, "hooks")
+
+        super().__init__(file_name)
 
         self.fam_key = c_upper(self.yaml.get('c-family-name', self.yaml["name"] + '_FAMILY_NAME'))
         self.ver_key = c_upper(self.yaml.get('c-version-name', self.yaml["name"] + '_FAMILY_VERSION'))
@@ -773,12 +768,18 @@ import yaml
         if 'definitions' not in self.yaml:
             self.yaml['definitions'] = []
 
-        self.name = self.yaml['name']
-        self.c_name = c_lower(self.name)
         if 'uapi-header' in self.yaml:
             self.uapi_header = self.yaml['uapi-header']
         else:
             self.uapi_header = f"linux/{self.name}.h"
+
+    def resolve(self):
+        self.resolve_up(super())
+
+        if self.yaml.get('protocol', 'genetlink') not in {'genetlink', 'genetlink-c', 'genetlink-legacy'}:
+            raise Exception("Codegen only supported for genetlink")
+
+        self.c_name = c_lower(self.name)
         if 'name-prefix' in self.yaml['operations']:
             self.op_prefix = c_upper(self.yaml['operations']['name-prefix'])
         else:
@@ -791,12 +792,6 @@ import yaml
         self.mcgrps = self.yaml.get('mcast-groups', {'list': []})
 
         self.consts = dict()
-        # list of all operations
-        self.msg_list = []
-        # dict of operations which have their own message type (have attributes)
-        self.ops = collections.OrderedDict()
-        self.attr_sets = dict()
-        self.attr_sets_list = []
 
         self.hooks = dict()
         for when in ['pre', 'post']:
@@ -824,11 +819,11 @@ import yaml
         if self.kernel_policy == 'global':
             self._load_global_policy()
 
-    def __getitem__(self, key):
-        return self.yaml[key]
+    def new_attr_set(self, elem):
+        return AttrSet(self, elem)
 
-    def get(self, key, default=None):
-        return self.yaml.get(key, default)
+    def new_operation(self, elem, req_value, rsp_value):
+        return Operation(self, elem, req_value, rsp_value)
 
     # Fake a 'do' equivalent of all events, so that we can render their response parsing
     def _mock_up_events(self):
@@ -847,27 +842,10 @@ import yaml
             else:
                 self.consts[elem['name']] = elem
 
-        for elem in self.yaml['attribute-sets']:
-            attr_set = AttrSet(self, elem)
-            self.attr_sets[elem['name']] = attr_set
-            self.attr_sets_list.append((elem['name'], attr_set), )
-
         ntf = []
-        val = 0
-        for elem in self.yaml['operations']['list']:
-            if 'value' in elem:
-                val = elem['value']
-
-            op = Operation(self, elem, val)
-            val += 1
-
-            self.msg_list.append(op)
-            if 'notify' in elem:
-                ntf.append(op)
-                continue
-            if 'attribute-set' not in elem:
-                continue
-            self.ops[elem['name']] = op
+        for msg in self.msgs.values():
+            if 'notify' in msg:
+                ntf.append(msg)
         for n in ntf:
             self.ops[n['notify']].add_notification(n)
 
@@ -2033,7 +2011,7 @@ _C_KW = {
 
     max_by_define = family.get('max-by-define', False)
 
-    for _, attr_set in family.attr_sets_list:
+    for _, attr_set in family.attr_sets.items():
         if attr_set.subset_of:
             continue
 
@@ -2044,9 +2022,9 @@ _C_KW = {
         uapi_enum_start(family, cw, attr_set.yaml, 'enum-name')
         for _, attr in attr_set.items():
             suffix = ','
-            if attr['value'] != val:
-                suffix = f" = {attr['value']},"
-                val = attr['value']
+            if attr.value != val:
+                suffix = f" = {attr.value},"
+                val = attr.value
             val += 1
             cw.p(attr.enum_name + suffix)
         cw.nl()
@@ -2066,7 +2044,7 @@ _C_KW = {
     max_value = f"({cnt_name} - 1)"
 
     uapi_enum_start(family, cw, family['operations'], 'enum-name')
-    for op in family.msg_list:
+    for op in family.msgs.values():
         if separate_ntf and ('notify' in op or 'event' in op):
             continue
 
@@ -2085,7 +2063,7 @@ _C_KW = {
 
     if separate_ntf:
         uapi_enum_start(family, cw, family['operations'], enum_name='async-enum')
-        for op in family.msg_list:
+        for op in family.msgs.values():
             if separate_ntf and not ('notify' in op or 'event' in op):
                 continue
 
-- 
2.39.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH net-next v2 05/14] tools: ynl: add support for types needed by ethtool
  2023-01-31  2:33 [PATCH net-next v2 00/14] tools: ynl: more docs and basic ethtool support Jakub Kicinski
                   ` (3 preceding siblings ...)
  2023-01-31  2:33 ` [PATCH net-next v2 04/14] tools: ynl: use the common YAML loading and validation code Jakub Kicinski
@ 2023-01-31  2:33 ` Jakub Kicinski
  2023-01-31  2:33 ` [PATCH net-next v2 06/14] tools: ynl: support directional enum-model in CLI Jakub Kicinski
                   ` (9 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: Jakub Kicinski @ 2023-01-31  2:33 UTC (permalink / raw)
  To: davem; +Cc: netdev, edumazet, pabeni, sdf, linux-doc, Jakub Kicinski

Ethtool needs support for handful of extra types.
It doesn't have the definitions section yet.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 tools/net/ynl/lib/ynl.py | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/tools/net/ynl/lib/ynl.py b/tools/net/ynl/lib/ynl.py
index 0ceb627ba686..a656b655d302 100644
--- a/tools/net/ynl/lib/ynl.py
+++ b/tools/net/ynl/lib/ynl.py
@@ -75,6 +75,9 @@ from .nlspec import SpecFamily
         self.full_len = (self.payload_len + 3) & ~3
         self.raw = raw[offset + 4:offset + self.payload_len]
 
+    def as_u8(self):
+        return struct.unpack("B", self.raw)[0]
+
     def as_u16(self):
         return struct.unpack("H", self.raw)[0]
 
@@ -302,7 +305,7 @@ genl_family_name_to_id = None
 
         self._types = dict()
 
-        for elem in self.yaml['definitions']:
+        for elem in self.yaml.get('definitions', []):
             self._types[elem['name']] = elem
 
         self.async_msg_ids = set()
@@ -334,6 +337,8 @@ genl_family_name_to_id = None
             attr_payload = b''
             for subname, subvalue in value.items():
                 attr_payload += self._add_attr(attr['nested-attributes'], subname, subvalue)
+        elif attr["type"] == 'flag':
+            attr_payload = b''
         elif attr["type"] == 'u32':
             attr_payload = struct.pack("I", int(value))
         elif attr["type"] == 'string':
@@ -369,6 +374,8 @@ genl_family_name_to_id = None
             if attr_spec["type"] == 'nest':
                 subdict = self._decode(NlAttrs(attr.raw), attr_spec['nested-attributes'])
                 rsp[attr_spec['name']] = subdict
+            elif attr_spec['type'] == 'u8':
+                rsp[attr_spec['name']] = attr.as_u8()
             elif attr_spec['type'] == 'u32':
                 rsp[attr_spec['name']] = attr.as_u32()
             elif attr_spec['type'] == 'u64':
@@ -377,6 +384,8 @@ genl_family_name_to_id = None
                 rsp[attr_spec['name']] = attr.as_strz()
             elif attr_spec["type"] == 'binary':
                 rsp[attr_spec['name']] = attr.as_bin()
+            elif attr_spec["type"] == 'flag':
+                rsp[attr_spec['name']] = True
             else:
                 raise Exception(f'Unknown {attr.type} {attr_spec["name"]} {attr_spec["type"]}')
 
-- 
2.39.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH net-next v2 06/14] tools: ynl: support directional enum-model in CLI
  2023-01-31  2:33 [PATCH net-next v2 00/14] tools: ynl: more docs and basic ethtool support Jakub Kicinski
                   ` (4 preceding siblings ...)
  2023-01-31  2:33 ` [PATCH net-next v2 05/14] tools: ynl: add support for types needed by ethtool Jakub Kicinski
@ 2023-01-31  2:33 ` Jakub Kicinski
  2023-01-31  2:33 ` [PATCH net-next v2 07/14] tools: ynl: support multi-attr Jakub Kicinski
                   ` (8 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: Jakub Kicinski @ 2023-01-31  2:33 UTC (permalink / raw)
  To: davem; +Cc: netdev, edumazet, pabeni, sdf, linux-doc, Jakub Kicinski

Support families which use different IDs for messages
to and from the kernel.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 tools/net/ynl/lib/ynl.py | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tools/net/ynl/lib/ynl.py b/tools/net/ynl/lib/ynl.py
index a656b655d302..690065003935 100644
--- a/tools/net/ynl/lib/ynl.py
+++ b/tools/net/ynl/lib/ynl.py
@@ -313,7 +313,7 @@ genl_family_name_to_id = None
 
         for msg in self.msgs.values():
             if msg.is_async:
-                self.async_msg_ids.add(msg.value)
+                self.async_msg_ids.add(msg.rsp_value)
 
         for op_name, op in self.ops.items():
             bound_f = functools.partial(self._op, op_name)
@@ -398,7 +398,7 @@ genl_family_name_to_id = None
         if self.include_raw:
             msg['nlmsg'] = nl_msg
             msg['genlmsg'] = genl_msg
-        op = self.msgs_by_value[genl_msg.genl_cmd]
+        op = self.rsp_by_value[genl_msg.genl_cmd]
         msg['name'] = op['name']
         msg['msg'] = self._decode(genl_msg.raw_attrs, op.attr_set.name)
         self.async_msg_queue.append(msg)
@@ -435,7 +435,7 @@ genl_family_name_to_id = None
             nl_flags |= Netlink.NLM_F_DUMP
 
         req_seq = random.randint(1024, 65535)
-        msg = _genl_msg(self.family.family_id, nl_flags, op.value, 1, req_seq)
+        msg = _genl_msg(self.family.family_id, nl_flags, op.req_value, 1, req_seq)
         for name, value in vals.items():
             msg += self._add_attr(op.attr_set.name, name, value)
         msg = _genl_msg_finalize(msg)
@@ -458,7 +458,7 @@ genl_family_name_to_id = None
 
                 gm = GenlMsg(nl_msg)
                 # Check if this is a reply to our request
-                if nl_msg.nl_seq != req_seq or gm.genl_cmd != op.value:
+                if nl_msg.nl_seq != req_seq or gm.genl_cmd != op.rsp_value:
                     if gm.genl_cmd in self.async_msg_ids:
                         self.handle_ntf(nl_msg, gm)
                         continue
-- 
2.39.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH net-next v2 07/14] tools: ynl: support multi-attr
  2023-01-31  2:33 [PATCH net-next v2 00/14] tools: ynl: more docs and basic ethtool support Jakub Kicinski
                   ` (5 preceding siblings ...)
  2023-01-31  2:33 ` [PATCH net-next v2 06/14] tools: ynl: support directional enum-model in CLI Jakub Kicinski
@ 2023-01-31  2:33 ` Jakub Kicinski
  2023-01-31  2:33 ` [PATCH net-next v2 08/14] tools: ynl: support pretty printing bad attribute names Jakub Kicinski
                   ` (7 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: Jakub Kicinski @ 2023-01-31  2:33 UTC (permalink / raw)
  To: davem; +Cc: netdev, edumazet, pabeni, sdf, linux-doc, Jakub Kicinski

Ethtool uses mutli-attr, add the support to YNL.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 tools/net/ynl/lib/ynl.py | 21 ++++++++++++++-------
 1 file changed, 14 insertions(+), 7 deletions(-)

diff --git a/tools/net/ynl/lib/ynl.py b/tools/net/ynl/lib/ynl.py
index 690065003935..c16326495cb7 100644
--- a/tools/net/ynl/lib/ynl.py
+++ b/tools/net/ynl/lib/ynl.py
@@ -373,22 +373,29 @@ genl_family_name_to_id = None
             attr_spec = attr_space.attrs_by_val[attr.type]
             if attr_spec["type"] == 'nest':
                 subdict = self._decode(NlAttrs(attr.raw), attr_spec['nested-attributes'])
-                rsp[attr_spec['name']] = subdict
+                decoded = subdict
             elif attr_spec['type'] == 'u8':
-                rsp[attr_spec['name']] = attr.as_u8()
+                decoded = attr.as_u8()
             elif attr_spec['type'] == 'u32':
-                rsp[attr_spec['name']] = attr.as_u32()
+                decoded = attr.as_u32()
             elif attr_spec['type'] == 'u64':
-                rsp[attr_spec['name']] = attr.as_u64()
+                decoded = attr.as_u64()
             elif attr_spec["type"] == 'string':
-                rsp[attr_spec['name']] = attr.as_strz()
+                decoded = attr.as_strz()
             elif attr_spec["type"] == 'binary':
-                rsp[attr_spec['name']] = attr.as_bin()
+                decoded = attr.as_bin()
             elif attr_spec["type"] == 'flag':
-                rsp[attr_spec['name']] = True
+                decoded = True
             else:
                 raise Exception(f'Unknown {attr.type} {attr_spec["name"]} {attr_spec["type"]}')
 
+            if not attr_spec.is_multi:
+                rsp[attr_spec['name']] = decoded
+            elif attr_spec.name in rsp:
+                rsp[attr_spec.name].append(decoded)
+            else:
+                rsp[attr_spec.name] = [decoded]
+
             if 'enum' in attr_spec:
                 self._decode_enum(rsp, attr_spec)
         return rsp
-- 
2.39.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH net-next v2 08/14] tools: ynl: support pretty printing bad attribute names
  2023-01-31  2:33 [PATCH net-next v2 00/14] tools: ynl: more docs and basic ethtool support Jakub Kicinski
                   ` (6 preceding siblings ...)
  2023-01-31  2:33 ` [PATCH net-next v2 07/14] tools: ynl: support multi-attr Jakub Kicinski
@ 2023-01-31  2:33 ` Jakub Kicinski
  2023-01-31  2:33 ` [PATCH net-next v2 09/14] tools: ynl: use operation names from spec on the CLI Jakub Kicinski
                   ` (6 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: Jakub Kicinski @ 2023-01-31  2:33 UTC (permalink / raw)
  To: davem; +Cc: netdev, edumazet, pabeni, sdf, linux-doc, Jakub Kicinski

One of my favorite features of the Netlink specs is that they
make decoding structured extack a ton easier.
Implement pretty printing bad attribute names in YNL.

For example it will now say:

  'bad-attr': '.header.flags'

rather than the useless:

  'bad-attr-offs': 32

Proof:

  $ ./cli.py --spec ethtool.yaml --do rings_get \
     --json '{"header":{"dev-index":1, "flags":4}}'
  Netlink error: Invalid argument
  nl_len = 68 (52) nl_flags = 0x300 nl_type = 2
	error: -22	extack: {'msg': 'reserved bit set',
				 'bad-attr': '.header.flags'}

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

diff --git a/tools/net/ynl/lib/ynl.py b/tools/net/ynl/lib/ynl.py
index c16326495cb7..2ff3e6dbdbf6 100644
--- a/tools/net/ynl/lib/ynl.py
+++ b/tools/net/ynl/lib/ynl.py
@@ -400,6 +400,40 @@ genl_family_name_to_id = None
                 self._decode_enum(rsp, attr_spec)
         return rsp
 
+    def _decode_extack_path(self, attrs, attr_set, offset, target):
+        for attr in attrs:
+            attr_spec = attr_set.attrs_by_val[attr.type]
+            if offset > target:
+                break
+            if offset == target:
+                return '.' + attr_spec.name
+
+            if offset + attr.full_len <= target:
+                offset += attr.full_len
+                continue
+            if attr_spec['type'] != 'nest':
+                raise Exception(f"Can't dive into {attr.type} ({attr_spec['name']}) for extack")
+            offset += 4
+            subpath = self._decode_extack_path(NlAttrs(attr.raw),
+                                               self.attr_sets[attr_spec['nested-attributes']],
+                                               offset, target)
+            if subpath is None:
+                return None
+            return '.' + attr_spec.name + subpath
+
+        return None
+
+    def _decode_extack(self, request, attr_space, extack):
+        if 'bad-attr-offs' not in extack:
+            return
+
+        genl_req = GenlMsg(NlMsg(request, 0, attr_space=attr_space))
+        path = self._decode_extack_path(genl_req.raw_attrs, attr_space,
+                                        20, extack['bad-attr-offs'])
+        if path:
+            del extack['bad-attr-offs']
+            extack['bad-attr'] = path
+
     def handle_ntf(self, nl_msg, genl_msg):
         msg = dict()
         if self.include_raw:
@@ -455,11 +489,17 @@ genl_family_name_to_id = None
             reply = self.sock.recv(128 * 1024)
             nms = NlMsgs(reply, attr_space=op.attr_set)
             for nl_msg in nms:
+                if nl_msg.extack:
+                    self._decode_extack(msg, op.attr_set, nl_msg.extack)
+
                 if nl_msg.error:
                     print("Netlink error:", os.strerror(-nl_msg.error))
                     print(nl_msg)
                     return
                 if nl_msg.done:
+                    if nl_msg.extack:
+                        print("Netlink warning:")
+                        print(nl_msg)
                     done = True
                     break
 
-- 
2.39.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH net-next v2 09/14] tools: ynl: use operation names from spec on the CLI
  2023-01-31  2:33 [PATCH net-next v2 00/14] tools: ynl: more docs and basic ethtool support Jakub Kicinski
                   ` (7 preceding siblings ...)
  2023-01-31  2:33 ` [PATCH net-next v2 08/14] tools: ynl: support pretty printing bad attribute names Jakub Kicinski
@ 2023-01-31  2:33 ` Jakub Kicinski
  2023-01-31  2:33 ` [PATCH net-next v2 10/14] tools: ynl: load jsonschema on demand Jakub Kicinski
                   ` (5 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: Jakub Kicinski @ 2023-01-31  2:33 UTC (permalink / raw)
  To: davem; +Cc: netdev, edumazet, pabeni, sdf, linux-doc, Jakub Kicinski

When I wrote the first version of the Python code I was quite
excited that we can generate class methods directly from the
spec. Unfortunately we need to use valid identifiers for method
names (specifically no dashes are allowed). Don't reuse those
names on the CLI, it's much more natural to use the operation
names exactly as listed in the spec.

Instead of:
  ./cli --do rings_get
use:
  ./cli --do rings-get

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 tools/net/ynl/cli.py     | 9 +++++----
 tools/net/ynl/lib/ynl.py | 6 ++++++
 2 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/tools/net/ynl/cli.py b/tools/net/ynl/cli.py
index 5c4eb5a68514..05d1f4069ce1 100755
--- a/tools/net/ynl/cli.py
+++ b/tools/net/ynl/cli.py
@@ -32,10 +32,11 @@ from lib import YnlFamily
     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:
diff --git a/tools/net/ynl/lib/ynl.py b/tools/net/ynl/lib/ynl.py
index 2ff3e6dbdbf6..1c7411ee04dc 100644
--- a/tools/net/ynl/lib/ynl.py
+++ b/tools/net/ynl/lib/ynl.py
@@ -520,3 +520,9 @@ genl_family_name_to_id = None
         if not dump and len(rsp) == 1:
             return rsp[0]
         return rsp
+
+    def do(self, method, vals):
+        return self._op(method, vals)
+
+    def dump(self, method, vals):
+        return self._op(method, vals, dump=True)
-- 
2.39.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH net-next v2 10/14] tools: ynl: load jsonschema on demand
  2023-01-31  2:33 [PATCH net-next v2 00/14] tools: ynl: more docs and basic ethtool support Jakub Kicinski
                   ` (8 preceding siblings ...)
  2023-01-31  2:33 ` [PATCH net-next v2 09/14] tools: ynl: use operation names from spec on the CLI Jakub Kicinski
@ 2023-01-31  2:33 ` Jakub Kicinski
  2023-01-31  2:33 ` [PATCH net-next v2 11/14] netlink: specs: finish up operation enum-models Jakub Kicinski
                   ` (4 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: Jakub Kicinski @ 2023-01-31  2:33 UTC (permalink / raw)
  To: davem; +Cc: netdev, edumazet, pabeni, sdf, linux-doc, Jakub Kicinski

The CLI script tries to validate jsonschema by default.
It's seems better to validate too many times than too few.
However, when copying the scripts to random servers having
to install jsonschema is tedious. Load jsonschema via
importlib, and let the user opt out.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 tools/net/ynl/cli.py        |  4 ++++
 tools/net/ynl/lib/nlspec.py | 11 ++++++++++-
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/tools/net/ynl/cli.py b/tools/net/ynl/cli.py
index 05d1f4069ce1..e64f1478764f 100755
--- a/tools/net/ynl/cli.py
+++ b/tools/net/ynl/cli.py
@@ -13,6 +13,7 @@ from lib import YnlFamily
     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 @@ from lib import YnlFamily
     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)
diff --git a/tools/net/ynl/lib/nlspec.py b/tools/net/ynl/lib/nlspec.py
index 4aa3b1ad97f0..e204679ad8b7 100644
--- a/tools/net/ynl/lib/nlspec.py
+++ b/tools/net/ynl/lib/nlspec.py
@@ -1,12 +1,16 @@
 # SPDX-License-Identifier: BSD-3-Clause
 
 import collections
-import jsonschema
+import importlib
 import os
 import traceback
 import yaml
 
 
+# To be loaded dynamically as needed
+jsonschema = None
+
+
 class SpecElement:
     """Netlink spec element.
 
@@ -197,9 +201,14 @@ import yaml
         if schema_path is None:
             schema_path = os.path.dirname(os.path.dirname(spec_path)) + f'/{self.proto}.yaml'
         if schema_path:
+            global jsonschema
+
             with open(schema_path, "r") as stream:
                 schema = yaml.safe_load(stream)
 
+            if jsonschema is None:
+                jsonschema = importlib.import_module("jsonschema")
+
             jsonschema.validate(self.yaml, schema)
 
         self.attr_sets = collections.OrderedDict()
-- 
2.39.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH net-next v2 11/14] netlink: specs: finish up operation enum-models
  2023-01-31  2:33 [PATCH net-next v2 00/14] tools: ynl: more docs and basic ethtool support Jakub Kicinski
                   ` (9 preceding siblings ...)
  2023-01-31  2:33 ` [PATCH net-next v2 10/14] tools: ynl: load jsonschema on demand Jakub Kicinski
@ 2023-01-31  2:33 ` Jakub Kicinski
  2023-01-31  2:33 ` [PATCH net-next v2 12/14] netlink: specs: add partial specification for ethtool Jakub Kicinski
                   ` (3 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: Jakub Kicinski @ 2023-01-31  2:33 UTC (permalink / raw)
  To: davem; +Cc: netdev, edumazet, pabeni, sdf, linux-doc, Jakub Kicinski

I had a (bright?) idea of introducing the concept of enum-models
to account for all the weird ways families enumerate their messages.
I've never finished it because generating C code for each of them
is pretty daunting. But for languages which can use ID values directly
the support is simple enough, so clean this up a bit.

"unified" model is what I recommend going forward.
"directional" model is what ethtool uses.
"notify-split" is used by the proposed DPLL code, but we can just
make them use "unified", it hasn't been merged :)

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
v2: spelling fixes
---
 Documentation/netlink/genetlink-c.yaml        |  4 +-
 Documentation/netlink/genetlink-legacy.yaml   | 11 ++-
 Documentation/netlink/genetlink.yaml          |  4 +-
 .../netlink/genetlink-legacy.rst              | 82 +++++++++++++++++++
 4 files changed, 92 insertions(+), 9 deletions(-)

diff --git a/Documentation/netlink/genetlink-c.yaml b/Documentation/netlink/genetlink-c.yaml
index e23e3c94a932..bbcfa2472b04 100644
--- a/Documentation/netlink/genetlink-c.yaml
+++ b/Documentation/netlink/genetlink-c.yaml
@@ -218,9 +218,7 @@ additionalProperties: False
           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
diff --git a/Documentation/netlink/genetlink-legacy.yaml b/Documentation/netlink/genetlink-legacy.yaml
index 88db2431ef26..5642925c4ceb 100644
--- a/Documentation/netlink/genetlink-legacy.yaml
+++ b/Documentation/netlink/genetlink-legacy.yaml
@@ -241,9 +241,7 @@ additionalProperties: False
           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 @@ additionalProperties: False
                       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).
diff --git a/Documentation/netlink/genetlink.yaml b/Documentation/netlink/genetlink.yaml
index b5e712bbe7e7..62a922755ce2 100644
--- a/Documentation/netlink/genetlink.yaml
+++ b/Documentation/netlink/genetlink.yaml
@@ -188,9 +188,7 @@ additionalProperties: False
           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
diff --git a/Documentation/userspace-api/netlink/genetlink-legacy.rst b/Documentation/userspace-api/netlink/genetlink-legacy.rst
index 65cbbffee0bf..3bf0bcdf21d8 100644
--- a/Documentation/userspace-api/netlink/genetlink-legacy.rst
+++ b/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)
 ===================
 
-- 
2.39.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH net-next v2 12/14] netlink: specs: add partial specification for ethtool
  2023-01-31  2:33 [PATCH net-next v2 00/14] tools: ynl: more docs and basic ethtool support Jakub Kicinski
                   ` (10 preceding siblings ...)
  2023-01-31  2:33 ` [PATCH net-next v2 11/14] netlink: specs: finish up operation enum-models Jakub Kicinski
@ 2023-01-31  2:33 ` Jakub Kicinski
  2023-01-31  2:33 ` [PATCH net-next v2 13/14] docs: netlink: add a starting guide for working with specs Jakub Kicinski
                   ` (2 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: Jakub Kicinski @ 2023-01-31  2:33 UTC (permalink / raw)
  To: davem; +Cc: netdev, edumazet, pabeni, sdf, linux-doc, Jakub Kicinski

Ethtool is one of the most actively developed families.
With the changes to the CLI it should be possible to use
the YNL based code for easy prototyping and development.
Add a partial family definition. I've tested the string
set and rings. I don't have any MAC Merge implementation
to test with, but I added the definition for it, anyway,
because it's last. New commands can simply be added at
the end without having to worry about manually providing
IDs / values.

Set (with notification support - None is the response,
the data is from the notification):

$ sudo ./tools/net/ynl/cli.py \
    --spec Documentation/netlink/specs/ethtool.yaml \
    --do rings-set \
    --json '{"header":{"dev-name":"enp0s31f6"}, "rx":129}' \
    --subscribe monitor
None
[{'msg': {'header': {'dev-index': 2, 'dev-name': 'enp0s31f6'},
          'rx': 136,
          'rx-max': 4096,
          'tx': 256,
          'tx-max': 4096,
          'tx-push': 0},
  'name': 'rings-ntf'}]

Do / dump (yes, the kernel requires that even for dump and even
if empty - the "header" nest must be there):

$ ./tools/net/ynl/cli.py \
    --spec Documentation/netlink/specs/ethtool.yaml \
    --do rings-get \
    --json '{"header":{"dev-index": 2}}'
{'header': {'dev-index': 2, 'dev-name': 'enp0s31f6'},
 'rx': 136,
 'rx-max': 4096,
 'tx': 256,
 'tx-max': 4096,
 'tx-push': 0}

$ ./tools/net/ynl/cli.py \
    --spec Documentation/netlink/specs/ethtool.yaml \
    --dump rings-get \
    --json '{"header":{}}'
[{'header': {'dev-index': 2, 'dev-name': 'enp0s31f6'},
  'rx': 136,
  'rx-max': 4096,
  'tx': 256,
  'tx-max': 4096,
  'tx-push': 0},
 {'header': {'dev-index': 3, 'dev-name': 'wlp0s20f3'}, 'tx-push': 0},
 {'header': {'dev-index': 19, 'dev-name': 'enp58s0u1u1'},
  'rx': 100,
  'rx-max': 4096,
  'tx-push': 0}]

And error reporting:

$ ./tools/net/ynl/cli.py \
    --spec Documentation/netlink/specs/ethtool.yaml \
    --dump rings-get \
    --json '{"header":{"flags":5}}'
Netlink error: Invalid argument
nl_len = 68 (52) nl_flags = 0x300 nl_type = 2
	error: -22	extack: {'msg': 'reserved bit set',
	                         'bad-attr-offs': 24,
				 'bad-attr': '.header.flags'}
None

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

diff --git a/Documentation/netlink/specs/ethtool.yaml b/Documentation/netlink/specs/ethtool.yaml
new file mode 100644
index 000000000000..82f4e6f8ddd3
--- /dev/null
+++ b/Documentation/netlink/specs/ethtool.yaml
@@ -0,0 +1,392 @@
+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
-- 
2.39.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH net-next v2 13/14] docs: netlink: add a starting guide for working with specs
  2023-01-31  2:33 [PATCH net-next v2 00/14] tools: ynl: more docs and basic ethtool support Jakub Kicinski
                   ` (11 preceding siblings ...)
  2023-01-31  2:33 ` [PATCH net-next v2 12/14] netlink: specs: add partial specification for ethtool Jakub Kicinski
@ 2023-01-31  2:33 ` Jakub Kicinski
  2023-01-31  2:33 ` [PATCH net-next v2 14/14] tools: net: use python3 explicitly Jakub Kicinski
  2023-02-01  4:40 ` [PATCH net-next v2 00/14] tools: ynl: more docs and basic ethtool support patchwork-bot+netdevbpf
  14 siblings, 0 replies; 16+ messages in thread
From: Jakub Kicinski @ 2023-01-31  2:33 UTC (permalink / raw)
  To: davem; +Cc: netdev, edumazet, pabeni, sdf, linux-doc, Jakub Kicinski

We have a bit of documentation about the internals of Netlink
and the specs, but really the goal is for most people to not
worry about those. Add a practical guide for beginners who
want to poke at the specs.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 Documentation/userspace-api/netlink/index.rst |  1 +
 .../userspace-api/netlink/intro-specs.rst     | 80 +++++++++++++++++++
 Documentation/userspace-api/netlink/specs.rst |  3 +
 3 files changed, 84 insertions(+)
 create mode 100644 Documentation/userspace-api/netlink/intro-specs.rst

diff --git a/Documentation/userspace-api/netlink/index.rst b/Documentation/userspace-api/netlink/index.rst
index be250110c8f6..26f3720cb3be 100644
--- a/Documentation/userspace-api/netlink/index.rst
+++ b/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
diff --git a/Documentation/userspace-api/netlink/intro-specs.rst b/Documentation/userspace-api/netlink/intro-specs.rst
new file mode 100644
index 000000000000..a3b847eafff7
--- /dev/null
+++ b/Documentation/userspace-api/netlink/intro-specs.rst
@@ -0,0 +1,80 @@
+.. 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.
diff --git a/Documentation/userspace-api/netlink/specs.rst b/Documentation/userspace-api/netlink/specs.rst
index 8394d74fc63a..6ffe8137cd90 100644
--- a/Documentation/userspace-api/netlink/specs.rst
+++ b/Documentation/userspace-api/netlink/specs.rst
@@ -21,6 +21,9 @@ kernel headers directly.
 
 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
 ====================
 
-- 
2.39.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH net-next v2 14/14] tools: net: use python3 explicitly
  2023-01-31  2:33 [PATCH net-next v2 00/14] tools: ynl: more docs and basic ethtool support Jakub Kicinski
                   ` (12 preceding siblings ...)
  2023-01-31  2:33 ` [PATCH net-next v2 13/14] docs: netlink: add a starting guide for working with specs Jakub Kicinski
@ 2023-01-31  2:33 ` Jakub Kicinski
  2023-02-01  4:40 ` [PATCH net-next v2 00/14] tools: ynl: more docs and basic ethtool support patchwork-bot+netdevbpf
  14 siblings, 0 replies; 16+ messages in thread
From: Jakub Kicinski @ 2023-01-31  2:33 UTC (permalink / raw)
  To: davem; +Cc: netdev, edumazet, pabeni, sdf, linux-doc, Jakub Kicinski

The scripts require Python 3 and some distros are dropping
Python 2 support.

Reported-by: Stanislav Fomichev <sdf@google.com>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 tools/net/ynl/cli.py       | 2 +-
 tools/net/ynl/ynl-gen-c.py | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/net/ynl/cli.py b/tools/net/ynl/cli.py
index e64f1478764f..db410b74d539 100755
--- a/tools/net/ynl/cli.py
+++ b/tools/net/ynl/cli.py
@@ -1,4 +1,4 @@
-#!/usr/bin/env python
+#!/usr/bin/env python3
 # SPDX-License-Identifier: BSD-3-Clause
 
 import argparse
diff --git a/tools/net/ynl/ynl-gen-c.py b/tools/net/ynl/ynl-gen-c.py
index dc14da634e8e..3942f24b9163 100755
--- a/tools/net/ynl/ynl-gen-c.py
+++ b/tools/net/ynl/ynl-gen-c.py
@@ -1,4 +1,4 @@
-#!/usr/bin/env python
+#!/usr/bin/env python3
 
 import argparse
 import collections
-- 
2.39.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH net-next v2 00/14] tools: ynl: more docs and basic ethtool support
  2023-01-31  2:33 [PATCH net-next v2 00/14] tools: ynl: more docs and basic ethtool support Jakub Kicinski
                   ` (13 preceding siblings ...)
  2023-01-31  2:33 ` [PATCH net-next v2 14/14] tools: net: use python3 explicitly Jakub Kicinski
@ 2023-02-01  4:40 ` patchwork-bot+netdevbpf
  14 siblings, 0 replies; 16+ messages in thread
From: patchwork-bot+netdevbpf @ 2023-02-01  4:40 UTC (permalink / raw)
  To: Jakub Kicinski; +Cc: davem, netdev, edumazet, pabeni, sdf, linux-doc

Hello:

This series was applied to netdev/net-next.git (master)
by Jakub Kicinski <kuba@kernel.org>:

On Mon, 30 Jan 2023 18:33:40 -0800 you wrote:
> 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.
> 
> [...]

Here is the summary with links:
  - [net-next,v2,01/14] tools: ynl-gen: prevent do / dump reordering
    https://git.kernel.org/netdev/net-next/c/eaf317e7d2bb
  - [net-next,v2,02/14] tools: ynl: move the cli and netlink code around
    https://git.kernel.org/netdev/net-next/c/4e4480e89c47
  - [net-next,v2,03/14] tools: ynl: add an object hierarchy to represent parsed spec
    https://git.kernel.org/netdev/net-next/c/3aacf8281336
  - [net-next,v2,04/14] tools: ynl: use the common YAML loading and validation code
    https://git.kernel.org/netdev/net-next/c/30a5c6c8104f
  - [net-next,v2,05/14] tools: ynl: add support for types needed by ethtool
    https://git.kernel.org/netdev/net-next/c/19b64b48a33e
  - [net-next,v2,06/14] tools: ynl: support directional enum-model in CLI
    https://git.kernel.org/netdev/net-next/c/fd0616d34274
  - [net-next,v2,07/14] tools: ynl: support multi-attr
    https://git.kernel.org/netdev/net-next/c/90256f3f8093
  - [net-next,v2,08/14] tools: ynl: support pretty printing bad attribute names
    https://git.kernel.org/netdev/net-next/c/4cd2796f3f8d
  - [net-next,v2,09/14] tools: ynl: use operation names from spec on the CLI
    https://git.kernel.org/netdev/net-next/c/8dfec0a88868
  - [net-next,v2,10/14] tools: ynl: load jsonschema on demand
    https://git.kernel.org/netdev/net-next/c/5c6674f6eb52
  - [net-next,v2,11/14] netlink: specs: finish up operation enum-models
    https://git.kernel.org/netdev/net-next/c/8403bf044530
  - [net-next,v2,12/14] netlink: specs: add partial specification for ethtool
    https://git.kernel.org/netdev/net-next/c/b784db7ae840
  - [net-next,v2,13/14] docs: netlink: add a starting guide for working with specs
    https://git.kernel.org/netdev/net-next/c/01e47a372268
  - [net-next,v2,14/14] tools: net: use python3 explicitly
    https://git.kernel.org/netdev/net-next/c/981cbcb030d9

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2023-02-01  4:40 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-31  2:33 [PATCH net-next v2 00/14] tools: ynl: more docs and basic ethtool support Jakub Kicinski
2023-01-31  2:33 ` [PATCH net-next v2 01/14] tools: ynl-gen: prevent do / dump reordering Jakub Kicinski
2023-01-31  2:33 ` [PATCH net-next v2 02/14] tools: ynl: move the cli and netlink code around Jakub Kicinski
2023-01-31  2:33 ` [PATCH net-next v2 03/14] tools: ynl: add an object hierarchy to represent parsed spec Jakub Kicinski
2023-01-31  2:33 ` [PATCH net-next v2 04/14] tools: ynl: use the common YAML loading and validation code Jakub Kicinski
2023-01-31  2:33 ` [PATCH net-next v2 05/14] tools: ynl: add support for types needed by ethtool Jakub Kicinski
2023-01-31  2:33 ` [PATCH net-next v2 06/14] tools: ynl: support directional enum-model in CLI Jakub Kicinski
2023-01-31  2:33 ` [PATCH net-next v2 07/14] tools: ynl: support multi-attr Jakub Kicinski
2023-01-31  2:33 ` [PATCH net-next v2 08/14] tools: ynl: support pretty printing bad attribute names Jakub Kicinski
2023-01-31  2:33 ` [PATCH net-next v2 09/14] tools: ynl: use operation names from spec on the CLI Jakub Kicinski
2023-01-31  2:33 ` [PATCH net-next v2 10/14] tools: ynl: load jsonschema on demand Jakub Kicinski
2023-01-31  2:33 ` [PATCH net-next v2 11/14] netlink: specs: finish up operation enum-models Jakub Kicinski
2023-01-31  2:33 ` [PATCH net-next v2 12/14] netlink: specs: add partial specification for ethtool Jakub Kicinski
2023-01-31  2:33 ` [PATCH net-next v2 13/14] docs: netlink: add a starting guide for working with specs Jakub Kicinski
2023-01-31  2:33 ` [PATCH net-next v2 14/14] tools: net: use python3 explicitly Jakub Kicinski
2023-02-01  4:40 ` [PATCH net-next v2 00/14] tools: ynl: more docs and basic ethtool support patchwork-bot+netdevbpf

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).