[patch net-next 00/13] netlink: specs: devlink: add the rest of missing attribute definitions

[patch net-next 00/13] netlink: specs: devlink: add the rest of missing attribute definitions

[Jiri Pirko]

From: Jiri Pirko <jiri@nvidia.com>

This patchset is adding the missing definitions of devlink attributes.

It got a bit tricky, as the param and fmsg value attributes have
different type according to a value of another attribute. Thankfully,
the selector infrastructure was recently introduced to ynl. This
patchset extends it a bit and uses it.

Another tricky bit was the fact that fmsg contains a list of attributes
that go as a stream and can be present multiple times. Also, it is
important to maintain the attribute position. For that, list output
needed to be added.

Also, nested devlink attributes definitions was added.

Examples:
$ sudo ./tools/net/ynl/cli.py --spec Documentation/netlink/specs/devlink.yaml \
                              --dump param-get
[{'bus-name': 'netdevsim',
  'dev-name': 'netdevsim1',
  'param': {'param-generic': True,
            'param-name': 'max_macs',
            'param-type': 'u32',
            'param-values-list': {'param-value': [{'param-value-cmode': 'driverinit',
                                                   'param-value-data': 32}]}}},
 {'bus-name': 'netdevsim',
  'dev-name': 'netdevsim1',
  'param': {'param-name': 'test1',
            'param-type': 'flag',
            'param-values-list': {'param-value': [{'param-value-cmode': 'driverinit',
                                                   'param-value-data': True}]}}}]
$ sudo ./tools/net/ynl/cli.py --spec Documentation/netlink/specs/devlink.yaml \
                              --do param-set \
			      --json '{"bus-name": "netdevsim", "dev-name": "netdevsim1", "param-name": "max_macs", "param-type": "u32", "param-value-data": 21, "param-value-cmode": "driverinit"}'
$ sudo ./tools/net/ynl/cli.py --spec Documentation/netlink/specs/devlink.yaml \
                              --do param-set \
			      --json '{"bus-name": "netdevsim", "dev-name": "netdevsim1", "param-name": "test1", "param-type": "flag", "param-value-data": false, "param-value-cmode": "driverinit"}'

$ sudo ./tools/net/ynl/cli.py --spec Documentation/netlink/specs/devlink.yaml \
                              --dump health-reporter-dump-get --json '{"bus-name": "netdevsim", "dev-name": "netdevsim1", "health-reporter-name": "dummy"}'
[{'fmsg': [{'fmsg-obj-nest-start': True},
           {'fmsg-pair-nest-start': True},
           {'fmsg-obj-name': 'test_bool'},
           {'fmsg-obj-value-type': 'flag'},
           {'fmsg-obj-value-data': True},
           {'fmsg-nest-end': True},
           {'fmsg-pair-nest-start': True},
           {'fmsg-obj-name': 'test_u8'},
           {'fmsg-obj-value-type': 'u8'},
           {'fmsg-obj-value-data': 1},
           {'fmsg-nest-end': True},
           {'fmsg-pair-nest-start': True},
           {'fmsg-obj-name': 'test_u32'},
           {'fmsg-obj-value-type': 'u32'},
           {'fmsg-obj-value-data': 3},
.....
           {'fmsg-nest-end': True}]}]

$ sudo ./tools/net/ynl/cli.py --spec Documentation/netlink/specs/devlink.yaml \
		              --do port-get \
			      --json '{"bus-name": "pci", "dev-name": "0000:08:00.1", "port-index": 98304}'
{'bus-name': 'pci',
 'dev-name': '0000:08:00.1',
 'port-controller-number': 0,
 'port-flavour': 'pci_sf',
 'port-function': {'caps': {'selector': {'roce-bit'}, 'value': {'roce-bit'}},
                   'devlink': {'bus-name': 'auxiliary',
                               'dev-name': 'mlx5_core.sf.2'},
                   'hw-addr': b'\x00\x00\x00\x00\x00\x00',
                   'opstate': 'attached',
                   'state': 'active'},
 'port-index': 98304,
 'port-netdev-ifindex': 7,
 'port-netdev-name': 'eth4',
 'port-pci-pf-number': 1,
 'port-pci-sf-number': 109,
 'port-splittable': 0,
 'port-type': 'eth'}

Jiri Pirko (13):
  tools: ynl: allow user to specify flag attr with bool values
  tools: ynl: process all scalar types encoding in single elif statement
  tools: ynl: allow user to pass enum string instead of scalar value
  netlink: specs: allow sub-messages in genetlink-legacy
  tools: ynl: allow attr in a subset to be of a different type
  tools: ynl: introduce attribute-replace for sub-message
  tools: ynl: add support for list in nested attribute
  netlink: specs: devlink: add enum for param-type attribute values
  netlink: specs: devlink: add missing param attribute definitions
  netlink: specs: devlink: treat dl-fmsg attribute as list
  netlink: specs: devlink: add enum for fmsg-obj-value-type attribute
    values
  netlink: specs: devlink: add missing fmsg-obj-value-data attribute
    definitions
  netlink: specs: devlink: add missing nested devlink definitions

 Documentation/netlink/genetlink-legacy.yaml   |  54 +++-
 Documentation/netlink/netlink-raw.yaml        |  10 +-
 Documentation/netlink/specs/devlink.yaml      | 260 +++++++++++++++++-
 .../netlink/genetlink-legacy.rst              | 126 +++++++++
 .../userspace-api/netlink/netlink-raw.rst     | 101 -------
 tools/net/ynl/lib/nlspec.py                   |   8 +
 tools/net/ynl/lib/ynl.py                      |  81 ++++--
 7 files changed, 500 insertions(+), 140 deletions(-)

-- 
2.43.2

[patch net-next 01/13] tools: ynl: allow user to specify flag attr with bool values

[Jiri Pirko]

From: Jiri Pirko <jiri@nvidia.com>

The flag attr presence in Netlink message indicates value "true",
if it is missing in the message it means "false".

Allow user to specify attrname with value "true"/"false"
in json for flag attrs, treat "false" value properly.

Signed-off-by: Jiri Pirko <jiri@nvidia.com>
---
 tools/net/ynl/lib/ynl.py | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tools/net/ynl/lib/ynl.py b/tools/net/ynl/lib/ynl.py
index f45ee5f29bed..108fe7eadd93 100644
--- a/tools/net/ynl/lib/ynl.py
+++ b/tools/net/ynl/lib/ynl.py
@@ -459,6 +459,8 @@ class YnlFamily(SpecFamily):
                 attr_payload += self._add_attr(attr['nested-attributes'],
                                                subname, subvalue, sub_attrs)
         elif attr["type"] == 'flag':
+            if value == False:
+                return b''
             attr_payload = b''
         elif attr["type"] == 'string':
             attr_payload = str(value).encode('ascii') + b'\x00'
-- 
2.43.2

[patch net-next 02/13] tools: ynl: process all scalar types encoding in single elif statement

[Jiri Pirko]

From: Jiri Pirko <jiri@nvidia.com>

As a preparation to handle enums for scalar values, unify the processing
of all scalar types in a single elif statement.

Signed-off-by: Jiri Pirko <jiri@nvidia.com>
---
 tools/net/ynl/lib/ynl.py | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/tools/net/ynl/lib/ynl.py b/tools/net/ynl/lib/ynl.py
index 108fe7eadd93..ccdb2f1e7379 100644
--- a/tools/net/ynl/lib/ynl.py
+++ b/tools/net/ynl/lib/ynl.py
@@ -473,14 +473,14 @@ class YnlFamily(SpecFamily):
                 attr_payload = self._encode_struct(attr.struct_name, value)
             else:
                 raise Exception(f'Unknown type for binary attribute, value: {value}')
-        elif attr.is_auto_scalar:
+        elif attr['type'] in NlAttr.type_formats or attr.is_auto_scalar:
             scalar = int(value)
-            real_type = attr["type"][0] + ('32' if scalar.bit_length() <= 32 else '64')
-            format = NlAttr.get_format(real_type, attr.byte_order)
-            attr_payload = format.pack(int(value))
-        elif attr['type'] in NlAttr.type_formats:
-            format = NlAttr.get_format(attr['type'], attr.byte_order)
-            attr_payload = format.pack(int(value))
+            if attr.is_auto_scalar:
+                attr_type = attr["type"][0] + ('32' if scalar.bit_length() <= 32 else '64')
+            else:
+                attr_type = attr["type"]
+            format = NlAttr.get_format(attr_type, attr.byte_order)
+            attr_payload = format.pack(scalar)
         elif attr['type'] in "bitfield32":
             attr_payload = struct.pack("II", int(value["value"]), int(value["selector"]))
         elif attr['type'] == 'sub-message':
-- 
2.43.2

[patch net-next 03/13] tools: ynl: allow user to pass enum string instead of scalar value

[Jiri Pirko]

From: Jiri Pirko <jiri@nvidia.com>

During decoding of messages coming from kernel, attribute values are
converted to enum names in case the attribute type is enum of bitfield32.

However, when user constructs json message, he has to pass plain scalar
values. See "state" "selector" and "value" attributes in following
examples:

$ sudo ./tools/net/ynl/cli.py --spec Documentation/netlink/specs/dpll.yaml --do pin-set --json '{"id": 0, "parent-device": {"parent-id": 0, "state": 1}}'
$ sudo ./tools/net/ynl/cli.py --spec Documentation/netlink/specs/devlink.yaml --do port-set --json '{"bus-name": "pci", "dev-name": "0000:08:00.1", "port-index": 98304, "port-function": {"caps": {"selector": 1, "value": 1 }}}'

Allow user to pass strings containing enum names, convert them to scalar
values to be encoded into Netlink message:

$ sudo ./tools/net/ynl/cli.py --spec Documentation/netlink/specs/dpll.yaml --do pin-set --json '{"id": 0, "parent-device": {"parent-id": 0, "state": "connected"}}'
$ sudo ./tools/net/ynl/cli.py --spec Documentation/netlink/specs/devlink.yaml --do port-set --json '{"bus-name": "pci", "dev-name": "0000:08:00.1", "port-index": 98304, "port-function": {"caps": {"selector": ["roce-bit"], "value": ["roce-bit"] }}}'

Signed-off-by: Jiri Pirko <jiri@nvidia.com>
---
 tools/net/ynl/lib/ynl.py | 21 +++++++++++++++++++--
 1 file changed, 19 insertions(+), 2 deletions(-)

diff --git a/tools/net/ynl/lib/ynl.py b/tools/net/ynl/lib/ynl.py
index ccdb2f1e7379..d2ea1571d00c 100644
--- a/tools/net/ynl/lib/ynl.py
+++ b/tools/net/ynl/lib/ynl.py
@@ -438,6 +438,21 @@ class YnlFamily(SpecFamily):
         self.sock.setsockopt(Netlink.SOL_NETLINK, Netlink.NETLINK_ADD_MEMBERSHIP,
                              mcast_id)
 
+    def _get_scalar(self, attr_spec, value):
+        try:
+            return int(value)
+        except (ValueError, TypeError) as e:
+            if 'enum' not in attr_spec:
+                raise e
+        enum = self.consts[attr_spec['enum']]
+        if enum.type == 'flags' or attr_spec.get('enum-as-flags', False):
+            scalar = 0
+            for single_value in value:
+                scalar += enum.entries[single_value].user_value(as_flags = True)
+            return scalar
+        else:
+            return enum.entries[value].user_value()
+
     def _add_attr(self, space, name, value, search_attrs):
         try:
             attr = self.attr_sets[space][name]
@@ -474,7 +489,7 @@ class YnlFamily(SpecFamily):
             else:
                 raise Exception(f'Unknown type for binary attribute, value: {value}')
         elif attr['type'] in NlAttr.type_formats or attr.is_auto_scalar:
-            scalar = int(value)
+            scalar = self._get_scalar(attr, value)
             if attr.is_auto_scalar:
                 attr_type = attr["type"][0] + ('32' if scalar.bit_length() <= 32 else '64')
             else:
@@ -482,7 +497,9 @@ class YnlFamily(SpecFamily):
             format = NlAttr.get_format(attr_type, attr.byte_order)
             attr_payload = format.pack(scalar)
         elif attr['type'] in "bitfield32":
-            attr_payload = struct.pack("II", int(value["value"]), int(value["selector"]))
+            scalar_value = self._get_scalar(attr, value["value"])
+            scalar_selector = self._get_scalar(attr, value["selector"])
+            attr_payload = struct.pack("II", scalar_value, scalar_selector)
         elif attr['type'] == 'sub-message':
             msg_format = self._resolve_selector(attr, search_attrs)
             attr_payload = b''
-- 
2.43.2

[patch net-next 04/13] netlink: specs: allow sub-messages in genetlink-legacy

[Jiri Pirko]

From: Jiri Pirko <jiri@nvidia.com>

Currently sub-messages are only supported in netlink-raw template.
To be able to utilize them in devlink spec, allow them in
genetlink-legacy as well.

Signed-off-by: Jiri Pirko <jiri@nvidia.com>
---
 Documentation/netlink/genetlink-legacy.yaml   |  47 +++++++-
 Documentation/netlink/netlink-raw.yaml        |   6 +-
 .../netlink/genetlink-legacy.rst              | 101 ++++++++++++++++++
 .../userspace-api/netlink/netlink-raw.rst     | 101 ------------------
 4 files changed, 148 insertions(+), 107 deletions(-)

diff --git a/Documentation/netlink/genetlink-legacy.yaml b/Documentation/netlink/genetlink-legacy.yaml
index 938703088306..6cb50e2cc021 100644
--- a/Documentation/netlink/genetlink-legacy.yaml
+++ b/Documentation/netlink/genetlink-legacy.yaml
@@ -200,7 +200,8 @@ properties:
                 description: The netlink attribute type
                 enum: [ unused, pad, flag, binary, bitfield32,
                         uint, sint, u8, u16, u32, u64, s32, s64,
-                        string, nest, array-nest, nest-type-value ]
+                        string, nest, array-nest, nest-type-value,
+                        sub-message ]
               doc:
                 description: Documentation of the attribute.
                 type: string
@@ -261,6 +262,15 @@ properties:
               struct:
                 description: Name of the struct type used for the attribute.
                 type: string
+              sub-message:
+                description: |
+                  Name of the sub-message definition to use for the attribute.
+                type: string
+              selector:
+                description: |
+                  Name of the attribute to use for dynamic selection of sub-message
+                  format specifier.
+                type: string
               # End genetlink-legacy
 
       # Make sure name-prefix does not appear in subsets (subsets inherit naming)
@@ -284,6 +294,41 @@ properties:
             items:
               required: [ type ]
 
+  sub-messages:
+    description: Definition of sub message attributes
+    type: array
+    items:
+      type: object
+      additionalProperties: False
+      required: [ name, formats ]
+      properties:
+        name:
+          description: Name of the sub-message definition
+          type: string
+        formats:
+          description: Dynamically selected format specifiers
+          type: array
+          items:
+            type: object
+            additionalProperties: False
+            required: [ value ]
+            properties:
+              value:
+                description: |
+                  Value to match for dynamic selection of sub-message format
+                  specifier.
+                type: string
+              fixed-header:
+                description: |
+                  Name of the struct definition to use as the fixed header
+                  for the sub message.
+                type: string
+              attribute-set:
+                description: |
+                  Name of the attribute space from which to resolve attributes
+                  in the sub message.
+                type: string
+
   operations:
     description: Operations supported by the protocol.
     type: object
diff --git a/Documentation/netlink/netlink-raw.yaml b/Documentation/netlink/netlink-raw.yaml
index ac4e05415f2f..cc38b026c451 100644
--- a/Documentation/netlink/netlink-raw.yaml
+++ b/Documentation/netlink/netlink-raw.yaml
@@ -280,8 +280,6 @@ properties:
               struct:
                 description: Name of the struct type used for the attribute.
                 type: string
-              # End genetlink-legacy
-              # Start netlink-raw
               sub-message:
                 description: |
                   Name of the sub-message definition to use for the attribute.
@@ -291,7 +289,7 @@ properties:
                   Name of the attribute to use for dynamic selection of sub-message
                   format specifier.
                 type: string
-              # End netlink-raw
+              # End genetlink-legacy
 
       # Make sure name-prefix does not appear in subsets (subsets inherit naming)
       dependencies:
@@ -314,7 +312,6 @@ properties:
             items:
               required: [ type ]
 
-  # Start netlink-raw
   sub-messages:
     description: Definition of sub message attributes
     type: array
@@ -349,7 +346,6 @@ properties:
                   Name of the attribute space from which to resolve attributes
                   in the sub message.
                 type: string
-  # End netlink-raw
 
   operations:
     description: Operations supported by the protocol.
diff --git a/Documentation/userspace-api/netlink/genetlink-legacy.rst b/Documentation/userspace-api/netlink/genetlink-legacy.rst
index 70a77387f6c4..7126b650090e 100644
--- a/Documentation/userspace-api/netlink/genetlink-legacy.rst
+++ b/Documentation/userspace-api/netlink/genetlink-legacy.rst
@@ -280,3 +280,104 @@ At the spec level we can define a ``dumps`` property for the ``do``,
 perhaps with values of ``combine`` and ``multi-object`` depending
 on how the parsing should be implemented (parse into a single reply
 vs list of objects i.e. pretty much a dump).
+
+Sub-messages
+------------
+
+Several raw netlink families such as
+:doc:`rt_link<../../networking/netlink_spec/rt_link>` and
+:doc:`tc<../../networking/netlink_spec/tc>` use attribute nesting as an
+abstraction to carry module specific information.
+
+Conceptually it looks as follows::
+
+    [OUTER NEST OR MESSAGE LEVEL]
+      [GENERIC ATTR 1]
+      [GENERIC ATTR 2]
+      [GENERIC ATTR 3]
+      [GENERIC ATTR - wrapper]
+        [MODULE SPECIFIC ATTR 1]
+        [MODULE SPECIFIC ATTR 2]
+
+The ``GENERIC ATTRs`` at the outer level are defined in the core (or rt_link or
+core TC), while specific drivers, TC classifiers, qdiscs etc. can carry their
+own information wrapped in the ``GENERIC ATTR - wrapper``. Even though the
+example above shows attributes nesting inside the wrapper, the modules generally
+have full freedom to define the format of the nest. In practice the payload of
+the wrapper attr has very similar characteristics to a netlink message. It may
+contain a fixed header / structure, netlink attributes, or both. Because of
+those shared characteristics we refer to the payload of the wrapper attribute as
+a sub-message.
+
+A sub-message attribute uses the value of another attribute as a selector key to
+choose the right sub-message format. For example if the following attribute has
+already been decoded:
+
+.. code-block:: json
+
+  { "kind": "gre" }
+
+and we encounter the following attribute spec:
+
+.. code-block:: yaml
+
+  -
+    name: data
+    type: sub-message
+    sub-message: linkinfo-data-msg
+    selector: kind
+
+Then we look for a sub-message definition called ``linkinfo-data-msg`` and use
+the value of the ``kind`` attribute i.e. ``gre`` as the key to choose the
+correct format for the sub-message:
+
+.. code-block:: yaml
+
+  sub-messages:
+    name: linkinfo-data-msg
+    formats:
+      -
+        value: bridge
+        attribute-set: linkinfo-bridge-attrs
+      -
+        value: gre
+        attribute-set: linkinfo-gre-attrs
+      -
+        value: geneve
+        attribute-set: linkinfo-geneve-attrs
+
+This would decode the attribute value as a sub-message with the attribute-set
+called ``linkinfo-gre-attrs`` as the attribute space.
+
+A sub-message can have an optional ``fixed-header`` followed by zero or more
+attributes from an ``attribute-set``. For example the following
+``tc-options-msg`` sub-message defines message formats that use a mixture of
+``fixed-header``, ``attribute-set`` or both together:
+
+.. code-block:: yaml
+
+  sub-messages:
+    -
+      name: tc-options-msg
+      formats:
+        -
+          value: bfifo
+          fixed-header: tc-fifo-qopt
+        -
+          value: cake
+          attribute-set: tc-cake-attrs
+        -
+          value: netem
+          fixed-header: tc-netem-qopt
+          attribute-set: tc-netem-attrs
+
+Note that a selector attribute must appear in a netlink message before any
+sub-message attributes that depend on it.
+
+If an attribute such as ``kind`` is defined at more than one nest level, then a
+sub-message selector will be resolved using the value 'closest' to the selector.
+For example, if the same attribute name is defined in a nested ``attribute-set``
+alongside a sub-message selector and also in a top level ``attribute-set``, then
+the selector will be resolved using the value 'closest' to the selector. If the
+value is not present in the message at the same level as defined in the spec
+then this is an error.
diff --git a/Documentation/userspace-api/netlink/netlink-raw.rst b/Documentation/userspace-api/netlink/netlink-raw.rst
index 1990eea772d0..5fb8b7cd6558 100644
--- a/Documentation/userspace-api/netlink/netlink-raw.rst
+++ b/Documentation/userspace-api/netlink/netlink-raw.rst
@@ -58,107 +58,6 @@ group registration.
         name: rtnlgrp-mctp-ifaddr
         value: 34
 
-Sub-messages
-------------
-
-Several raw netlink families such as
-:doc:`rt_link<../../networking/netlink_spec/rt_link>` and
-:doc:`tc<../../networking/netlink_spec/tc>` use attribute nesting as an
-abstraction to carry module specific information.
-
-Conceptually it looks as follows::
-
-    [OUTER NEST OR MESSAGE LEVEL]
-      [GENERIC ATTR 1]
-      [GENERIC ATTR 2]
-      [GENERIC ATTR 3]
-      [GENERIC ATTR - wrapper]
-        [MODULE SPECIFIC ATTR 1]
-        [MODULE SPECIFIC ATTR 2]
-
-The ``GENERIC ATTRs`` at the outer level are defined in the core (or rt_link or
-core TC), while specific drivers, TC classifiers, qdiscs etc. can carry their
-own information wrapped in the ``GENERIC ATTR - wrapper``. Even though the
-example above shows attributes nesting inside the wrapper, the modules generally
-have full freedom to define the format of the nest. In practice the payload of
-the wrapper attr has very similar characteristics to a netlink message. It may
-contain a fixed header / structure, netlink attributes, or both. Because of
-those shared characteristics we refer to the payload of the wrapper attribute as
-a sub-message.
-
-A sub-message attribute uses the value of another attribute as a selector key to
-choose the right sub-message format. For example if the following attribute has
-already been decoded:
-
-.. code-block:: json
-
-  { "kind": "gre" }
-
-and we encounter the following attribute spec:
-
-.. code-block:: yaml
-
-  -
-    name: data
-    type: sub-message
-    sub-message: linkinfo-data-msg
-    selector: kind
-
-Then we look for a sub-message definition called ``linkinfo-data-msg`` and use
-the value of the ``kind`` attribute i.e. ``gre`` as the key to choose the
-correct format for the sub-message:
-
-.. code-block:: yaml
-
-  sub-messages:
-    name: linkinfo-data-msg
-    formats:
-      -
-        value: bridge
-        attribute-set: linkinfo-bridge-attrs
-      -
-        value: gre
-        attribute-set: linkinfo-gre-attrs
-      -
-        value: geneve
-        attribute-set: linkinfo-geneve-attrs
-
-This would decode the attribute value as a sub-message with the attribute-set
-called ``linkinfo-gre-attrs`` as the attribute space.
-
-A sub-message can have an optional ``fixed-header`` followed by zero or more
-attributes from an ``attribute-set``. For example the following
-``tc-options-msg`` sub-message defines message formats that use a mixture of
-``fixed-header``, ``attribute-set`` or both together:
-
-.. code-block:: yaml
-
-  sub-messages:
-    -
-      name: tc-options-msg
-      formats:
-        -
-          value: bfifo
-          fixed-header: tc-fifo-qopt
-        -
-          value: cake
-          attribute-set: tc-cake-attrs
-        -
-          value: netem
-          fixed-header: tc-netem-qopt
-          attribute-set: tc-netem-attrs
-
-Note that a selector attribute must appear in a netlink message before any
-sub-message attributes that depend on it.
-
-If an attribute such as ``kind`` is defined at more than one nest level, then a
-sub-message selector will be resolved using the value 'closest' to the selector.
-For example, if the same attribute name is defined in a nested ``attribute-set``
-alongside a sub-message selector and also in a top level ``attribute-set``, then
-the selector will be resolved using the value 'closest' to the selector. If the
-value is not present in the message at the same level as defined in the spec
-then this is an error.
-
 Nested struct definitions
 -------------------------
 
-- 
2.43.2

[patch net-next 05/13] tools: ynl: allow attr in a subset to be of a different type

[Jiri Pirko]

From: Jiri Pirko <jiri@nvidia.com>

Currently, when subset attribute is defined, it inherits the whole
definition of the attribute in original set. To be able to achieve
sub message decoding of the same attribute of different type, allow
spec to define different type for the same attribute.

Signed-off-by: Jiri Pirko <jiri@nvidia.com>
---
 tools/net/ynl/lib/nlspec.py | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tools/net/ynl/lib/nlspec.py b/tools/net/ynl/lib/nlspec.py
index fbce52395b3b..5e48ee0fb8b4 100644
--- a/tools/net/ynl/lib/nlspec.py
+++ b/tools/net/ynl/lib/nlspec.py
@@ -217,6 +217,8 @@ class SpecAttrSet(SpecElement):
             real_set = family.attr_sets[self.subset_of]
             for elem in self.yaml['attributes']:
                 attr = real_set[elem['name']]
+                if 'type' in elem and attr.type != elem['type']:
+                    attr = self.new_attr(elem, attr.value)
                 self.attrs[attr.name] = attr
                 self.attrs_by_val[attr.value] = attr
 
-- 
2.43.2

[patch net-next 06/13] tools: ynl: introduce attribute-replace for sub-message

[Jiri Pirko]

From: Jiri Pirko <jiri@nvidia.com>

For devlink param, param-value-data attr is used by kernel to fill
different attribute type according to param-type attribute value.

Currently the sub-message feature allows spec to embed custom message
selected by another attribute. The sub-message is then nested inside the
attr of sub-message type.

Benefit from the sub-message feature and extend it. Introduce
attribute-replace spec flag by which the spec indicates that ynl should
consider sub-message as not nested in the original attribute, but rather
to consider the original attribute as the sub-message right away.

Signed-off-by: Jiri Pirko <jiri@nvidia.com>
---
 Documentation/netlink/genetlink-legacy.yaml   |  4 +++
 Documentation/netlink/netlink-raw.yaml        |  4 +++
 .../netlink/genetlink-legacy.rst              | 25 ++++++++++++++++++
 tools/net/ynl/lib/nlspec.py                   |  6 +++++
 tools/net/ynl/lib/ynl.py                      | 26 ++++++++++++++-----
 5 files changed, 59 insertions(+), 6 deletions(-)

diff --git a/Documentation/netlink/genetlink-legacy.yaml b/Documentation/netlink/genetlink-legacy.yaml
index 6cb50e2cc021..77d89f81c71f 100644
--- a/Documentation/netlink/genetlink-legacy.yaml
+++ b/Documentation/netlink/genetlink-legacy.yaml
@@ -328,6 +328,10 @@ properties:
                   Name of the attribute space from which to resolve attributes
                   in the sub message.
                 type: string
+              attribute-replace:
+                description: |
+                  Replace the parent nested attribute with attribute set
+                type: boolean
 
   operations:
     description: Operations supported by the protocol.
diff --git a/Documentation/netlink/netlink-raw.yaml b/Documentation/netlink/netlink-raw.yaml
index cc38b026c451..e32660fbe6c3 100644
--- a/Documentation/netlink/netlink-raw.yaml
+++ b/Documentation/netlink/netlink-raw.yaml
@@ -346,6 +346,10 @@ properties:
                   Name of the attribute space from which to resolve attributes
                   in the sub message.
                 type: string
+              attribute-replace:
+                description: |
+                  Replace the parent nested attribute with attribute set
+                type: boolean
 
   operations:
     description: Operations supported by the protocol.
diff --git a/Documentation/userspace-api/netlink/genetlink-legacy.rst b/Documentation/userspace-api/netlink/genetlink-legacy.rst
index 7126b650090e..a9ccbfbb4a8d 100644
--- a/Documentation/userspace-api/netlink/genetlink-legacy.rst
+++ b/Documentation/userspace-api/netlink/genetlink-legacy.rst
@@ -381,3 +381,28 @@ alongside a sub-message selector and also in a top level ``attribute-set``, then
 the selector will be resolved using the value 'closest' to the selector. If the
 value is not present in the message at the same level as defined in the spec
 then this is an error.
+
+Some users, like devlink param, fill different attribute type according to
+selector attribute value. ``replace-attribute`` set to ``true`` indicates,
+that sub-message is not nested inside the attribute, but rather replacing
+the attribute. This allows to treat the attribute type differently according
+to the selector:
+
+.. code-block:: yaml
+
+  sub-messages:
+    -
+      name: dl-param-value-data-msg
+      formats:
+        -
+          value: u32
+          attribute-set: dl-param-value-data-u32-attrs
+          attribute-replace: true
+        -
+          value: string
+          attribute-set: dl-param-value-data-string-attrs
+          attribute-replace: true
+        -
+          value: bool
+          attribute-set: dl-param-value-data-bool-attrs
+          attribute-replace: true
diff --git a/tools/net/ynl/lib/nlspec.py b/tools/net/ynl/lib/nlspec.py
index 5e48ee0fb8b4..280d50e9079c 100644
--- a/tools/net/ynl/lib/nlspec.py
+++ b/tools/net/ynl/lib/nlspec.py
@@ -237,6 +237,9 @@ class SpecAttrSet(SpecElement):
     def items(self):
         return self.attrs.items()
 
+    def keys(self):
+        return self.attrs.keys()
+
 
 class SpecStructMember(SpecElement):
     """Struct member attribute
@@ -318,6 +321,8 @@ class SpecSubMessageFormat(SpecElement):
         value         attribute value to match against type selector
         fixed_header  string, name of fixed header, or None
         attr_set      string, name of attribute set, or None
+        attr_replace  bool, indicates replacement of parent attribute with
+                      attr_set decode, or None
     """
     def __init__(self, family, yaml):
         super().__init__(family, yaml)
@@ -325,6 +330,7 @@ class SpecSubMessageFormat(SpecElement):
         self.value = yaml.get('value')
         self.fixed_header = yaml.get('fixed-header')
         self.attr_set = yaml.get('attribute-set')
+        self.attr_replace = yaml.get('attribute-replace')
 
 
 class SpecOperation(SpecElement):
diff --git a/tools/net/ynl/lib/ynl.py b/tools/net/ynl/lib/ynl.py
index d2ea1571d00c..8591e6bfe40b 100644
--- a/tools/net/ynl/lib/ynl.py
+++ b/tools/net/ynl/lib/ynl.py
@@ -507,11 +507,16 @@ class YnlFamily(SpecFamily):
                 attr_payload += self._encode_struct(msg_format.fixed_header, value)
             if msg_format.attr_set:
                 if msg_format.attr_set in self.attr_sets:
-                    nl_type |= Netlink.NLA_F_NESTED
-                    sub_attrs = SpaceAttrs(msg_format.attr_set, value, search_attrs)
-                    for subname, subvalue in value.items():
-                        attr_payload += self._add_attr(msg_format.attr_set,
-                                                       subname, subvalue, sub_attrs)
+                    if msg_format.attr_replace:
+                        first_attr_name = list(self.attr_sets[msg_format.attr_set].keys())[0]
+                        return self._add_attr(msg_format.attr_set, first_attr_name,
+                                              value, search_attrs)
+                    else:
+                        nl_type |= Netlink.NLA_F_NESTED
+                        sub_attrs = SpaceAttrs(msg_format.attr_set, value, search_attrs)
+                        for subname, subvalue in value.items():
+                            attr_payload += self._add_attr(msg_format.attr_set,
+                                                           subname, subvalue, sub_attrs)
                 else:
                     raise Exception(f"Unknown attribute-set '{msg_format.attr_set}'")
         else:
@@ -600,8 +605,17 @@ class YnlFamily(SpecFamily):
             offset = self._struct_size(msg_format.fixed_header)
         if msg_format.attr_set:
             if msg_format.attr_set in self.attr_sets:
-                subdict = self._decode(NlAttrs(attr.raw, offset), msg_format.attr_set)
+                if msg_format.attr_replace:
+                    attrs = [attr]
+                else:
+                    attrs = NlAttrs(attr.raw, offset);
+                subdict = self._decode(attrs, msg_format.attr_set)
                 decoded.update(subdict)
+                if msg_format.attr_replace:
+                    try:
+                        decoded = decoded[attr_spec.name]
+                    except KeyError:
+                        raise Exception(f"Attribute-set '{attr_space}' does not contain '{attr_spec.name}'")
             else:
                 raise Exception(f"Unknown attribute-set '{attr_space}' when decoding '{attr_spec.name}'")
         return decoded
-- 
2.43.2

[patch net-next 07/13] tools: ynl: add support for list in nested attribute

[Jiri Pirko]

From: Jiri Pirko <jiri@nvidia.com>

Some nested attributes, like devlink-fmg, contain plain list of
attributes that may repeat. Current decode rsp implementation is
dictionary which causes duplicate attributes to be re-written.

To solve this, introduce "nested-list" flag for "nest" type which
indicates the _decode() function to use list instead of dictionary and
store each decoded attribute into this list as a separate dictionary.

Signed-off-by: Jiri Pirko <jiri@nvidia.com>
---
 Documentation/netlink/genetlink-legacy.yaml |  3 +++
 tools/net/ynl/lib/ynl.py                    | 18 ++++++++++++++----
 2 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/Documentation/netlink/genetlink-legacy.yaml b/Documentation/netlink/genetlink-legacy.yaml
index 77d89f81c71f..0e0ee3b4ff5f 100644
--- a/Documentation/netlink/genetlink-legacy.yaml
+++ b/Documentation/netlink/genetlink-legacy.yaml
@@ -220,6 +220,9 @@ properties:
               nested-attributes:
                 description: Name of the space (sub-space) used inside the attribute.
                 type: string
+              nested-list:
+                description: The nested attribute contains a list of attributes.
+                type: boolean
               enum:
                 description: Name of the enum type used for the attribute.
                 type: string
diff --git a/tools/net/ynl/lib/ynl.py b/tools/net/ynl/lib/ynl.py
index 8591e6bfe40b..08fe27c8dec7 100644
--- a/tools/net/ynl/lib/ynl.py
+++ b/tools/net/ynl/lib/ynl.py
@@ -386,8 +386,12 @@ class SpaceAttrs:
     def lookup(self, name):
         for scope in self.scopes:
             if name in scope.spec:
-                if name in scope.values:
+                if isinstance(scope.values, dict) and name in scope.values:
                     return scope.values[name]
+                if isinstance(scope.values, list):
+                    for item in reversed(scope.values):
+                        if name in item:
+                            return item[name]
                 spec_name = scope.spec.yaml['name']
                 raise Exception(
                     f"No value for '{name}' in attribute space '{spec_name}'")
@@ -568,6 +572,10 @@ class YnlFamily(SpecFamily):
             return attr.as_bin()
 
     def _rsp_add(self, rsp, name, is_multi, decoded):
+        if isinstance(rsp, list):
+            rsp.append({name: decoded})
+            return
+
         if is_multi == None:
             if name in rsp and type(rsp[name]) is not list:
                 rsp[name] = [rsp[name]]
@@ -620,8 +628,8 @@ class YnlFamily(SpecFamily):
                 raise Exception(f"Unknown attribute-set '{attr_space}' when decoding '{attr_spec.name}'")
         return decoded
 
-    def _decode(self, attrs, space, outer_attrs = None):
-        rsp = dict()
+    def _decode(self, attrs, space, outer_attrs = None, to_list = False):
+        rsp = list() if to_list else dict()
         if space:
             attr_space = self.attr_sets[space]
             search_attrs = SpaceAttrs(attr_space, rsp, outer_attrs)
@@ -637,7 +645,9 @@ class YnlFamily(SpecFamily):
                 continue
 
             if attr_spec["type"] == 'nest':
-                subdict = self._decode(NlAttrs(attr.raw), attr_spec['nested-attributes'], search_attrs)
+                nested_list = attr_spec['nested-list'] if 'nested-list' in attr_spec else False
+                subdict = self._decode(NlAttrs(attr.raw), attr_spec['nested-attributes'],
+                                       search_attrs, to_list = nested_list)
                 decoded = subdict
             elif attr_spec["type"] == 'string':
                 decoded = attr.as_strz()
-- 
2.43.2

[patch net-next 08/13] netlink: specs: devlink: add enum for param-type attribute values

[Jiri Pirko]

From: Jiri Pirko <jiri@nvidia.com>

For devlink param values, devlink relies on NLA_* values what are used
internally in kernel to indicate which type the attribute is.
This is not exposed over UAPI. Add devlink-param-type enum that defines
these values as part of devlink yaml spec.

Signed-off-by: Jiri Pirko <jiri@nvidia.com>
---
 Documentation/netlink/specs/devlink.yaml | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/Documentation/netlink/specs/devlink.yaml b/Documentation/netlink/specs/devlink.yaml
index cf6eaa0da821..88abe137c8ef 100644
--- a/Documentation/netlink/specs/devlink.yaml
+++ b/Documentation/netlink/specs/devlink.yaml
@@ -164,6 +164,22 @@ definitions:
         value: 1
       -
         name: fw-activate
+  -
+    type: enum
+    name: param-type
+    entries:
+      -
+        name: u8
+        value: 1
+      -
+        name: u16
+      -
+        name: u32
+      -
+        name: string
+        value: 5
+      -
+        name: flag
   -
     type: enum
     name: param-cmode
@@ -498,6 +514,7 @@ attribute-sets:
       -
         name: param-type
         type: u8
+        enum: param-type
 
       # TODO: fill in the attributes in between
 
-- 
2.43.2

[patch net-next 09/13] netlink: specs: devlink: add missing param attribute definitions

[Jiri Pirko]

From: Jiri Pirko <jiri@nvidia.com>

Add missing values list attribute and all nested attributes definition,
including param-value-data. For this one, use newly introduced
sub-message replace-attribute infrastructure to allow to process
attribute type selected by param-value-type.

Signed-off-by: Jiri Pirko <jiri@nvidia.com>
---
 Documentation/netlink/specs/devlink.yaml | 108 +++++++++++++++++++++--
 1 file changed, 102 insertions(+), 6 deletions(-)

diff --git a/Documentation/netlink/specs/devlink.yaml b/Documentation/netlink/specs/devlink.yaml
index 88abe137c8ef..71a95163c419 100644
--- a/Documentation/netlink/specs/devlink.yaml
+++ b/Documentation/netlink/specs/devlink.yaml
@@ -515,14 +515,24 @@ attribute-sets:
         name: param-type
         type: u8
         enum: param-type
-
-      # TODO: fill in the attributes in between
-
+      -
+        name: param-values-list
+        type: nest
+        nested-attributes: dl-param-values-list
+      -
+        name: param-value
+        type: nest
+        multi-attr: true
+        nested-attributes: dl-param-value
+      -
+        name: param-value-data
+        type: sub-message
+        sub-message: dl-param-value-data-msg
+        selector: param-type
       -
         name: param-value-cmode
         type: u8
         enum: param-cmode
-        value: 87
       -
         name: region-name
         type: string
@@ -1131,8 +1141,24 @@ attribute-sets:
         name: param-generic
       -
         name: param-type
+      -
+        name: param-values-list
+
+  -
+    name: dl-param-values-list
+    subset-of: devlink
+    attributes:
+      -
+        name: param-value
 
-      # TODO: fill in the attribute param-value-list
+  -
+    name: dl-param-value
+    subset-of: devlink
+    attributes:
+      -
+        name: param-value-data
+      -
+        name: param-value-cmode
 
   -
     name: dl-region-snapshots
@@ -1243,6 +1269,71 @@ attribute-sets:
         name: flash
         type: flag
 
+  -
+    name: dl-param-value-data-u8-attrs
+    subset-of: devlink
+    attributes:
+      -
+        name: param-value-data
+        type: u8
+
+  -
+    name: dl-param-value-data-u16-attrs
+    subset-of: devlink
+    attributes:
+      -
+        name: param-value-data
+        type: u16
+
+  -
+    name: dl-param-value-data-u32-attrs
+    subset-of: devlink
+    attributes:
+      -
+        name: param-value-data
+        type: u32
+
+  -
+    name: dl-param-value-data-string-attrs
+    subset-of: devlink
+    attributes:
+      -
+        name: param-value-data
+        type: string
+
+  -
+    name: dl-param-value-data-flag-attrs
+    subset-of: devlink
+    attributes:
+      -
+        name: param-value-data
+        type: flag
+
+sub-messages:
+  -
+    name: dl-param-value-data-msg
+    formats:
+      -
+        value: u8
+        attribute-set: dl-param-value-data-u8-attrs
+        attribute-replace: true
+      -
+        value: u16
+        attribute-set: dl-param-value-data-u16-attrs
+        attribute-replace: true
+      -
+        value: u32
+        attribute-set: dl-param-value-data-u32-attrs
+        attribute-replace: true
+      -
+        value: string
+        attribute-set: dl-param-value-data-string-attrs
+        attribute-replace: true
+      -
+        value: flag
+        attribute-set: dl-param-value-data-flag-attrs
+        attribute-replace: true
+
 operations:
   enum-model: directional
   list:
@@ -1731,7 +1822,12 @@ operations:
             - dev-name
             - param-name
         reply: &param-get-reply
-          attributes: *param-id-attrs
+          attributes:
+            - bus-name
+            - dev-name
+            - param-name
+            - param-type
+            - param-values-list
       dump:
         request:
           attributes: *dev-id-attrs
-- 
2.43.2

[patch net-next 10/13] netlink: specs: devlink: treat dl-fmsg attribute as list

[Jiri Pirko]

From: Jiri Pirko <jiri@nvidia.com>

Benefit from the previously introduces "nested-list" flag for "nest"
type and use it to indicate dl-fmsg attribute contains a list.

Signed-off-by: Jiri Pirko <jiri@nvidia.com>
---
 Documentation/netlink/specs/devlink.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/netlink/specs/devlink.yaml b/Documentation/netlink/specs/devlink.yaml
index 71a95163c419..df1afdf06068 100644
--- a/Documentation/netlink/specs/devlink.yaml
+++ b/Documentation/netlink/specs/devlink.yaml
@@ -601,6 +601,7 @@ attribute-sets:
         name: fmsg
         type: nest
         nested-attributes: dl-fmsg
+        nested-list: true
       -
         name: fmsg-obj-nest-start
         type: flag
-- 
2.43.2

[patch net-next 11/13] netlink: specs: devlink: add enum for fmsg-obj-value-type attribute values

[Jiri Pirko]

From: Jiri Pirko <jiri@nvidia.com>

For devlink fmsg object values, devlink relies on NLA_* values what
are used internally in kernel to indicate which type the attribute is.
This is not exposed over UAPI. Add devlink-param-type enum that defines
these values as part of devlink yaml spec.

Signed-off-by: Jiri Pirko <jiri@nvidia.com>
---
 Documentation/netlink/specs/devlink.yaml | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/Documentation/netlink/specs/devlink.yaml b/Documentation/netlink/specs/devlink.yaml
index df1afdf06068..fa4440141b05 100644
--- a/Documentation/netlink/specs/devlink.yaml
+++ b/Documentation/netlink/specs/devlink.yaml
@@ -218,6 +218,26 @@ definitions:
         name: exception
       -
         name: control
+  -
+    type: enum
+    name: fmsg-obj-value-type
+    entries:
+      -
+        name: u8
+        value: 1
+      -
+        name: u32
+        value: 3
+      -
+        name: u64
+      -
+        name: flag
+        value: 6
+      -
+        name: string
+        value: 10
+      -
+        name: binary
 
 attribute-sets:
   -
@@ -620,6 +640,7 @@ attribute-sets:
       -
         name: fmsg-obj-value-type
         type: u8
+        enum: fmsg-obj-value-type
 
       # TODO: fill in the attributes in between
 
-- 
2.43.2

[patch net-next 12/13] netlink: specs: devlink: add missing fmsg-obj-value-data attribute definitions

[Jiri Pirko]

From: Jiri Pirko <jiri@nvidia.com>

Add missing fmsg-obj-value-data definition. Use newly introduced
sub-message replace-attribute infrastructure to allow to process
attribute type selected by fmsg-obj-value-type.

Signed-off-by: Jiri Pirko <jiri@nvidia.com>
---
 Documentation/netlink/specs/devlink.yaml | 89 ++++++++++++++++++++++--
 1 file changed, 85 insertions(+), 4 deletions(-)

diff --git a/Documentation/netlink/specs/devlink.yaml b/Documentation/netlink/specs/devlink.yaml
index fa4440141b05..d2bc0e366d09 100644
--- a/Documentation/netlink/specs/devlink.yaml
+++ b/Documentation/netlink/specs/devlink.yaml
@@ -641,13 +641,14 @@ attribute-sets:
         name: fmsg-obj-value-type
         type: u8
         enum: fmsg-obj-value-type
-
-      # TODO: fill in the attributes in between
-
+      -
+        name: fmsg-obj-value-data
+        type: sub-message
+        sub-message: dl-fmsg-obj-value-data-msg
+        selector: fmsg-obj-value-type
       -
         name: health-reporter
         type: nest
-        value: 114
         nested-attributes: dl-health-reporter
       -
         name: health-reporter-name
@@ -1226,6 +1227,10 @@ attribute-sets:
         name: fmsg-nest-end
       -
         name: fmsg-obj-name
+      -
+        name: fmsg-obj-value-type
+      -
+        name: fmsg-obj-value-data
 
   -
     name: dl-health-reporter
@@ -1331,6 +1336,54 @@ attribute-sets:
         name: param-value-data
         type: flag
 
+  -
+    name: dl-fmsg-obj-value-data-u8-attrs
+    subset-of: devlink
+    attributes:
+      -
+        name: fmsg-obj-value-data
+        type: u8
+
+  -
+    name: dl-fmsg-obj-value-data-u32-attrs
+    subset-of: devlink
+    attributes:
+      -
+        name: fmsg-obj-value-data
+        type: u32
+
+  -
+    name: dl-fmsg-obj-value-data-u64-attrs
+    subset-of: devlink
+    attributes:
+      -
+        name: fmsg-obj-value-data
+        type: u64
+
+  -
+    name: dl-fmsg-obj-value-data-flag-attrs
+    subset-of: devlink
+    attributes:
+      -
+        name: fmsg-obj-value-data
+        type: flag
+
+  -
+    name: dl-fmsg-obj-value-data-string-attrs
+    subset-of: devlink
+    attributes:
+      -
+        name: fmsg-obj-value-data
+        type: string
+
+  -
+    name: dl-fmsg-obj-value-data-binary-attrs
+    subset-of: devlink
+    attributes:
+      -
+        name: fmsg-obj-value-data
+        type: binary
+
 sub-messages:
   -
     name: dl-param-value-data-msg
@@ -1356,6 +1409,34 @@ sub-messages:
         attribute-set: dl-param-value-data-flag-attrs
         attribute-replace: true
 
+  -
+    name: dl-fmsg-obj-value-data-msg
+    formats:
+      -
+        value: u8
+        attribute-set: dl-fmsg-obj-value-data-u8-attrs
+        attribute-replace: true
+      -
+        value: u32
+        attribute-set: dl-fmsg-obj-value-data-u32-attrs
+        attribute-replace: true
+      -
+        value: u64
+        attribute-set: dl-fmsg-obj-value-data-u64-attrs
+        attribute-replace: true
+      -
+        value: flag
+        attribute-set: dl-fmsg-obj-value-data-flag-attrs
+        attribute-replace: true
+      -
+        value: string
+        attribute-set: dl-fmsg-obj-value-data-string-attrs
+        attribute-replace: true
+      -
+        value: binary
+        attribute-set: dl-fmsg-obj-value-data-binary-attrs
+        attribute-replace: true
+
 operations:
   enum-model: directional
   list:
-- 
2.43.2

[patch net-next 13/13] netlink: specs: devlink: add missing nested devlink definitions

[Jiri Pirko]

From: Jiri Pirko <jiri@nvidia.com>

Add missing nested devlink subspace definition with definition of two
attributes using it.

Signed-off-by: Jiri Pirko <jiri@nvidia.com>
---
 Documentation/netlink/specs/devlink.yaml | 24 ++++++++++++++++++++----
 1 file changed, 20 insertions(+), 4 deletions(-)

diff --git a/Documentation/netlink/specs/devlink.yaml b/Documentation/netlink/specs/devlink.yaml
index d2bc0e366d09..c416339b69a6 100644
--- a/Documentation/netlink/specs/devlink.yaml
+++ b/Documentation/netlink/specs/devlink.yaml
@@ -853,13 +853,14 @@ attribute-sets:
         name: linecard-supported-types
         type: nest
         nested-attributes: dl-linecard-supported-types
-
-      # TODO: fill in the attributes in between
-
+      -
+        name: nested-devlink
+        type: nest
+        multi-attr: true
+        nested-attributes: dl-nested-devlink
       -
         name: selftests
         type: nest
-        value: 176
         nested-attributes: dl-selftest-id
       -
         name: rate-tx-priority
@@ -944,6 +945,10 @@ attribute-sets:
         type: bitfield32
         enum: port-fn-attr-cap
         enum-as-flags: True
+      -
+        name: devlink
+        type: nest
+        nested-attributes: dl-nested-devlink
 
   -
     name: dl-dpipe-tables
@@ -1288,6 +1293,17 @@ attribute-sets:
       -
         name: linecard-type
 
+  -
+    name: dl-nested-devlink
+    subset-of: devlink
+    attributes:
+      -
+        name: bus-name
+      -
+        name: dev-name
+      -
+        name: netns-id
+
   -
     name: dl-selftest-id
     name-prefix: devlink-attr-selftest-id-
-- 
2.43.2

Re: [patch net-next 01/13] tools: ynl: allow user to specify flag attr with bool values

[Jakub Kicinski]

On Mon, 19 Feb 2024 18:25:17 +0100 Jiri Pirko wrote:
>          elif attr["type"] == 'flag':
> +            if value == False:
> +                return b''

how about "if value:" ? It could also be null / None or some other
"false" object.

Re: [patch net-next 03/13] tools: ynl: allow user to pass enum string instead of scalar value

[Jakub Kicinski]

On Mon, 19 Feb 2024 18:25:19 +0100 Jiri Pirko wrote:
> +    def _get_scalar(self, attr_spec, value):

It'd be cleaner to make it more symmetric with _decode_enum(), and call
it _encode_enum().

Re: [patch net-next 03/13] tools: ynl: allow user to pass enum string instead of scalar value

[Jakub Kicinski]

On Mon, 19 Feb 2024 18:25:19 +0100 Jiri Pirko wrote:
> +        if enum.type == 'flags' or attr_spec.get('enum-as-flags', False):
> +            scalar = 0
> +            for single_value in value:
> +                scalar += enum.entries[single_value].user_value(as_flags = True)

If the user mistakenly passes a single value for a flag, rather than 
a set, this is going to generate a hard to understand error.
How about we check isinstance(, str) and handle that directly,
whether a flag or not.

Re: [patch net-next 04/13] netlink: specs: allow sub-messages in genetlink-legacy

[Jakub Kicinski]

On Mon, 19 Feb 2024 18:25:20 +0100 Jiri Pirko wrote:
> Currently sub-messages are only supported in netlink-raw template.
> To be able to utilize them in devlink spec, allow them in
> genetlink-legacy as well.

Why missing in the commit message.

Re: [patch net-next 06/13] tools: ynl: introduce attribute-replace for sub-message

[Jakub Kicinski]

On Mon, 19 Feb 2024 18:25:22 +0100 Jiri Pirko wrote:
> For devlink param, param-value-data attr is used by kernel to fill
> different attribute type according to param-type attribute value.
> 
> Currently the sub-message feature allows spec to embed custom message
> selected by another attribute. The sub-message is then nested inside the
> attr of sub-message type.
> 
> Benefit from the sub-message feature and extend it. Introduce
> attribute-replace spec flag by which the spec indicates that ynl should
> consider sub-message as not nested in the original attribute, but rather
> to consider the original attribute as the sub-message right away.

The "type agnostic" / generic style of devlink params and fmsg
are contrary to YNL's direction and goals. YNL abstracts parsing
and typing using external spec. devlink params and fmsg go in a
different direction where all information is carried by netlink values
and netlink typing is just to create "JSON-like" formatting.
These are incompatible ideas, and combining these two abstractions
in one library provides little value - devlink CLI already has an
implementation for fmsg and params. YNL doesn't have to cover
everything.

Re: [patch net-next 01/13] tools: ynl: allow user to specify flag attr with bool values

[Jiri Pirko]

Mon, Feb 19, 2024 at 09:42:22PM CET, kuba@kernel.org wrote:
>On Mon, 19 Feb 2024 18:25:17 +0100 Jiri Pirko wrote:
>>          elif attr["type"] == 'flag':
>> +            if value == False:
>> +                return b''
>
>how about "if value:" ? It could also be null / None or some other
>"false" object.

Sure, why not.

Re: [patch net-next 03/13] tools: ynl: allow user to pass enum string instead of scalar value

[Jiri Pirko]

Mon, Feb 19, 2024 at 09:49:14PM CET, kuba@kernel.org wrote:
>On Mon, 19 Feb 2024 18:25:19 +0100 Jiri Pirko wrote:
>> +    def _get_scalar(self, attr_spec, value):
>
>It'd be cleaner to make it more symmetric with _decode_enum(), and call
>it _encode_enum().

That is misleading name, as it does not have to be enum.

Re: [patch net-next 03/13] tools: ynl: allow user to pass enum string instead of scalar value

[Jiri Pirko]

Mon, Feb 19, 2024 at 09:51:00PM CET, kuba@kernel.org wrote:
>On Mon, 19 Feb 2024 18:25:19 +0100 Jiri Pirko wrote:
>> +        if enum.type == 'flags' or attr_spec.get('enum-as-flags', False):
>> +            scalar = 0
>> +            for single_value in value:
>> +                scalar += enum.entries[single_value].user_value(as_flags = True)
>
>If the user mistakenly passes a single value for a flag, rather than 
>a set, this is going to generate a hard to understand error.
>How about we check isinstance(, str) and handle that directly,
>whether a flag or not.

Yeah, I was thinking about that as well. But as the flag output is
always list, here we expect also always list. I can either do what you
suggest of Errout with some sane message in case of the variable is not
a list. I didn't find ynl to be particularly forgiving in case of input
and error messages, that is why I didn't bother here.

Re: [patch net-next 04/13] netlink: specs: allow sub-messages in genetlink-legacy

[Jiri Pirko]

Mon, Feb 19, 2024 at 09:51:18PM CET, kuba@kernel.org wrote:
>On Mon, 19 Feb 2024 18:25:20 +0100 Jiri Pirko wrote:
>> Currently sub-messages are only supported in netlink-raw template.
>> To be able to utilize them in devlink spec, allow them in
>> genetlink-legacy as well.
>
>Why missing in the commit message.

Sure.

Re: [patch net-next 06/13] tools: ynl: introduce attribute-replace for sub-message

[Jiri Pirko]

Mon, Feb 19, 2024 at 11:52:04PM CET, kuba@kernel.org wrote:
>On Mon, 19 Feb 2024 18:25:22 +0100 Jiri Pirko wrote:
>> For devlink param, param-value-data attr is used by kernel to fill
>> different attribute type according to param-type attribute value.
>> 
>> Currently the sub-message feature allows spec to embed custom message
>> selected by another attribute. The sub-message is then nested inside the
>> attr of sub-message type.
>> 
>> Benefit from the sub-message feature and extend it. Introduce
>> attribute-replace spec flag by which the spec indicates that ynl should
>> consider sub-message as not nested in the original attribute, but rather
>> to consider the original attribute as the sub-message right away.
>
>The "type agnostic" / generic style of devlink params and fmsg
>are contrary to YNL's direction and goals. YNL abstracts parsing

True, but that's what we have. Similar to what we have in TC where
sub-messages are present, that is also against ynl's direction and
goals.

>and typing using external spec. devlink params and fmsg go in a
>different direction where all information is carried by netlink values
>and netlink typing is just to create "JSON-like" formatting.

Only fmsg, not params.

>These are incompatible ideas, and combining these two abstractions
>in one library provides little value - devlink CLI already has an
>implementation for fmsg and params. YNL doesn't have to cover
>everything.

True. In this patchset, I'm just using the existing sub-message
infrastructure with a bit of extension. The json-like thing of fmsg is
ignored, I don't try to reconstruct json from netlink message of fmsg.

Re: [patch net-next 03/13] tools: ynl: allow user to pass enum string instead of scalar value

[Jakub Kicinski]

On Tue, 20 Feb 2024 08:25:14 +0100 Jiri Pirko wrote:
> >It'd be cleaner to make it more symmetric with _decode_enum(), and call
> >it _encode_enum().  
> 
> That is misleading name, as it does not have to be enum.

It's a variant of a encode function using enum metadata.

Re: [patch net-next 03/13] tools: ynl: allow user to pass enum string instead of scalar value

[Jakub Kicinski]

On Tue, 20 Feb 2024 08:27:57 +0100 Jiri Pirko wrote:
> >If the user mistakenly passes a single value for a flag, rather than 
> >a set, this is going to generate a hard to understand error.
> >How about we check isinstance(, str) and handle that directly,
> >whether a flag or not.  
> 
> Yeah, I was thinking about that as well. But as the flag output is
> always list, here we expect also always list. I can either do what you
> suggest of Errout with some sane message in case of the variable is not
> a list. I didn't find ynl to be particularly forgiving in case of input
> and error messages, that is why I didn't bother here.

It's not the same thing, but (without looking at the code IIRC)
for multi-attr we do accept both a list and direct value.
Here we don't have to be lenient in what we accept.
Clear error message would be good enough.

Some of the sharp edges in Python YNL are because I very much
anticipated the pyroute2 maintainer to do a proper implementation, 
and this tool was just a very crude PoC :D

Re: [patch net-next 06/13] tools: ynl: introduce attribute-replace for sub-message

[Jakub Kicinski]

On Tue, 20 Feb 2024 08:31:23 +0100 Jiri Pirko wrote:
> >The "type agnostic" / generic style of devlink params and fmsg
> >are contrary to YNL's direction and goals. YNL abstracts parsing  
> 
> True, but that's what we have. Similar to what we have in TC where
> sub-messages are present, that is also against ynl's direction and
> goals.

But TC and ip-link are raw netlink, meaning genetlink-legacy remains
fairly straightforward. BTW since we currently have full parity in C
code gen adding this series will break build for tools/net/ynl.

Plus ip-link is a really high value target. I had been pondering how 
to solve it myself. There's probably a hundred different implementations
out there of container management systems which spawn veths using odd
hacks because "netlink is scary". Once I find the time to finish
rtnetlink codegen we can replace all  the unholy libbpf netlink hacks
with ynl, too.

So at this stage I'd really like to focus YNL on language coverage
(adding more codegens), packaging and usability polish, not extending
the spec definitions to cover not-so-often used corner cases.
Especially those which will barely benefit because they are in
themselves built to be an abstraction.

Re: [patch net-next 03/13] tools: ynl: allow user to pass enum string instead of scalar value

[Donald Hunter]

On Wed, 21 Feb 2024 at 01:59, Jakub Kicinski <kuba@kernel.org> wrote:
>
> Some of the sharp edges in Python YNL are because I very much
> anticipated the pyroute2 maintainer to do a proper implementation,
> and this tool was just a very crude PoC :D

Hah yeah, I looked at pyroute2 a while back and thought there was a
bit of an impedance mismatch between the dynamic schema driven
approach of ynl and the declarative / procedural code in iproute2. I
think a code generator would be the way to target iproute2.

https://github.com/svinota/pyroute2/blob/34d0768f89fd232126c49e2f7c94e6da6582795b/pyroute2/netlink/rtnl/rtmsg.py#L102-L139

I find ynl to be a very useful tool when writing and testing spec
files and have been happy to contribute to it for that purpose. I
think we have started to remove some of the sharp edges, but there is
more to do :-)

Re: [patch net-next 06/13] tools: ynl: introduce attribute-replace for sub-message

[Jiri Pirko]

Wed, Feb 21, 2024 at 03:10:04AM CET, kuba@kernel.org wrote:
>On Tue, 20 Feb 2024 08:31:23 +0100 Jiri Pirko wrote:
>> >The "type agnostic" / generic style of devlink params and fmsg
>> >are contrary to YNL's direction and goals. YNL abstracts parsing  
>> 
>> True, but that's what we have. Similar to what we have in TC where
>> sub-messages are present, that is also against ynl's direction and
>> goals.
>
>But TC and ip-link are raw netlink, meaning genetlink-legacy remains
>fairly straightforward. BTW since we currently have full parity in C
>code gen adding this series will break build for tools/net/ynl.
>
>Plus ip-link is a really high value target. I had been pondering how 
>to solve it myself. There's probably a hundred different implementations
>out there of container management systems which spawn veths using odd
>hacks because "netlink is scary". Once I find the time to finish
>rtnetlink codegen we can replace all  the unholy libbpf netlink hacks
>with ynl, too.
>
>So at this stage I'd really like to focus YNL on language coverage
>(adding more codegens), packaging and usability polish, not extending
>the spec definitions to cover not-so-often used corner cases.
>Especially those which will barely benefit because they are in
>themselves built to be an abstraction.

That leaves devlink.yaml incomplete, which I'm not happy about. It is a
legacy, it should be covered by genetlink-legacy I believe.

To undestand you correctly, should I wait until codegen for raw netlink
is done and then to rebase-repost this? Or do you say this will never be
acceptable?

Re: [patch net-next 03/13] tools: ynl: allow user to pass enum string instead of scalar value

[Jiri Pirko]

Wed, Feb 21, 2024 at 02:55:56AM CET, kuba@kernel.org wrote:
>On Tue, 20 Feb 2024 08:25:14 +0100 Jiri Pirko wrote:
>> >It'd be cleaner to make it more symmetric with _decode_enum(), and call
>> >it _encode_enum().  
>> 
>> That is misleading name, as it does not have to be enum.
>
>It's a variant of a encode function using enum metadata.

Not really. "decode_enum()" is exactly that, it only works with enum
case. Here, the enum is optional. If user passes scalar directly or if
the attribute does not have "enum" defined, this has nothing to do with
enum. But, as you wish.

Re: [patch net-next 06/13] tools: ynl: introduce attribute-replace for sub-message

[Jakub Kicinski]

On Wed, 21 Feb 2024 13:48:13 +0100 Jiri Pirko wrote:
> >But TC and ip-link are raw netlink, meaning genetlink-legacy remains
> >fairly straightforward. BTW since we currently have full parity in C
> >code gen adding this series will break build for tools/net/ynl.
> >
> >Plus ip-link is a really high value target. I had been pondering how 
> >to solve it myself. There's probably a hundred different implementations
> >out there of container management systems which spawn veths using odd
> >hacks because "netlink is scary". Once I find the time to finish
> >rtnetlink codegen we can replace all  the unholy libbpf netlink hacks
> >with ynl, too.
> >
> >So at this stage I'd really like to focus YNL on language coverage
> >(adding more codegens), packaging and usability polish, not extending
> >the spec definitions to cover not-so-often used corner cases.
> >Especially those which will barely benefit because they are in
> >themselves built to be an abstraction.  
> 
> That leaves devlink.yaml incomplete, which I'm not happy about. It is a
> legacy, it should be covered by genetlink-legacy I believe.
> 
> To undestand you correctly, should I wait until codegen for raw netlink
> is done and then to rebase-repost this? Or do you say this will never be
> acceptable?

It'd definitely not acceptable before the rtnetlink C codegen is
complete, and at least two other code gens for genetlink-legacy.
At that point we can reconsider complicating the schema further.

Re: [patch net-next 06/13] tools: ynl: introduce attribute-replace for sub-message

[Jiri Pirko]

Wed, Feb 21, 2024 at 07:45:05PM CET, kuba@kernel.org wrote:
>On Wed, 21 Feb 2024 13:48:13 +0100 Jiri Pirko wrote:
>> >But TC and ip-link are raw netlink, meaning genetlink-legacy remains
>> >fairly straightforward. BTW since we currently have full parity in C
>> >code gen adding this series will break build for tools/net/ynl.
>> >
>> >Plus ip-link is a really high value target. I had been pondering how 
>> >to solve it myself. There's probably a hundred different implementations
>> >out there of container management systems which spawn veths using odd
>> >hacks because "netlink is scary". Once I find the time to finish
>> >rtnetlink codegen we can replace all  the unholy libbpf netlink hacks
>> >with ynl, too.
>> >
>> >So at this stage I'd really like to focus YNL on language coverage
>> >(adding more codegens), packaging and usability polish, not extending
>> >the spec definitions to cover not-so-often used corner cases.
>> >Especially those which will barely benefit because they are in
>> >themselves built to be an abstraction.  
>> 
>> That leaves devlink.yaml incomplete, which I'm not happy about. It is a
>> legacy, it should be covered by genetlink-legacy I believe.
>> 
>> To undestand you correctly, should I wait until codegen for raw netlink
>> is done and then to rebase-repost this? Or do you say this will never be
>> acceptable?
>
>It'd definitely not acceptable before the rtnetlink C codegen is
>complete, and at least two other code gens for genetlink-legacy.
>At that point we can reconsider complicating the schema further.

Okay, will keep it in the cupboard for now. I would definitelly love to
get the devlink.yaml complete. Next step is to generate the uapi header
from it replacing the existing one.