From eaf317e7d2bbb04486c9842aea9be1e94bd416ed Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Mon, 30 Jan 2023 18:33:41 -0800 Subject: [PATCH 01/14] tools: ynl-gen: prevent do / dump reordering An earlier fix tried to address generated code jumping around one code-gen run to another. Turns out dict()s are already ordered since Python 3.7, the problem is that we iterate over operation modes using a set(). Sets are unordered in Python. Signed-off-by: Jakub Kicinski --- tools/net/ynl/ynl-gen-c.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/net/ynl/ynl-gen-c.py b/tools/net/ynl/ynl-gen-c.py index 1aa872e582ab..e5002d420961 100755 --- a/tools/net/ynl/ynl-gen-c.py +++ b/tools/net/ynl/ynl-gen-c.py @@ -933,7 +933,7 @@ class Family: if attr_set_name != op['attribute-set']: raise Exception('For a global policy all ops must use the same set') - for op_mode in {'do', 'dump'}: + for op_mode in ['do', 'dump']: if op_mode in op: global_set.update(op[op_mode].get('request', [])) @@ -2244,7 +2244,7 @@ def main(): for op_name, op in parsed.ops.items(): if parsed.kernel_policy in {'per-op', 'split'}: - for op_mode in {'do', 'dump'}: + for op_mode in ['do', 'dump']: if op_mode in op and 'request' in op[op_mode]: cw.p(f"/* {op.enum_name} - {op_mode} */") ri = RenderInfo(cw, parsed, args.mode, op, op_name, op_mode) From 4e4480e89c47b52b3f4fbc1ddf07a7ce541f0839 Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Mon, 30 Jan 2023 18:33:42 -0800 Subject: [PATCH 02/14] tools: ynl: move the cli and netlink code around Move the CLI code out of samples/ and the library part of it into tools/net/ynl/lib/. This way we can start sharing some code with the code gen. Initially I thought that code gen is too C-specific to share anything but basic stuff like calculating values for enums can easily be shared. Signed-off-by: Jakub Kicinski --- tools/net/ynl/{samples => }/cli.py | 2 +- tools/net/ynl/lib/__init__.py | 5 +++++ tools/net/ynl/{samples => lib}/ynl.py | 0 3 files changed, 6 insertions(+), 1 deletion(-) rename tools/net/ynl/{samples => }/cli.py (97%) create mode 100644 tools/net/ynl/lib/__init__.py rename tools/net/ynl/{samples => lib}/ynl.py (100%) diff --git a/tools/net/ynl/samples/cli.py b/tools/net/ynl/cli.py similarity index 97% rename from tools/net/ynl/samples/cli.py rename to tools/net/ynl/cli.py index b27159c70710..5c4eb5a68514 100755 --- a/tools/net/ynl/samples/cli.py +++ b/tools/net/ynl/cli.py @@ -6,7 +6,7 @@ import json import pprint import time -from ynl import YnlFamily +from lib import YnlFamily def main(): diff --git a/tools/net/ynl/lib/__init__.py b/tools/net/ynl/lib/__init__.py new file mode 100644 index 000000000000..0a6102758ebe --- /dev/null +++ b/tools/net/ynl/lib/__init__.py @@ -0,0 +1,5 @@ +# SPDX-License-Identifier: BSD-3-Clause + +from .ynl import YnlFamily + +__all__ = ["YnlFamily"] diff --git a/tools/net/ynl/samples/ynl.py b/tools/net/ynl/lib/ynl.py similarity index 100% rename from tools/net/ynl/samples/ynl.py rename to tools/net/ynl/lib/ynl.py From 3aacf8281336ac57fe4a4e85fa55a68218e90b5c Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Mon, 30 Jan 2023 18:33:43 -0800 Subject: [PATCH 03/14] tools: ynl: add an object hierarchy to represent parsed spec There's a lot of copy and pasting going on between the "cli" and code gen when it comes to representing the parsed spec. Create a library which both can use. Signed-off-by: Jakub Kicinski --- tools/net/ynl/lib/__init__.py | 4 +- tools/net/ynl/lib/nlspec.py | 301 ++++++++++++++++++++++++++++++++++ 2 files changed, 304 insertions(+), 1 deletion(-) create mode 100644 tools/net/ynl/lib/nlspec.py diff --git a/tools/net/ynl/lib/__init__.py b/tools/net/ynl/lib/__init__.py index 0a6102758ebe..3c73f59eabab 100644 --- a/tools/net/ynl/lib/__init__.py +++ b/tools/net/ynl/lib/__init__.py @@ -1,5 +1,7 @@ # SPDX-License-Identifier: BSD-3-Clause +from .nlspec import SpecAttr, SpecAttrSet, SpecFamily, SpecOperation from .ynl import YnlFamily -__all__ = ["YnlFamily"] +__all__ = ["SpecAttr", "SpecAttrSet", "SpecFamily", "SpecOperation", + "YnlFamily"] diff --git a/tools/net/ynl/lib/nlspec.py b/tools/net/ynl/lib/nlspec.py new file mode 100644 index 000000000000..4aa3b1ad97f0 --- /dev/null +++ b/tools/net/ynl/lib/nlspec.py @@ -0,0 +1,301 @@ +# SPDX-License-Identifier: BSD-3-Clause + +import collections +import jsonschema +import os +import traceback +import yaml + + +class SpecElement: + """Netlink spec element. + + Abstract element of the Netlink spec. Implements the dictionary interface + for access to the raw spec. Supports iterative resolution of dependencies + across elements and class inheritance levels. The elements of the spec + may refer to each other, and although loops should be very rare, having + to maintain correct ordering of instantiation is painful, so the resolve() + method should be used to perform parts of init which require access to + other parts of the spec. + + Attributes: + yaml raw spec as loaded from the spec file + family back reference to the full family + + name name of the entity as listed in the spec (optional) + ident_name name which can be safely used as identifier in code (optional) + """ + def __init__(self, family, yaml): + self.yaml = yaml + self.family = family + + if 'name' in self.yaml: + self.name = self.yaml['name'] + self.ident_name = self.name.replace('-', '_') + + self._super_resolved = False + family.add_unresolved(self) + + def __getitem__(self, key): + return self.yaml[key] + + def __contains__(self, key): + return key in self.yaml + + def get(self, key, default=None): + return self.yaml.get(key, default) + + def resolve_up(self, up): + if not self._super_resolved: + up.resolve() + self._super_resolved = True + + def resolve(self): + pass + + +class SpecAttr(SpecElement): + """ Single Netlink atttribute type + + Represents a single attribute type within an attr space. + + Attributes: + value numerical ID when serialized + attr_set Attribute Set containing this attr + """ + def __init__(self, family, attr_set, yaml, value): + super().__init__(family, yaml) + + self.value = value + self.attr_set = attr_set + self.is_multi = yaml.get('multi-attr', False) + + +class SpecAttrSet(SpecElement): + """ Netlink Attribute Set class. + + Represents a ID space of attributes within Netlink. + + Note that unlike other elements, which expose contents of the raw spec + via the dictionary interface Attribute Set exposes attributes by name. + + Attributes: + attrs ordered dict of all attributes (indexed by name) + attrs_by_val ordered dict of all attributes (indexed by value) + subset_of parent set if this is a subset, otherwise None + """ + def __init__(self, family, yaml): + super().__init__(family, yaml) + + self.subset_of = self.yaml.get('subset-of', None) + + self.attrs = collections.OrderedDict() + self.attrs_by_val = collections.OrderedDict() + + val = 0 + for elem in self.yaml['attributes']: + if 'value' in elem: + val = elem['value'] + + attr = self.new_attr(elem, val) + self.attrs[attr.name] = attr + self.attrs_by_val[attr.value] = attr + val += 1 + + def new_attr(self, elem, value): + return SpecAttr(self.family, self, elem, value) + + def __getitem__(self, key): + return self.attrs[key] + + def __contains__(self, key): + return key in self.attrs + + def __iter__(self): + yield from self.attrs + + def items(self): + return self.attrs.items() + + +class SpecOperation(SpecElement): + """Netlink Operation + + Information about a single Netlink operation. + + Attributes: + value numerical ID when serialized, None if req/rsp values differ + + req_value numerical ID when serialized, user -> kernel + rsp_value numerical ID when serialized, user <- kernel + is_call bool, whether the operation is a call + is_async bool, whether the operation is a notification + is_resv bool, whether the operation does not exist (it's just a reserved ID) + attr_set attribute set name + + yaml raw spec as loaded from the spec file + """ + def __init__(self, family, yaml, req_value, rsp_value): + super().__init__(family, yaml) + + self.value = req_value if req_value == rsp_value else None + self.req_value = req_value + self.rsp_value = rsp_value + + self.is_call = 'do' in yaml or 'dump' in yaml + self.is_async = 'notify' in yaml or 'event' in yaml + self.is_resv = not self.is_async and not self.is_call + + # Added by resolve: + self.attr_set = None + delattr(self, "attr_set") + + def resolve(self): + self.resolve_up(super()) + + if 'attribute-set' in self.yaml: + attr_set_name = self.yaml['attribute-set'] + elif 'notify' in self.yaml: + msg = self.family.msgs[self.yaml['notify']] + attr_set_name = msg['attribute-set'] + elif self.is_resv: + attr_set_name = '' + else: + raise Exception(f"Can't resolve attribute set for op '{self.name}'") + if attr_set_name: + self.attr_set = self.family.attr_sets[attr_set_name] + + +class SpecFamily(SpecElement): + """ Netlink Family Spec class. + + Netlink family information loaded from a spec (e.g. in YAML). + Takes care of unfolding implicit information which can be skipped + in the spec itself for brevity. + + The class can be used like a dictionary to access the raw spec + elements but that's usually a bad idea. + + Attributes: + proto protocol type (e.g. genetlink) + + attr_sets dict of attribute sets + msgs dict of all messages (index by name) + msgs_by_value dict of all messages (indexed by name) + ops dict of all valid requests / responses + """ + def __init__(self, spec_path, schema_path=None): + with open(spec_path, "r") as stream: + spec = yaml.safe_load(stream) + + self._resolution_list = [] + + super().__init__(self, spec) + + self.proto = self.yaml.get('protocol', 'genetlink') + + if schema_path is None: + schema_path = os.path.dirname(os.path.dirname(spec_path)) + f'/{self.proto}.yaml' + if schema_path: + with open(schema_path, "r") as stream: + schema = yaml.safe_load(stream) + + jsonschema.validate(self.yaml, schema) + + self.attr_sets = collections.OrderedDict() + self.msgs = collections.OrderedDict() + self.req_by_value = collections.OrderedDict() + self.rsp_by_value = collections.OrderedDict() + self.ops = collections.OrderedDict() + + last_exception = None + while len(self._resolution_list) > 0: + resolved = [] + unresolved = self._resolution_list + self._resolution_list = [] + + for elem in unresolved: + try: + elem.resolve() + except (KeyError, AttributeError) as e: + self._resolution_list.append(elem) + last_exception = e + continue + + resolved.append(elem) + + if len(resolved) == 0: + traceback.print_exception(last_exception) + raise Exception("Could not resolve any spec element, infinite loop?") + + def new_attr_set(self, elem): + return SpecAttrSet(self, elem) + + def new_operation(self, elem, req_val, rsp_val): + return SpecOperation(self, elem, req_val, rsp_val) + + def add_unresolved(self, elem): + self._resolution_list.append(elem) + + def _dictify_ops_unified(self): + val = 0 + for elem in self.yaml['operations']['list']: + if 'value' in elem: + val = elem['value'] + + op = self.new_operation(elem, val, val) + val += 1 + + self.msgs[op.name] = op + + def _dictify_ops_directional(self): + req_val = rsp_val = 0 + for elem in self.yaml['operations']['list']: + if 'notify' in elem: + if 'value' in elem: + rsp_val = elem['value'] + req_val_next = req_val + rsp_val_next = rsp_val + 1 + req_val = None + elif 'do' in elem or 'dump' in elem: + mode = elem['do'] if 'do' in elem else elem['dump'] + + v = mode.get('request', {}).get('value', None) + if v: + req_val = v + v = mode.get('reply', {}).get('value', None) + if v: + rsp_val = v + + rsp_inc = 1 if 'reply' in mode else 0 + req_val_next = req_val + 1 + rsp_val_next = rsp_val + rsp_inc + else: + raise Exception("Can't parse directional ops") + + op = self.new_operation(elem, req_val, rsp_val) + req_val = req_val_next + rsp_val = rsp_val_next + + self.msgs[op.name] = op + + def resolve(self): + self.resolve_up(super()) + + for elem in self.yaml['attribute-sets']: + attr_set = self.new_attr_set(elem) + self.attr_sets[elem['name']] = attr_set + + msg_id_model = self.yaml['operations'].get('enum-model', 'unified') + if msg_id_model == 'unified': + self._dictify_ops_unified() + elif msg_id_model == 'directional': + self._dictify_ops_directional() + + for op in self.msgs.values(): + if op.req_value is not None: + self.req_by_value[op.req_value] = op + if op.rsp_value is not None: + self.rsp_by_value[op.rsp_value] = op + if not op.is_async and 'attribute-set' in op: + self.ops[op.name] = op From 30a5c6c8104f34ed02c50041b82f3ef80924de6d Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Mon, 30 Jan 2023 18:33:44 -0800 Subject: [PATCH 04/14] tools: ynl: use the common YAML loading and validation code Adapt the common object hierarchy in code gen and CLI. Signed-off-by: Jakub Kicinski --- tools/net/ynl/lib/ynl.py | 118 ++++------------ tools/net/ynl/ynl-gen-c.py | 268 +++++++++++++++++-------------------- 2 files changed, 148 insertions(+), 238 deletions(-) diff --git a/tools/net/ynl/lib/ynl.py b/tools/net/ynl/lib/ynl.py index b71523d71d46..0ceb627ba686 100644 --- a/tools/net/ynl/lib/ynl.py +++ b/tools/net/ynl/lib/ynl.py @@ -1,13 +1,14 @@ # SPDX-License-Identifier: BSD-3-Clause import functools -import jsonschema import os import random import socket import struct import yaml +from .nlspec import SpecFamily + # # Generic Netlink code which should really be in some library, but I can't quickly find one. # @@ -158,8 +159,8 @@ class NlMsg: # We don't have the ability to parse nests yet, so only do global if 'miss-type' in self.extack and 'miss-nest' not in self.extack: miss_type = self.extack['miss-type'] - if len(attr_space.attr_list) > miss_type: - spec = attr_space.attr_list[miss_type] + if miss_type in attr_space.attrs_by_val: + spec = attr_space.attrs_by_val[miss_type] desc = spec['name'] if 'doc' in spec: desc += f" ({spec['doc']})" @@ -289,100 +290,31 @@ class GenlFamily: # -class YnlAttrSpace: - def __init__(self, family, yaml): - self.yaml = yaml - - self.attrs = dict() - self.name = self.yaml['name'] - self.subspace_of = self.yaml['subset-of'] if 'subspace-of' in self.yaml else None - - val = 0 - max_val = 0 - for elem in self.yaml['attributes']: - if 'value' in elem: - val = elem['value'] - else: - elem['value'] = val - if val > max_val: - max_val = val - val += 1 - - self.attrs[elem['name']] = elem - - self.attr_list = [None] * (max_val + 1) - for elem in self.yaml['attributes']: - self.attr_list[elem['value']] = elem - - def __getitem__(self, key): - return self.attrs[key] - - def __contains__(self, key): - return key in self.yaml - - def __iter__(self): - yield from self.attrs - - def items(self): - return self.attrs.items() - - -class YnlFamily: +class YnlFamily(SpecFamily): def __init__(self, def_path, schema=None): + super().__init__(def_path, schema) + self.include_raw = False - with open(def_path, "r") as stream: - self.yaml = yaml.safe_load(stream) - - if schema: - with open(schema, "r") as stream: - schema = yaml.safe_load(stream) - - jsonschema.validate(self.yaml, schema) - self.sock = socket.socket(socket.AF_NETLINK, socket.SOCK_RAW, Netlink.NETLINK_GENERIC) self.sock.setsockopt(Netlink.SOL_NETLINK, Netlink.NETLINK_CAP_ACK, 1) self.sock.setsockopt(Netlink.SOL_NETLINK, Netlink.NETLINK_EXT_ACK, 1) - self._ops = dict() - self._spaces = dict() self._types = dict() - for elem in self.yaml['attribute-sets']: - self._spaces[elem['name']] = YnlAttrSpace(self, elem) - for elem in self.yaml['definitions']: self._types[elem['name']] = elem - async_separation = 'async-prefix' in self.yaml['operations'] self.async_msg_ids = set() self.async_msg_queue = [] - val = 0 - max_val = 0 - for elem in self.yaml['operations']['list']: - if not (async_separation and ('notify' in elem or 'event' in elem)): - if 'value' in elem: - val = elem['value'] - else: - elem['value'] = val - val += 1 - max_val = max(val, max_val) - if 'notify' in elem or 'event' in elem: - self.async_msg_ids.add(elem['value']) + for msg in self.msgs.values(): + if msg.is_async: + self.async_msg_ids.add(msg.value) - self._ops[elem['name']] = elem - - op_name = elem['name'].replace('-', '_') - - bound_f = functools.partial(self._op, elem['name']) - setattr(self, op_name, bound_f) - - self._op_array = [None] * max_val - for _, op in self._ops.items(): - self._op_array[op['value']] = op - if 'notify' in op: - op['attribute-set'] = self._ops[op['notify']]['attribute-set'] + for op_name, op in self.ops.items(): + bound_f = functools.partial(self._op, op_name) + setattr(self, op.ident_name, bound_f) self.family = GenlFamily(self.yaml['name']) @@ -395,8 +327,8 @@ class YnlFamily: self.family.genl_family['mcast'][mcast_name]) def _add_attr(self, space, name, value): - attr = self._spaces[space][name] - nl_type = attr['value'] + attr = self.attr_sets[space][name] + nl_type = attr.value if attr["type"] == 'nest': nl_type |= Netlink.NLA_F_NESTED attr_payload = b'' @@ -430,10 +362,10 @@ class YnlFamily: rsp[attr_spec['name']] = value def _decode(self, attrs, space): - attr_space = self._spaces[space] + attr_space = self.attr_sets[space] rsp = dict() for attr in attrs: - attr_spec = attr_space.attr_list[attr.type] + attr_spec = attr_space.attrs_by_val[attr.type] if attr_spec["type"] == 'nest': subdict = self._decode(NlAttrs(attr.raw), attr_spec['nested-attributes']) rsp[attr_spec['name']] = subdict @@ -457,9 +389,9 @@ class YnlFamily: if self.include_raw: msg['nlmsg'] = nl_msg msg['genlmsg'] = genl_msg - op = self._op_array[genl_msg.genl_cmd] + op = self.msgs_by_value[genl_msg.genl_cmd] msg['name'] = op['name'] - msg['msg'] = self._decode(genl_msg.raw_attrs, op['attribute-set']) + msg['msg'] = self._decode(genl_msg.raw_attrs, op.attr_set.name) self.async_msg_queue.append(msg) def check_ntf(self): @@ -487,16 +419,16 @@ class YnlFamily: self.handle_ntf(nl_msg, gm) def _op(self, method, vals, dump=False): - op = self._ops[method] + op = self.ops[method] nl_flags = Netlink.NLM_F_REQUEST | Netlink.NLM_F_ACK if dump: nl_flags |= Netlink.NLM_F_DUMP req_seq = random.randint(1024, 65535) - msg = _genl_msg(self.family.family_id, nl_flags, op['value'], 1, req_seq) + msg = _genl_msg(self.family.family_id, nl_flags, op.value, 1, req_seq) for name, value in vals.items(): - msg += self._add_attr(op['attribute-set'], name, value) + msg += self._add_attr(op.attr_set.name, name, value) msg = _genl_msg_finalize(msg) self.sock.send(msg, 0) @@ -505,7 +437,7 @@ class YnlFamily: rsp = [] while not done: reply = self.sock.recv(128 * 1024) - nms = NlMsgs(reply, attr_space=self._spaces[op['attribute-set']]) + nms = NlMsgs(reply, attr_space=op.attr_set) for nl_msg in nms: if nl_msg.error: print("Netlink error:", os.strerror(-nl_msg.error)) @@ -517,7 +449,7 @@ class YnlFamily: gm = GenlMsg(nl_msg) # Check if this is a reply to our request - if nl_msg.nl_seq != req_seq or gm.genl_cmd != op['value']: + if nl_msg.nl_seq != req_seq or gm.genl_cmd != op.value: if gm.genl_cmd in self.async_msg_ids: self.handle_ntf(nl_msg, gm) continue @@ -525,7 +457,7 @@ class YnlFamily: print('Unexpected message: ' + repr(gm)) continue - rsp.append(self._decode(gm.raw_attrs, op['attribute-set'])) + rsp.append(self._decode(gm.raw_attrs, op.attr_set.name)) if not rsp: return None diff --git a/tools/net/ynl/ynl-gen-c.py b/tools/net/ynl/ynl-gen-c.py index e5002d420961..dc14da634e8e 100755 --- a/tools/net/ynl/ynl-gen-c.py +++ b/tools/net/ynl/ynl-gen-c.py @@ -2,10 +2,11 @@ import argparse import collections -import jsonschema import os import yaml +from lib import SpecFamily, SpecAttrSet, SpecAttr, SpecOperation + def c_upper(name): return name.upper().replace('-', '_') @@ -28,12 +29,12 @@ class BaseNlLib: "ynl_cb_array, NLMSG_MIN_TYPE)" -class Type: - def __init__(self, family, attr_set, attr): - self.family = family +class Type(SpecAttr): + def __init__(self, family, attr_set, attr, value): + super().__init__(family, attr_set, attr, value) + self.attr = attr - self.value = attr['value'] - self.name = c_lower(attr['name']) + self.attr_set = attr_set self.type = attr['type'] self.checks = attr.get('checks', {}) @@ -46,17 +47,17 @@ class Type: else: self.nested_render_name = f"{family.name}_{c_lower(self.nested_attrs)}" - self.enum_name = f"{attr_set.name_prefix}{self.name}" - self.enum_name = c_upper(self.enum_name) self.c_name = c_lower(self.name) if self.c_name in _C_KW: self.c_name += '_' - def __getitem__(self, key): - return self.attr[key] + # Added by resolve(): + self.enum_name = None + delattr(self, "enum_name") - def __contains__(self, key): - return key in self.attr + def resolve(self): + self.enum_name = f"{self.attr_set.name_prefix}{self.name}" + self.enum_name = c_upper(self.enum_name) def is_multi_val(self): return None @@ -214,24 +215,34 @@ class TypePad(Type): class TypeScalar(Type): - def __init__(self, family, attr_set, attr): - super().__init__(family, attr_set, attr) - - self.is_bitfield = False - if 'enum' in self.attr: - self.is_bitfield = family.consts[self.attr['enum']]['type'] == 'flags' - if 'enum-as-flags' in self.attr and self.attr['enum-as-flags']: - self.is_bitfield = True - - if 'enum' in self.attr and not self.is_bitfield: - self.type_name = f"enum {family.name}_{c_lower(self.attr['enum'])}" - else: - self.type_name = '__' + self.type + def __init__(self, family, attr_set, attr, value): + super().__init__(family, attr_set, attr, value) self.byte_order_comment = '' if 'byte-order' in attr: self.byte_order_comment = f" /* {attr['byte-order']} */" + # Added by resolve(): + self.is_bitfield = None + delattr(self, "is_bitfield") + self.type_name = None + delattr(self, "type_name") + + def resolve(self): + self.resolve_up(super()) + + if 'enum-as-flags' in self.attr and self.attr['enum-as-flags']: + self.is_bitfield = True + elif 'enum' in self.attr: + self.is_bitfield = self.family.consts[self.attr['enum']]['type'] == 'flags' + else: + self.is_bitfield = False + + if 'enum' in self.attr and not self.is_bitfield: + self.type_name = f"enum {self.family.name}_{c_lower(self.attr['enum'])}" + else: + self.type_name = '__' + self.type + def _mnl_type(self): t = self.type # mnl does not have a helper for signed types @@ -648,14 +659,11 @@ class EnumSet: return mask -class AttrSet: +class AttrSet(SpecAttrSet): def __init__(self, family, yaml): - self.yaml = yaml + super().__init__(family, yaml) - self.attrs = dict() - self.name = self.yaml['name'] - if 'subset-of' not in yaml: - self.subset_of = None + if self.subset_of is None: if 'name-prefix' in yaml: pfx = yaml['name-prefix'] elif self.name == family.name: @@ -665,83 +673,68 @@ class AttrSet: self.name_prefix = c_upper(pfx) self.max_name = c_upper(self.yaml.get('attr-max-name', f"{self.name_prefix}max")) else: - self.subset_of = self.yaml['subset-of'] self.name_prefix = family.attr_sets[self.subset_of].name_prefix self.max_name = family.attr_sets[self.subset_of].max_name + # Added by resolve: + self.c_name = None + delattr(self, "c_name") + + def resolve(self): self.c_name = c_lower(self.name) if self.c_name in _C_KW: self.c_name += '_' - if self.c_name == family.c_name: + if self.c_name == self.family.c_name: self.c_name = '' - val = 0 - for elem in self.yaml['attributes']: - if 'value' in elem: - val = elem['value'] - else: - elem['value'] = val - val += 1 - - if 'multi-attr' in elem and elem['multi-attr']: - attr = TypeMultiAttr(family, self, elem) - elif elem['type'] in scalars: - attr = TypeScalar(family, self, elem) - elif elem['type'] == 'unused': - attr = TypeUnused(family, self, elem) - elif elem['type'] == 'pad': - attr = TypePad(family, self, elem) - elif elem['type'] == 'flag': - attr = TypeFlag(family, self, elem) - elif elem['type'] == 'string': - attr = TypeString(family, self, elem) - elif elem['type'] == 'binary': - attr = TypeBinary(family, self, elem) - elif elem['type'] == 'nest': - attr = TypeNest(family, self, elem) - elif elem['type'] == 'array-nest': - attr = TypeArrayNest(family, self, elem) - elif elem['type'] == 'nest-type-value': - attr = TypeNestTypeValue(family, self, elem) - else: - raise Exception(f"No typed class for type {elem['type']}") - - self.attrs[elem['name']] = attr - - def __getitem__(self, key): - return self.attrs[key] - - def __contains__(self, key): - return key in self.yaml - - def __iter__(self): - yield from self.attrs - - def items(self): - return self.attrs.items() - - -class Operation: - def __init__(self, family, yaml, value): - self.yaml = yaml - self.value = value - - self.name = self.yaml['name'] - self.render_name = family.name + '_' + c_lower(self.name) - self.is_async = 'notify' in yaml or 'event' in yaml - if not self.is_async: - self.enum_name = family.op_prefix + c_upper(self.name) + def new_attr(self, elem, value): + if 'multi-attr' in elem and elem['multi-attr']: + return TypeMultiAttr(self.family, self, elem, value) + elif elem['type'] in scalars: + return TypeScalar(self.family, self, elem, value) + elif elem['type'] == 'unused': + return TypeUnused(self.family, self, elem, value) + elif elem['type'] == 'pad': + return TypePad(self.family, self, elem, value) + elif elem['type'] == 'flag': + return TypeFlag(self.family, self, elem, value) + elif elem['type'] == 'string': + return TypeString(self.family, self, elem, value) + elif elem['type'] == 'binary': + return TypeBinary(self.family, self, elem, value) + elif elem['type'] == 'nest': + return TypeNest(self.family, self, elem, value) + elif elem['type'] == 'array-nest': + return TypeArrayNest(self.family, self, elem, value) + elif elem['type'] == 'nest-type-value': + return TypeNestTypeValue(self.family, self, elem, value) else: - self.enum_name = family.async_op_prefix + c_upper(self.name) + raise Exception(f"No typed class for type {elem['type']}") + + +class Operation(SpecOperation): + def __init__(self, family, yaml, req_value, rsp_value): + super().__init__(family, yaml, req_value, rsp_value) + + if req_value != rsp_value: + raise Exception("Directional messages not supported by codegen") + + self.render_name = family.name + '_' + c_lower(self.name) self.dual_policy = ('do' in yaml and 'request' in yaml['do']) and \ ('dump' in yaml and 'request' in yaml['dump']) - def __getitem__(self, key): - return self.yaml[key] + # Added by resolve: + self.enum_name = None + delattr(self, "enum_name") - def __contains__(self, key): - return key in self.yaml + def resolve(self): + self.resolve_up(super()) + + if not self.is_async: + self.enum_name = self.family.op_prefix + c_upper(self.name) + else: + self.enum_name = self.family.async_op_prefix + c_upper(self.name) def add_notification(self, op): if 'notify' not in self.yaml: @@ -751,21 +744,23 @@ class Operation: self.yaml['notify']['cmds'].append(op) -class Family: +class Family(SpecFamily): def __init__(self, file_name): - with open(file_name, "r") as stream: - self.yaml = yaml.safe_load(stream) + # Added by resolve: + self.c_name = None + delattr(self, "c_name") + self.op_prefix = None + delattr(self, "op_prefix") + self.async_op_prefix = None + delattr(self, "async_op_prefix") + self.mcgrps = None + delattr(self, "mcgrps") + self.consts = None + delattr(self, "consts") + self.hooks = None + delattr(self, "hooks") - self.proto = self.yaml.get('protocol', 'genetlink') - - with open(os.path.dirname(os.path.dirname(file_name)) + - f'/{self.proto}.yaml', "r") as stream: - schema = yaml.safe_load(stream) - - jsonschema.validate(self.yaml, schema) - - if self.yaml.get('protocol', 'genetlink') not in {'genetlink', 'genetlink-c', 'genetlink-legacy'}: - raise Exception("Codegen only supported for genetlink") + super().__init__(file_name) self.fam_key = c_upper(self.yaml.get('c-family-name', self.yaml["name"] + '_FAMILY_NAME')) self.ver_key = c_upper(self.yaml.get('c-version-name', self.yaml["name"] + '_FAMILY_VERSION')) @@ -773,12 +768,18 @@ class Family: if 'definitions' not in self.yaml: self.yaml['definitions'] = [] - self.name = self.yaml['name'] - self.c_name = c_lower(self.name) if 'uapi-header' in self.yaml: self.uapi_header = self.yaml['uapi-header'] else: self.uapi_header = f"linux/{self.name}.h" + + def resolve(self): + self.resolve_up(super()) + + if self.yaml.get('protocol', 'genetlink') not in {'genetlink', 'genetlink-c', 'genetlink-legacy'}: + raise Exception("Codegen only supported for genetlink") + + self.c_name = c_lower(self.name) if 'name-prefix' in self.yaml['operations']: self.op_prefix = c_upper(self.yaml['operations']['name-prefix']) else: @@ -791,12 +792,6 @@ class Family: self.mcgrps = self.yaml.get('mcast-groups', {'list': []}) self.consts = dict() - # list of all operations - self.msg_list = [] - # dict of operations which have their own message type (have attributes) - self.ops = collections.OrderedDict() - self.attr_sets = dict() - self.attr_sets_list = [] self.hooks = dict() for when in ['pre', 'post']: @@ -824,11 +819,11 @@ class Family: if self.kernel_policy == 'global': self._load_global_policy() - def __getitem__(self, key): - return self.yaml[key] + def new_attr_set(self, elem): + return AttrSet(self, elem) - def get(self, key, default=None): - return self.yaml.get(key, default) + def new_operation(self, elem, req_value, rsp_value): + return Operation(self, elem, req_value, rsp_value) # Fake a 'do' equivalent of all events, so that we can render their response parsing def _mock_up_events(self): @@ -847,27 +842,10 @@ class Family: else: self.consts[elem['name']] = elem - for elem in self.yaml['attribute-sets']: - attr_set = AttrSet(self, elem) - self.attr_sets[elem['name']] = attr_set - self.attr_sets_list.append((elem['name'], attr_set), ) - ntf = [] - val = 0 - for elem in self.yaml['operations']['list']: - if 'value' in elem: - val = elem['value'] - - op = Operation(self, elem, val) - val += 1 - - self.msg_list.append(op) - if 'notify' in elem: - ntf.append(op) - continue - if 'attribute-set' not in elem: - continue - self.ops[elem['name']] = op + for msg in self.msgs.values(): + if 'notify' in msg: + ntf.append(msg) for n in ntf: self.ops[n['notify']].add_notification(n) @@ -2033,7 +2011,7 @@ def render_uapi(family, cw): max_by_define = family.get('max-by-define', False) - for _, attr_set in family.attr_sets_list: + for _, attr_set in family.attr_sets.items(): if attr_set.subset_of: continue @@ -2044,9 +2022,9 @@ def render_uapi(family, cw): uapi_enum_start(family, cw, attr_set.yaml, 'enum-name') for _, attr in attr_set.items(): suffix = ',' - if attr['value'] != val: - suffix = f" = {attr['value']}," - val = attr['value'] + if attr.value != val: + suffix = f" = {attr.value}," + val = attr.value val += 1 cw.p(attr.enum_name + suffix) cw.nl() @@ -2066,7 +2044,7 @@ def render_uapi(family, cw): max_value = f"({cnt_name} - 1)" uapi_enum_start(family, cw, family['operations'], 'enum-name') - for op in family.msg_list: + for op in family.msgs.values(): if separate_ntf and ('notify' in op or 'event' in op): continue @@ -2085,7 +2063,7 @@ def render_uapi(family, cw): if separate_ntf: uapi_enum_start(family, cw, family['operations'], enum_name='async-enum') - for op in family.msg_list: + for op in family.msgs.values(): if separate_ntf and not ('notify' in op or 'event' in op): continue From 19b64b48a33e986dd21430e759629af2562373b2 Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Mon, 30 Jan 2023 18:33:45 -0800 Subject: [PATCH 05/14] tools: ynl: add support for types needed by ethtool Ethtool needs support for handful of extra types. It doesn't have the definitions section yet. Signed-off-by: Jakub Kicinski --- tools/net/ynl/lib/ynl.py | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/tools/net/ynl/lib/ynl.py b/tools/net/ynl/lib/ynl.py index 0ceb627ba686..a656b655d302 100644 --- a/tools/net/ynl/lib/ynl.py +++ b/tools/net/ynl/lib/ynl.py @@ -75,6 +75,9 @@ class NlAttr: self.full_len = (self.payload_len + 3) & ~3 self.raw = raw[offset + 4:offset + self.payload_len] + def as_u8(self): + return struct.unpack("B", self.raw)[0] + def as_u16(self): return struct.unpack("H", self.raw)[0] @@ -302,7 +305,7 @@ class YnlFamily(SpecFamily): self._types = dict() - for elem in self.yaml['definitions']: + for elem in self.yaml.get('definitions', []): self._types[elem['name']] = elem self.async_msg_ids = set() @@ -334,6 +337,8 @@ class YnlFamily(SpecFamily): attr_payload = b'' for subname, subvalue in value.items(): attr_payload += self._add_attr(attr['nested-attributes'], subname, subvalue) + elif attr["type"] == 'flag': + attr_payload = b'' elif attr["type"] == 'u32': attr_payload = struct.pack("I", int(value)) elif attr["type"] == 'string': @@ -369,6 +374,8 @@ class YnlFamily(SpecFamily): if attr_spec["type"] == 'nest': subdict = self._decode(NlAttrs(attr.raw), attr_spec['nested-attributes']) rsp[attr_spec['name']] = subdict + elif attr_spec['type'] == 'u8': + rsp[attr_spec['name']] = attr.as_u8() elif attr_spec['type'] == 'u32': rsp[attr_spec['name']] = attr.as_u32() elif attr_spec['type'] == 'u64': @@ -377,6 +384,8 @@ class YnlFamily(SpecFamily): rsp[attr_spec['name']] = attr.as_strz() elif attr_spec["type"] == 'binary': rsp[attr_spec['name']] = attr.as_bin() + elif attr_spec["type"] == 'flag': + rsp[attr_spec['name']] = True else: raise Exception(f'Unknown {attr.type} {attr_spec["name"]} {attr_spec["type"]}') From fd0616d34274fb489e3ea9aed089414d533b58e6 Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Mon, 30 Jan 2023 18:33:46 -0800 Subject: [PATCH 06/14] tools: ynl: support directional enum-model in CLI Support families which use different IDs for messages to and from the kernel. Signed-off-by: Jakub Kicinski --- tools/net/ynl/lib/ynl.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tools/net/ynl/lib/ynl.py b/tools/net/ynl/lib/ynl.py index a656b655d302..690065003935 100644 --- a/tools/net/ynl/lib/ynl.py +++ b/tools/net/ynl/lib/ynl.py @@ -313,7 +313,7 @@ class YnlFamily(SpecFamily): for msg in self.msgs.values(): if msg.is_async: - self.async_msg_ids.add(msg.value) + self.async_msg_ids.add(msg.rsp_value) for op_name, op in self.ops.items(): bound_f = functools.partial(self._op, op_name) @@ -398,7 +398,7 @@ class YnlFamily(SpecFamily): if self.include_raw: msg['nlmsg'] = nl_msg msg['genlmsg'] = genl_msg - op = self.msgs_by_value[genl_msg.genl_cmd] + op = self.rsp_by_value[genl_msg.genl_cmd] msg['name'] = op['name'] msg['msg'] = self._decode(genl_msg.raw_attrs, op.attr_set.name) self.async_msg_queue.append(msg) @@ -435,7 +435,7 @@ class YnlFamily(SpecFamily): nl_flags |= Netlink.NLM_F_DUMP req_seq = random.randint(1024, 65535) - msg = _genl_msg(self.family.family_id, nl_flags, op.value, 1, req_seq) + msg = _genl_msg(self.family.family_id, nl_flags, op.req_value, 1, req_seq) for name, value in vals.items(): msg += self._add_attr(op.attr_set.name, name, value) msg = _genl_msg_finalize(msg) @@ -458,7 +458,7 @@ class YnlFamily(SpecFamily): gm = GenlMsg(nl_msg) # Check if this is a reply to our request - if nl_msg.nl_seq != req_seq or gm.genl_cmd != op.value: + if nl_msg.nl_seq != req_seq or gm.genl_cmd != op.rsp_value: if gm.genl_cmd in self.async_msg_ids: self.handle_ntf(nl_msg, gm) continue From 90256f3f8093284ef4f162229438ff1eea3a928e Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Mon, 30 Jan 2023 18:33:47 -0800 Subject: [PATCH 07/14] tools: ynl: support multi-attr Ethtool uses mutli-attr, add the support to YNL. Signed-off-by: Jakub Kicinski --- tools/net/ynl/lib/ynl.py | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/tools/net/ynl/lib/ynl.py b/tools/net/ynl/lib/ynl.py index 690065003935..c16326495cb7 100644 --- a/tools/net/ynl/lib/ynl.py +++ b/tools/net/ynl/lib/ynl.py @@ -373,22 +373,29 @@ class YnlFamily(SpecFamily): attr_spec = attr_space.attrs_by_val[attr.type] if attr_spec["type"] == 'nest': subdict = self._decode(NlAttrs(attr.raw), attr_spec['nested-attributes']) - rsp[attr_spec['name']] = subdict + decoded = subdict elif attr_spec['type'] == 'u8': - rsp[attr_spec['name']] = attr.as_u8() + decoded = attr.as_u8() elif attr_spec['type'] == 'u32': - rsp[attr_spec['name']] = attr.as_u32() + decoded = attr.as_u32() elif attr_spec['type'] == 'u64': - rsp[attr_spec['name']] = attr.as_u64() + decoded = attr.as_u64() elif attr_spec["type"] == 'string': - rsp[attr_spec['name']] = attr.as_strz() + decoded = attr.as_strz() elif attr_spec["type"] == 'binary': - rsp[attr_spec['name']] = attr.as_bin() + decoded = attr.as_bin() elif attr_spec["type"] == 'flag': - rsp[attr_spec['name']] = True + decoded = True else: raise Exception(f'Unknown {attr.type} {attr_spec["name"]} {attr_spec["type"]}') + if not attr_spec.is_multi: + rsp[attr_spec['name']] = decoded + elif attr_spec.name in rsp: + rsp[attr_spec.name].append(decoded) + else: + rsp[attr_spec.name] = [decoded] + if 'enum' in attr_spec: self._decode_enum(rsp, attr_spec) return rsp From 4cd2796f3f8d82aa3a13d3eca39950fe6fe9d672 Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Mon, 30 Jan 2023 18:33:48 -0800 Subject: [PATCH 08/14] tools: ynl: support pretty printing bad attribute names One of my favorite features of the Netlink specs is that they make decoding structured extack a ton easier. Implement pretty printing bad attribute names in YNL. For example it will now say: 'bad-attr': '.header.flags' rather than the useless: 'bad-attr-offs': 32 Proof: $ ./cli.py --spec ethtool.yaml --do rings_get \ --json '{"header":{"dev-index":1, "flags":4}}' Netlink error: Invalid argument nl_len = 68 (52) nl_flags = 0x300 nl_type = 2 error: -22 extack: {'msg': 'reserved bit set', 'bad-attr': '.header.flags'} Signed-off-by: Jakub Kicinski --- tools/net/ynl/lib/ynl.py | 40 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/tools/net/ynl/lib/ynl.py b/tools/net/ynl/lib/ynl.py index c16326495cb7..2ff3e6dbdbf6 100644 --- a/tools/net/ynl/lib/ynl.py +++ b/tools/net/ynl/lib/ynl.py @@ -400,6 +400,40 @@ class YnlFamily(SpecFamily): self._decode_enum(rsp, attr_spec) return rsp + def _decode_extack_path(self, attrs, attr_set, offset, target): + for attr in attrs: + attr_spec = attr_set.attrs_by_val[attr.type] + if offset > target: + break + if offset == target: + return '.' + attr_spec.name + + if offset + attr.full_len <= target: + offset += attr.full_len + continue + if attr_spec['type'] != 'nest': + raise Exception(f"Can't dive into {attr.type} ({attr_spec['name']}) for extack") + offset += 4 + subpath = self._decode_extack_path(NlAttrs(attr.raw), + self.attr_sets[attr_spec['nested-attributes']], + offset, target) + if subpath is None: + return None + return '.' + attr_spec.name + subpath + + return None + + def _decode_extack(self, request, attr_space, extack): + if 'bad-attr-offs' not in extack: + return + + genl_req = GenlMsg(NlMsg(request, 0, attr_space=attr_space)) + path = self._decode_extack_path(genl_req.raw_attrs, attr_space, + 20, extack['bad-attr-offs']) + if path: + del extack['bad-attr-offs'] + extack['bad-attr'] = path + def handle_ntf(self, nl_msg, genl_msg): msg = dict() if self.include_raw: @@ -455,11 +489,17 @@ class YnlFamily(SpecFamily): reply = self.sock.recv(128 * 1024) nms = NlMsgs(reply, attr_space=op.attr_set) for nl_msg in nms: + if nl_msg.extack: + self._decode_extack(msg, op.attr_set, nl_msg.extack) + if nl_msg.error: print("Netlink error:", os.strerror(-nl_msg.error)) print(nl_msg) return if nl_msg.done: + if nl_msg.extack: + print("Netlink warning:") + print(nl_msg) done = True break From 8dfec0a8886880868802094967c6a769b6d15737 Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Mon, 30 Jan 2023 18:33:49 -0800 Subject: [PATCH 09/14] tools: ynl: use operation names from spec on the CLI When I wrote the first version of the Python code I was quite excited that we can generate class methods directly from the spec. Unfortunately we need to use valid identifiers for method names (specifically no dashes are allowed). Don't reuse those names on the CLI, it's much more natural to use the operation names exactly as listed in the spec. Instead of: ./cli --do rings_get use: ./cli --do rings-get Signed-off-by: Jakub Kicinski --- tools/net/ynl/cli.py | 9 +++++---- tools/net/ynl/lib/ynl.py | 6 ++++++ 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/tools/net/ynl/cli.py b/tools/net/ynl/cli.py index 5c4eb5a68514..05d1f4069ce1 100755 --- a/tools/net/ynl/cli.py +++ b/tools/net/ynl/cli.py @@ -32,10 +32,11 @@ def main(): if args.sleep: time.sleep(args.sleep) - if args.do or args.dump: - method = getattr(ynl, args.do if args.do else args.dump) - - reply = method(attrs, dump=bool(args.dump)) + if args.do: + reply = ynl.do(args.do, attrs) + pprint.PrettyPrinter().pprint(reply) + if args.dump: + reply = ynl.dump(args.dump, attrs) pprint.PrettyPrinter().pprint(reply) if args.ntf: diff --git a/tools/net/ynl/lib/ynl.py b/tools/net/ynl/lib/ynl.py index 2ff3e6dbdbf6..1c7411ee04dc 100644 --- a/tools/net/ynl/lib/ynl.py +++ b/tools/net/ynl/lib/ynl.py @@ -520,3 +520,9 @@ class YnlFamily(SpecFamily): if not dump and len(rsp) == 1: return rsp[0] return rsp + + def do(self, method, vals): + return self._op(method, vals) + + def dump(self, method, vals): + return self._op(method, vals, dump=True) From 5c6674f6eb52f7968b805b25c7478b3d96b6b4f7 Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Mon, 30 Jan 2023 18:33:50 -0800 Subject: [PATCH 10/14] tools: ynl: load jsonschema on demand The CLI script tries to validate jsonschema by default. It's seems better to validate too many times than too few. However, when copying the scripts to random servers having to install jsonschema is tedious. Load jsonschema via importlib, and let the user opt out. Signed-off-by: Jakub Kicinski --- tools/net/ynl/cli.py | 4 ++++ tools/net/ynl/lib/nlspec.py | 11 ++++++++++- 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/tools/net/ynl/cli.py b/tools/net/ynl/cli.py index 05d1f4069ce1..e64f1478764f 100755 --- a/tools/net/ynl/cli.py +++ b/tools/net/ynl/cli.py @@ -13,6 +13,7 @@ def main(): parser = argparse.ArgumentParser(description='YNL CLI sample') parser.add_argument('--spec', dest='spec', type=str, required=True) parser.add_argument('--schema', dest='schema', type=str) + parser.add_argument('--no-schema', action='store_true') parser.add_argument('--json', dest='json_text', type=str) parser.add_argument('--do', dest='do', type=str) parser.add_argument('--dump', dest='dump', type=str) @@ -20,6 +21,9 @@ def main(): parser.add_argument('--subscribe', dest='ntf', type=str) args = parser.parse_args() + if args.no_schema: + args.schema = '' + attrs = {} if args.json_text: attrs = json.loads(args.json_text) diff --git a/tools/net/ynl/lib/nlspec.py b/tools/net/ynl/lib/nlspec.py index 4aa3b1ad97f0..e204679ad8b7 100644 --- a/tools/net/ynl/lib/nlspec.py +++ b/tools/net/ynl/lib/nlspec.py @@ -1,12 +1,16 @@ # SPDX-License-Identifier: BSD-3-Clause import collections -import jsonschema +import importlib import os import traceback import yaml +# To be loaded dynamically as needed +jsonschema = None + + class SpecElement: """Netlink spec element. @@ -197,9 +201,14 @@ class SpecFamily(SpecElement): if schema_path is None: schema_path = os.path.dirname(os.path.dirname(spec_path)) + f'/{self.proto}.yaml' if schema_path: + global jsonschema + with open(schema_path, "r") as stream: schema = yaml.safe_load(stream) + if jsonschema is None: + jsonschema = importlib.import_module("jsonschema") + jsonschema.validate(self.yaml, schema) self.attr_sets = collections.OrderedDict() From 8403bf04453044397219bc70715a3a94730199cc Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Mon, 30 Jan 2023 18:33:51 -0800 Subject: [PATCH 11/14] netlink: specs: finish up operation enum-models I had a (bright?) idea of introducing the concept of enum-models to account for all the weird ways families enumerate their messages. I've never finished it because generating C code for each of them is pretty daunting. But for languages which can use ID values directly the support is simple enough, so clean this up a bit. "unified" model is what I recommend going forward. "directional" model is what ethtool uses. "notify-split" is used by the proposed DPLL code, but we can just make them use "unified", it hasn't been merged :) Signed-off-by: Jakub Kicinski --- Documentation/netlink/genetlink-c.yaml | 4 +- Documentation/netlink/genetlink-legacy.yaml | 11 ++- Documentation/netlink/genetlink.yaml | 4 +- .../netlink/genetlink-legacy.rst | 82 +++++++++++++++++++ 4 files changed, 92 insertions(+), 9 deletions(-) diff --git a/Documentation/netlink/genetlink-c.yaml b/Documentation/netlink/genetlink-c.yaml index e23e3c94a932..bbcfa2472b04 100644 --- a/Documentation/netlink/genetlink-c.yaml +++ b/Documentation/netlink/genetlink-c.yaml @@ -218,9 +218,7 @@ properties: to a single enum. "directional" has the messages sent to the kernel and from the kernel enumerated separately. - "notify-split" has the notifications and request-response types in - different enums. - enum: [ unified, directional, notify-split ] + enum: [ unified ] name-prefix: description: | Prefix for the C enum name of the command. The name is formed by concatenating diff --git a/Documentation/netlink/genetlink-legacy.yaml b/Documentation/netlink/genetlink-legacy.yaml index 88db2431ef26..5642925c4ceb 100644 --- a/Documentation/netlink/genetlink-legacy.yaml +++ b/Documentation/netlink/genetlink-legacy.yaml @@ -241,9 +241,7 @@ properties: to a single enum. "directional" has the messages sent to the kernel and from the kernel enumerated separately. - "notify-split" has the notifications and request-response types in - different enums. - enum: [ unified, directional, notify-split ] + enum: [ unified, directional ] # Trim name-prefix: description: | Prefix for the C enum name of the command. The name is formed by concatenating @@ -307,6 +305,13 @@ properties: type: array items: type: string + # Start genetlink-legacy + value: + description: | + ID of this message if value for request and response differ, + i.e. requests and responses have different message enums. + $ref: '#/$defs/uint' + # End genetlink-legacy reply: *subop-attr-list pre: description: Hook for a function to run before the main callback (pre_doit or start). diff --git a/Documentation/netlink/genetlink.yaml b/Documentation/netlink/genetlink.yaml index b5e712bbe7e7..62a922755ce2 100644 --- a/Documentation/netlink/genetlink.yaml +++ b/Documentation/netlink/genetlink.yaml @@ -188,9 +188,7 @@ properties: to a single enum. "directional" has the messages sent to the kernel and from the kernel enumerated separately. - "notify-split" has the notifications and request-response types in - different enums. - enum: [ unified, directional, notify-split ] + enum: [ unified ] name-prefix: description: | Prefix for the C enum name of the command. The name is formed by concatenating diff --git a/Documentation/userspace-api/netlink/genetlink-legacy.rst b/Documentation/userspace-api/netlink/genetlink-legacy.rst index 65cbbffee0bf..3bf0bcdf21d8 100644 --- a/Documentation/userspace-api/netlink/genetlink-legacy.rst +++ b/Documentation/userspace-api/netlink/genetlink-legacy.rst @@ -74,6 +74,88 @@ type. Inside the attr-index nest are the policy attributes. Modern Netlink families should have instead defined this as a flat structure, the nesting serves no good purpose here. +Operations +========== + +Enum (message ID) model +----------------------- + +unified +~~~~~~~ + +Modern families use the ``unified`` message ID model, which uses +a single enumeration for all messages within family. Requests and +responses share the same message ID. Notifications have separate +IDs from the same space. For example given the following list +of operations: + +.. code-block:: yaml + + - + name: a + value: 1 + do: ... + - + name: b + do: ... + - + name: c + value: 4 + notify: a + - + name: d + do: ... + +Requests and responses for operation ``a`` will have the ID of 1, +the requests and responses of ``b`` - 2 (since there is no explicit +``value`` it's previous operation ``+ 1``). Notification ``c`` will +use the ID of 4, operation ``d`` 5 etc. + +directional +~~~~~~~~~~~ + +The ``directional`` model splits the ID assignment by the direction of +the message. Messages from and to the kernel can't be confused with +each other so this conserves the ID space (at the cost of making +the programming more cumbersome). + +In this case ``value`` attribute should be specified in the ``request`` +``reply`` sections of the operations (if an operation has both ``do`` +and ``dump`` the IDs are shared, ``value`` should be set in ``do``). +For notifications the ``value`` is provided at the op level but it +only allocates a ``reply`` (i.e. a "from-kernel" ID). Let's look +at an example: + +.. code-block:: yaml + + - + name: a + do: + request: + value: 2 + attributes: ... + reply: + value: 1 + attributes: ... + - + name: b + notify: a + - + name: c + notify: a + value: 7 + - + name: d + do: ... + +In this case ``a`` will use 2 when sending the message to the kernel +and expects message with ID 1 in response. Notification ``b`` allocates +a "from-kernel" ID which is 2. ``c`` allocates "from-kernel" ID of 7. +If operation ``d`` does not set ``values`` explicitly in the spec +it will be allocated 3 for the request (``a`` is the previous operation +with a request section and the value of 2) and 8 for response (``c`` is +the previous operation in the "from-kernel" direction). + Other quirks (todo) =================== From b784db7ae840811dfb3eea900620f6bb4ae6e0b1 Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Mon, 30 Jan 2023 18:33:52 -0800 Subject: [PATCH 12/14] netlink: specs: add partial specification for ethtool Ethtool is one of the most actively developed families. With the changes to the CLI it should be possible to use the YNL based code for easy prototyping and development. Add a partial family definition. I've tested the string set and rings. I don't have any MAC Merge implementation to test with, but I added the definition for it, anyway, because it's last. New commands can simply be added at the end without having to worry about manually providing IDs / values. Set (with notification support - None is the response, the data is from the notification): $ sudo ./tools/net/ynl/cli.py \ --spec Documentation/netlink/specs/ethtool.yaml \ --do rings-set \ --json '{"header":{"dev-name":"enp0s31f6"}, "rx":129}' \ --subscribe monitor None [{'msg': {'header': {'dev-index': 2, 'dev-name': 'enp0s31f6'}, 'rx': 136, 'rx-max': 4096, 'tx': 256, 'tx-max': 4096, 'tx-push': 0}, 'name': 'rings-ntf'}] Do / dump (yes, the kernel requires that even for dump and even if empty - the "header" nest must be there): $ ./tools/net/ynl/cli.py \ --spec Documentation/netlink/specs/ethtool.yaml \ --do rings-get \ --json '{"header":{"dev-index": 2}}' {'header': {'dev-index': 2, 'dev-name': 'enp0s31f6'}, 'rx': 136, 'rx-max': 4096, 'tx': 256, 'tx-max': 4096, 'tx-push': 0} $ ./tools/net/ynl/cli.py \ --spec Documentation/netlink/specs/ethtool.yaml \ --dump rings-get \ --json '{"header":{}}' [{'header': {'dev-index': 2, 'dev-name': 'enp0s31f6'}, 'rx': 136, 'rx-max': 4096, 'tx': 256, 'tx-max': 4096, 'tx-push': 0}, {'header': {'dev-index': 3, 'dev-name': 'wlp0s20f3'}, 'tx-push': 0}, {'header': {'dev-index': 19, 'dev-name': 'enp58s0u1u1'}, 'rx': 100, 'rx-max': 4096, 'tx-push': 0}] And error reporting: $ ./tools/net/ynl/cli.py \ --spec Documentation/netlink/specs/ethtool.yaml \ --dump rings-get \ --json '{"header":{"flags":5}}' Netlink error: Invalid argument nl_len = 68 (52) nl_flags = 0x300 nl_type = 2 error: -22 extack: {'msg': 'reserved bit set', 'bad-attr-offs': 24, 'bad-attr': '.header.flags'} None Signed-off-by: Jakub Kicinski --- Documentation/netlink/specs/ethtool.yaml | 392 +++++++++++++++++++++++ 1 file changed, 392 insertions(+) create mode 100644 Documentation/netlink/specs/ethtool.yaml diff --git a/Documentation/netlink/specs/ethtool.yaml b/Documentation/netlink/specs/ethtool.yaml new file mode 100644 index 000000000000..82f4e6f8ddd3 --- /dev/null +++ b/Documentation/netlink/specs/ethtool.yaml @@ -0,0 +1,392 @@ +name: ethtool + +protocol: genetlink-legacy + +doc: Partial family for Ethtool Netlink. + +attribute-sets: + - + name: header + attributes: + - + name: dev-index + type: u32 + value: 1 + - + name: dev-name + type: string + - + name: flags + type: u32 + + - + name: bitset-bit + attributes: + - + name: index + type: u32 + value: 1 + - + name: name + type: string + - + name: value + type: flag + - + name: bitset-bits + attributes: + - + name: bit + type: nest + nested-attributes: bitset-bit + value: 1 + - + name: bitset + attributes: + - + name: nomask + type: flag + value: 1 + - + name: size + type: u32 + - + name: bits + type: nest + nested-attributes: bitset-bits + + - + name: string + attributes: + - + name: index + type: u32 + value: 1 + - + name: value + type: string + - + name: strings + attributes: + - + name: string + type: nest + value: 1 + multi-attr: true + nested-attributes: string + - + name: stringset + attributes: + - + name: id + type: u32 + value: 1 + - + name: count + type: u32 + - + name: strings + type: nest + multi-attr: true + nested-attributes: strings + - + name: stringsets + attributes: + - + name: stringset + type: nest + multi-attr: true + value: 1 + nested-attributes: stringset + - + name: strset + attributes: + - + name: header + value: 1 + type: nest + nested-attributes: header + - + name: stringsets + type: nest + nested-attributes: stringsets + - + name: counts-only + type: flag + + - + name: privflags + attributes: + - + name: header + value: 1 + type: nest + nested-attributes: header + - + name: flags + type: nest + nested-attributes: bitset + + - + name: rings + attributes: + - + name: header + value: 1 + type: nest + nested-attributes: header + - + name: rx-max + type: u32 + - + name: rx-mini-max + type: u32 + - + name: rx-jumbo-max + type: u32 + - + name: tx-max + type: u32 + - + name: rx + type: u32 + - + name: rx-mini + type: u32 + - + name: rx-jumbo + type: u32 + - + name: tx + type: u32 + - + name: rx-buf-len + type: u32 + - + name: tcp-data-split + type: u8 + - + name: cqe-size + type: u32 + - + name: tx-push + type: u8 + + - + name: mm-stat + attributes: + - + name: pad + value: 1 + type: pad + - + name: reassembly-errors + type: u64 + - + name: smd-errors + type: u64 + - + name: reassembly-ok + type: u64 + - + name: rx-frag-count + type: u64 + - + name: tx-frag-count + type: u64 + - + name: hold-count + type: u64 + - + name: mm + attributes: + - + name: header + value: 1 + type: nest + nested-attributes: header + - + name: pmac-enabled + type: u8 + - + name: tx-enabled + type: u8 + - + name: tx-active + type: u8 + - + name: tx-min-frag-size + type: u32 + - + name: tx-min-frag-size + type: u32 + - + name: verify-enabled + type: u8 + - + name: verify-status + type: u8 + - + name: verify-time + type: u32 + - + name: max-verify-time + type: u32 + - + name: stats + type: nest + nested-attributes: mm-stat + +operations: + enum-model: directional + list: + - + name: strset-get + doc: Get string set from the kernel. + + attribute-set: strset + + do: &strset-get-op + request: + value: 1 + attributes: + - header + - stringsets + - counts-only + reply: + value: 1 + attributes: + - header + - stringsets + dump: *strset-get-op + + # TODO: fill in the requests in between + + - + name: privflags-get + doc: Get device private flags. + + attribute-set: privflags + + do: &privflag-get-op + request: + value: 13 + attributes: + - header + reply: + value: 14 + attributes: + - header + - flags + dump: *privflag-get-op + - + name: privflags-set + doc: Set device private flags. + + attribute-set: privflags + + do: + request: + attributes: + - header + - flags + - + name: privflags-ntf + doc: Notification for change in device private flags. + notify: privflags-get + + - + name: rings-get + doc: Get ring params. + + attribute-set: rings + + do: &ring-get-op + request: + attributes: + - header + reply: + attributes: + - header + - rx-max + - rx-mini-max + - rx-jumbo-max + - tx-max + - rx + - rx-mini + - rx-jumbo + - tx + - rx-buf-len + - tcp-data-split + - cqe-size + - tx-push + dump: *ring-get-op + - + name: rings-set + doc: Set ring params. + + attribute-set: rings + + do: + request: + attributes: + - header + - rx + - rx-mini + - rx-jumbo + - tx + - rx-buf-len + - tcp-data-split + - cqe-size + - tx-push + - + name: rings-ntf + doc: Notification for change in ring params. + notify: rings-get + + # TODO: fill in the requests in between + + - + name: mm-get + doc: Get MAC Merge configuration and state + + attribute-set: mm + + do: &mm-get-op + request: + value: 42 + attributes: + - header + reply: + value: 42 + attributes: + - header + - pmac-enabled + - tx-enabled + - tx-active + - tx-min-frag-size + - rx-min-frag-size + - verify-enabled + - verify-time + - max-verify-time + - stats + dump: *mm-get-op + - + name: mm-set + doc: Set MAC Merge configuration + + attribute-set: mm + + do: + request: + attributes: + - header + - verify-enabled + - verify-time + - tx-enabled + - pmac-enabled + - tx-min-frag-size + - + name: mm-ntf + doc: Notification for change in MAC Merge configuration. + notify: mm-get From 01e47a372268beaec517dc0ea8d9101550600df4 Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Mon, 30 Jan 2023 18:33:53 -0800 Subject: [PATCH 13/14] docs: netlink: add a starting guide for working with specs We have a bit of documentation about the internals of Netlink and the specs, but really the goal is for most people to not worry about those. Add a practical guide for beginners who want to poke at the specs. Signed-off-by: Jakub Kicinski --- Documentation/userspace-api/netlink/index.rst | 1 + .../userspace-api/netlink/intro-specs.rst | 80 +++++++++++++++++++ Documentation/userspace-api/netlink/specs.rst | 3 + 3 files changed, 84 insertions(+) create mode 100644 Documentation/userspace-api/netlink/intro-specs.rst diff --git a/Documentation/userspace-api/netlink/index.rst b/Documentation/userspace-api/netlink/index.rst index be250110c8f6..26f3720cb3be 100644 --- a/Documentation/userspace-api/netlink/index.rst +++ b/Documentation/userspace-api/netlink/index.rst @@ -10,6 +10,7 @@ Netlink documentation for users. :maxdepth: 2 intro + intro-specs specs c-code-gen genetlink-legacy diff --git a/Documentation/userspace-api/netlink/intro-specs.rst b/Documentation/userspace-api/netlink/intro-specs.rst new file mode 100644 index 000000000000..a3b847eafff7 --- /dev/null +++ b/Documentation/userspace-api/netlink/intro-specs.rst @@ -0,0 +1,80 @@ +.. SPDX-License-Identifier: BSD-3-Clause + +===================================== +Using Netlink protocol specifications +===================================== + +This document is a quick starting guide for using Netlink protocol +specifications. For more detailed description of the specs see :doc:`specs`. + +Simple CLI +========== + +Kernel comes with a simple CLI tool which should be useful when +developing Netlink related code. The tool is implemented in Python +and can use a YAML specification to issue Netlink requests +to the kernel. Only Generic Netlink is supported. + +The tool is located at ``tools/net/ynl/cli.py``. It accepts +a handul of arguments, the most important ones are: + + - ``--spec`` - point to the spec file + - ``--do $name`` / ``--dump $name`` - issue request ``$name`` + - ``--json $attrs`` - provide attributes for the request + - ``--subscribe $group`` - receive notifications from ``$group`` + +YAML specs can be found under ``Documentation/netlink/specs/``. + +Example use:: + + $ ./tools/net/ynl/cli.py --spec Documentation/netlink/specs/ethtool.yaml \ + --do rings-get \ + --json '{"header":{"dev-index": 18}}' + {'header': {'dev-index': 18, 'dev-name': 'eni1np1'}, + 'rx': 0, + 'rx-jumbo': 0, + 'rx-jumbo-max': 4096, + 'rx-max': 4096, + 'rx-mini': 0, + 'rx-mini-max': 4096, + 'tx': 0, + 'tx-max': 4096, + 'tx-push': 0} + +The input arguments are parsed as JSON, while the output is only +Python-pretty-printed. This is because some Netlink types can't +be expressed as JSON directly. If such attributes are needed in +the input some hacking of the script will be necessary. + +The spec and Netlink internals are factored out as a standalone +library - it should be easy to write Python tools / tests reusing +code from ``cli.py``. + +Generating kernel code +====================== + +``tools/net/ynl/ynl-regen.sh`` scans the kernel tree in search of +auto-generated files which need to be updated. Using this tool is the easiest +way to generate / update auto-generated code. + +By default code is re-generated only if spec is newer than the source, +to force regeneration use ``-f``. + +``ynl-regen.sh`` searches for ``YNL-GEN`` in the contents of files +(note that it only scans files in the git index, that is only files +tracked by git!) For instance the ``fou_nl.c`` kernel source contains:: + + /* Documentation/netlink/specs/fou.yaml */ + /* YNL-GEN kernel source */ + +``ynl-regen.sh`` will find this marker and replace the file with +kernel source based on fou.yaml. + +The simplest way to generate a new file based on a spec is to add +the two marker lines like above to a file, add that file to git, +and run the regeneration tool. Grep the tree for ``YNL-GEN`` +to see other examples. + +The code generation itself is performed by ``tools/net/ynl/ynl-gen-c.py`` +but it takes a few arguments so calling it directly for each file +quickly becomes tedious. diff --git a/Documentation/userspace-api/netlink/specs.rst b/Documentation/userspace-api/netlink/specs.rst index 8394d74fc63a..6ffe8137cd90 100644 --- a/Documentation/userspace-api/netlink/specs.rst +++ b/Documentation/userspace-api/netlink/specs.rst @@ -21,6 +21,9 @@ Internally kernel uses the YAML specs to generate: YAML specifications can be found under ``Documentation/netlink/specs/`` +This document describes details of the schema. +See :doc:`intro-specs` for a practical starting guide. + Compatibility levels ==================== From 981cbcb030d919ee49ebbfc2889839c6882d9ea7 Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Mon, 30 Jan 2023 18:33:54 -0800 Subject: [PATCH 14/14] tools: net: use python3 explicitly The scripts require Python 3 and some distros are dropping Python 2 support. Reported-by: Stanislav Fomichev Signed-off-by: Jakub Kicinski --- tools/net/ynl/cli.py | 2 +- tools/net/ynl/ynl-gen-c.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/net/ynl/cli.py b/tools/net/ynl/cli.py index e64f1478764f..db410b74d539 100755 --- a/tools/net/ynl/cli.py +++ b/tools/net/ynl/cli.py @@ -1,4 +1,4 @@ -#!/usr/bin/env python +#!/usr/bin/env python3 # SPDX-License-Identifier: BSD-3-Clause import argparse diff --git a/tools/net/ynl/ynl-gen-c.py b/tools/net/ynl/ynl-gen-c.py index dc14da634e8e..3942f24b9163 100755 --- a/tools/net/ynl/ynl-gen-c.py +++ b/tools/net/ynl/ynl-gen-c.py @@ -1,4 +1,4 @@ -#!/usr/bin/env python +#!/usr/bin/env python3 import argparse import collections