netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net-next v1 00/12] tools/net/ynl: Add features for tc family
@ 2024-01-23 16:05 Donald Hunter
  2024-01-23 16:05 ` [PATCH net-next v1 01/12] tools/net/ynl: Add --output-json arg to ynl cli Donald Hunter
                   ` (11 more replies)
  0 siblings, 12 replies; 31+ messages in thread
From: Donald Hunter @ 2024-01-23 16:05 UTC (permalink / raw)
  To: netdev, Jakub Kicinski, David S. Miller, Eric Dumazet,
	Paolo Abeni, Jonathan Corbet, linux-doc, Jacob Keller,
	Breno Leitao, Jiri Pirko, Alessandro Marcolini
  Cc: donald.hunter, Donald Hunter

Add features to ynl for tc and update the tc spec to use them.

Patch 1 adds an option to output json instead of python pretty printing.
Patch 2 adds support for sub-messages in nested attribute spaces that
reference keys from a parent space.
Patches 3 and 6-8 refactor ynl in support of nested struct definitions
Patch 4 implements sub-message encoding for write ops.
Patch 5 adds logic to set default zero values for binary blobs
Patches 9, 10 adds support and docs for nested struct definitions
Patch 11 updates the ynl doc generator to include type information for
struct members.
Patch 12 updates the tc spec - still a work in progress but more complete

Donald Hunter (12):
  tools/net/ynl: Add --output-json arg to ynl cli
  tools/net/ynl: Support sub-messages in nested attribute spaces
  tools/net/ynl: Refactor fixed header encoding into separate method
  tools/net/ynl: Add support for encoding sub-messages
  tools/net/ynl: Encode default values for binary blobs
  tools/net/ynl: Combine struct decoding logic in ynl
  tools/net/ynl: Rename _fixed_header_size() to _struct_size()
  tools/net/ynl: Move formatted_string method out of NlAttr
  tools/net/ynl: Add support for nested structs
  doc/netlink: Describe nested structs in netlink raw docs
  tools/net/ynl: Add type info to struct members in generated docs
  doc/netlink/specs: Update the tc spec

 Documentation/netlink/netlink-raw.yaml        |   15 +-
 Documentation/netlink/specs/tc.yaml           | 2218 +++++++++++++++--
 .../userspace-api/netlink/netlink-raw.rst     |   34 +
 tools/net/ynl/cli.py                          |   22 +-
 tools/net/ynl/lib/nlspec.py                   |    2 +
 tools/net/ynl/lib/ynl.py                      |  174 +-
 tools/net/ynl/ynl-gen-rst.py                  |   10 +-
 7 files changed, 2241 insertions(+), 234 deletions(-)

-- 
2.42.0


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

* [PATCH net-next v1 01/12] tools/net/ynl: Add --output-json arg to ynl cli
  2024-01-23 16:05 [PATCH net-next v1 00/12] tools/net/ynl: Add features for tc family Donald Hunter
@ 2024-01-23 16:05 ` Donald Hunter
  2024-01-25 13:50   ` Breno Leitao
  2024-01-23 16:05 ` [PATCH net-next v1 02/12] tools/net/ynl: Support sub-messages in nested attribute spaces Donald Hunter
                   ` (10 subsequent siblings)
  11 siblings, 1 reply; 31+ messages in thread
From: Donald Hunter @ 2024-01-23 16:05 UTC (permalink / raw)
  To: netdev, Jakub Kicinski, David S. Miller, Eric Dumazet,
	Paolo Abeni, Jonathan Corbet, linux-doc, Jacob Keller,
	Breno Leitao, Jiri Pirko, Alessandro Marcolini
  Cc: donald.hunter, Donald Hunter

The ynl cli currently emits python pretty printed structures which is
hard to consume. Add a new --output-json argument to emit JSON.

Signed-off-by: Donald Hunter <donald.hunter@gmail.com>
---
 tools/net/ynl/cli.py | 22 +++++++++++++++++++---
 1 file changed, 19 insertions(+), 3 deletions(-)

diff --git a/tools/net/ynl/cli.py b/tools/net/ynl/cli.py
index 2ad9ec0f5545..0f8239979670 100755
--- a/tools/net/ynl/cli.py
+++ b/tools/net/ynl/cli.py
@@ -9,6 +9,15 @@ import time
 from lib import YnlFamily, Netlink
 
 
+class YnlEncoder(json.JSONEncoder):
+    def default(self, obj):
+        if isinstance(obj, bytes):
+            return bytes.hex(obj)
+        if isinstance(obj, set):
+            return list(obj)
+        return json.JSONEncoder.default(self, obj)
+
+
 def main():
     parser = argparse.ArgumentParser(description='YNL CLI sample')
     parser.add_argument('--spec', dest='spec', type=str, required=True)
@@ -28,8 +37,15 @@ def main():
     parser.add_argument('--append', dest='flags', action='append_const',
                         const=Netlink.NLM_F_APPEND)
     parser.add_argument('--process-unknown', action=argparse.BooleanOptionalAction)
+    parser.add_argument('--output-json', action='store_true')
     args = parser.parse_args()
 
+    def output(msg):
+        if args.output_json:
+            print(json.dumps(msg, cls=YnlEncoder))
+        else:
+            pprint.PrettyPrinter().pprint(msg)
+
     if args.no_schema:
         args.schema = ''
 
@@ -47,14 +63,14 @@ def main():
 
     if args.do:
         reply = ynl.do(args.do, attrs, args.flags)
-        pprint.PrettyPrinter().pprint(reply)
+        output(reply)
     if args.dump:
         reply = ynl.dump(args.dump, attrs)
-        pprint.PrettyPrinter().pprint(reply)
+        output(reply)
 
     if args.ntf:
         ynl.check_ntf()
-        pprint.PrettyPrinter().pprint(ynl.async_msg_queue)
+        output(ynl.async_msg_queue)
 
 
 if __name__ == "__main__":
-- 
2.42.0


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

* [PATCH net-next v1 02/12] tools/net/ynl: Support sub-messages in nested attribute spaces
  2024-01-23 16:05 [PATCH net-next v1 00/12] tools/net/ynl: Add features for tc family Donald Hunter
  2024-01-23 16:05 ` [PATCH net-next v1 01/12] tools/net/ynl: Add --output-json arg to ynl cli Donald Hunter
@ 2024-01-23 16:05 ` Donald Hunter
  2024-01-24  0:18   ` Jakub Kicinski
  2024-01-23 16:05 ` [PATCH net-next v1 03/12] tools/net/ynl: Refactor fixed header encoding into separate method Donald Hunter
                   ` (9 subsequent siblings)
  11 siblings, 1 reply; 31+ messages in thread
From: Donald Hunter @ 2024-01-23 16:05 UTC (permalink / raw)
  To: netdev, Jakub Kicinski, David S. Miller, Eric Dumazet,
	Paolo Abeni, Jonathan Corbet, linux-doc, Jacob Keller,
	Breno Leitao, Jiri Pirko, Alessandro Marcolini
  Cc: donald.hunter, Donald Hunter

Sub-message selectors could only be resolved using values from the
current nest level. Enable value lookup in outer scopes by using
collections.ChainMap to implement an ordered lookup from nested to
outer scopes.

Signed-off-by: Donald Hunter <donald.hunter@gmail.com>
---
 tools/net/ynl/lib/ynl.py | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/tools/net/ynl/lib/ynl.py b/tools/net/ynl/lib/ynl.py
index 1e10512b2117..b00cde5d29e5 100644
--- a/tools/net/ynl/lib/ynl.py
+++ b/tools/net/ynl/lib/ynl.py
@@ -1,6 +1,6 @@
 # SPDX-License-Identifier: GPL-2.0 OR BSD-3-Clause
 
-from collections import namedtuple
+from collections import namedtuple, ChainMap
 import functools
 import os
 import random
@@ -564,8 +564,8 @@ class YnlFamily(SpecFamily):
         spec = sub_msg_spec.formats[value]
         return spec
 
-    def _decode_sub_msg(self, attr, attr_spec, rsp):
-        msg_format = self._resolve_selector(attr_spec, rsp)
+    def _decode_sub_msg(self, attr, attr_spec, vals):
+        msg_format = self._resolve_selector(attr_spec, vals)
         decoded = {}
         offset = 0
         if msg_format.fixed_header:
@@ -579,10 +579,11 @@ class YnlFamily(SpecFamily):
                 raise Exception(f"Unknown attribute-set '{attr_space}' when decoding '{attr_spec.name}'")
         return decoded
 
-    def _decode(self, attrs, space):
+    def _decode(self, attrs, space, outer_vals = ChainMap()):
         if space:
             attr_space = self.attr_sets[space]
         rsp = dict()
+        vals = ChainMap(rsp, outer_vals)
         for attr in attrs:
             try:
                 attr_spec = attr_space.attrs_by_val[attr.type]
@@ -594,7 +595,7 @@ class YnlFamily(SpecFamily):
                 continue
 
             if attr_spec["type"] == 'nest':
-                subdict = self._decode(NlAttrs(attr.raw), attr_spec['nested-attributes'])
+                subdict = self._decode(NlAttrs(attr.raw), attr_spec['nested-attributes'], vals)
                 decoded = subdict
             elif attr_spec["type"] == 'string':
                 decoded = attr.as_strz()
@@ -617,7 +618,7 @@ class YnlFamily(SpecFamily):
                     selector = self._decode_enum(selector, attr_spec)
                 decoded = {"value": value, "selector": selector}
             elif attr_spec["type"] == 'sub-message':
-                decoded = self._decode_sub_msg(attr, attr_spec, rsp)
+                decoded = self._decode_sub_msg(attr, attr_spec, vals)
             else:
                 if not self.process_unknown:
                     raise Exception(f'Unknown {attr_spec["type"]} with name {attr_spec["name"]}')
-- 
2.42.0


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

* [PATCH net-next v1 03/12] tools/net/ynl: Refactor fixed header encoding into separate method
  2024-01-23 16:05 [PATCH net-next v1 00/12] tools/net/ynl: Add features for tc family Donald Hunter
  2024-01-23 16:05 ` [PATCH net-next v1 01/12] tools/net/ynl: Add --output-json arg to ynl cli Donald Hunter
  2024-01-23 16:05 ` [PATCH net-next v1 02/12] tools/net/ynl: Support sub-messages in nested attribute spaces Donald Hunter
@ 2024-01-23 16:05 ` Donald Hunter
  2024-01-23 16:05 ` [PATCH net-next v1 04/12] tools/net/ynl: Add support for encoding sub-messages Donald Hunter
                   ` (8 subsequent siblings)
  11 siblings, 0 replies; 31+ messages in thread
From: Donald Hunter @ 2024-01-23 16:05 UTC (permalink / raw)
  To: netdev, Jakub Kicinski, David S. Miller, Eric Dumazet,
	Paolo Abeni, Jonathan Corbet, linux-doc, Jacob Keller,
	Breno Leitao, Jiri Pirko, Alessandro Marcolini
  Cc: donald.hunter, Donald Hunter

Refactor the fixed header encoding into a separate _encode_struct method
so that it can be reused for fixed headers in sub-messages and for
encoding structs.

Signed-off-by: Donald Hunter <donald.hunter@gmail.com>
---
 tools/net/ynl/lib/ynl.py | 26 +++++++++++++++-----------
 1 file changed, 15 insertions(+), 11 deletions(-)

diff --git a/tools/net/ynl/lib/ynl.py b/tools/net/ynl/lib/ynl.py
index b00cde5d29e5..dd54205f866f 100644
--- a/tools/net/ynl/lib/ynl.py
+++ b/tools/net/ynl/lib/ynl.py
@@ -701,6 +701,20 @@ class YnlFamily(SpecFamily):
                 fixed_header_attrs[m.name] = value
         return fixed_header_attrs
 
+    def _encode_struct(self, name, vals):
+        members = self.consts[name].members
+        attr_payload = b''
+        for m in members:
+            value = vals.pop(m.name) if m.name in vals else 0
+            if m.type == 'pad':
+                attr_payload += bytearray(m.len)
+            elif m.type == 'binary':
+                attr_payload += bytes.fromhex(value)
+            else:
+                format = NlAttr.get_format(m.type, m.byte_order)
+                attr_payload += format.pack(value)
+        return attr_payload
+
     def handle_ntf(self, decoded):
         msg = dict()
         if self.include_raw:
@@ -760,18 +774,8 @@ class YnlFamily(SpecFamily):
 
         req_seq = random.randint(1024, 65535)
         msg = self.nlproto.message(nl_flags, op.req_value, 1, req_seq)
-        fixed_header_members = []
         if op.fixed_header:
-            fixed_header_members = self.consts[op.fixed_header].members
-            for m in fixed_header_members:
-                value = vals.pop(m.name) if m.name in vals else 0
-                if m.type == 'pad':
-                    msg += bytearray(m.len)
-                elif m.type == 'binary':
-                    msg += bytes.fromhex(value)
-                else:
-                    format = NlAttr.get_format(m.type, m.byte_order)
-                    msg += format.pack(value)
+            msg += self._encode_struct(op.fixed_header, vals)
         for name, value in vals.items():
             msg += self._add_attr(op.attr_set.name, name, value)
         msg = _genl_msg_finalize(msg)
-- 
2.42.0


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

* [PATCH net-next v1 04/12] tools/net/ynl: Add support for encoding sub-messages
  2024-01-23 16:05 [PATCH net-next v1 00/12] tools/net/ynl: Add features for tc family Donald Hunter
                   ` (2 preceding siblings ...)
  2024-01-23 16:05 ` [PATCH net-next v1 03/12] tools/net/ynl: Refactor fixed header encoding into separate method Donald Hunter
@ 2024-01-23 16:05 ` Donald Hunter
  2024-01-23 16:05 ` [PATCH net-next v1 05/12] tools/net/ynl: Encode default values for binary blobs Donald Hunter
                   ` (7 subsequent siblings)
  11 siblings, 0 replies; 31+ messages in thread
From: Donald Hunter @ 2024-01-23 16:05 UTC (permalink / raw)
  To: netdev, Jakub Kicinski, David S. Miller, Eric Dumazet,
	Paolo Abeni, Jonathan Corbet, linux-doc, Jacob Keller,
	Breno Leitao, Jiri Pirko, Alessandro Marcolini
  Cc: donald.hunter, Donald Hunter

Add sub-message encoding to ynl. This makes it possible to create
tc qdiscs and other polymorphic netlink objects.

Signed-off-by: Donald Hunter <donald.hunter@gmail.com>
---
 tools/net/ynl/lib/ynl.py | 24 +++++++++++++++++++++---
 1 file changed, 21 insertions(+), 3 deletions(-)

diff --git a/tools/net/ynl/lib/ynl.py b/tools/net/ynl/lib/ynl.py
index dd54205f866f..d1005e662d52 100644
--- a/tools/net/ynl/lib/ynl.py
+++ b/tools/net/ynl/lib/ynl.py
@@ -449,7 +449,7 @@ class YnlFamily(SpecFamily):
         self.sock.setsockopt(Netlink.SOL_NETLINK, Netlink.NETLINK_ADD_MEMBERSHIP,
                              mcast_id)
 
-    def _add_attr(self, space, name, value):
+    def _add_attr(self, space, name, value, vals):
         try:
             attr = self.attr_sets[space][name]
         except KeyError:
@@ -458,8 +458,10 @@ class YnlFamily(SpecFamily):
         if attr["type"] == 'nest':
             nl_type |= Netlink.NLA_F_NESTED
             attr_payload = b''
+            subvals = ChainMap(value, vals)
             for subname, subvalue in value.items():
-                attr_payload += self._add_attr(attr['nested-attributes'], subname, subvalue)
+                attr_payload += self._add_attr(attr['nested-attributes'],
+                                               subname, subvalue, subvals)
         elif attr["type"] == 'flag':
             attr_payload = b''
         elif attr["type"] == 'string':
@@ -469,6 +471,8 @@ class YnlFamily(SpecFamily):
                 attr_payload = value
             elif isinstance(value, str):
                 attr_payload = bytes.fromhex(value)
+            elif isinstance(value, dict) and attr.struct_name:
+                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:
@@ -481,6 +485,20 @@ class YnlFamily(SpecFamily):
             attr_payload = format.pack(int(value))
         elif attr['type'] in "bitfield32":
             attr_payload = struct.pack("II", int(value["value"]), int(value["selector"]))
+        elif attr['type'] == 'sub-message':
+            msg_format = self._resolve_selector(attr, vals)
+            attr_payload = b''
+            if msg_format.fixed_header:
+                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
+                    subvals = ChainMap(value, vals)
+                    for subname, subvalue in value.items():
+                        attr_payload += self._add_attr(msg_format.attr_set,
+                                                       subname, subvalue, subvals)
+                else:
+                    raise Exception(f"Unknown attribute-set '{msg_format.attr_set}'")
         else:
             raise Exception(f'Unknown type at {space} {name} {value} {attr["type"]}')
 
@@ -777,7 +795,7 @@ class YnlFamily(SpecFamily):
         if op.fixed_header:
             msg += self._encode_struct(op.fixed_header, vals)
         for name, value in vals.items():
-            msg += self._add_attr(op.attr_set.name, name, value)
+            msg += self._add_attr(op.attr_set.name, name, value, vals)
         msg = _genl_msg_finalize(msg)
 
         self.sock.send(msg, 0)
-- 
2.42.0


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

* [PATCH net-next v1 05/12] tools/net/ynl: Encode default values for binary blobs
  2024-01-23 16:05 [PATCH net-next v1 00/12] tools/net/ynl: Add features for tc family Donald Hunter
                   ` (3 preceding siblings ...)
  2024-01-23 16:05 ` [PATCH net-next v1 04/12] tools/net/ynl: Add support for encoding sub-messages Donald Hunter
@ 2024-01-23 16:05 ` Donald Hunter
  2024-01-23 16:05 ` [PATCH net-next v1 06/12] tools/net/ynl: Combine struct decoding logic in ynl Donald Hunter
                   ` (6 subsequent siblings)
  11 siblings, 0 replies; 31+ messages in thread
From: Donald Hunter @ 2024-01-23 16:05 UTC (permalink / raw)
  To: netdev, Jakub Kicinski, David S. Miller, Eric Dumazet,
	Paolo Abeni, Jonathan Corbet, linux-doc, Jacob Keller,
	Breno Leitao, Jiri Pirko, Alessandro Marcolini
  Cc: donald.hunter, Donald Hunter

Add support for defaulting binary byte arrays to all zeros as well as
defaulting scalar values to 0 when encoding input parameters.

Signed-off-by: Donald Hunter <donald.hunter@gmail.com>
---
 tools/net/ynl/lib/ynl.py | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/tools/net/ynl/lib/ynl.py b/tools/net/ynl/lib/ynl.py
index d1005e662d52..ea4638c56802 100644
--- a/tools/net/ynl/lib/ynl.py
+++ b/tools/net/ynl/lib/ynl.py
@@ -723,12 +723,17 @@ class YnlFamily(SpecFamily):
         members = self.consts[name].members
         attr_payload = b''
         for m in members:
-            value = vals.pop(m.name) if m.name in vals else 0
+            value = vals.pop(m.name) if m.name in vals else None
             if m.type == 'pad':
                 attr_payload += bytearray(m.len)
             elif m.type == 'binary':
-                attr_payload += bytes.fromhex(value)
+                if value is None:
+                    attr_payload += bytearray(m.len)
+                else:
+                    attr_payload += bytes.fromhex(value)
             else:
+                if value is None:
+                    value = 0
                 format = NlAttr.get_format(m.type, m.byte_order)
                 attr_payload += format.pack(value)
         return attr_payload
-- 
2.42.0


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

* [PATCH net-next v1 06/12] tools/net/ynl: Combine struct decoding logic in ynl
  2024-01-23 16:05 [PATCH net-next v1 00/12] tools/net/ynl: Add features for tc family Donald Hunter
                   ` (4 preceding siblings ...)
  2024-01-23 16:05 ` [PATCH net-next v1 05/12] tools/net/ynl: Encode default values for binary blobs Donald Hunter
@ 2024-01-23 16:05 ` Donald Hunter
  2024-01-23 16:05 ` [PATCH net-next v1 07/12] tools/net/ynl: Rename _fixed_header_size() to _struct_size() Donald Hunter
                   ` (5 subsequent siblings)
  11 siblings, 0 replies; 31+ messages in thread
From: Donald Hunter @ 2024-01-23 16:05 UTC (permalink / raw)
  To: netdev, Jakub Kicinski, David S. Miller, Eric Dumazet,
	Paolo Abeni, Jonathan Corbet, linux-doc, Jacob Keller,
	Breno Leitao, Jiri Pirko, Alessandro Marcolini
  Cc: donald.hunter, Donald Hunter

_decode_fixed_header() and NlAttr.as_struct() both implemented struct
decoding logic. Deduplicate the code into newly named _decode_struct()
method.

Signed-off-by: Donald Hunter <donald.hunter@gmail.com>
---
 tools/net/ynl/lib/ynl.py | 47 ++++++++++++----------------------------
 1 file changed, 14 insertions(+), 33 deletions(-)

diff --git a/tools/net/ynl/lib/ynl.py b/tools/net/ynl/lib/ynl.py
index ea4638c56802..cc1106cbe8a6 100644
--- a/tools/net/ynl/lib/ynl.py
+++ b/tools/net/ynl/lib/ynl.py
@@ -148,23 +148,6 @@ class NlAttr:
         format = self.get_format(type)
         return [ x[0] for x in format.iter_unpack(self.raw) ]
 
-    def as_struct(self, members):
-        value = dict()
-        offset = 0
-        for m in members:
-            # TODO: handle non-scalar members
-            if m.type == 'binary':
-                decoded = self.raw[offset : offset + m['len']]
-                offset += m['len']
-            elif m.type in NlAttr.type_formats:
-                format = self.get_format(m.type, m.byte_order)
-                [ decoded ] = format.unpack_from(self.raw, offset)
-                offset += format.size
-            if m.display_hint:
-                decoded = self.formatted_string(decoded, m.display_hint)
-            value[m.name] = decoded
-        return value
-
     def __repr__(self):
         return f"[type:{self.type} len:{self._len}] {self.raw}"
 
@@ -521,11 +504,7 @@ class YnlFamily(SpecFamily):
 
     def _decode_binary(self, attr, attr_spec):
         if attr_spec.struct_name:
-            members = self.consts[attr_spec.struct_name]
-            decoded = attr.as_struct(members)
-            for m in members:
-                if m.enum:
-                    decoded[m.name] = self._decode_enum(decoded[m.name], m)
+            decoded = self._decode_struct(attr.raw, attr_spec.struct_name)
         elif attr_spec.sub_type:
             decoded = attr.as_c_array(attr_spec.sub_type)
         else:
@@ -587,7 +566,7 @@ class YnlFamily(SpecFamily):
         decoded = {}
         offset = 0
         if msg_format.fixed_header:
-            decoded.update(self._decode_fixed_header(attr, msg_format.fixed_header));
+            decoded.update(self._decode_struct(attr.raw, msg_format.fixed_header));
             offset = self._fixed_header_size(msg_format.fixed_header)
         if msg_format.attr_set:
             if msg_format.attr_set in self.attr_sets:
@@ -698,26 +677,28 @@ class YnlFamily(SpecFamily):
         else:
             return 0
 
-    def _decode_fixed_header(self, msg, name):
-        fixed_header_members = self.consts[name].members
-        fixed_header_attrs = dict()
+    def _decode_struct(self, data, name):
+        members = self.consts[name].members
+        attrs = dict()
         offset = 0
-        for m in fixed_header_members:
+        for m in members:
             value = None
             if m.type == 'pad':
                 offset += m.len
             elif m.type == 'binary':
-                value = msg.raw[offset : offset + m.len]
+                value = data[offset : offset + m.len]
                 offset += m.len
             else:
                 format = NlAttr.get_format(m.type, m.byte_order)
-                [ value ] = format.unpack_from(msg.raw, offset)
+                [ value ] = format.unpack_from(data, offset)
                 offset += format.size
             if value is not None:
                 if m.enum:
                     value = self._decode_enum(value, m)
-                fixed_header_attrs[m.name] = value
-        return fixed_header_attrs
+                elif m.display_hint:
+                    value = NlAttr.formatted_string(value, m.display_hint)
+                attrs[m.name] = value
+        return attrs
 
     def _encode_struct(self, name, vals):
         members = self.consts[name].members
@@ -745,7 +726,7 @@ class YnlFamily(SpecFamily):
         op = self.rsp_by_value[decoded.cmd()]
         attrs = self._decode(decoded.raw_attrs, op.attr_set.name)
         if op.fixed_header:
-            attrs.update(self._decode_fixed_header(decoded, op.fixed_header))
+            attrs.update(self._decode_struct(decoded.raw, op.fixed_header))
 
         msg['name'] = op['name']
         msg['msg'] = attrs
@@ -836,7 +817,7 @@ class YnlFamily(SpecFamily):
 
                 rsp_msg = self._decode(decoded.raw_attrs, op.attr_set.name)
                 if op.fixed_header:
-                    rsp_msg.update(self._decode_fixed_header(decoded, op.fixed_header))
+                    rsp_msg.update(self._decode_struct(decoded.raw, op.fixed_header))
                 rsp.append(rsp_msg)
 
         if not rsp:
-- 
2.42.0


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

* [PATCH net-next v1 07/12] tools/net/ynl: Rename _fixed_header_size() to _struct_size()
  2024-01-23 16:05 [PATCH net-next v1 00/12] tools/net/ynl: Add features for tc family Donald Hunter
                   ` (5 preceding siblings ...)
  2024-01-23 16:05 ` [PATCH net-next v1 06/12] tools/net/ynl: Combine struct decoding logic in ynl Donald Hunter
@ 2024-01-23 16:05 ` Donald Hunter
  2024-01-23 16:05 ` [PATCH net-next v1 08/12] tools/net/ynl: Move formatted_string method out of NlAttr Donald Hunter
                   ` (4 subsequent siblings)
  11 siblings, 0 replies; 31+ messages in thread
From: Donald Hunter @ 2024-01-23 16:05 UTC (permalink / raw)
  To: netdev, Jakub Kicinski, David S. Miller, Eric Dumazet,
	Paolo Abeni, Jonathan Corbet, linux-doc, Jacob Keller,
	Breno Leitao, Jiri Pirko, Alessandro Marcolini
  Cc: donald.hunter, Donald Hunter

Refactor the _fixed_header_size() method to be _struct_size() so that
naming is consistent with _encode_struct() and _decode_struct().

Signed-off-by: Donald Hunter <donald.hunter@gmail.com>
---
 tools/net/ynl/lib/ynl.py | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/tools/net/ynl/lib/ynl.py b/tools/net/ynl/lib/ynl.py
index cc1106cbe8a6..f040c8a2a575 100644
--- a/tools/net/ynl/lib/ynl.py
+++ b/tools/net/ynl/lib/ynl.py
@@ -353,7 +353,7 @@ class NetlinkProtocol:
         fixed_header_size = 0
         if ynl:
             op = ynl.rsp_by_value[msg.cmd()]
-            fixed_header_size = ynl._fixed_header_size(op.fixed_header)
+            fixed_header_size = ynl._struct_size(op.fixed_header)
         msg.raw_attrs = NlAttrs(msg.raw, fixed_header_size)
         return msg
 
@@ -567,7 +567,7 @@ class YnlFamily(SpecFamily):
         offset = 0
         if msg_format.fixed_header:
             decoded.update(self._decode_struct(attr.raw, msg_format.fixed_header));
-            offset = self._fixed_header_size(msg_format.fixed_header)
+            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)
@@ -656,18 +656,18 @@ class YnlFamily(SpecFamily):
             return
 
         msg = self.nlproto.decode(self, NlMsg(request, 0, op.attr_set))
-        offset = 20 + self._fixed_header_size(op.fixed_header)
+        offset = 20 + self._struct_size(op.fixed_header)
         path = self._decode_extack_path(msg.raw_attrs, op.attr_set, offset,
                                         extack['bad-attr-offs'])
         if path:
             del extack['bad-attr-offs']
             extack['bad-attr'] = path
 
-    def _fixed_header_size(self, name):
+    def _struct_size(self, name):
         if name:
-            fixed_header_members = self.consts[name].members
+            members = self.consts[name].members
             size = 0
-            for m in fixed_header_members:
+            for m in members:
                 if m.type in ['pad', 'binary']:
                     size += m.len
                 else:
-- 
2.42.0


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

* [PATCH net-next v1 08/12] tools/net/ynl: Move formatted_string method out of NlAttr
  2024-01-23 16:05 [PATCH net-next v1 00/12] tools/net/ynl: Add features for tc family Donald Hunter
                   ` (6 preceding siblings ...)
  2024-01-23 16:05 ` [PATCH net-next v1 07/12] tools/net/ynl: Rename _fixed_header_size() to _struct_size() Donald Hunter
@ 2024-01-23 16:05 ` Donald Hunter
  2024-01-25 14:24   ` Breno Leitao
  2024-01-23 16:05 ` [PATCH net-next v1 09/12] tools/net/ynl: Add support for nested structs Donald Hunter
                   ` (3 subsequent siblings)
  11 siblings, 1 reply; 31+ messages in thread
From: Donald Hunter @ 2024-01-23 16:05 UTC (permalink / raw)
  To: netdev, Jakub Kicinski, David S. Miller, Eric Dumazet,
	Paolo Abeni, Jonathan Corbet, linux-doc, Jacob Keller,
	Breno Leitao, Jiri Pirko, Alessandro Marcolini
  Cc: donald.hunter, Donald Hunter

The formatted_string() class method was in NlAttr so that it could be
accessed by NlAttr.as_struct(). Now that as_struct() has been removed,
move formatted_string() to YnlFamily as an internal helper method.

Signed-off-by: Donald Hunter <donald.hunter@gmail.com>
---
 tools/net/ynl/lib/ynl.py | 31 +++++++++++++++----------------
 1 file changed, 15 insertions(+), 16 deletions(-)

diff --git a/tools/net/ynl/lib/ynl.py b/tools/net/ynl/lib/ynl.py
index f040c8a2a575..c9c5b1fcc6f4 100644
--- a/tools/net/ynl/lib/ynl.py
+++ b/tools/net/ynl/lib/ynl.py
@@ -113,20 +113,6 @@ class NlAttr:
                 else format.little
         return format.native
 
-    @classmethod
-    def formatted_string(cls, raw, display_hint):
-        if display_hint == 'mac':
-            formatted = ':'.join('%02x' % b for b in raw)
-        elif display_hint == 'hex':
-            formatted = bytes.hex(raw, ' ')
-        elif display_hint in [ 'ipv4', 'ipv6' ]:
-            formatted = format(ipaddress.ip_address(raw))
-        elif display_hint == 'uuid':
-            formatted = str(uuid.UUID(bytes=raw))
-        else:
-            formatted = raw
-        return formatted
-
     def as_scalar(self, attr_type, byte_order=None):
         format = self.get_format(attr_type, byte_order)
         return format.unpack(self.raw)[0]
@@ -510,7 +496,7 @@ class YnlFamily(SpecFamily):
         else:
             decoded = attr.as_bin()
             if attr_spec.display_hint:
-                decoded = NlAttr.formatted_string(decoded, attr_spec.display_hint)
+                decoded = self._formatted_string(decoded, attr_spec.display_hint)
         return decoded
 
     def _decode_array_nest(self, attr, attr_spec):
@@ -696,7 +682,7 @@ class YnlFamily(SpecFamily):
                 if m.enum:
                     value = self._decode_enum(value, m)
                 elif m.display_hint:
-                    value = NlAttr.formatted_string(value, m.display_hint)
+                    value = self._formatted_string(value, m.display_hint)
                 attrs[m.name] = value
         return attrs
 
@@ -719,6 +705,19 @@ class YnlFamily(SpecFamily):
                 attr_payload += format.pack(value)
         return attr_payload
 
+    def _formatted_string(self, raw, display_hint):
+        if display_hint == 'mac':
+            formatted = ':'.join('%02x' % b for b in raw)
+        elif display_hint == 'hex':
+            formatted = bytes.hex(raw, ' ')
+        elif display_hint in [ 'ipv4', 'ipv6' ]:
+            formatted = format(ipaddress.ip_address(raw))
+        elif display_hint == 'uuid':
+            formatted = str(uuid.UUID(bytes=raw))
+        else:
+            formatted = raw
+        return formatted
+
     def handle_ntf(self, decoded):
         msg = dict()
         if self.include_raw:
-- 
2.42.0


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

* [PATCH net-next v1 09/12] tools/net/ynl: Add support for nested structs
  2024-01-23 16:05 [PATCH net-next v1 00/12] tools/net/ynl: Add features for tc family Donald Hunter
                   ` (7 preceding siblings ...)
  2024-01-23 16:05 ` [PATCH net-next v1 08/12] tools/net/ynl: Move formatted_string method out of NlAttr Donald Hunter
@ 2024-01-23 16:05 ` Donald Hunter
  2024-01-23 16:05 ` [PATCH net-next v1 10/12] doc/netlink: Describe nested structs in netlink raw docs Donald Hunter
                   ` (2 subsequent siblings)
  11 siblings, 0 replies; 31+ messages in thread
From: Donald Hunter @ 2024-01-23 16:05 UTC (permalink / raw)
  To: netdev, Jakub Kicinski, David S. Miller, Eric Dumazet,
	Paolo Abeni, Jonathan Corbet, linux-doc, Jacob Keller,
	Breno Leitao, Jiri Pirko, Alessandro Marcolini
  Cc: donald.hunter, Donald Hunter

Make it possible for struct definitions to reference other struct
definitions ofr binary members. For example, the tbf qdisc uses this
struct definition for its parms attribute:

  -
    name: tc-tbf-qopt
    type: struct
    members:
      -
        name: rate
        type: binary
        struct: tc-ratespec
      -
        name: peakrate
        type: binary
        struct: tc-ratespec
      -
        name: limit
        type: u32
      -
        name: buffer
        type: u32
      -
        name: mtu
        type: u32

This adds the necessary schema changes and adds nested struct encoding
and decoding to ynl.

Signed-off-by: Donald Hunter <donald.hunter@gmail.com>
---
 Documentation/netlink/netlink-raw.yaml | 15 ++++++++++++---
 tools/net/ynl/lib/nlspec.py            |  2 ++
 tools/net/ynl/lib/ynl.py               | 26 ++++++++++++++++++++------
 3 files changed, 34 insertions(+), 9 deletions(-)

diff --git a/Documentation/netlink/netlink-raw.yaml b/Documentation/netlink/netlink-raw.yaml
index 04b92f1a5cd6..ac4e05415f2f 100644
--- a/Documentation/netlink/netlink-raw.yaml
+++ b/Documentation/netlink/netlink-raw.yaml
@@ -152,14 +152,23 @@ properties:
                   the right formatting mechanism when displaying values of this
                   type.
                 enum: [ hex, mac, fddi, ipv4, ipv6, uuid ]
+              struct:
+                description: Name of the nested struct type.
+                type: string
             if:
               properties:
                 type:
-                  oneOf:
-                    - const: binary
-                    - const: pad
+                  const: pad
             then:
               required: [ len ]
+            if:
+              properties:
+                type:
+                  const: binary
+            then:
+              oneOf:
+                - required: [ len ]
+                - required: [ struct ]
         # End genetlink-legacy
 
   attribute-sets:
diff --git a/tools/net/ynl/lib/nlspec.py b/tools/net/ynl/lib/nlspec.py
index 44f13e383e8a..5d197a12ab8d 100644
--- a/tools/net/ynl/lib/nlspec.py
+++ b/tools/net/ynl/lib/nlspec.py
@@ -248,6 +248,7 @@ class SpecStructMember(SpecElement):
         len         integer, optional byte length of binary types
         display_hint  string, hint to help choose format specifier
                       when displaying the value
+        struct      string, name of nested struct type
     """
     def __init__(self, family, yaml):
         super().__init__(family, yaml)
@@ -256,6 +257,7 @@ class SpecStructMember(SpecElement):
         self.enum = yaml.get('enum')
         self.len = yaml.get('len')
         self.display_hint = yaml.get('display-hint')
+        self.struct = yaml.get('struct')
 
 
 class SpecStruct(SpecElement):
diff --git a/tools/net/ynl/lib/ynl.py b/tools/net/ynl/lib/ynl.py
index c9c5b1fcc6f4..dff2c042e6c3 100644
--- a/tools/net/ynl/lib/ynl.py
+++ b/tools/net/ynl/lib/ynl.py
@@ -655,7 +655,10 @@ class YnlFamily(SpecFamily):
             size = 0
             for m in members:
                 if m.type in ['pad', 'binary']:
-                    size += m.len
+                    if m.struct:
+                        size += self._struct_size(m.struct)
+                    else:
+                        size += m.len
                 else:
                     format = NlAttr.get_format(m.type, m.byte_order)
                     size += format.size
@@ -672,8 +675,14 @@ class YnlFamily(SpecFamily):
             if m.type == 'pad':
                 offset += m.len
             elif m.type == 'binary':
-                value = data[offset : offset + m.len]
-                offset += m.len
+                if m.struct:
+                    len = self._struct_size(m.struct)
+                    value = self._decode_struct(data[offset : offset + len],
+                                                m.struct)
+                    offset += len
+                else:
+                    value = data[offset : offset + m.len]
+                    offset += m.len
             else:
                 format = NlAttr.get_format(m.type, m.byte_order)
                 [ value ] = format.unpack_from(data, offset)
@@ -694,10 +703,15 @@ class YnlFamily(SpecFamily):
             if m.type == 'pad':
                 attr_payload += bytearray(m.len)
             elif m.type == 'binary':
-                if value is None:
-                    attr_payload += bytearray(m.len)
+                if m.struct:
+                    if value is None:
+                        value = dict()
+                    attr_payload += self._encode_struct(m.struct, value)
                 else:
-                    attr_payload += bytes.fromhex(value)
+                    if value is None:
+                        attr_payload += bytearray(m.len)
+                    else:
+                        attr_payload += bytes.fromhex(value)
             else:
                 if value is None:
                     value = 0
-- 
2.42.0


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

* [PATCH net-next v1 10/12] doc/netlink: Describe nested structs in netlink raw docs
  2024-01-23 16:05 [PATCH net-next v1 00/12] tools/net/ynl: Add features for tc family Donald Hunter
                   ` (8 preceding siblings ...)
  2024-01-23 16:05 ` [PATCH net-next v1 09/12] tools/net/ynl: Add support for nested structs Donald Hunter
@ 2024-01-23 16:05 ` Donald Hunter
  2024-01-23 16:05 ` [PATCH net-next v1 11/12] tools/net/ynl: Add type info to struct members in generated docs Donald Hunter
  2024-01-23 16:05 ` [PATCH net-next v1 12/12] doc/netlink/specs: Update the tc spec Donald Hunter
  11 siblings, 0 replies; 31+ messages in thread
From: Donald Hunter @ 2024-01-23 16:05 UTC (permalink / raw)
  To: netdev, Jakub Kicinski, David S. Miller, Eric Dumazet,
	Paolo Abeni, Jonathan Corbet, linux-doc, Jacob Keller,
	Breno Leitao, Jiri Pirko, Alessandro Marcolini
  Cc: donald.hunter, Donald Hunter

Add a description and example of nested struct definitions
to the netlink raw documentation.

Signed-off-by: Donald Hunter <donald.hunter@gmail.com>
---
 .../userspace-api/netlink/netlink-raw.rst     | 34 +++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/Documentation/userspace-api/netlink/netlink-raw.rst b/Documentation/userspace-api/netlink/netlink-raw.rst
index 1e14f5f22b8e..ecf04c91f70e 100644
--- a/Documentation/userspace-api/netlink/netlink-raw.rst
+++ b/Documentation/userspace-api/netlink/netlink-raw.rst
@@ -150,3 +150,37 @@ attributes from an ``attribute-set``. For example the following
 
 Note that a selector attribute must appear in a netlink message before any
 sub-message attributes that depend on it.
+
+Nested struct definitions
+-------------------------
+
+Many raw netlink families such as :doc:`tc<../../networking/netlink_spec/tc>`
+make use of nested struct definitions. The ``netlink-raw`` schema makes it
+possible to embed a struct within a struct definition using the ``struct``
+property. For example, the following struct definition embeds the
+``tc-ratespec`` struct definition for both the ``rate`` and the ``peakrate``
+members of ``struct tc-tbf-qopt``.
+
+.. code-block:: yaml
+
+  -
+    name: tc-tbf-qopt
+    type: struct
+    members:
+      -
+        name: rate
+        type: binary
+        struct: tc-ratespec
+      -
+        name: peakrate
+        type: binary
+        struct: tc-ratespec
+      -
+        name: limit
+        type: u32
+      -
+        name: buffer
+        type: u32
+      -
+        name: mtu
+        type: u32
-- 
2.42.0


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

* [PATCH net-next v1 11/12] tools/net/ynl: Add type info to struct members in generated docs
  2024-01-23 16:05 [PATCH net-next v1 00/12] tools/net/ynl: Add features for tc family Donald Hunter
                   ` (9 preceding siblings ...)
  2024-01-23 16:05 ` [PATCH net-next v1 10/12] doc/netlink: Describe nested structs in netlink raw docs Donald Hunter
@ 2024-01-23 16:05 ` Donald Hunter
  2024-01-25 13:59   ` Breno Leitao
  2024-01-23 16:05 ` [PATCH net-next v1 12/12] doc/netlink/specs: Update the tc spec Donald Hunter
  11 siblings, 1 reply; 31+ messages in thread
From: Donald Hunter @ 2024-01-23 16:05 UTC (permalink / raw)
  To: netdev, Jakub Kicinski, David S. Miller, Eric Dumazet,
	Paolo Abeni, Jonathan Corbet, linux-doc, Jacob Keller,
	Breno Leitao, Jiri Pirko, Alessandro Marcolini
  Cc: donald.hunter, Donald Hunter

Extend the ynl doc generator to include type information for struct
members, ignoring the pad type.

Signed-off-by: Donald Hunter <donald.hunter@gmail.com>
---
 tools/net/ynl/ynl-gen-rst.py | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/tools/net/ynl/ynl-gen-rst.py b/tools/net/ynl/ynl-gen-rst.py
index 262d88f88696..75c969d36b6a 100755
--- a/tools/net/ynl/ynl-gen-rst.py
+++ b/tools/net/ynl/ynl-gen-rst.py
@@ -189,12 +189,20 @@ def parse_operations(operations: List[Dict[str, Any]]) -> str:
 
 def parse_entries(entries: List[Dict[str, Any]], level: int) -> str:
     """Parse a list of entries"""
+    ignored = ["pad"]
     lines = []
     for entry in entries:
         if isinstance(entry, dict):
             # entries could be a list or a dictionary
+            field_name = entry.get("name", "")
+            if field_name in ignored:
+                continue
+            type_ = entry.get("type")
+            struct_ = entry.get("struct")
+            if type_:
+                field_name += f" ({inline(type_)})"
             lines.append(
-                rst_fields(entry.get("name", ""), sanitize(entry.get("doc", "")), level)
+                rst_fields(field_name, sanitize(entry.get("doc", "")), level)
             )
         elif isinstance(entry, list):
             lines.append(rst_list_inline(entry, level))
-- 
2.42.0


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

* [PATCH net-next v1 12/12] doc/netlink/specs: Update the tc spec
  2024-01-23 16:05 [PATCH net-next v1 00/12] tools/net/ynl: Add features for tc family Donald Hunter
                   ` (10 preceding siblings ...)
  2024-01-23 16:05 ` [PATCH net-next v1 11/12] tools/net/ynl: Add type info to struct members in generated docs Donald Hunter
@ 2024-01-23 16:05 ` Donald Hunter
  11 siblings, 0 replies; 31+ messages in thread
From: Donald Hunter @ 2024-01-23 16:05 UTC (permalink / raw)
  To: netdev, Jakub Kicinski, David S. Miller, Eric Dumazet,
	Paolo Abeni, Jonathan Corbet, linux-doc, Jacob Keller,
	Breno Leitao, Jiri Pirko, Alessandro Marcolini
  Cc: donald.hunter, Donald Hunter

Fill in many of the gaps in the tc netlink spec, including stats attrs,
classes and actions. Many documentation strings have also been added.

This is still a work in progress, albeit fairly complete:
 - there are still many attributes left as binary blobs.
 - actions have not had much testing

Signed-off-by: Donald Hunter <donald.hunter@gmail.com>
---
 Documentation/netlink/specs/tc.yaml | 2218 +++++++++++++++++++++++++--
 1 file changed, 2067 insertions(+), 151 deletions(-)

diff --git a/Documentation/netlink/specs/tc.yaml b/Documentation/netlink/specs/tc.yaml
index 4346fa402fc9..4b21b00dbebe 100644
--- a/Documentation/netlink/specs/tc.yaml
+++ b/Documentation/netlink/specs/tc.yaml
@@ -48,21 +48,28 @@ definitions:
       -
         name: bytes
         type: u64
+        doc: Number of enqueued bytes
       -
         name: packets
         type: u32
+        doc: Number of enqueued packets
       -
         name: drops
         type: u32
+        doc: Packets dropped because of lack of resources
       -
         name: overlimits
         type: u32
+        doc: |
+          Number of throttle events when this flow goes out of allocated bandwidth
       -
         name: bps
         type: u32
+        doc: Current flow byte rate
       -
         name: pps
         type: u32
+        doc: Current flow packet rate
       -
         name: qlen
         type: u32
@@ -112,6 +119,7 @@ definitions:
       -
         name: limit
         type: u32
+        doc: Queue length; bytes for bfifo, packets for pfifo
   -
     name: tc-htb-opt
     type: struct
@@ -119,11 +127,11 @@ definitions:
       -
         name: rate
         type: binary
-        len: 12
+        struct: tc-ratespec
       -
         name: ceil
         type: binary
-        len: 12
+        struct: tc-ratespec
       -
         name: buffer
         type: u32
@@ -149,15 +157,19 @@ definitions:
       -
         name: rate2quantum
         type: u32
+        doc: bps->quantum divisor
       -
         name: defcls
         type: u32
+        doc: Default class number
       -
         name: debug
         type: u32
+        doc: Debug flags
       -
         name: direct-pkts
         type: u32
+        doc: Count of non shaped packets
   -
     name: tc-gred-qopt
     type: struct
@@ -165,15 +177,19 @@ definitions:
       -
         name: limit
         type: u32
+        doc: HARD maximal queue length in bytes
       -
         name: qth-min
         type: u32
+        doc: Min average length threshold in bytes
       -
         name: qth-max
         type: u32
+        doc: Max average length threshold in bytes
       -
         name: DP
         type: u32
+        doc: Up to 2^32 DPs
       -
         name: backlog
         type: u32
@@ -195,15 +211,19 @@ definitions:
       -
         name: Wlog
         type: u8
+        doc: log(W)
       -
         name: Plog
         type: u8
+        doc: log(P_max / (qth-max - qth-min))
       -
         name: Scell_log
         type: u8
+        doc: cell size for idle damping
       -
         name: prio
         type: u8
+        doc: Priority of this VQ
       -
         name: packets
         type: u32
@@ -266,9 +286,11 @@ definitions:
       -
         name: bands
         type: u16
+        doc: Number of bands
       -
         name: max-bands
         type: u16
+        doc: Maximum number of queues
   -
     name: tc-netem-qopt
     type: struct
@@ -276,21 +298,138 @@ definitions:
       -
         name: latency
         type: u32
+        doc: Added delay in microseconds
       -
         name: limit
         type: u32
+        doc: Fifo limit in packets
       -
         name: loss
         type: u32
+        doc: Random packet loss (0=none, ~0=100%)
       -
         name: gap
         type: u32
+        doc: Re-ordering gap (0 for none)
       -
         name: duplicate
         type: u32
+        doc: Random packet duplication (0=none, ~0=100%)
       -
         name: jitter
         type: u32
+        doc: Random jitter latency in microseconds
+  -
+    name: tc-netem-gimodel
+    doc: State transition probabilities for 4 state model
+    type: struct
+    members:
+      -
+        name: p13
+        type: u32
+      -
+        name: p31
+        type: u32
+      -
+        name: p32
+        type: u32
+      -
+        name: p14
+        type: u32
+      -
+        name: p23
+        type: u32
+  -
+    name: tc-netem-gemodel
+    doc: Gilbert-Elliot models
+    type: struct
+    members:
+      -
+        name: p
+        type: u32
+      -
+        name: r
+        type: u32
+      -
+        name: h
+        type: u32
+      -
+        name: k1
+        type: u32
+  -
+    name: tc-netem-corr
+    type: struct
+    members:
+      -
+        name: delay-corr
+        type: u32
+        doc: Delay correlation
+      -
+        name: loss-corr
+        type: u32
+        doc: Packet loss correlation
+      -
+        name: dup-corr
+        type: u32
+        doc: Duplicate correlation
+  -
+    name: tc-netem-reorder
+    type: struct
+    members:
+      -
+        name: probability
+        type: u32
+      -
+        name: correlation
+        type: u32
+  -
+    name: tc-netem-corrupt
+    type: struct
+    members:
+      -
+        name: probability
+        type: u32
+      -
+        name: correlation
+        type: u32
+  -
+    name: tc-netem-rate
+    type: struct
+    members:
+      -
+        name: rate
+        type: u32
+      -
+        name: packet-overhead
+        type: s32
+      -
+        name: cell-size
+        type: u32
+      -
+        name: cell-overhead
+        type: s32
+  -
+    name: tc-netem-slot
+    type: struct
+    members:
+      -
+        name: min-delay
+        type: s64
+      -
+        name: max-delay
+        type: s64
+      -
+        name: max-packets
+        type: s32
+      -
+        name: max-bytes
+        type: s32
+      -
+        name: dist-delay
+        type: s64
+      -
+        name: dist-jitter
+        type: s64
   -
     name: tc-plug-qopt
     type: struct
@@ -307,11 +446,13 @@ definitions:
     members:
       -
         name: bands
-        type: u16
+        type: u32
+        doc: Number of bands
       -
         name: priomap
         type: binary
         len: 16
+        doc: Map of logical priority -> PRIO band
   -
     name: tc-red-qopt
     type: struct
@@ -319,21 +460,27 @@ definitions:
       -
         name: limit
         type: u32
+        doc: Hard queue length in packets
       -
         name: qth-min
         type: u32
+        doc: Min average threshold in packets
       -
         name: qth-max
         type: u32
+        doc: Max average threshold in packets
       -
         name: Wlog
         type: u8
+        doc: log(W)
       -
         name: Plog
         type: u8
+        doc: log(P_max / (qth-max - qth-min))
       -
         name: Scell-log
         type: u8
+        doc: Cell size for idle damping
       -
         name: flags
         type: u8
@@ -369,71 +516,128 @@ definitions:
         name: penalty-burst
         type: u32
   -
-    name: tc-sfq-qopt-v1 # TODO nested structs
+    name: tc-sfq-qopt
     type: struct
     members:
       -
         name: quantum
         type: u32
+        doc: Bytes per round allocated to flow
       -
         name: perturb-period
         type: s32
+        doc: Period of hash perturbation
       -
         name: limit
         type: u32
+        doc: Maximal packets in queue
       -
         name: divisor
         type: u32
+        doc: Hash divisor
       -
         name: flows
         type: u32
+        doc: Maximal number of flows
+  -
+    name: tc-sfqred-stats
+    type: struct
+    members:
+      -
+        name: prob-drop
+        type: u32
+        doc: Early drops, below max threshold
+      -
+        name: forced-drop
+        type: u32
+        doc: Early drops, after max threshold
+      -
+        name: prob-mark
+        type: u32
+        doc: Marked packets, below max threshold
+      -
+        name: forced-mark
+        type: u32
+        doc: Marked packets, after max threshold
+      -
+        name: prob-mark-head
+        type: u32
+        doc: Marked packets, below max threshold
+      -
+        name: forced-mark-head
+        type: u32
+        doc: Marked packets, after max threshold
+  -
+    name: tc-sfq-qopt-v1
+    type: struct
+    members:
+      -
+        name: v0
+        type: binary
+        struct: tc-sfq-qopt
       -
         name: depth
         type: u32
+        doc: Maximum number of packets per flow
       -
         name: headdrop
         type: u32
       -
         name: limit
         type: u32
+        doc: HARD maximal flow queue length in bytes
       -
         name: qth-min
         type: u32
+        doc: Min average length threshold in bytes
       -
-        name: qth-mac
+        name: qth-max
         type: u32
+        doc: Max average length threshold in bytes
       -
         name: Wlog
         type: u8
+        doc: log(W)
       -
         name: Plog
         type: u8
+        doc: log(P_max / (qth-max - qth-min))
       -
         name: Scell-log
         type: u8
+        doc: Cell size for idle damping
       -
         name: flags
         type: u8
       -
         name: max-P
         type: u32
+        doc: probabilty, high resolution
       -
-        name: prob-drop
-        type: u32
+        name: stats
+        type: binary
+        struct: tc-sfqred-stats
+  -
+    name: tc-ratespec
+    type: struct
+    members:
       -
-        name: forced-drop
-        type: u32
+        name: cell-log
+        type: u8
       -
-        name: prob-mark
-        type: u32
+        name: linklayer
+        type: u8
       -
-        name: forced-mark
-        type: u32
+        name: overhead
+        type: u8
       -
-        name: prob-mark-head
-        type: u32
+        name: cell-align
+        type: u8
       -
-        name: forced-mark-head
+        name: mpu
+        type: u8
+      -
+        name: rate
         type: u32
   -
     name: tc-tbf-qopt
@@ -441,12 +645,12 @@ definitions:
     members:
       -
         name: rate
-        type: binary # TODO nested struct tc_ratespec
-        len: 12
+        type: binary
+        struct: tc-ratespec
       -
         name: peakrate
-        type: binary # TODO nested struct tc_ratespec
-        len: 12
+        type: binary
+        struct: tc-ratespec
       -
         name: limit
         type: u32
@@ -491,67 +695,1299 @@ definitions:
       -
         name: interval
         type: s8
+        doc: Sampling period
       -
         name: ewma-log
         type: u8
-attribute-sets:
+        doc: The log() of measurement window weight
   -
-    name: tc-attrs
+    name: tc-choke-xstats
+    type: struct
+    members:
+      -
+        name: early
+        type: u32
+        doc: Early drops
+      -
+        name: pdrop
+        type: u32
+        doc: Drops due to queue limits
+      -
+        name: other
+        type: u32
+        doc: Drops due to drop() calls
+      -
+        name: marked
+        type: u32
+        doc: Marked packets
+      -
+        name: matched
+        type: u32
+        doc: Drops due to flow match
+  -
+    name: tc-codel-xstats
+    type: struct
+    members:
+      -
+        name: maxpacket
+        type: u32
+        doc: Largest packet we've seen so far
+      -
+        name: count
+        type: u32
+        doc: How many drops we've done since the last time we entered dropping state
+      -
+        name: lastcount
+        type: u32
+        doc: Count at entry to dropping state
+      -
+        name: ldelay
+        type: u32
+        doc: in-queue delay seen by most recently dequeued packet
+      -
+        name: drop-next
+        type: s32
+        doc: Time to drop next packet
+      -
+        name: drop-overlimit
+        type: u32
+        doc: Number of times max qdisc packet limit was hit
+      -
+        name: ecn-mark
+        type: u32
+        doc: Number of packets we've ECN marked instead of dropped
+      -
+        name: dropping
+        type: u32
+        doc: Are we in a dropping state?
+      -
+        name: ce-mark
+        type: u32
+        doc: Number of CE marked packets because of ce-threshold
+  -
+    name: tc-fq-codel-xstats
+    type: struct
+    members:
+      -
+        name: type
+        type: u32
+      -
+        name: maxpacket
+        type: u32
+        doc: Largest packet we've seen so far
+      -
+        name: drop-overlimit
+        type: u32
+        doc: Number of times max qdisc packet limit was hit
+      -
+        name: ecn-mark
+        type: u32
+        doc: Number of packets we ECN marked instead of being dropped
+      -
+        name: new-flow-count
+        type: u32
+        doc: Number of times packets created a new flow
+      -
+        name: new-flows-len
+        type: u32
+        doc: Count of flows in new list
+      -
+        name: old-flows-len
+        type: u32
+        doc: Count of flows in old list
+      -
+        name: ce-mark
+        type: u32
+        doc: Packets above ce-threshold
+      -
+        name: memory-usage
+        type: u32
+        doc: Memory usage in bytes
+      -
+        name: drop-overmemory
+        type: u32
+  -
+    name: tc-fq-pie-xstats
+    type: struct
+    members:
+      -
+        name: packets-in
+        type: u32
+        doc: Total number of packets enqueued
+      -
+        name: dropped
+        type: u32
+        doc: Packets dropped due to fq_pie_action
+      -
+        name: overlimit
+        type: u32
+        doc: Dropped due to lack of space in queue
+      -
+        name: overmemory
+        type: u32
+        doc: Dropped due to lack of memory in queue
+      -
+        name: ecn-mark
+        type: u32
+        doc: Packets marked with ecn
+      -
+        name: new-flow-count
+        type: u32
+        doc: Count of new flows created by packets
+      -
+        name: new-flows-len
+        type: u32
+        doc: Count of flows in new list
+      -
+        name: old-flows-len
+        type: u32
+        doc: Count of flows in old list
+      -
+        name: memory-usage
+        type: u32
+        doc: Total memory across all queues
+  -
+    name: tc-fq-qd-stats
+    type: struct
+    members:
+      -
+        name: gc-flows
+        type: u64
+      -
+        name: highprio-packets
+        type: u64
+        doc: obsolete
+      -
+        name: tcp-retrans
+        type: u64
+        doc: obsolete
+      -
+        name: throttled
+        type: u64
+      -
+        name: flows-plimit
+        type: u64
+      -
+        name: pkts-too-long
+        type: u64
+      -
+        name: allocation-errors
+        type: u64
+      -
+        name: time-next-delayed-flow
+        type: s64
+      -
+        name: flows
+        type: u32
+      -
+        name: inactive-flows
+        type: u32
+      -
+        name: throttled-flows
+        type: u32
+      -
+        name: unthrottle-latency-ns
+        type: u32
+      -
+        name: ce-mark
+        type: u64
+        doc: Packets above ce-threshold
+      -
+        name: horizon-drops
+        type: u64
+      -
+        name: horizon-caps
+        type: u64
+      -
+        name: fastpath-packets
+        type: u64
+      -
+        name: band-drops
+        type: binary
+        len: 24
+      -
+        name: band-pkt-count
+        type: binary
+        len: 12
+      -
+        name: pad
+        type: pad
+        len: 4
+  -
+    name: tc-hhf-xstats
+    type: struct
+    members:
+      -
+        name: drop-overlimit
+        type: u32
+        doc: Number of times max qdisc packet limit was hit
+      -
+        name: hh-overlimit
+        type: u32
+        doc: Number of times max heavy-hitters was hit
+      -
+        name: hh-tot-count
+        type: u32
+        doc: Number of captured heavy-hitters so far
+      -
+        name: hh-cur-count
+        type: u32
+        doc: Number of current heavy-hitters
+  -
+    name: tc-pie-xstats
+    type: struct
+    members:
+      -
+        name: prob
+        type: u64
+        doc: Current probability
+      -
+        name: delay
+        type: u32
+        doc: Current delay in ms
+      -
+        name: avg-dq-rate
+        type: u32
+        doc: Current average dq rate in bits/pie-time
+      -
+        name: dq-rate-estimating
+        type: u32
+        doc: Is avg-dq-rate being calculated?
+      -
+        name: packets-in
+        type: u32
+        doc: Total number of packets enqueued
+      -
+        name: dropped
+        type: u32
+        doc: Packets dropped due to pie action
+      -
+        name: overlimit
+        type: u32
+        doc: Dropped due to lack of space in queue
+      -
+        name: maxq
+        type: u32
+        doc: Maximum queue size
+      -
+        name: ecn-mark
+        type: u32
+        doc: Packets marked with ecn
+  -
+    name: tc-red-xstats
+    type: struct
+    members:
+      -
+        name: early
+        type: u32
+        doc: Early drops
+      -
+        name: pdrop
+        type: u32
+        doc: Drops due to queue limits
+      -
+        name: other
+        type: u32
+        doc: Drops due to drop() calls
+      -
+        name: marked
+        type: u32
+        doc: Marked packets
+  -
+    name: tc-sfb-xstats
+    type: struct
+    members:
+      -
+        name: earlydrop
+        type: u32
+      -
+        name: penaltydrop
+        type: u32
+      -
+        name: bucketdrop
+        type: u32
+      -
+        name: queuedrop
+        type: u32
+      -
+        name: childdrop
+        type: u32
+        doc: drops in child qdisc
+      -
+        name: marked
+        type: u32
+      -
+        name: maxqlen
+        type: u32
+      -
+        name: maxprob
+        type: u32
+      -
+        name: avgprob
+        type: u32
+  -
+    name: tc-sfq-xstats
+    type: struct
+    members:
+      -
+        name: allot
+        type: s32
+  -
+    name: gnet-stats-basic
+    type: struct
+    members:
+      -
+        name: bytes
+        type: u64
+      -
+        name: packets
+        type: u32
+  -
+    name: gnet-stats-rate-est
+    type: struct
+    members:
+      -
+        name: bps
+        type: u32
+      -
+        name: pps
+        type: u32
+  -
+    name: gnet-stats-rate-est64
+    type: struct
+    members:
+      -
+        name: bps
+        type: u64
+      -
+        name: pps
+        type: u64
+  -
+    name: gnet-stats-queue
+    type: struct
+    members:
+      -
+        name: qlen
+        type: u32
+      -
+        name: backlog
+        type: u32
+      -
+        name: drops
+        type: u32
+      -
+        name: requeues
+        type: u32
+      -
+        name: overlimits
+        type: u32
+  -
+    name: tc-u32-key
+    type: struct
+    members:
+      -
+        name: mask
+        type: u32
+        byte-order: big-endian
+      -
+        name: val
+        type: u32
+        byte-order: big-endian
+      -
+        name: "off"
+        type: s32
+      -
+        name: offmask
+        type: s32
+  -
+    name: tc-u32-sel
+    type: struct
+    members:
+      -
+        name: flags
+        type: u8
+      -
+        name: offshift
+        type: u8
+      -
+        name: nkeys
+        type: u8
+      -
+        name: offmask
+        type: u16
+        byte-order: big-endian
+      -
+        name: "off"
+        type: u16
+      -
+        name: offoff
+        type: s16
+      -
+        name: hoff
+        type: s16
+      -
+        name: hmask
+        type: u32
+        byte-order: big-endian
+      -
+        name: keys
+        type: binary
+        struct: tc-u32-key # TODO: array
+  -
+    name: tc-u32-pcnt
+    type: struct
+    members:
+      -
+        name: rcnt
+        type: u64
+      -
+        name: rhit
+        type: u64
+      -
+        name: kcnts
+        type: u64 # TODO: array
+  -
+    name: tcf-t
+    type: struct
+    members:
+      -
+        name: install
+        type: u64
+      -
+        name: lastuse
+        type: u64
+      -
+        name: expires
+        type: u64
+      -
+        name: firstuse
+        type: u64
+  -
+    name: tc-gen
+    type: struct
+    members:
+      -
+        name: index
+        type: u32
+      -
+        name: capab
+        type: u32
+      -
+        name: action
+        type: s32
+      -
+        name: refcnt
+        type: s32
+      -
+        name: bindcnt
+        type: s32
+  -
+    name: tc-gact-p
+    type: struct
+    members:
+      -
+        name: ptype
+        type: u16
+      -
+        name: pval
+        type: u16
+      -
+        name: paction
+        type: s32
+  -
+    name: tcf-ematch-tree-hdr
+    type: struct
+    members:
+      -
+        name: nmatches
+        type: u16
+      -
+        name: progid
+        type: u16
+  -
+    name: tc-basic-pcnt
+    type: struct
+    members:
+      -
+        name: rcnt
+        type: u64
+      -
+        name: rhit
+        type: u64
+  -
+    name: tc-matchall-pcnt
+    type: struct
+    members:
+      -
+        name: rhit
+        type: u64
+  -
+    name: tc-mpls
+    type: struct
+    members:
+      -
+        name: index
+        type: u32
+      -
+        name: capab
+        type: u32
+      -
+        name: action
+        type: s32
+      -
+        name: refcnt
+        type: s32
+      -
+        name: bindcnt
+        type: s32
+      -
+        name: m-action
+        type: s32
+  -
+    name: tc-police
+    type: struct
+    members:
+      -
+        name: index
+        type: u32
+      -
+        name: action
+        type: s32
+      -
+        name: limit
+        type: u32
+      -
+        name: burst
+        type: u32
+      -
+        name: mtu
+        type: u32
+      -
+        name: rate
+        type: binary
+        struct: tc-ratespec
+      -
+        name: peakrate
+        type: binary
+        struct: tc-ratespec
+      -
+        name: refcnt
+        type: s32
+      -
+        name: bindcnt
+        type: s32
+      -
+        name: capab
+        type: u32
+  -
+    name: tc-pedit-sel
+    type: struct
+    members:
+      -
+        name: index
+        type: u32
+      -
+        name: capab
+        type: u32
+      -
+        name: action
+        type: s32
+      -
+        name: refcnt
+        type: s32
+      -
+        name: bindcnt
+        type: s32
+      -
+        name: nkeys
+        type: u8
+      -
+        name: flags
+        type: u8
+      -
+        name: keys
+        type: binary
+        struct: tc-pedit-key # TODO: array
+  -
+    name: tc-pedit-key
+    type: struct
+    members:
+      -
+        name: mask
+        type: u32
+      -
+        name: val
+        type: u32
+      -
+        name: "off"
+        type: u32
+      -
+        name: at
+        type: u32
+      -
+        name: offmask
+        type: u32
+      -
+        name: shift
+        type: u32
+  -
+    name: tc-vlan
+    type: struct
+    members:
+      -
+        name: index
+        type: u32
+      -
+        name: capab
+        type: u32
+      -
+        name: action
+        type: s32
+      -
+        name: refcnt
+        type: s32
+      -
+        name: bindcnt
+        type: s32
+      -
+        name: v-action
+        type: s32
+attribute-sets:
+  -
+    name: tc-attrs
+    attributes:
+      -
+        name: kind
+        type: string
+      -
+        name: options
+        type: sub-message
+        sub-message: tc-options-msg
+        selector: kind
+      -
+        name: stats
+        type: binary
+        struct: tc-stats
+      -
+        name: xstats
+        type: sub-message
+        sub-message: tca-stats-app-msg
+        selector: kind
+      -
+        name: rate
+        type: binary
+        struct: gnet-estimator
+      -
+        name: fcnt
+        type: u32
+      -
+        name: stats2
+        type: nest
+        nested-attributes: tca-stats-attrs
+      -
+        name: stab
+        type: nest
+        nested-attributes: tca-stab-attrs
+      -
+        name: pad
+        type: pad
+      -
+        name: dump-invisible
+        type: flag
+      -
+        name: chain
+        type: u32
+      -
+        name: hw-offload
+        type: u8
+      -
+        name: ingress-block
+        type: u32
+      -
+        name: egress-block
+        type: u32
+      -
+        name: dump-flags
+        type: bitfield32
+      -
+        name: ext-warn-msg
+        type: string
+  -
+    name: tc-act-attrs
+    attributes:
+      -
+        name: kind
+        type: string
+      -
+        name: options
+        type: sub-message
+        sub-message: tc-act-options-msg
+        selector: kind
+      -
+        name: index
+        type: u32
+      -
+        name: stats
+        type: nest
+        nested-attributes: tc-act-stats-attrs
+      -
+        name: pad
+        type: pad
+      -
+        name: cookie
+        type: binary
+      -
+        name: flags
+        type: bitfield32
+      -
+        name: hw-stats
+        type: bitfield32
+      -
+        name: used-hw-stats
+        type: bitfield32
+      -
+        name: in-hw-count
+        type: u32
+  -
+    name: tc-act-stats-attrs
+    attributes:
+      -
+        name: basic
+        type: binary
+        struct: gnet-stats-basic
+      -
+        name: rate-est
+        type: binary
+        struct: gnet-stats-rate-est
+      -
+        name: queue
+        type: binary
+        struct: gnet-stats-queue
+      -
+        name: app
+        type: binary
+      -
+        name: rate-est64
+        type: binary
+        struct: gnet-stats-rate-est64
+      -
+        name: pad
+        type: pad
+      -
+        name: basic-hw
+        type: binary
+        struct: gnet-stats-basic
+      -
+        name: pkt64
+        type: u64
+  -
+    name: tc-act-bpf-attrs
+    attributes:
+      -
+        name: tm
+        type: binary
+        struct: tcf-t
+      -
+        name: parms
+        type: binary
+      -
+        name: ops-len
+        type: u16
+      -
+        name: ops
+        type: binary
+      -
+        name: fd
+        type: u32
+      -
+        name: name
+        type: string
+      -
+        name: pad
+        type: pad
+      -
+        name: tag
+        type: binary
+      -
+        name: id
+        type: binary
+  -
+    name: tc-act-connmark-attrs
+    attributes:
+      -
+        name: parms
+        type: binary
+      -
+        name: tm
+        type: binary
+        struct: tcf-t
+      -
+        name: pad
+        type: pad
+  -
+    name: tc-act-csum-attrs
+    attributes:
+      -
+        name: parms
+        type: binary
+      -
+        name: tm
+        type: binary
+        struct: tcf-t
+      -
+        name: pad
+        type: pad
+  -
+    name: tc-act-ct-attrs
+    attributes:
+      -
+        name: parms
+        type: binary
+      -
+        name: tm
+        type: binary
+        struct: tcf-t
+      -
+        name: action
+        type: u16
+      -
+        name: zone
+        type: u16
+      -
+        name: mark
+        type: u32
+      -
+        name: mark-mask
+        type: u32
+      -
+        name: labels
+        type: binary
+      -
+        name: labels-mask
+        type: binary
+      -
+        name: nat-ipv4-min
+        type: u32
+        byte-order: big-endian
+      -
+        name: nat-ipv4-max
+        type: u32
+        byte-order: big-endian
+      -
+        name: nat-ipv6-min
+        type: binary
+      -
+        name: nat-ipv6-max
+        type: binary
+      -
+        name: nat-port-min
+        type: u16
+        byte-order: big-endian
+      -
+        name: nat-port-max
+        type: u16
+        byte-order: big-endian
+      -
+        name: pad
+        type: pad
+      -
+        name: helper-name
+        type: string
+      -
+        name: helper-family
+        type: u8
+      -
+        name: helper-proto
+        type: u8
+  -
+    name: tc-act-ctinfo-attrs
+    attributes:
+      -
+        name: pad
+        type: pad
+      -
+        name: tm
+        type: binary
+        struct: tcf-t
+      -
+        name: act
+        type: binary
+      -
+        name: zone
+        type: u16
+      -
+        name: parms-dscp-mask
+        type: u32
+      -
+        name: parms-dscp-statemask
+        type: u32
+      -
+        name: parms-cpmark-mask
+        type: u32
+      -
+        name: stats-dscp-set
+        type: u64
+      -
+        name: stats-dscp-error
+        type: u64
+      -
+        name: stats-cpmark-set
+        type: u64
+  -
+    name: tc-act-gate-attrs
+    attributes:
+      -
+        name: tm
+        type: binary
+        struct: tcf-t
+      -
+        name: parms
+        type: binary
+      -
+        name: pad
+        type: pad
+      -
+        name: priority
+        type: s32
+      -
+        name: entry-list
+        type: binary
+      -
+        name: base-time
+        type: u64
+      -
+        name: cycle-time
+        type: u64
+      -
+        name: cycle-time-ext
+        type: u64
+      -
+        name: flags
+        type: u32
+      -
+        name: clockid
+        type: s32
+  -
+    name: tc-act-ife-attrs
+    attributes:
+      -
+        name: parms
+        type: binary
+      -
+        name: tm
+        type: binary
+        struct: tcf-t
+      -
+        name: dmac
+        type: binary
+      -
+        name: smac
+        type: binary
+      -
+        name: type
+        type: u16
+      -
+        name: metalst
+        type: binary
+      -
+        name: pad
+        type: pad
+  -
+    name: tc-act-mirred-attrs
+    attributes:
+      -
+        name: tm
+        type: binary
+        struct: tcf-t
+      -
+        name: parms
+        type: binary
+      -
+        name: pad
+        type: pad
+      -
+        name: blockid
+        type: binary
+  -
+    name: tc-act-mpls-attrs
+    attributes:
+      -
+        name: tm
+        type: binary
+        struct: tcf-t
+      -
+        name: parms
+        type: binary
+        struct: tc-mpls
+      -
+        name: pad
+        type: pad
+      -
+        name: proto
+        type: u16
+        byte-order: big-endian
+      -
+        name: label
+        type: u32
+      -
+        name: tc
+        type: u8
+      -
+        name: ttl
+        type: u8
+      -
+        name: bos
+        type: u8
+  -
+    name: tc-act-nat-attrs
+    attributes:
+      -
+        name: parms
+        type: binary
+      -
+        name: tm
+        type: binary
+        struct: tcf-t
+      -
+        name: pad
+        type: pad
+  -
+    name: tc-act-pedit-attrs
+    attributes:
+      -
+        name: tm
+        type: binary
+        struct: tcf-t
+      -
+        name: parms
+        type: binary
+        struct: tc-pedit-sel
+      -
+        name: pad
+        type: pad
+      -
+        name: parms-ex
+        type: binary
+      -
+        name: keys-ex
+        type: binary
+      -
+        name: key-ex
+        type: binary
+  -
+    name: tc-act-simple-attrs
+    attributes:
+      -
+        name: tm
+        type: binary
+        struct: tcf-t
+      -
+        name: parms
+        type: binary
+      -
+        name: data
+        type: binary
+      -
+        name: pad
+        type: pad
+  -
+    name: tc-act-skbedit-attrs
+    attributes:
+      -
+        name: tm
+        type: binary
+        struct: tcf-t
+      -
+        name: parms
+        type: binary
+      -
+        name: priority
+        type: u32
+      -
+        name: queue-mapping
+        type: u16
+      -
+        name: mark
+        type: u32
+      -
+        name: pad
+        type: pad
+      -
+        name: ptype
+        type: u16
+      -
+        name: mask
+        type: u32
+      -
+        name: flags
+        type: u64
+      -
+        name: queue-mapping-max
+        type: u16
+  -
+    name: tc-act-skbmod-attrs
+    attributes:
+      -
+        name: tm
+        type: binary
+        struct: tcf-t
+      -
+        name: parms
+        type: binary
+      -
+        name: dmac
+        type: binary
+      -
+        name: smac
+        type: binary
+      -
+        name: etype
+        type: binary
+      -
+        name: pad
+        type: pad
+  -
+    name: tc-act-tunnel-key-attrs
+    attributes:
+      -
+        name: tm
+        type: binary
+        struct: tcf-t
+      -
+        name: parms
+        type: binary
+      -
+        name: enc-ipv4-src
+        type: u32
+        byte-order: big-endian
+      -
+        name: enc-ipv4-dst
+        type: u32
+        byte-order: big-endian
+      -
+        name: enc-ipv6-src
+        type: binary
+      -
+        name: enc-ipv6-dst
+        type: binary
+      -
+        name: enc-key-id
+        type: u64
+        byte-order: big-endian
+      -
+        name: pad
+        type: pad
+      -
+        name: enc-dst-port
+        type: u16
+        byte-order: big-endian
+      -
+        name: no-csum
+        type: u8
+      -
+        name: enc-opts
+        type: binary
+      -
+        name: enc-tos
+        type: u8
+      -
+        name: enc-ttl
+        type: u8
+      -
+        name: no-frag
+        type: flag
+  -
+    name: tc-act-vlan-attrs
     attributes:
       -
-        name: kind
-        type: string
-      -
-        name: options
-        type: sub-message
-        sub-message: tc-options-msg
-        selector: kind
+        name: tm
+        type: binary
+        struct: tcf-t
       -
-        name: stats
+        name: parms
         type: binary
-        struct: tc-stats
+        struct: tc-vlan
       -
-        name: xstats
+        name: push-vlan-id
+        type: u16
+      -
+        name: push-vlan-protocol
+        type: u16
+      -
+        name: pad
+        type: pad
+      -
+        name: push-vlan-priority
+        type: u8
+      -
+        name: push-eth-dst
         type: binary
       -
-        name: rate
+        name: push-eth-src
         type: binary
-        struct: gnet-estimator
+  -
+    name: tc-basic-attrs
+    attributes:
       -
-        name: fcnt
+        name: classid
         type: u32
       -
-        name: stats2
+        name: ematches
         type: nest
-        nested-attributes: tca-stats-attrs
+        nested-attributes: tc-ematch-attrs
       -
-        name: stab
+        name: act
+        type: array-nest
+        nested-attributes: tc-act-attrs
+      -
+        name: police
         type: nest
-        nested-attributes: tca-stab-attrs
+        nested-attributes: tc-police-attrs
+      -
+        name: pcnt
+        type: binary
+        struct: tc-basic-pcnt
       -
         name: pad
         type: pad
+  -
+    name: tc-bpf-attrs
+    attributes:
       -
-        name: dump-invisible
-        type: flag
+        name: act
+        type: nest
+        nested-attributes: tc-act-attrs
       -
-        name: chain
+        name: police
+        type: nest
+        nested-attributes: tc-police-attrs
+      -
+        name: classid
         type: u32
       -
-        name: hw-offload
-        type: u8
+        name: ops-len
+        type: u16
       -
-        name: ingress-block
+        name: ops
+        type: binary
+      -
+        name: fd
         type: u32
       -
-        name: egress-block
+        name: name
+        type: string
+      -
+        name: flags
         type: u32
       -
-        name: dump-flags
-        type: bitfield32
+        name: flags-gen
+        type: u32
       -
-        name: ext-warn-msg
-        type: string
+        name: tag
+        type: binary
+      -
+        name: id
+        type: u32
   -
     name: tc-cake-attrs
     attributes:
@@ -641,7 +2077,8 @@ attribute-sets:
         type: u32
       -
         name: tin-stats
-        type: binary
+        type: array-nest
+        nested-attributes: tc-cake-tin-stats-attrs
       -
         name: deficit
         type: s32
@@ -660,6 +2097,84 @@ attribute-sets:
       -
         name: blue-timer-us
         type: s32
+  -
+    name: tc-cake-tin-stats-attrs
+    attributes:
+      -
+        name: pad
+        type: pad
+      -
+        name: sent-packets
+        type: u32
+      -
+        name: sent-bytes64
+        type: u64
+      -
+        name: dropped-packets
+        type: u32
+      -
+        name: dropped-bytes64
+        type: u64
+      -
+        name: acks-dropped-packets
+        type: u32
+      -
+        name: acks-dropped-bytes64
+        type: u64
+      -
+        name: ecn-marked-packets
+        type: u32
+      -
+        name: ecn-marked-bytes64
+        type: u64
+      -
+        name: backlog-packets
+        type: u32
+      -
+        name: backlog-bytes
+        type: u32
+      -
+        name: threshold-rate64
+        type: u64
+      -
+        name: target-us
+        type: u32
+      -
+        name: interval-us
+        type: u32
+      -
+        name: way-indirect-hits
+        type: u32
+      -
+        name: way-misses
+        type: u32
+      -
+        name: way-collisions
+        type: u32
+      -
+        name: peak-delay-us
+        type: u32
+      -
+        name: avg-delay-us
+        type: u32
+      -
+        name: base-delay-us
+        type: u32
+      -
+        name: sparse-flows
+        type: u32
+      -
+        name: bulk-flows
+        type: u32
+      -
+        name: unresponsive-flows
+        type: u32
+      -
+        name: max-skblen
+        type: u32
+      -
+        name: flow-quantum
+        type: u32
   -
     name: tc-cbs-attrs
     attributes:
@@ -667,6 +2182,20 @@ attribute-sets:
         name: parms
         type: binary
         struct: tc-cbs-qopt
+  -
+    name: tc-cgroup-attrs
+    attributes:
+      -
+        name: act
+        type: nest
+        nested-attributes: tc-act-attrs
+      -
+        name: police
+        type: nest
+        nested-attributes: tc-police-attrs
+      -
+        name: ematches
+        type: binary
   -
     name: tc-choke-attrs
     attributes:
@@ -677,6 +2206,9 @@ attribute-sets:
       -
         name: stab
         type: binary
+        checks:
+          min-len: 256
+          max-len: 256
       -
         name: max-p
         type: u32
@@ -704,6 +2236,56 @@ attribute-sets:
       -
         name: quantum
         type: u32
+  -
+    name: tc-ematch-attrs
+    attributes:
+      -
+        name: tree-hdr
+        type: binary
+        struct: tcf-ematch-tree-hdr
+      -
+        name: tree-list
+        type: binary
+  -
+    name: tc-flow-attrs
+    attributes:
+      -
+        name: keys
+        type: u32
+      -
+        name: mode
+        type: u32
+      -
+        name: baseclass
+        type: u32
+      -
+        name: rshift
+        type: u32
+      -
+        name: addend
+        type: u32
+      -
+        name: mask
+        type: u32
+      -
+        name: xor
+        type: u32
+      -
+        name: divisor
+        type: u32
+      -
+        name: act
+        type: binary
+      -
+        name: police
+        type: nest
+        nested-attributes: tc-police-attrs
+      -
+        name: ematches
+        type: binary
+      -
+        name: perturb
+        type: u32
   -
     name: tc-flower-attrs
     attributes:
@@ -953,15 +2535,19 @@ attribute-sets:
       -
         name: key-arp-sha
         type: binary
+        display-hint: mac
       -
         name: key-arp-sha-mask
         type: binary
+        display-hint: mac
       -
         name: key-arp-tha
         type: binary
+        display-hint: mac
       -
         name: key-arp-tha-mask
         type: binary
+        display-hint: mac
       -
         name: key-mpls-ttl
         type: u8
@@ -1020,10 +2606,12 @@ attribute-sets:
         type: u8
       -
         name: key-enc-opts
-        type: binary
+        type: nest
+        nested-attributes: tc-flower-key-enc-opts-attrs
       -
         name: key-enc-opts-mask
-        type: binary
+        type: nest
+        nested-attributes: tc-flower-key-enc-opts-attrs
       -
         name: in-hw-count
         type: u32
@@ -1056,41 +2644,165 @@ attribute-sets:
         name: key-ct-zone-mask
         type: u16
       -
-        name: key-ct-mark
-        type: u32
+        name: key-ct-mark
+        type: u32
+      -
+        name: key-ct-mark-mask
+        type: u32
+      -
+        name: key-ct-labels
+        type: binary
+      -
+        name: key-ct-labels-mask
+        type: binary
+      -
+        name: key-mpls-opts
+        type: nest
+        nested-attributes: tc-flower-key-mpls-opt-attrs
+      -
+        name: key-hash
+        type: u32
+      -
+        name: key-hash-mask
+        type: u32
+      -
+        name: key-num-of-vlans
+        type: u8
+      -
+        name: key-pppoe-sid
+        type: u16
+        byte-order: big-endian
+      -
+        name: key-ppp-proto
+        type: u16
+        byte-order: big-endian
+      -
+        name: key-l2-tpv3-sid
+        type: u32
+        byte-order: big-endian
+      -
+        name: l2-miss
+        type: u8
+      -
+        name: key-cfm
+        type: nest
+        nested-attributes: tc-flower-key-cfm-attrs
+      -
+        name: key-spi
+        type: u32
+        byte-order: big-endian
+      -
+        name: key-spi-mask
+        type: u32
+        byte-order: big-endian
+  -
+    name: tc-flower-key-enc-opts-attrs
+    attributes:
+      -
+        name: geneve
+        type: nest
+        nested-attributes: tc-flower-key-enc-opt-geneve-attrs
+      -
+        name: vxlan
+        type: nest
+        nested-attributes: tc-flower-key-enc-opt-vxlan-attrs
+      -
+        name: erspan
+        type: nest
+        nested-attributes: tc-flower-key-enc-opt-erspan-attrs
+      -
+        name: gtp
+        type: nest
+        nested-attributes: tc-flower-key-enc-opt-gtp-attrs
+  -
+    name: tc-flower-key-enc-opt-geneve-attrs
+    attributes:
+      -
+        name: class
+        type: u16
+      -
+        name: type
+        type: u8
+      -
+        name: data
+        type: binary
+  -
+    name: tc-flower-key-enc-opt-vxlan-attrs
+    attributes:
+      -
+        name: gbp
+        type: u32
+  -
+    name: tc-flower-key-enc-opt-erspan-attrs
+    attributes:
+      -
+        name: ver
+        type: u8
+      -
+        name: index
+        type: u32
+      -
+        name: dir
+        type: u8
+      -
+        name: hwid
+        type: u8
+  -
+    name: tc-flower-key-enc-opt-gtp-attrs
+    attributes:
+      -
+        name: pdu-type
+        type: u8
       -
-        name: key-ct-mark-mask
-        type: u32
+        name: qfi
+        type: u8
+  -
+    name: tc-flower-key-mpls-opt-attrs
+    attributes:
       -
-        name: key-ct-labels
-        type: binary
+        name: lse-depth
+        type: u8
       -
-        name: key-ct-labels-mask
-        type: binary
+        name: lse-ttl
+        type: u8
       -
-        name: key-mpls-opts
-        type: binary
+        name: lse-bos
+        type: u8
       -
-        name: key-hash
-        type: u32
+        name: lse-tc
+        type: u8
       -
-        name: key-hash-mask
+        name: lse-label
         type: u32
+  -
+    name: tc-flower-key-cfm-attrs
+    attributes:
       -
-        name: key-num-of-vlans
+        name: md-level
         type: u8
       -
-        name: key-pppoe-sid
-        type: u16
-        byte-order: big-endian
+        name: opcode
+        type: u8
+  -
+    name: tc-fw-attrs
+    attributes:
       -
-        name: key-ppp-proto
-        type: u16
-        byte-order: big-endian
+        name: classid
+        type: u32
       -
-        name: key-l2-tpv3-sid
+        name: police
+        type: nest
+        nested-attributes: tc-police-attrs
+      -
+        name: indev
+        type: string
+      -
+        name: act
+        type: array-nest
+        nested-attributes: tc-act-attrs
+      -
+        name: mask
         type: u32
-        byte-order: big-endian
   -
     name: tc-gred-attrs
     attributes:
@@ -1135,7 +2847,7 @@ attribute-sets:
         type: u32
       -
         name: stat-bytes
-        type: u32
+        type: u64
       -
         name: stat-packets
         type: u32
@@ -1232,40 +2944,25 @@ attribute-sets:
         name: offload
         type: flag
   -
-    name: tc-act-attrs
+    name: tc-matchall-attrs
     attributes:
       -
-        name: kind
-        type: string
+        name: classid
+        type: u32
       -
-        name: options
-        type: sub-message
-        sub-message: tc-act-options-msg
-        selector: kind
+        name: act
+        type: array-nest
+        nested-attributes: tc-act-attrs
       -
-        name: index
+        name: flags
         type: u32
       -
-        name: stats
+        name: pcnt
         type: binary
+        struct: tc-matchall-pcnt
       -
         name: pad
         type: pad
-      -
-        name: cookie
-        type: binary
-      -
-        name: flags
-        type: bitfield32
-      -
-        name: hw-stats
-        type: bitfield32
-      -
-        name: used-hw-stats
-        type: bitfield32
-      -
-        name: in-hw-count
-        type: u32
   -
     name: tc-etf-attrs
     attributes:
@@ -1304,48 +3001,71 @@ attribute-sets:
       -
         name: plimit
         type: u32
+        doc: Limit of total number of packets in queue
       -
         name: flow-plimit
         type: u32
+        doc: Limit of packets per flow
       -
         name: quantum
         type: u32
+        doc: RR quantum
       -
         name: initial-quantum
         type: u32
+        doc: RR quantum for new flow
       -
         name: rate-enable
         type: u32
+        doc: Enable / disable rate limiting
       -
         name: flow-default-rate
         type: u32
+        doc: Obsolete, do not use
       -
         name: flow-max-rate
         type: u32
+        doc: Per flow max rate
       -
         name: buckets-log
         type: u32
+        doc: log2(number of buckets)
       -
         name: flow-refill-delay
         type: u32
+        doc: Flow credit refill delay in usec
       -
         name: orphan-mask
         type: u32
+        doc: Mask applied to orphaned skb hashes
       -
         name: low-rate-threshold
         type: u32
+        doc: Per packet delay under this rate
       -
         name: ce-threshold
         type: u32
+        doc: DCTCP-like CE marking threshold
       -
         name: timer-slack
         type: u32
       -
         name: horizon
         type: u32
+        doc: Time horizon in usec
       -
         name: horizon-drop
         type: u8
+        doc: Drop packets beyond horizon, or cap their EDT
+      -
+        name: priomap
+        type: binary
+        struct: tc-prio-qopt
+      -
+        name: weights
+        type: binary
+        sub-type: s32
+        doc: Weights for each band
   -
     name: tc-fq-codel-attrs
     attributes:
@@ -1427,6 +3147,7 @@ attribute-sets:
       -
         name: corr
         type: binary
+        struct: tc-netem-corr
       -
         name: delay-dist
         type: binary
@@ -1434,15 +3155,19 @@ attribute-sets:
       -
         name: reorder
         type: binary
+        struct: tc-netem-reorder
       -
         name: corrupt
         type: binary
+        struct: tc-netem-corrupt
       -
         name: loss
-        type: binary
+        type: nest
+        nested-attributes: tc-netem-loss-attrs
       -
         name: rate
         type: binary
+        struct: tc-netem-rate
       -
         name: ecn
         type: u32
@@ -1461,10 +3186,27 @@ attribute-sets:
       -
         name: slot
         type: binary
+        struct: tc-netem-slot
       -
         name: slot-dist
         type: binary
         sub-type: s16
+      -
+        name: prng-seed
+        type: u64
+  -
+    name: tc-netem-loss-attrs
+    attributes:
+      -
+        name: gi
+        type: binary
+        doc: General Intuitive - 4 state model
+        struct: tc-netem-gimodel
+      -
+        name: ge
+        type: binary
+        doc: Gilbert Elliot models
+        struct: tc-netem-gemodel
   -
     name: tc-pie-attrs
     attributes:
@@ -1492,6 +3234,44 @@ attribute-sets:
       -
         name: dq-rate-estimator
         type: u32
+  -
+    name: tc-police-attrs
+    attributes:
+      -
+        name: tbf
+        type: binary
+        struct: tc-police
+      -
+        name: rate
+        type: binary
+      -
+        name: peakrate
+        type: binary
+      -
+        name: avrate
+        type: u32
+      -
+        name: result
+        type: u32
+      -
+        name: tm
+        type: binary
+        struct: tcf-t
+      -
+        name: pad
+        type: pad
+      -
+        name: rate64
+        type: u64
+      -
+        name: peakrate64
+        type: u64
+      -
+        name: pktrate64
+        type: u64
+      -
+        name: pktburst64
+        type: u64
   -
     name: tc-qfq-attrs
     attributes:
@@ -1516,13 +3296,36 @@ attribute-sets:
         type: u32
       -
         name: flags
-        type: binary
+        type: bitfield32
       -
         name: early-drop-block
         type: u32
       -
         name: mark-block
         type: u32
+  -
+    name: tc-route-attrs
+    attributes:
+      -
+        name: classid
+        type: u32
+      -
+        name: to
+        type: u32
+      -
+        name: from
+        type: u32
+      -
+        name: iif
+        type: u32
+      -
+        name: police
+        type: nest
+        nested-attributes: tc-police-attrs
+      -
+        name: act
+        type: array-nest
+        nested-attributes: tc-act-attrs
   -
     name: tc-taprio-attrs
     attributes:
@@ -1629,17 +3432,43 @@ attribute-sets:
         name: pad
         type: pad
   -
-    name: tca-gact-attrs
+    name: tc-act-sample-attrs
+    attributes:
+      -
+        name: tm
+        type: binary
+        struct: tcf-t
+      -
+        name: parms
+        type: binary
+        struct: tc-gen
+      -
+        name: rate
+        type: u32
+      -
+        name: trunc-size
+        type: u32
+      -
+        name: psample-group
+        type: u32
+      -
+        name: pad
+        type: pad
+  -
+    name: tc-act-gact-attrs
     attributes:
       -
         name: tm
         type: binary
+        struct: tcf-t
       -
         name: parms
         type: binary
+        struct: tc-gen
       -
         name: prob
         type: binary
+        struct: tc-gact-p
       -
         name: pad
         type: pad
@@ -1659,34 +3488,89 @@ attribute-sets:
       -
         name: basic
         type: binary
+        struct: gnet-stats-basic
       -
         name: rate-est
         type: binary
+        struct: gnet-stats-rate-est
       -
         name: queue
         type: binary
+        struct: gnet-stats-queue
       -
         name: app
-        type: binary # TODO sub-message needs 2+ level deep lookup
+        type: sub-message
         sub-message: tca-stats-app-msg
         selector: kind
       -
         name: rate-est64
         type: binary
+        struct: gnet-stats-rate-est64
       -
         name: pad
         type: pad
       -
         name: basic-hw
         type: binary
+        struct: gnet-stats-basic
       -
         name: pkt64
+        type: u64
+  -
+    name: tc-u32-attrs
+    attributes:
+      -
+        name: classid
+        type: u32
+      -
+        name: hash
+        type: u32
+      -
+        name: link
+        type: u32
+      -
+        name: divisor
+        type: u32
+      -
+        name: sel
+        type: binary
+        struct: tc-u32-sel
+      -
+        name: police
+        type: nest
+        nested-attributes: tc-police-attrs
+      -
+        name: act
+        type: array-nest
+        nested-attributes: tc-act-attrs
+      -
+        name: indev
+        type: string
+      -
+        name: pcnt
+        type: binary
+        struct: tc-u32-pcnt
+      -
+        name: mark
         type: binary
+        struct: tc-u32-mark
+      -
+        name: flags
+        type: u32
+      -
+        name: pad
+        type: pad
 
 sub-messages:
   -
     name: tc-options-msg
     formats:
+      -
+        value: basic
+        attribute-set: tc-basic-attrs
+      -
+        value: bpf
+        attribute-set: tc-bpf-attrs
       -
         value: bfifo
         fixed-header: tc-fifo-qopt
@@ -1696,6 +3580,9 @@ sub-messages:
       -
         value: cbs
         attribute-set: tc-cbs-attrs
+      -
+        value: cgroup
+        attribute-set: tc-cgroup-attrs
       -
         value: choke
         attribute-set: tc-choke-attrs
@@ -1713,6 +3600,12 @@ sub-messages:
       -
         value: ets
         attribute-set: tc-ets-attrs
+      -
+        value: flow
+        attribute-set: tc-flow-attrs
+      -
+        value: flower
+        attribute-set: tc-flower-attrs
       -
         value: fq
         attribute-set: tc-fq-attrs
@@ -1723,8 +3616,8 @@ sub-messages:
         value: fq_pie
         attribute-set: tc-fq-pie-attrs
       -
-        value: flower
-        attribute-set: tc-flower-attrs
+        value: fw
+        attribute-set: tc-fw-attrs
       -
         value: gred
         attribute-set: tc-gred-attrs
@@ -1739,6 +3632,9 @@ sub-messages:
         attribute-set: tc-htb-attrs
       -
         value: ingress # no content
+      -
+        value: matchall
+        attribute-set: tc-matchall-attrs
       -
         value: mq # no content
       -
@@ -1775,6 +3671,9 @@ sub-messages:
       -
         value: red
         attribute-set: tc-red-attrs
+      -
+        value: route
+        attribute-set: tc-route-attrs
       -
         value: sfb
         fixed-header: tc-sfb-qopt
@@ -1787,88 +3686,105 @@ sub-messages:
       -
         value: tbf
         attribute-set: tc-tbf-attrs
-  -
-    name: tc-act-options-msg
-    formats:
       -
-        value: gact
-        attribute-set: tca-gact-attrs
+        value: u32
+        attribute-set: tc-u32-attrs
   -
-    name: tca-stats-app-msg
+    name: tc-act-options-msg
     formats:
       -
-        value: bfifo
-      -
-        value: blackhole
+        value: bpf
+        attribute-set: tc-act-bpf-attrs
       -
-        value: cake
-        attribute-set: tc-cake-stats-attrs
+        value: connmark
+        attribute-set: tc-act-connmark-attrs
       -
-        value: cbs
+        value: csum
+        attribute-set: tc-act-csum-attrs
       -
-        value: choke
+        value: ct
+        attribute-set: tc-act-ct-attrs
       -
-        value: clsact
+        value: ctinfo
+        attribute-set: tc-act-ctinfo-attrs
       -
-        value: codel
+        value: gact
+        attribute-set: tc-act-gact-attrs
       -
-        value: drr
+        value: gate
+        attribute-set: tc-act-gate-attrs
       -
-        value: etf
+        value: ife
+        attribute-set: tc-act-ife-attrs
       -
-        value: ets
+        value: mirred
+        attribute-set: tc-act-mirred-attrs
       -
-        value: fq
+        value: mpls
+        attribute-set: tc-act-mpls-attrs
       -
-        value: fq_codel
+        value: nat
+        attribute-set: tc-act-nat-attrs
       -
-        value: fq_pie
+        value: pedit
+        attribute-set: tc-act-pedit-attrs
       -
-        value: flower
+        value: police
+        attribute-set: tc-act-police-attrs
       -
-        value: gred
+        value: sample
+        attribute-set: tc-act-sample-attrs
       -
-        value: hfsc
+        value: simple
+        attribute-set: tc-act-simple-attrs
       -
-        value: hhf
+        value: skbedit
+        attribute-set: tc-act-skbedit-attrs
       -
-        value: htb
+        value: skbmod
+        attribute-set: tc-act-skbmod-attrs
       -
-        value: ingress
+        value: tunnel_key
+        attribute-set: tc-act-tunnel-key-attrs
       -
-        value: mq
+        value: vlan
+        attribute-set: tc-act-vlan-attrs
+  -
+    name: tca-stats-app-msg
+    formats:
       -
-        value: mqprio
+        value: cake
+        attribute-set: tc-cake-stats-attrs
       -
-        value: multiq
+        value: choke
+        fixed-header: tc-choke-xstats
       -
-        value: netem
+        value: codel
+        fixed-header: tc-codel-xstats
       -
-        value: noqueue
+        value: fq
+        fixed-header: tc-fq-qd-stats
       -
-        value: pfifo
+        value: fq_codel
+        fixed-header: tc-fq-codel-xstats
       -
-        value: pfifo_fast
+        value: fq_pie
+        fixed-header: tc-fq-pie-xstats
       -
-        value: pfifo_head_drop
+        value: hhf
+        fixed-header: tc-hhf-xstats
       -
         value: pie
-      -
-        value: plug
-      -
-        value: prio
-      -
-        value: qfq
+        fixed-header: tc-pie-xstats
       -
         value: red
+        fixed-header: tc-red-xstats
       -
         value: sfb
+        fixed-header: tc-sfb-xstats
       -
         value: sfq
-      -
-        value: taprio
-      -
-        value: tbf
+        fixed-header: tc-sfq-xstats
 
 operations:
   enum-model: directional
-- 
2.42.0


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

* Re: [PATCH net-next v1 02/12] tools/net/ynl: Support sub-messages in nested attribute spaces
  2024-01-23 16:05 ` [PATCH net-next v1 02/12] tools/net/ynl: Support sub-messages in nested attribute spaces Donald Hunter
@ 2024-01-24  0:18   ` Jakub Kicinski
  2024-01-24  9:37     ` Donald Hunter
  0 siblings, 1 reply; 31+ messages in thread
From: Jakub Kicinski @ 2024-01-24  0:18 UTC (permalink / raw)
  To: Donald Hunter
  Cc: netdev, David S. Miller, Eric Dumazet, Paolo Abeni,
	Jonathan Corbet, linux-doc, Jacob Keller, Breno Leitao,
	Jiri Pirko, Alessandro Marcolini, donald.hunter

On Tue, 23 Jan 2024 16:05:28 +0000 Donald Hunter wrote:
> Sub-message selectors could only be resolved using values from the
> current nest level. Enable value lookup in outer scopes by using
> collections.ChainMap to implement an ordered lookup from nested to
> outer scopes.

Meaning if the key is not found in current scope we'll silently and
recursively try outer scopes? Did we already document that?
I remember we discussed it, can you share a link to that discussion?

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

* Re: [PATCH net-next v1 02/12] tools/net/ynl: Support sub-messages in nested attribute spaces
  2024-01-24  0:18   ` Jakub Kicinski
@ 2024-01-24  9:37     ` Donald Hunter
  2024-01-24 15:32       ` Jakub Kicinski
  0 siblings, 1 reply; 31+ messages in thread
From: Donald Hunter @ 2024-01-24  9:37 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: netdev, David S. Miller, Eric Dumazet, Paolo Abeni,
	Jonathan Corbet, linux-doc, Jacob Keller, Breno Leitao,
	Jiri Pirko, Alessandro Marcolini, donald.hunter

Jakub Kicinski <kuba@kernel.org> writes:

> On Tue, 23 Jan 2024 16:05:28 +0000 Donald Hunter wrote:
>> Sub-message selectors could only be resolved using values from the
>> current nest level. Enable value lookup in outer scopes by using
>> collections.ChainMap to implement an ordered lookup from nested to
>> outer scopes.
>
> Meaning if the key is not found in current scope we'll silently and
> recursively try outer scopes? Did we already document that?
> I remember we discussed it, can you share a link to that discussion?

Yes, it silently tries outer scopes. The previous discussion is here:

https://patchwork.kernel.org/project/netdevbpf/patch/20231130214959.27377-7-donald.hunter@gmail.com/#25622101

This is the doc patch that describes sub-messages:

https://patchwork.kernel.org/project/netdevbpf/patch/20231215093720.18774-4-donald.hunter@gmail.com/

It doesn't mention searching outer scopes so I can add that to the docs.

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

* Re: [PATCH net-next v1 02/12] tools/net/ynl: Support sub-messages in nested attribute spaces
  2024-01-24  9:37     ` Donald Hunter
@ 2024-01-24 15:32       ` Jakub Kicinski
  2024-01-26 12:44         ` Donald Hunter
  0 siblings, 1 reply; 31+ messages in thread
From: Jakub Kicinski @ 2024-01-24 15:32 UTC (permalink / raw)
  To: Donald Hunter
  Cc: netdev, David S. Miller, Eric Dumazet, Paolo Abeni,
	Jonathan Corbet, linux-doc, Jacob Keller, Breno Leitao,
	Jiri Pirko, Alessandro Marcolini, donald.hunter

On Wed, 24 Jan 2024 09:37:31 +0000 Donald Hunter wrote:
> > Meaning if the key is not found in current scope we'll silently and
> > recursively try outer scopes? Did we already document that?
> > I remember we discussed it, can you share a link to that discussion?  
> 
> Yes, it silently tries outer scopes. The previous discussion is here:
> 
> https://patchwork.kernel.org/project/netdevbpf/patch/20231130214959.27377-7-donald.hunter@gmail.com/#25622101
> 
> This is the doc patch that describes sub-messages:
> 
> https://patchwork.kernel.org/project/netdevbpf/patch/20231215093720.18774-4-donald.hunter@gmail.com/
> 
> It doesn't mention searching outer scopes so I can add that to the docs.

I'm a tiny bit worried about the mis-ordered case. If the selector attr
is after the sub-msg but outer scope has an attr of the same name we'll
silently use the wrong one. It shouldn't happen in practice but can we
notice the wrong ordering and error out cleanly?

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

* Re: [PATCH net-next v1 01/12] tools/net/ynl: Add --output-json arg to ynl cli
  2024-01-23 16:05 ` [PATCH net-next v1 01/12] tools/net/ynl: Add --output-json arg to ynl cli Donald Hunter
@ 2024-01-25 13:50   ` Breno Leitao
  0 siblings, 0 replies; 31+ messages in thread
From: Breno Leitao @ 2024-01-25 13:50 UTC (permalink / raw)
  To: Donald Hunter
  Cc: netdev, Jakub Kicinski, David S. Miller, Eric Dumazet,
	Paolo Abeni, Jonathan Corbet, linux-doc, Jacob Keller, Jiri Pirko,
	Alessandro Marcolini, donald.hunter

On Tue, Jan 23, 2024 at 04:05:27PM +0000, Donald Hunter wrote:
> The ynl cli currently emits python pretty printed structures which is
> hard to consume. Add a new --output-json argument to emit JSON.
> 
> Signed-off-by: Donald Hunter <donald.hunter@gmail.com>

Reviewed-by: Breno Leitao <leitao@debian.org>

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

* Re: [PATCH net-next v1 11/12] tools/net/ynl: Add type info to struct members in generated docs
  2024-01-23 16:05 ` [PATCH net-next v1 11/12] tools/net/ynl: Add type info to struct members in generated docs Donald Hunter
@ 2024-01-25 13:59   ` Breno Leitao
  0 siblings, 0 replies; 31+ messages in thread
From: Breno Leitao @ 2024-01-25 13:59 UTC (permalink / raw)
  To: Donald Hunter
  Cc: netdev, Jakub Kicinski, David S. Miller, Eric Dumazet,
	Paolo Abeni, Jonathan Corbet, linux-doc, Jacob Keller, Jiri Pirko,
	Alessandro Marcolini, donald.hunter

On Tue, Jan 23, 2024 at 04:05:37PM +0000, Donald Hunter wrote:
> Extend the ynl doc generator to include type information for struct
> members, ignoring the pad type.
> 
> Signed-off-by: Donald Hunter <donald.hunter@gmail.com>
> ---
>  tools/net/ynl/ynl-gen-rst.py | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/net/ynl/ynl-gen-rst.py b/tools/net/ynl/ynl-gen-rst.py
> index 262d88f88696..75c969d36b6a 100755
> --- a/tools/net/ynl/ynl-gen-rst.py
> +++ b/tools/net/ynl/ynl-gen-rst.py
> @@ -189,12 +189,20 @@ def parse_operations(operations: List[Dict[str, Any]]) -> str:
>  
>  def parse_entries(entries: List[Dict[str, Any]], level: int) -> str:
>      """Parse a list of entries"""
> +    ignored = ["pad"]
>      lines = []
>      for entry in entries:
>          if isinstance(entry, dict):
>              # entries could be a list or a dictionary
> +            field_name = entry.get("name", "")
> +            if field_name in ignored:
> +                continue
> +            type_ = entry.get("type")
> +            struct_ = entry.get("struct")

Where are you using this `struct_` variable ?

Rest of the code it looks good.

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

* Re: [PATCH net-next v1 08/12] tools/net/ynl: Move formatted_string method out of NlAttr
  2024-01-23 16:05 ` [PATCH net-next v1 08/12] tools/net/ynl: Move formatted_string method out of NlAttr Donald Hunter
@ 2024-01-25 14:24   ` Breno Leitao
  0 siblings, 0 replies; 31+ messages in thread
From: Breno Leitao @ 2024-01-25 14:24 UTC (permalink / raw)
  To: Donald Hunter
  Cc: netdev, Jakub Kicinski, David S. Miller, Eric Dumazet,
	Paolo Abeni, Jonathan Corbet, linux-doc, Jacob Keller, Jiri Pirko,
	Alessandro Marcolini, donald.hunter

On Tue, Jan 23, 2024 at 04:05:34PM +0000, Donald Hunter wrote:
> The formatted_string() class method was in NlAttr so that it could be
> accessed by NlAttr.as_struct(). Now that as_struct() has been removed,
> move formatted_string() to YnlFamily as an internal helper method.
> 
> Signed-off-by: Donald Hunter <donald.hunter@gmail.com>

Reviewed-by: Breno Leitao <leitao@debian.org>

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

* Re: [PATCH net-next v1 02/12] tools/net/ynl: Support sub-messages in nested attribute spaces
  2024-01-24 15:32       ` Jakub Kicinski
@ 2024-01-26 12:44         ` Donald Hunter
  2024-01-26 18:50           ` Jakub Kicinski
  0 siblings, 1 reply; 31+ messages in thread
From: Donald Hunter @ 2024-01-26 12:44 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: netdev, David S. Miller, Eric Dumazet, Paolo Abeni,
	Jonathan Corbet, linux-doc, Jacob Keller, Breno Leitao,
	Jiri Pirko, Alessandro Marcolini, donald.hunter

Jakub Kicinski <kuba@kernel.org> writes:

> On Wed, 24 Jan 2024 09:37:31 +0000 Donald Hunter wrote:
>> > Meaning if the key is not found in current scope we'll silently and
>> > recursively try outer scopes? Did we already document that?
>> > I remember we discussed it, can you share a link to that discussion?  
>> 
>> Yes, it silently tries outer scopes. The previous discussion is here:
>> 
>> https://patchwork.kernel.org/project/netdevbpf/patch/20231130214959.27377-7-donald.hunter@gmail.com/#25622101
>> 
>> This is the doc patch that describes sub-messages:
>> 
>> https://patchwork.kernel.org/project/netdevbpf/patch/20231215093720.18774-4-donald.hunter@gmail.com/
>> 
>> It doesn't mention searching outer scopes so I can add that to the docs.
>
> I'm a tiny bit worried about the mis-ordered case. If the selector attr
> is after the sub-msg but outer scope has an attr of the same name we'll
> silently use the wrong one. It shouldn't happen in practice but can we
> notice the wrong ordering and error out cleanly?

I was quite pleased with how simple the patch turned out to be when I
used ChainMap, but it does have this weakness. In practice, the only
place this could be a problem is with tc-act-attrs which has the same
attribute name 'kind' in the nest and in tc-attrs at the top level. If
you send a create message with ynl, you could omit the 'kind' attr in
the 'act' nest and ynl would incorrectly resolve to the top level
'kind'. The kernel would reject the action with a missing 'kind' but the
rest of payload would be encoded wrongly and/or could break ynl.

My initial thought is that this might be better handled as input
validation, e.g. adding 'required: true' to the spec for 'act/kind'.
After using ynl for a while, I think it would help to specify required
attributes for messages, nests and sub-messsages. It's very hard to
discover the required attributes for families that don't provide extack
responses for errors.

Thoughts?

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

* Re: [PATCH net-next v1 02/12] tools/net/ynl: Support sub-messages in nested attribute spaces
  2024-01-26 12:44         ` Donald Hunter
@ 2024-01-26 18:50           ` Jakub Kicinski
  2024-01-27 17:18             ` Donald Hunter
  0 siblings, 1 reply; 31+ messages in thread
From: Jakub Kicinski @ 2024-01-26 18:50 UTC (permalink / raw)
  To: Donald Hunter
  Cc: netdev, David S. Miller, Eric Dumazet, Paolo Abeni,
	Jonathan Corbet, linux-doc, Jacob Keller, Breno Leitao,
	Jiri Pirko, Alessandro Marcolini, donald.hunter

On Fri, 26 Jan 2024 12:44:57 +0000 Donald Hunter wrote:
> I was quite pleased with how simple the patch turned out to be when I
> used ChainMap, but it does have this weakness.

It is very neat, no question about it :(

> In practice, the only place this could be a problem is with
> tc-act-attrs which has the same attribute name 'kind' in the nest and
> in tc-attrs at the top level. If you send a create message with ynl,
> you could omit the 'kind' attr in the 'act' nest and ynl would
> incorrectly resolve to the top level 'kind'. The kernel would reject
> the action with a missing 'kind' but the rest of payload would be
> encoded wrongly and/or could break ynl.

We can detect the problem post-fact and throw an exception. I primarily
care about removing the ambiguity.

Is it possible to check at which "level" of the chainmap the key was
found? If so we can also construct a 'chainmap of attr sets' and make
sure that the key level == attr set level. I.e. that we got a hit at
the first level which declares a key of that name.

More crude option - we could construct a list of dicts (the levels
within the chainmap) and keys they can't contain. Once we got a hit 
for a sub-message key at level A, all dicts currently on top of A
are not allowed to add that key. Once we're done with the message we
scan thru the list and make sure the keys haven't appeared?

Another random thought, should we mark the keys which can "descend"
somehow? IDK, put a ~ in front?

	selector: ~kind

or some other char?

> My initial thought is that this might be better handled as input
> validation, e.g. adding 'required: true' to the spec for 'act/kind'.
> After using ynl for a while, I think it would help to specify required
> attributes for messages, nests and sub-messsages. It's very hard to
> discover the required attributes for families that don't provide
> extack responses for errors.

Hah, required attrs. I have been sitting on patches for the kernel for
over a year - https://github.com/kuba-moo/linux/tree/req-args
Not sure if they actually work but for the kernel I was curious if it's
possible to do the validation in constant time (in relation to the
policy size, i.e. without scanning the entire policy at the end to
confirm that all required attrs are present). And that's what I came up
with.

I haven't posted it because I was a tiny bit worried that required args
will cause bugs (people forgetting to null check attrs) and may cause
uAPI breakage down the line (we should clearly state that "required"
status is just advisory, and can go away in future kernel release).
But that was more of a on-the-fence situation. If you find them useful
feel free to move forward!

I do think that's a separate story, tho. For sub-message selector
- isn't the key _implicitly_ required, in the first attr set where 
it is defined? Conversely if the sub-message isn't present the key
isn't required any more either?

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

* Re: [PATCH net-next v1 02/12] tools/net/ynl: Support sub-messages in nested attribute spaces
  2024-01-26 18:50           ` Jakub Kicinski
@ 2024-01-27 17:18             ` Donald Hunter
  2024-01-27 18:52               ` Alessandro Marcolini
  2024-01-30  1:42               ` Jakub Kicinski
  0 siblings, 2 replies; 31+ messages in thread
From: Donald Hunter @ 2024-01-27 17:18 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: netdev, David S. Miller, Eric Dumazet, Paolo Abeni,
	Jonathan Corbet, linux-doc, Jacob Keller, Breno Leitao,
	Jiri Pirko, Alessandro Marcolini, donald.hunter

Jakub Kicinski <kuba@kernel.org> writes:

> On Fri, 26 Jan 2024 12:44:57 +0000 Donald Hunter wrote:
>> I was quite pleased with how simple the patch turned out to be when I
>> used ChainMap, but it does have this weakness.
>
> It is very neat, no question about it :(
>
>> In practice, the only place this could be a problem is with
>> tc-act-attrs which has the same attribute name 'kind' in the nest and
>> in tc-attrs at the top level. If you send a create message with ynl,
>> you could omit the 'kind' attr in the 'act' nest and ynl would
>> incorrectly resolve to the top level 'kind'. The kernel would reject
>> the action with a missing 'kind' but the rest of payload would be
>> encoded wrongly and/or could break ynl.
>
> We can detect the problem post-fact and throw an exception. I primarily
> care about removing the ambiguity.

Agreed.

> Is it possible to check at which "level" of the chainmap the key was
> found? If so we can also construct a 'chainmap of attr sets' and make
> sure that the key level == attr set level. I.e. that we got a hit at
> the first level which declares a key of that name.
>
> More crude option - we could construct a list of dicts (the levels
> within the chainmap) and keys they can't contain. Once we got a hit
> for a sub-message key at level A, all dicts currently on top of A
> are not allowed to add that key. Once we're done with the message we
> scan thru the list and make sure the keys haven't appeared?
>
> Another random thought, should we mark the keys which can "descend"
> somehow? IDK, put a ~ in front?
>
> 	selector: ~kind
>
> or some other char?

Okay, so I think the behaviour we need is to either search current scope
or search the outermost scope. My suggestion would be to replace the
ChainMap approach with just choosing between current and outermost
scope. The unusual case is needing to search the outermost scope so
using a prefix e.g. '/' for that would work.

We can have 'selector: kind' continue to refer to current scope and then
have 'selector: /kind' refer to the outermost scope.

If we run into a case that requires something other than current or
outermost then we could add e.g. '../kind' so that the scope to search
is always explicitly identified.

>> My initial thought is that this might be better handled as input
>> validation, e.g. adding 'required: true' to the spec for 'act/kind'.
>> After using ynl for a while, I think it would help to specify required
>> attributes for messages, nests and sub-messsages. It's very hard to
>> discover the required attributes for families that don't provide
>> extack responses for errors.
>
> Hah, required attrs. I have been sitting on patches for the kernel for
> over a year - https://github.com/kuba-moo/linux/tree/req-args
> Not sure if they actually work but for the kernel I was curious if it's
> possible to do the validation in constant time (in relation to the
> policy size, i.e. without scanning the entire policy at the end to
> confirm that all required attrs are present). And that's what I came up
> with.

Interesting. It's definitely a thorny problem with varying sets of
'required' attributes. It could be useful to report the absolutely
required attributes in policy responses, without any actual enforcement.
Would it be possible to report policy for legacy netlink-raw families?

Thinking about it, usability would probably be most improved by adding
extack messages to more of the tc error paths.

> I haven't posted it because I was a tiny bit worried that required args
> will cause bugs (people forgetting to null check attrs) and may cause
> uAPI breakage down the line (we should clearly state that "required"
> status is just advisory, and can go away in future kernel release).
> But that was more of a on-the-fence situation. If you find them useful
> feel free to move forward!
>
> I do think that's a separate story, tho. For sub-message selector
> - isn't the key _implicitly_ required, in the first attr set where
> it is defined? Conversely if the sub-message isn't present the key
> isn't required any more either?

Yes, the key is implicitly required for sub-messages. The toplevel key
is probably required regardless of the presence of a sub-message.

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

* Re: [PATCH net-next v1 02/12] tools/net/ynl: Support sub-messages in nested attribute spaces
  2024-01-27 17:18             ` Donald Hunter
@ 2024-01-27 18:52               ` Alessandro Marcolini
  2024-01-28 19:36                 ` Donald Hunter
  2024-01-30  1:42               ` Jakub Kicinski
  1 sibling, 1 reply; 31+ messages in thread
From: Alessandro Marcolini @ 2024-01-27 18:52 UTC (permalink / raw)
  To: Donald Hunter, Jakub Kicinski
  Cc: netdev, David S. Miller, Eric Dumazet, Paolo Abeni,
	Jonathan Corbet, linux-doc, Jacob Keller, Breno Leitao,
	Jiri Pirko, donald.hunter


On 1/27/24 18:18, Donald Hunter wrote:
> Jakub Kicinski <kuba@kernel.org> writes:
>> Is it possible to check at which "level" of the chainmap the key was
>> found? If so we can also construct a 'chainmap of attr sets' and make
>> sure that the key level == attr set level. I.e. that we got a hit at
>> the first level which declares a key of that name.
>>
>> More crude option - we could construct a list of dicts (the levels
>> within the chainmap) and keys they can't contain. Once we got a hit
>> for a sub-message key at level A, all dicts currently on top of A
>> are not allowed to add that key. Once we're done with the message we
>> scan thru the list and make sure the keys haven't appeared?
>>
>> Another random thought, should we mark the keys which can "descend"
>> somehow? IDK, put a ~ in front?
>>
>> 	selector: ~kind
>>
>> or some other char?
> Okay, so I think the behaviour we need is to either search current scope
> or search the outermost scope. My suggestion would be to replace the
> ChainMap approach with just choosing between current and outermost
> scope. The unusual case is needing to search the outermost scope so
> using a prefix e.g. '/' for that would work.
>
> We can have 'selector: kind' continue to refer to current scope and then
> have 'selector: /kind' refer to the outermost scope.
>
> If we run into a case that requires something other than current or
> outermost then we could add e.g. '../kind' so that the scope to search
> is always explicitly identified.

Wouldn't add different chars in front of the selctor value be confusing?

IMHO the solution of using a ChainMap with levels could be an easier solution. We could just modify the __getitem__() method to output both the value and the level, and the get() method to add the chance to specify a level (in our case the level found in the spec) and error out if the specified level doesn't match with the found one. Something like this:

from collections import ChainMap

class LevelChainMap(ChainMap):
    def __getitem__(self, key):
        for mapping in self.maps:
            try:
                return mapping[key], self.maps[::-1].index(mapping)
            except KeyError:
                pass
        return self.__missing__(key)

    def get(self, key, default=None, level=None):
        val, lvl = self[key] if key in self else (default, None)
        if level:
            if lvl != level:
                raise Exception("Level mismatch")
        return val, lvl

# example usage
c = LevelChainMap({'a':1}, {'inner':{'a':1}}, {'outer': {'inner':{'a':1}}})
print(c.get('a', level=2))
print(c.get('a', level=1)) #raise err

This will leave the spec as it is and will require small changes.

What do you think?


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

* Re: [PATCH net-next v1 02/12] tools/net/ynl: Support sub-messages in nested attribute spaces
  2024-01-27 18:52               ` Alessandro Marcolini
@ 2024-01-28 19:36                 ` Donald Hunter
  2024-01-29 20:35                   ` Alessandro Marcolini
  2024-01-30  1:32                   ` Jakub Kicinski
  0 siblings, 2 replies; 31+ messages in thread
From: Donald Hunter @ 2024-01-28 19:36 UTC (permalink / raw)
  To: Alessandro Marcolini
  Cc: Jakub Kicinski, netdev, David S. Miller, Eric Dumazet,
	Paolo Abeni, Jonathan Corbet, linux-doc, Jacob Keller,
	Breno Leitao, Jiri Pirko, donald.hunter

Alessandro Marcolini <alessandromarcolini99@gmail.com> writes:

> On 1/27/24 18:18, Donald Hunter wrote:
>> Okay, so I think the behaviour we need is to either search current scope
>> or search the outermost scope. My suggestion would be to replace the
>> ChainMap approach with just choosing between current and outermost
>> scope. The unusual case is needing to search the outermost scope so
>> using a prefix e.g. '/' for that would work.
>>
>> We can have 'selector: kind' continue to refer to current scope and then
>> have 'selector: /kind' refer to the outermost scope.
>>
>> If we run into a case that requires something other than current or
>> outermost then we could add e.g. '../kind' so that the scope to search
>> is always explicitly identified.
>
> Wouldn't add different chars in front of the selctor value be confusing?
>
> IMHO the solution of using a ChainMap with levels could be an easier solution. We could just
> modify the __getitem__() method to output both the value and the level, and the get() method to
> add the chance to specify a level (in our case the level found in the spec) and error out if the
> specified level doesn't match with the found one. Something like this:

If we take the approach of resolving the level from the spec then I
wouldn't use ChainMap. Per the Python docs [1]: "A ChainMap class is
provided for quickly linking a number of mappings so they can be treated
as a single unit."

I think we could instead pass a list of mappings from current to
outermost and then just reference the correct level that was resolved
from the spec.

> from collections import ChainMap
>
> class LevelChainMap(ChainMap):
>     def __getitem__(self, key):
>         for mapping in self.maps:
>             try:
>                 return mapping[key], self.maps[::-1].index(mapping)
>             except KeyError:
>                 pass
>         return self.__missing__(key)
>
>     def get(self, key, default=None, level=None):
>         val, lvl = self[key] if key in self else (default, None)
>         if level:
>             if lvl != level:
>                 raise Exception("Level mismatch")
>         return val, lvl
>
> # example usage
> c = LevelChainMap({'a':1}, {'inner':{'a':1}}, {'outer': {'inner':{'a':1}}})
> print(c.get('a', level=2))
> print(c.get('a', level=1)) #raise err
>
> This will leave the spec as it is and will require small changes.
>
> What do you think?

The more I think about it, the more I agree that using path-like syntax
in the selector is overkill. It makes sense to resolve the selector
level from the spec and then directly access the mappings from the
correct scope level.

[1] https://docs.python.org/3/library/collections.html#collections.ChainMap

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

* Re: [PATCH net-next v1 02/12] tools/net/ynl: Support sub-messages in nested attribute spaces
  2024-01-28 19:36                 ` Donald Hunter
@ 2024-01-29 20:35                   ` Alessandro Marcolini
  2024-01-30  1:32                   ` Jakub Kicinski
  1 sibling, 0 replies; 31+ messages in thread
From: Alessandro Marcolini @ 2024-01-29 20:35 UTC (permalink / raw)
  To: Donald Hunter
  Cc: Jakub Kicinski, netdev, David S. Miller, Eric Dumazet,
	Paolo Abeni, Jonathan Corbet, linux-doc, Jacob Keller,
	Breno Leitao, Jiri Pirko, donald.hunter

On 1/28/24 20:36, Donald Hunter wrote:
> Alessandro Marcolini <alessandromarcolini99@gmail.com> writes:
>
>> On 1/27/24 18:18, Donald Hunter wrote:
>>> Okay, so I think the behaviour we need is to either search current scope
>>> or search the outermost scope. My suggestion would be to replace the
>>> ChainMap approach with just choosing between current and outermost
>>> scope. The unusual case is needing to search the outermost scope so
>>> using a prefix e.g. '/' for that would work.
>>>
>>> We can have 'selector: kind' continue to refer to current scope and then
>>> have 'selector: /kind' refer to the outermost scope.
>>>
>>> If we run into a case that requires something other than current or
>>> outermost then we could add e.g. '../kind' so that the scope to search
>>> is always explicitly identified.
>> Wouldn't add different chars in front of the selctor value be confusing?
>>
>> IMHO the solution of using a ChainMap with levels could be an easier solution. We could just
>> modify the __getitem__() method to output both the value and the level, and the get() method to
>> add the chance to specify a level (in our case the level found in the spec) and error out if the
>> specified level doesn't match with the found one. Something like this:
> If we take the approach of resolving the level from the spec then I
> wouldn't use ChainMap. Per the Python docs [1]: "A ChainMap class is
> provided for quickly linking a number of mappings so they can be treated
> as a single unit."
>
> I think we could instead pass a list of mappings from current to
> outermost and then just reference the correct level that was resolved
> from the spec.

Yes, you're right. There is no need to use a ChainMap at all. The implementation I proposed is in fact a list of mappings with unnecessary complications.

>> from collections import ChainMap
>>
>> class LevelChainMap(ChainMap):
>>     def __getitem__(self, key):
>>         for mapping in self.maps:
>>             try:
>>                 return mapping[key], self.maps[::-1].index(mapping)
>>             except KeyError:
>>                 pass
>>         return self.__missing__(key)
>>
>>     def get(self, key, default=None, level=None):
>>         val, lvl = self[key] if key in self else (default, None)
>>         if level:
>>             if lvl != level:
>>                 raise Exception("Level mismatch")
>>         return val, lvl
>>
>> # example usage
>> c = LevelChainMap({'a':1}, {'inner':{'a':1}}, {'outer': {'inner':{'a':1}}})
>> print(c.get('a', level=2))
>> print(c.get('a', level=1)) #raise err
>>
>> This will leave the spec as it is and will require small changes.
>>
>> What do you think?
> The more I think about it, the more I agree that using path-like syntax
> in the selector is overkill. It makes sense to resolve the selector
> level from the spec and then directly access the mappings from the
> correct scope level.
>
> [1] https://docs.python.org/3/library/collections.html#collections.ChainMap
Agreed.

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

* Re: [PATCH net-next v1 02/12] tools/net/ynl: Support sub-messages in nested attribute spaces
  2024-01-28 19:36                 ` Donald Hunter
  2024-01-29 20:35                   ` Alessandro Marcolini
@ 2024-01-30  1:32                   ` Jakub Kicinski
  1 sibling, 0 replies; 31+ messages in thread
From: Jakub Kicinski @ 2024-01-30  1:32 UTC (permalink / raw)
  To: Donald Hunter
  Cc: Alessandro Marcolini, netdev, David S. Miller, Eric Dumazet,
	Paolo Abeni, Jonathan Corbet, linux-doc, Jacob Keller,
	Breno Leitao, Jiri Pirko, donald.hunter

On Sun, 28 Jan 2024 19:36:29 +0000 Donald Hunter wrote:
> > from collections import ChainMap
> >
> > class LevelChainMap(ChainMap):
> >     def __getitem__(self, key):
> >         for mapping in self.maps:
> >             try:
> >                 return mapping[key], self.maps[::-1].index(mapping)
> >             except KeyError:
> >                 pass
> >         return self.__missing__(key)
> >
> >     def get(self, key, default=None, level=None):
> >         val, lvl = self[key] if key in self else (default, None)
> >         if level:
> >             if lvl != level:
> >                 raise Exception("Level mismatch")
> >         return val, lvl
> >
> > # example usage
> > c = LevelChainMap({'a':1}, {'inner':{'a':1}}, {'outer': {'inner':{'a':1}}})
> > print(c.get('a', level=2))
> > print(c.get('a', level=1)) #raise err
> >
> > This will leave the spec as it is and will require small changes.
> >
> > What do you think?  
> 
> The more I think about it, the more I agree that using path-like syntax
> in the selector is overkill. It makes sense to resolve the selector
> level from the spec and then directly access the mappings from the
> correct scope level.

Plus if we resolve from the spec that's easily reusable in C / C++ 
code gen :)

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

* Re: [PATCH net-next v1 02/12] tools/net/ynl: Support sub-messages in nested attribute spaces
  2024-01-27 17:18             ` Donald Hunter
  2024-01-27 18:52               ` Alessandro Marcolini
@ 2024-01-30  1:42               ` Jakub Kicinski
  2024-01-30  9:12                 ` Donald Hunter
  2024-02-01 20:53                 ` Jacob Keller
  1 sibling, 2 replies; 31+ messages in thread
From: Jakub Kicinski @ 2024-01-30  1:42 UTC (permalink / raw)
  To: Donald Hunter
  Cc: netdev, David S. Miller, Eric Dumazet, Paolo Abeni,
	Jonathan Corbet, linux-doc, Jacob Keller, Breno Leitao,
	Jiri Pirko, Alessandro Marcolini, donald.hunter

On Sat, 27 Jan 2024 17:18:59 +0000 Donald Hunter wrote:
> > Hah, required attrs. I have been sitting on patches for the kernel for
> > over a year - https://github.com/kuba-moo/linux/tree/req-args
> > Not sure if they actually work but for the kernel I was curious if it's
> > possible to do the validation in constant time (in relation to the
> > policy size, i.e. without scanning the entire policy at the end to
> > confirm that all required attrs are present). And that's what I came up
> > with.  
> 
> Interesting. It's definitely a thorny problem with varying sets of
> 'required' attributes. It could be useful to report the absolutely
> required attributes in policy responses, without any actual enforcement.
> Would it be possible to report policy for legacy netlink-raw families?

It's a simple matter of plumbing. We care reuse the genetlink policy
dumping, just need to add a new attr to make "classic" family IDs
distinct from genetlink ones.

The policy vs spec is another interesting question. When I started
thinking about YNL my intuition was to extend policies to carry all
relevant info. But the more I thought about it the less sense it made.

Whether YNL specs should replace policy dumps completely (by building
the YAML into the kernel, and exposing via sysfs like kheaders or btf)
 - I'm not sure. I think I used policy dumps twice in my life. They
are not all that useful, IMVHO...

> Thinking about it, usability would probably be most improved by adding
> extack messages to more of the tc error paths.

TC was one of the first netlink families, so we shouldn't judge it too
harshly. With that preface - it should only be used as "lessons learned"
not to inform modern designs.

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

* Re: [PATCH net-next v1 02/12] tools/net/ynl: Support sub-messages in nested attribute spaces
  2024-01-30  1:42               ` Jakub Kicinski
@ 2024-01-30  9:12                 ` Donald Hunter
  2024-02-01 20:53                 ` Jacob Keller
  1 sibling, 0 replies; 31+ messages in thread
From: Donald Hunter @ 2024-01-30  9:12 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: netdev, David S. Miller, Eric Dumazet, Paolo Abeni,
	Jonathan Corbet, linux-doc, Jacob Keller, Breno Leitao,
	Jiri Pirko, Alessandro Marcolini, donald.hunter

Jakub Kicinski <kuba@kernel.org> writes:

> On Sat, 27 Jan 2024 17:18:59 +0000 Donald Hunter wrote:
>> > Hah, required attrs. I have been sitting on patches for the kernel for
>> > over a year - https://github.com/kuba-moo/linux/tree/req-args
>> > Not sure if they actually work but for the kernel I was curious if it's
>> > possible to do the validation in constant time (in relation to the
>> > policy size, i.e. without scanning the entire policy at the end to
>> > confirm that all required attrs are present). And that's what I came up
>> > with.  
>> 
>> Interesting. It's definitely a thorny problem with varying sets of
>> 'required' attributes. It could be useful to report the absolutely
>> required attributes in policy responses, without any actual enforcement.
>> Would it be possible to report policy for legacy netlink-raw families?
>
> It's a simple matter of plumbing. We care reuse the genetlink policy
> dumping, just need to add a new attr to make "classic" family IDs
> distinct from genetlink ones.
>
> The policy vs spec is another interesting question. When I started
> thinking about YNL my intuition was to extend policies to carry all
> relevant info. But the more I thought about it the less sense it made.
>
> Whether YNL specs should replace policy dumps completely (by building
> the YAML into the kernel, and exposing via sysfs like kheaders or btf)
>  - I'm not sure. I think I used policy dumps twice in my life. They
> are not all that useful, IMVHO...

Yeah, fair point. I don't think I've used policy dumps in any meaningful
way either. Maybe no real value in exporting it for netlink-raw.

>> Thinking about it, usability would probably be most improved by adding
>> extack messages to more of the tc error paths.
>
> TC was one of the first netlink families, so we shouldn't judge it too
> harshly. With that preface - it should only be used as "lessons learned"
> not to inform modern designs.

Oh, not judging TC, just considering whether it would be useful to throw
some extack patches at it.

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

* Re: [PATCH net-next v1 02/12] tools/net/ynl: Support sub-messages in nested attribute spaces
  2024-01-30  1:42               ` Jakub Kicinski
  2024-01-30  9:12                 ` Donald Hunter
@ 2024-02-01 20:53                 ` Jacob Keller
  2024-02-02  0:04                   ` Jakub Kicinski
  1 sibling, 1 reply; 31+ messages in thread
From: Jacob Keller @ 2024-02-01 20:53 UTC (permalink / raw)
  To: Jakub Kicinski, Donald Hunter
  Cc: netdev, David S. Miller, Eric Dumazet, Paolo Abeni,
	Jonathan Corbet, linux-doc, Breno Leitao, Jiri Pirko,
	Alessandro Marcolini, donald.hunter



On 1/29/2024 5:42 PM, Jakub Kicinski wrote:
> Whether YNL specs should replace policy dumps completely (by building
> the YAML into the kernel, and exposing via sysfs like kheaders or btf)
>  - I'm not sure. I think I used policy dumps twice in my life. They
> are not all that useful, IMVHO...

Many older genetlink/netlink families don't have a super robust or
specific policy. For example, devlink has a single enum for all
attributes, and the policy is not specified per command. The policy
simply accepts all attributes for every command. This means that you
can't rely on policy to decide whether an attribute has meaning for a
given command.

Unfortunately, we can't really change this because it ultimately counts
as uAPI and we require that existing working functionality continues
working in the future. I personally find this too stringent as sending
such junk attributes requires someone going out of their way to write
the messages and add extra attributes. In most cases I think sane
users/software would rather be informed that they are sending data which
is not relevant.

However, I can understand the point that the userspace software
"worked", and we don't want to break existing applications just because
of a kernel upgrade.

The YNL spec does this by telling you at every layer of nesting which
set of attributes are allowed and with what values. Even if we can't
enforce this for older families its still useful information to report
in some manner.

In addition, the YNL spec is more readable than the policy dumps which
essentially require a separate tool to parse out everything and convert
to something useful.

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

* Re: [PATCH net-next v1 02/12] tools/net/ynl: Support sub-messages in nested attribute spaces
  2024-02-01 20:53                 ` Jacob Keller
@ 2024-02-02  0:04                   ` Jakub Kicinski
  2024-02-02 17:12                     ` Jacob Keller
  0 siblings, 1 reply; 31+ messages in thread
From: Jakub Kicinski @ 2024-02-02  0:04 UTC (permalink / raw)
  To: Jacob Keller
  Cc: Donald Hunter, netdev, David S. Miller, Eric Dumazet, Paolo Abeni,
	Jonathan Corbet, linux-doc, Breno Leitao, Jiri Pirko,
	Alessandro Marcolini, donald.hunter

On Thu, 1 Feb 2024 12:53:08 -0800 Jacob Keller wrote:
> On 1/29/2024 5:42 PM, Jakub Kicinski wrote:
> > Whether YNL specs should replace policy dumps completely (by building
> > the YAML into the kernel, and exposing via sysfs like kheaders or btf)
> >  - I'm not sure. I think I used policy dumps twice in my life. They
> > are not all that useful, IMVHO...  
> 
> Many older genetlink/netlink families don't have a super robust or
> specific policy. For example, devlink has a single enum for all
> attributes, and the policy is not specified per command. The policy
> simply accepts all attributes for every command. This means that you
> can't rely on policy to decide whether an attribute has meaning for a
> given command.

FWIW Jiri converted devlink to use ynl policy generation. AFAIU it now
only accepts what's used and nobody complained, yet, knock wood.

Agreed on other points :)

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

* Re: [PATCH net-next v1 02/12] tools/net/ynl: Support sub-messages in nested attribute spaces
  2024-02-02  0:04                   ` Jakub Kicinski
@ 2024-02-02 17:12                     ` Jacob Keller
  0 siblings, 0 replies; 31+ messages in thread
From: Jacob Keller @ 2024-02-02 17:12 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: Donald Hunter, netdev, David S. Miller, Eric Dumazet, Paolo Abeni,
	Jonathan Corbet, linux-doc, Breno Leitao, Jiri Pirko,
	Alessandro Marcolini, donald.hunter



On 2/1/2024 4:04 PM, Jakub Kicinski wrote:
> On Thu, 1 Feb 2024 12:53:08 -0800 Jacob Keller wrote:
>> On 1/29/2024 5:42 PM, Jakub Kicinski wrote:
>>> Whether YNL specs should replace policy dumps completely (by building
>>> the YAML into the kernel, and exposing via sysfs like kheaders or btf)
>>>  - I'm not sure. I think I used policy dumps twice in my life. They
>>> are not all that useful, IMVHO...  
>>
>> Many older genetlink/netlink families don't have a super robust or
>> specific policy. For example, devlink has a single enum for all
>> attributes, and the policy is not specified per command. The policy
>> simply accepts all attributes for every command. This means that you
>> can't rely on policy to decide whether an attribute has meaning for a
>> given command.
> 
> FWIW Jiri converted devlink to use ynl policy generation. AFAIU it now
> only accepts what's used and nobody complained, yet, knock wood.
> 

Oh, I guess I missed that. That's awesome.

> Agreed on other points :)
> 

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

end of thread, other threads:[~2024-02-02 17:12 UTC | newest]

Thread overview: 31+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-01-23 16:05 [PATCH net-next v1 00/12] tools/net/ynl: Add features for tc family Donald Hunter
2024-01-23 16:05 ` [PATCH net-next v1 01/12] tools/net/ynl: Add --output-json arg to ynl cli Donald Hunter
2024-01-25 13:50   ` Breno Leitao
2024-01-23 16:05 ` [PATCH net-next v1 02/12] tools/net/ynl: Support sub-messages in nested attribute spaces Donald Hunter
2024-01-24  0:18   ` Jakub Kicinski
2024-01-24  9:37     ` Donald Hunter
2024-01-24 15:32       ` Jakub Kicinski
2024-01-26 12:44         ` Donald Hunter
2024-01-26 18:50           ` Jakub Kicinski
2024-01-27 17:18             ` Donald Hunter
2024-01-27 18:52               ` Alessandro Marcolini
2024-01-28 19:36                 ` Donald Hunter
2024-01-29 20:35                   ` Alessandro Marcolini
2024-01-30  1:32                   ` Jakub Kicinski
2024-01-30  1:42               ` Jakub Kicinski
2024-01-30  9:12                 ` Donald Hunter
2024-02-01 20:53                 ` Jacob Keller
2024-02-02  0:04                   ` Jakub Kicinski
2024-02-02 17:12                     ` Jacob Keller
2024-01-23 16:05 ` [PATCH net-next v1 03/12] tools/net/ynl: Refactor fixed header encoding into separate method Donald Hunter
2024-01-23 16:05 ` [PATCH net-next v1 04/12] tools/net/ynl: Add support for encoding sub-messages Donald Hunter
2024-01-23 16:05 ` [PATCH net-next v1 05/12] tools/net/ynl: Encode default values for binary blobs Donald Hunter
2024-01-23 16:05 ` [PATCH net-next v1 06/12] tools/net/ynl: Combine struct decoding logic in ynl Donald Hunter
2024-01-23 16:05 ` [PATCH net-next v1 07/12] tools/net/ynl: Rename _fixed_header_size() to _struct_size() Donald Hunter
2024-01-23 16:05 ` [PATCH net-next v1 08/12] tools/net/ynl: Move formatted_string method out of NlAttr Donald Hunter
2024-01-25 14:24   ` Breno Leitao
2024-01-23 16:05 ` [PATCH net-next v1 09/12] tools/net/ynl: Add support for nested structs Donald Hunter
2024-01-23 16:05 ` [PATCH net-next v1 10/12] doc/netlink: Describe nested structs in netlink raw docs Donald Hunter
2024-01-23 16:05 ` [PATCH net-next v1 11/12] tools/net/ynl: Add type info to struct members in generated docs Donald Hunter
2024-01-25 13:59   ` Breno Leitao
2024-01-23 16:05 ` [PATCH net-next v1 12/12] doc/netlink/specs: Update the tc spec Donald Hunter

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