From 7cf93538e087a792cb476008a757ab7c1c23b68c Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Wed, 1 Mar 2023 10:36:40 -0800 Subject: [PATCH 1/3] tools: ynl: fully inherit attrs in subsets To avoid having to repeat the entire definition of an attribute (including the value) use the Attr object from the original set. In fact this is already the documented expectation. Fixes: be5bea1cc0bf ("net: add basic C code generators for Netlink") Signed-off-by: Jakub Kicinski Signed-off-by: David S. Miller --- Documentation/userspace-api/netlink/specs.rst | 3 ++- tools/net/ynl/lib/nlspec.py | 23 ++++++++++++------- 2 files changed, 17 insertions(+), 9 deletions(-) diff --git a/Documentation/userspace-api/netlink/specs.rst b/Documentation/userspace-api/netlink/specs.rst index 6ffe8137cd902..1424ab1b9b337 100644 --- a/Documentation/userspace-api/netlink/specs.rst +++ b/Documentation/userspace-api/netlink/specs.rst @@ -199,7 +199,8 @@ The ``value`` property can be skipped, in which case the attribute ID will be the value of the previous attribute plus one (recursively) and ``0`` for the first attribute in the attribute set. -Note that the ``value`` of an attribute is defined only in its main set. +Note that the ``value`` of an attribute is defined only in its main set +(not in subsets). enum ~~~~ diff --git a/tools/net/ynl/lib/nlspec.py b/tools/net/ynl/lib/nlspec.py index 71da568e2c28f..dff31dad36c50 100644 --- a/tools/net/ynl/lib/nlspec.py +++ b/tools/net/ynl/lib/nlspec.py @@ -95,15 +95,22 @@ def __init__(self, family, yaml): self.attrs = collections.OrderedDict() self.attrs_by_val = collections.OrderedDict() - val = 0 - for elem in self.yaml['attributes']: - if 'value' in elem: - val = elem['value'] + if self.subset_of is None: + val = 0 + for elem in self.yaml['attributes']: + if 'value' in elem: + val = elem['value'] - attr = self.new_attr(elem, val) - self.attrs[attr.name] = attr - self.attrs_by_val[attr.value] = attr - val += 1 + attr = self.new_attr(elem, val) + self.attrs[attr.name] = attr + self.attrs_by_val[attr.value] = attr + val += 1 + else: + real_set = family.attr_sets[self.subset_of] + for elem in self.yaml['attributes']: + attr = real_set[elem['name']] + self.attrs[attr.name] = attr + self.attrs_by_val[attr.value] = attr def new_attr(self, elem, value): return SpecAttr(self.family, self, elem, value) From ad4fafcde5bc1badb8fc2c7f260a5d6b83a038e4 Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Wed, 1 Mar 2023 10:36:41 -0800 Subject: [PATCH 2/3] tools: ynl: use 1 as the default for first entry in attrs/ops Pretty much all families use value: 1 or reserve as unspec the first entry in attribute set and the first operation. Make this the default. Update documentation (the doc for values of operations just refers back to doc for attrs so updating only attrs). Signed-off-by: Jakub Kicinski Signed-off-by: David S. Miller --- Documentation/userspace-api/netlink/specs.rst | 7 ++++++- tools/net/ynl/lib/nlspec.py | 6 +++--- tools/net/ynl/ynl-gen-c.py | 7 +++++-- 3 files changed, 14 insertions(+), 6 deletions(-) diff --git a/Documentation/userspace-api/netlink/specs.rst b/Documentation/userspace-api/netlink/specs.rst index 1424ab1b9b337..32e53328d113d 100644 --- a/Documentation/userspace-api/netlink/specs.rst +++ b/Documentation/userspace-api/netlink/specs.rst @@ -197,7 +197,12 @@ value Numerical attribute ID, used in serialized Netlink messages. The ``value`` property can be skipped, in which case the attribute ID will be the value of the previous attribute plus one (recursively) -and ``0`` for the first attribute in the attribute set. +and ``1`` for the first attribute in the attribute set. + +Attributes (and operations) use ``1`` as the default value for the first +entry (unlike enums in definitions which start from ``0``) because +entry ``0`` is almost always reserved as undefined. Spec can explicitly +set value to ``0`` if needed. Note that the ``value`` of an attribute is defined only in its main set (not in subsets). diff --git a/tools/net/ynl/lib/nlspec.py b/tools/net/ynl/lib/nlspec.py index dff31dad36c50..9d394e50de23d 100644 --- a/tools/net/ynl/lib/nlspec.py +++ b/tools/net/ynl/lib/nlspec.py @@ -96,7 +96,7 @@ def __init__(self, family, yaml): self.attrs_by_val = collections.OrderedDict() if self.subset_of is None: - val = 0 + val = 1 for elem in self.yaml['attributes']: if 'value' in elem: val = elem['value'] @@ -252,7 +252,7 @@ def add_unresolved(self, elem): self._resolution_list.append(elem) def _dictify_ops_unified(self): - val = 0 + val = 1 for elem in self.yaml['operations']['list']: if 'value' in elem: val = elem['value'] @@ -263,7 +263,7 @@ def _dictify_ops_unified(self): self.msgs[op.name] = op def _dictify_ops_directional(self): - req_val = rsp_val = 0 + req_val = rsp_val = 1 for elem in self.yaml['operations']['list']: if 'notify' in elem: if 'value' in elem: diff --git a/tools/net/ynl/ynl-gen-c.py b/tools/net/ynl/ynl-gen-c.py index 274e9c566f61b..62f8f2c3c56c8 100755 --- a/tools/net/ynl/ynl-gen-c.py +++ b/tools/net/ynl/ynl-gen-c.py @@ -2044,14 +2044,17 @@ def render_uapi(family, cw): max_value = f"({cnt_name} - 1)" uapi_enum_start(family, cw, family['operations'], 'enum-name') + val = 0 for op in family.msgs.values(): if separate_ntf and ('notify' in op or 'event' in op): continue suffix = ',' - if 'value' in op: - suffix = f" = {op['value']}," + if op.value != val: + suffix = f" = {op.value}," + val = op.value cw.p(op.enum_name + suffix) + val += 1 cw.nl() cw.p(cnt_name + ('' if max_by_define else ',')) if not max_by_define: From bcec7171eba901cc5f427ebbad9fe17ed4749b8f Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Wed, 1 Mar 2023 10:36:42 -0800 Subject: [PATCH 3/3] netlink: specs: update for codegen enumerating from 1 Now that the codegen rules had been changed we can update the specs to reflect the new default. Signed-off-by: Jakub Kicinski Signed-off-by: David S. Miller --- Documentation/netlink/specs/ethtool.yaml | 15 --------------- Documentation/netlink/specs/fou.yaml | 2 ++ Documentation/netlink/specs/netdev.yaml | 2 -- 3 files changed, 2 insertions(+), 17 deletions(-) diff --git a/Documentation/netlink/specs/ethtool.yaml b/Documentation/netlink/specs/ethtool.yaml index 08b776908d152..35c462bce56f6 100644 --- a/Documentation/netlink/specs/ethtool.yaml +++ b/Documentation/netlink/specs/ethtool.yaml @@ -11,7 +11,6 @@ attribute-sets: - name: dev-index type: u32 - value: 1 - name: dev-name type: string @@ -25,7 +24,6 @@ attribute-sets: - name: index type: u32 - value: 1 - name: name type: string @@ -39,14 +37,12 @@ attribute-sets: name: bit type: nest nested-attributes: bitset-bit - value: 1 - name: bitset attributes: - name: nomask type: flag - value: 1 - name: size type: u32 @@ -61,7 +57,6 @@ attribute-sets: - name: index type: u32 - value: 1 - name: value type: string @@ -71,7 +66,6 @@ attribute-sets: - name: string type: nest - value: 1 multi-attr: true nested-attributes: string - @@ -80,7 +74,6 @@ attribute-sets: - name: id type: u32 - value: 1 - name: count type: u32 @@ -96,14 +89,12 @@ attribute-sets: name: stringset type: nest multi-attr: true - value: 1 nested-attributes: stringset - name: strset attributes: - name: header - value: 1 type: nest nested-attributes: header - @@ -119,7 +110,6 @@ attribute-sets: attributes: - name: header - value: 1 type: nest nested-attributes: header - @@ -132,7 +122,6 @@ attribute-sets: attributes: - name: header - value: 1 type: nest nested-attributes: header - @@ -180,7 +169,6 @@ attribute-sets: attributes: - name: pad - value: 1 type: pad - name: reassembly-errors @@ -205,7 +193,6 @@ attribute-sets: attributes: - name: header - value: 1 type: nest nested-attributes: header - @@ -251,13 +238,11 @@ operations: do: &strset-get-op request: - value: 1 attributes: - header - stringsets - counts-only reply: - value: 1 attributes: - header - stringsets diff --git a/Documentation/netlink/specs/fou.yaml b/Documentation/netlink/specs/fou.yaml index 266c386eedf3a..cca4cf98f03ab 100644 --- a/Documentation/netlink/specs/fou.yaml +++ b/Documentation/netlink/specs/fou.yaml @@ -26,6 +26,7 @@ attribute-sets: - name: unspec type: unused + value: 0 - name: port type: u16 @@ -71,6 +72,7 @@ operations: - name: unspec doc: unused + value: 0 - name: add diff --git a/Documentation/netlink/specs/netdev.yaml b/Documentation/netlink/specs/netdev.yaml index cffef09729f15..ba9ee13cf7296 100644 --- a/Documentation/netlink/specs/netdev.yaml +++ b/Documentation/netlink/specs/netdev.yaml @@ -48,7 +48,6 @@ attribute-sets: name: ifindex doc: netdev ifindex type: u32 - value: 1 checks: min: 1 - @@ -66,7 +65,6 @@ operations: - name: dev-get doc: Get / dump information about a netdev. - value: 1 attribute-set: dev do: request: