Skip to content

Commit

Permalink
Merge branch 'net-tools-ynl-fixes'
Browse files Browse the repository at this point in the history
Jakub Kicinski says:

====================
tools: ynl: fix subset use and change default value for attrs/ops

Fix a problem in subsetting, which will become apparent when
the devlink family comes after the merge window. Even tho none
of the existing families need this, we don't want someone to
get "inspired" by the current, incorrect code when using specs
in other languages.

Change the default value for the first attr/op. This is a slight
behavior change so needs to go in now. The diffstat of the last
patch should serve as the clearest justification there..
====================

Signed-off-by: David S. Miller <davem@davemloft.net>
  • Loading branch information
David S. Miller committed Mar 3, 2023
2 parents ad93bab + bcec717 commit 8f632a0
Show file tree
Hide file tree
Showing 6 changed files with 32 additions and 31 deletions.
15 changes: 0 additions & 15 deletions Documentation/netlink/specs/ethtool.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ attribute-sets:
-
name: dev-index
type: u32
value: 1
-
name: dev-name
type: string
Expand All @@ -25,7 +24,6 @@ attribute-sets:
-
name: index
type: u32
value: 1
-
name: name
type: string
Expand All @@ -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
Expand All @@ -61,7 +57,6 @@ attribute-sets:
-
name: index
type: u32
value: 1
-
name: value
type: string
Expand All @@ -71,7 +66,6 @@ attribute-sets:
-
name: string
type: nest
value: 1
multi-attr: true
nested-attributes: string
-
Expand All @@ -80,7 +74,6 @@ attribute-sets:
-
name: id
type: u32
value: 1
-
name: count
type: u32
Expand All @@ -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
-
Expand All @@ -119,7 +110,6 @@ attribute-sets:
attributes:
-
name: header
value: 1
type: nest
nested-attributes: header
-
Expand All @@ -132,7 +122,6 @@ attribute-sets:
attributes:
-
name: header
value: 1
type: nest
nested-attributes: header
-
Expand Down Expand Up @@ -180,7 +169,6 @@ attribute-sets:
attributes:
-
name: pad
value: 1
type: pad
-
name: reassembly-errors
Expand All @@ -205,7 +193,6 @@ attribute-sets:
attributes:
-
name: header
value: 1
type: nest
nested-attributes: header
-
Expand Down Expand Up @@ -251,13 +238,11 @@ operations:

do: &strset-get-op
request:
value: 1
attributes:
- header
- stringsets
- counts-only
reply:
value: 1
attributes:
- header
- stringsets
Expand Down
2 changes: 2 additions & 0 deletions Documentation/netlink/specs/fou.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ attribute-sets:
-
name: unspec
type: unused
value: 0
-
name: port
type: u16
Expand Down Expand Up @@ -71,6 +72,7 @@ operations:
-
name: unspec
doc: unused
value: 0

-
name: add
Expand Down
2 changes: 0 additions & 2 deletions Documentation/netlink/specs/netdev.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@ attribute-sets:
name: ifindex
doc: netdev ifindex
type: u32
value: 1
checks:
min: 1
-
Expand All @@ -66,7 +65,6 @@ operations:
-
name: dev-get
doc: Get / dump information about a netdev.
value: 1
attribute-set: dev
do:
request:
Expand Down
10 changes: 8 additions & 2 deletions Documentation/userspace-api/netlink/specs.rst
Original file line number Diff line number Diff line change
Expand Up @@ -197,9 +197,15 @@ 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.

Note that the ``value`` of an attribute is defined only in its main 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).

enum
~~~~
Expand Down
27 changes: 17 additions & 10 deletions tools/net/ynl/lib/nlspec.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 = 1
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)
Expand Down Expand Up @@ -245,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']
Expand All @@ -256,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:
Expand Down
7 changes: 5 additions & 2 deletions tools/net/ynl/ynl-gen-c.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down

0 comments on commit 8f632a0

Please sign in to comment.