From b334f5ed3d914d46652db2b8aad7d135ad4a50ad Mon Sep 17 00:00:00 2001 From: Hangbin Liu Date: Wed, 27 Mar 2024 20:31:28 +0800 Subject: ynl: support hex display_hint for integer Some times it would be convenient to read the integer as hex, like mask values. Suggested-by: Donald Hunter Reviewed-by: Donald Hunter Signed-off-by: Hangbin Liu Link: https://lore.kernel.org/r/20240327123130.1322921-2-liuhangbin@gmail.com Signed-off-by: Jakub Kicinski --- tools/net/ynl/lib/ynl.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'tools/net/ynl/lib/ynl.py') diff --git a/tools/net/ynl/lib/ynl.py b/tools/net/ynl/lib/ynl.py index 5fa7957f6e0f..e73b027c5624 100644 --- a/tools/net/ynl/lib/ynl.py +++ b/tools/net/ynl/lib/ynl.py @@ -819,7 +819,10 @@ class YnlFamily(SpecFamily): if display_hint == 'mac': formatted = ':'.join('%02x' % b for b in raw) elif display_hint == 'hex': - formatted = bytes.hex(raw, ' ') + if isinstance(raw, int): + formatted = hex(raw) + else: + formatted = bytes.hex(raw, ' ') elif display_hint in [ 'ipv4', 'ipv6' ]: formatted = format(ipaddress.ip_address(raw)) elif display_hint == 'uuid': -- cgit v1.2.3-59-g8ed1b From bd3ce405fecc6f2f9ce09c789386e326346899a0 Mon Sep 17 00:00:00 2001 From: Donald Hunter Date: Thu, 28 Mar 2024 15:56:36 +0000 Subject: tools/net/ynl: Add extack policy attribute decoding The NLMSGERR_ATTR_POLICY extack attribute has been ignored by ynl up to now. Extend extack decoding to include _POLICY and the nested NL_POLICY_TYPE_ATTR_* attributes. For example: ./tools/net/ynl/cli.py \ --spec Documentation/netlink/specs/rt_link.yaml \ --create --do newlink --json '{ "ifname": "12345678901234567890", "linkinfo": {"kind": "bridge"} }' Netlink error: Numerical result out of range nl_len = 104 (88) nl_flags = 0x300 nl_type = 2 error: -34 extack: {'msg': 'Attribute failed policy validation', 'policy': {'max-length': 15, 'type': 'string'}, 'bad-attr': '.ifname'} Signed-off-by: Donald Hunter Link: https://lore.kernel.org/r/20240328155636.64688-1-donald.hunter@gmail.com Signed-off-by: Jakub Kicinski --- tools/net/ynl/lib/ynl.py | 46 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) (limited to 'tools/net/ynl/lib/ynl.py') diff --git a/tools/net/ynl/lib/ynl.py b/tools/net/ynl/lib/ynl.py index e73b027c5624..82d3c98067aa 100644 --- a/tools/net/ynl/lib/ynl.py +++ b/tools/net/ynl/lib/ynl.py @@ -1,6 +1,7 @@ # SPDX-License-Identifier: GPL-2.0 OR BSD-3-Clause from collections import namedtuple +from enum import Enum import functools import os import random @@ -76,6 +77,25 @@ class Netlink: NLMSGERR_ATTR_MISS_TYPE = 5 NLMSGERR_ATTR_MISS_NEST = 6 + # Policy types + NL_POLICY_TYPE_ATTR_TYPE = 1 + NL_POLICY_TYPE_ATTR_MIN_VALUE_S = 2 + NL_POLICY_TYPE_ATTR_MAX_VALUE_S = 3 + NL_POLICY_TYPE_ATTR_MIN_VALUE_U = 4 + NL_POLICY_TYPE_ATTR_MAX_VALUE_U = 5 + NL_POLICY_TYPE_ATTR_MIN_LENGTH = 6 + NL_POLICY_TYPE_ATTR_MAX_LENGTH = 7 + NL_POLICY_TYPE_ATTR_POLICY_IDX = 8 + NL_POLICY_TYPE_ATTR_POLICY_MAXTYPE = 9 + NL_POLICY_TYPE_ATTR_BITFIELD32_MASK = 10 + NL_POLICY_TYPE_ATTR_PAD = 11 + NL_POLICY_TYPE_ATTR_MASK = 12 + + AttrType = Enum('AttrType', ['flag', 'u8', 'u16', 'u32', 'u64', + 's8', 's16', 's32', 's64', + 'binary', 'string', 'nul-string', + 'nested', 'nested-array', + 'bitfield32', 'sint', 'uint']) class NlError(Exception): def __init__(self, nl_msg): @@ -198,6 +218,8 @@ class NlMsg: self.extack['miss-nest'] = extack.as_scalar('u32') elif extack.type == Netlink.NLMSGERR_ATTR_OFFS: self.extack['bad-attr-offs'] = extack.as_scalar('u32') + elif extack.type == Netlink.NLMSGERR_ATTR_POLICY: + self.extack['policy'] = self._decode_policy(extack.raw) else: if 'unknown' not in self.extack: self.extack['unknown'] = [] @@ -214,6 +236,30 @@ class NlMsg: desc += f" ({spec['doc']})" self.extack['miss-type'] = desc + def _decode_policy(self, raw): + policy = {} + for attr in NlAttrs(raw): + if attr.type == Netlink.NL_POLICY_TYPE_ATTR_TYPE: + type = attr.as_scalar('u32') + policy['type'] = Netlink.AttrType(type).name + elif attr.type == Netlink.NL_POLICY_TYPE_ATTR_MIN_VALUE_S: + policy['min-value'] = attr.as_scalar('s64') + elif attr.type == Netlink.NL_POLICY_TYPE_ATTR_MAX_VALUE_S: + policy['max-value'] = attr.as_scalar('s64') + elif attr.type == Netlink.NL_POLICY_TYPE_ATTR_MIN_VALUE_U: + policy['min-value'] = attr.as_scalar('u64') + elif attr.type == Netlink.NL_POLICY_TYPE_ATTR_MAX_VALUE_U: + policy['max-value'] = attr.as_scalar('u64') + elif attr.type == Netlink.NL_POLICY_TYPE_ATTR_MIN_LENGTH: + policy['min-length'] = attr.as_scalar('u32') + elif attr.type == Netlink.NL_POLICY_TYPE_ATTR_MAX_LENGTH: + policy['max-length'] = attr.as_scalar('u32') + elif attr.type == Netlink.NL_POLICY_TYPE_ATTR_BITFIELD32_MASK: + policy['bitfield32-mask'] = attr.as_scalar('u32') + elif attr.type == Netlink.NL_POLICY_TYPE_ATTR_MASK: + policy['mask'] = attr.as_scalar('u64') + return policy + def cmd(self): return self.nl_type -- cgit v1.2.3-59-g8ed1b From b269d2b4a5232c6130b5df83fb112f4029df4544 Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Tue, 2 Apr 2024 19:34:21 -0700 Subject: tools: ynl: copy netlink error to NlError Typing e.nl_msg.error when processing exception is a bit tedious and counter-intuitive. Set a local .error member to the positive value of the netlink level error. Reviewed-by: Petr Machata Link: https://lore.kernel.org/r/20240403023426.1762996-3-kuba@kernel.org Signed-off-by: Jakub Kicinski --- tools/net/ynl/lib/ynl.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'tools/net/ynl/lib/ynl.py') diff --git a/tools/net/ynl/lib/ynl.py b/tools/net/ynl/lib/ynl.py index 82d3c98067aa..b30210f537f7 100644 --- a/tools/net/ynl/lib/ynl.py +++ b/tools/net/ynl/lib/ynl.py @@ -100,9 +100,10 @@ class Netlink: class NlError(Exception): def __init__(self, nl_msg): self.nl_msg = nl_msg + self.error = -nl_msg.error def __str__(self): - return f"Netlink error: {os.strerror(-self.nl_msg.error)}\n{self.nl_msg}" + return f"Netlink error: {os.strerror(self.error)}\n{self.nl_msg}" class ConfigError(Exception): -- cgit v1.2.3-59-g8ed1b From aa6485d813ad6d884d23e1e9cf3913be29a1f229 Mon Sep 17 00:00:00 2001 From: Hangbin Liu Date: Thu, 4 Apr 2024 14:31:12 +0800 Subject: ynl: rename array-nest to indexed-array Some implementations, like bonding, has nest array with same attr type. To support all kinds of entries under one nest array. As discussed[1], let's rename array-nest to indexed-array, and assuming the value is a nest by passing the type via sub-type. [1] https://lore.kernel.org/netdev/20240312100105.16a59086@kernel.org/ Suggested-by: Jakub Kicinski Signed-off-by: Hangbin Liu Link: https://lore.kernel.org/r/20240404063114.1221532-2-liuhangbin@gmail.com Signed-off-by: Jakub Kicinski --- Documentation/netlink/genetlink-c.yaml | 2 +- Documentation/netlink/genetlink-legacy.yaml | 2 +- Documentation/netlink/genetlink.yaml | 2 +- Documentation/netlink/netlink-raw.yaml | 2 +- Documentation/netlink/specs/nlctrl.yaml | 6 ++++-- Documentation/netlink/specs/rt_link.yaml | 3 ++- Documentation/netlink/specs/tc.yaml | 21 ++++++++++++++------- .../userspace-api/netlink/genetlink-legacy.rst | 12 +++++++++--- tools/net/ynl/lib/ynl.py | 13 ++++++++----- tools/net/ynl/ynl-gen-c.py | 18 ++++++++++++------ 10 files changed, 53 insertions(+), 28 deletions(-) (limited to 'tools/net/ynl/lib/ynl.py') diff --git a/Documentation/netlink/genetlink-c.yaml b/Documentation/netlink/genetlink-c.yaml index 4dfd899a1661..4f803eaac6d8 100644 --- a/Documentation/netlink/genetlink-c.yaml +++ b/Documentation/netlink/genetlink-c.yaml @@ -158,7 +158,7 @@ properties: type: &attr-type enum: [ unused, pad, flag, binary, uint, sint, u8, u16, u32, u64, s32, s64, - string, nest, array-nest, nest-type-value ] + string, nest, indexed-array, nest-type-value ] doc: description: Documentation of the attribute. type: string diff --git a/Documentation/netlink/genetlink-legacy.yaml b/Documentation/netlink/genetlink-legacy.yaml index b48ad3b1cc32..8db0e22fa72c 100644 --- a/Documentation/netlink/genetlink-legacy.yaml +++ b/Documentation/netlink/genetlink-legacy.yaml @@ -201,7 +201,7 @@ properties: description: The netlink attribute type enum: [ unused, pad, flag, binary, bitfield32, uint, sint, u8, u16, u32, u64, s32, s64, - string, nest, array-nest, nest-type-value ] + string, nest, indexed-array, nest-type-value ] doc: description: Documentation of the attribute. type: string diff --git a/Documentation/netlink/genetlink.yaml b/Documentation/netlink/genetlink.yaml index ebd6ee743fcc..b036227b46f1 100644 --- a/Documentation/netlink/genetlink.yaml +++ b/Documentation/netlink/genetlink.yaml @@ -124,7 +124,7 @@ properties: type: &attr-type enum: [ unused, pad, flag, binary, uint, sint, u8, u16, u32, u64, s32, s64, - string, nest, array-nest, nest-type-value ] + string, nest, indexed-array, nest-type-value ] doc: description: Documentation of the attribute. type: string diff --git a/Documentation/netlink/netlink-raw.yaml b/Documentation/netlink/netlink-raw.yaml index a76e54cbadbc..914aa1c0a273 100644 --- a/Documentation/netlink/netlink-raw.yaml +++ b/Documentation/netlink/netlink-raw.yaml @@ -222,7 +222,7 @@ properties: description: The netlink attribute type enum: [ unused, pad, flag, binary, bitfield32, u8, u16, u32, u64, s8, s16, s32, s64, - string, nest, array-nest, nest-type-value, + string, nest, indexed-array, nest-type-value, sub-message ] doc: description: Documentation of the attribute. diff --git a/Documentation/netlink/specs/nlctrl.yaml b/Documentation/netlink/specs/nlctrl.yaml index b1632b95f725..a36535350bdb 100644 --- a/Documentation/netlink/specs/nlctrl.yaml +++ b/Documentation/netlink/specs/nlctrl.yaml @@ -65,11 +65,13 @@ attribute-sets: type: u32 - name: ops - type: array-nest + type: indexed-array + sub-type: nest nested-attributes: op-attrs - name: mcast-groups - type: array-nest + type: indexed-array + sub-type: nest nested-attributes: mcast-group-attrs - name: policy diff --git a/Documentation/netlink/specs/rt_link.yaml b/Documentation/netlink/specs/rt_link.yaml index 81a5a3d1b04d..e5dcb2cf1724 100644 --- a/Documentation/netlink/specs/rt_link.yaml +++ b/Documentation/netlink/specs/rt_link.yaml @@ -1690,7 +1690,8 @@ attribute-sets: type: binary - name: hw-s-info - type: array-nest + type: indexed-array + sub-type: nest nested-attributes: hw-s-info-one - name: l3-stats diff --git a/Documentation/netlink/specs/tc.yaml b/Documentation/netlink/specs/tc.yaml index 6068c105c5ee..8c01e4e13195 100644 --- a/Documentation/netlink/specs/tc.yaml +++ b/Documentation/netlink/specs/tc.yaml @@ -1988,7 +1988,8 @@ attribute-sets: nested-attributes: tc-ematch-attrs - name: act - type: array-nest + type: indexed-array + sub-type: nest nested-attributes: tc-act-attrs - name: police @@ -2128,7 +2129,8 @@ attribute-sets: type: u32 - name: tin-stats - type: array-nest + type: indexed-array + sub-type: nest nested-attributes: tc-cake-tin-stats-attrs - name: deficit @@ -2348,7 +2350,8 @@ attribute-sets: type: string - name: act - type: array-nest + type: indexed-array + sub-type: nest nested-attributes: tc-act-attrs - name: key-eth-dst @@ -2849,7 +2852,8 @@ attribute-sets: type: string - name: act - type: array-nest + type: indexed-array + sub-type: nest nested-attributes: tc-act-attrs - name: mask @@ -3002,7 +3006,8 @@ attribute-sets: type: u32 - name: act - type: array-nest + type: indexed-array + sub-type: nest nested-attributes: tc-act-attrs - name: flags @@ -3375,7 +3380,8 @@ attribute-sets: nested-attributes: tc-police-attrs - name: act - type: array-nest + type: indexed-array + sub-type: nest nested-attributes: tc-act-attrs - name: tc-taprio-attrs @@ -3593,7 +3599,8 @@ attribute-sets: nested-attributes: tc-police-attrs - name: act - type: array-nest + type: indexed-array + sub-type: nest nested-attributes: tc-act-attrs - name: indev diff --git a/Documentation/userspace-api/netlink/genetlink-legacy.rst b/Documentation/userspace-api/netlink/genetlink-legacy.rst index 70a77387f6c4..54e8fb25e093 100644 --- a/Documentation/userspace-api/netlink/genetlink-legacy.rst +++ b/Documentation/userspace-api/netlink/genetlink-legacy.rst @@ -46,10 +46,16 @@ For reference the ``multi-attr`` array may look like this:: where ``ARRAY-ATTR`` is the array entry type. -array-nest -~~~~~~~~~~ +indexed-array +~~~~~~~~~~~~~ + +``indexed-array`` wraps the entire array in an extra attribute (hence +limiting its size to 64kB). The ``ENTRY`` nests are special and have the +index of the entry as their type instead of normal attribute type. -``array-nest`` creates the following structure:: +A ``sub-type`` is needed to describe what type in the ``ENTRY``. A ``nest`` +``sub-type`` means there are nest arrays in the ``ENTRY``, with the structure +looks like:: [SOME-OTHER-ATTR] [ARRAY-ATTR] diff --git a/tools/net/ynl/lib/ynl.py b/tools/net/ynl/lib/ynl.py index b30210f537f7..d671efea808a 100644 --- a/tools/net/ynl/lib/ynl.py +++ b/tools/net/ynl/lib/ynl.py @@ -631,15 +631,18 @@ class YnlFamily(SpecFamily): decoded = self._formatted_string(decoded, attr_spec.display_hint) return decoded - def _decode_array_nest(self, attr, attr_spec): + def _decode_array_attr(self, attr, attr_spec): decoded = [] offset = 0 while offset < len(attr.raw): item = NlAttr(attr.raw, offset) offset += item.full_len - subattrs = self._decode(NlAttrs(item.raw), attr_spec['nested-attributes']) - decoded.append({ item.type: subattrs }) + if attr_spec["sub-type"] == 'nest': + subattrs = self._decode(NlAttrs(item.raw), attr_spec['nested-attributes']) + decoded.append({ item.type: subattrs }) + else: + raise Exception(f'Unknown {attr_spec["sub-type"]} with name {attr_spec["name"]}') return decoded def _decode_nest_type_value(self, attr, attr_spec): @@ -733,8 +736,8 @@ class YnlFamily(SpecFamily): decoded = attr.as_scalar(attr_spec['type'], attr_spec.byte_order) if 'enum' in attr_spec: decoded = self._decode_enum(decoded, attr_spec) - elif attr_spec["type"] == 'array-nest': - decoded = self._decode_array_nest(attr, attr_spec) + elif attr_spec["type"] == 'indexed-array': + decoded = self._decode_array_attr(attr, attr_spec) elif attr_spec["type"] == 'bitfield32': value, selector = struct.unpack("II", attr.raw) if 'enum' in attr_spec: diff --git a/tools/net/ynl/ynl-gen-c.py b/tools/net/ynl/ynl-gen-c.py index a451cbfbd781..c0b90c104d92 100755 --- a/tools/net/ynl/ynl-gen-c.py +++ b/tools/net/ynl/ynl-gen-c.py @@ -841,8 +841,11 @@ class AttrSet(SpecAttrSet): t = TypeBitfield32(self.family, self, elem, value) elif elem['type'] == 'nest': t = TypeNest(self.family, self, elem, value) - elif elem['type'] == 'array-nest': - t = TypeArrayNest(self.family, self, elem, value) + elif elem['type'] == 'indexed-array' and 'sub-type' in elem: + if elem["sub-type"] == 'nest': + t = TypeArrayNest(self.family, self, elem, value) + else: + raise Exception(f'new_attr: unsupported sub-type {elem["sub-type"]}') elif elem['type'] == 'nest-type-value': t = TypeNestTypeValue(self.family, self, elem, value) else: @@ -1055,7 +1058,7 @@ class Family(SpecFamily): if nested in self.root_sets: raise Exception("Inheriting members to a space used as root not supported") inherit.update(set(spec['type-value'])) - elif spec['type'] == 'array-nest': + elif spec['type'] == 'indexed-array': inherit.add('idx') self.pure_nested_structs[nested].set_inherited(inherit) @@ -1619,9 +1622,12 @@ def _multi_parse(ri, struct, init_lines, local_vars): multi_attrs = set() needs_parg = False for arg, aspec in struct.member_list(): - if aspec['type'] == 'array-nest': - local_vars.append(f'const struct nlattr *attr_{aspec.c_name};') - array_nests.add(arg) + if aspec['type'] == 'indexed-array' and 'sub-type' in aspec: + if aspec["sub-type"] == 'nest': + local_vars.append(f'const struct nlattr *attr_{aspec.c_name};') + array_nests.add(arg) + else: + raise Exception(f'Not supported sub-type {aspec["sub-type"]}') if 'multi-attr' in aspec: multi_attrs.add(arg) needs_parg |= 'nested-attributes' in aspec -- cgit v1.2.3-59-g8ed1b From a7408b56e5f9e96c486cec9aaf7490452b713ebf Mon Sep 17 00:00:00 2001 From: Hangbin Liu Date: Thu, 4 Apr 2024 14:31:13 +0800 Subject: ynl: support binary and integer sub-type for indexed-array Add binary and integer sub-type support for indexed-array to display bond arp and ns targets. Here is what the result looks like: # ip link add bond0 type bond mode 1 \ arp_ip_target 192.168.1.1,192.168.1.2 ns_ip6_target 2001::1,2001::2 # ./tools/net/ynl/cli.py --spec Documentation/netlink/specs/rt_link.yaml \ --do getlink --json '{"ifname": "bond0"}' --output-json | jq '.linkinfo' "arp-ip-target": [ "192.168.1.1", "192.168.1.2" ], [...] "ns-ip6-target": [ "2001::1", "2001::2" ], Signed-off-by: Hangbin Liu Link: https://lore.kernel.org/r/20240404063114.1221532-3-liuhangbin@gmail.com Signed-off-by: Jakub Kicinski --- Documentation/userspace-api/netlink/genetlink-legacy.rst | 10 +++++++--- tools/net/ynl/lib/ynl.py | 10 ++++++++++ 2 files changed, 17 insertions(+), 3 deletions(-) (limited to 'tools/net/ynl/lib/ynl.py') diff --git a/Documentation/userspace-api/netlink/genetlink-legacy.rst b/Documentation/userspace-api/netlink/genetlink-legacy.rst index 54e8fb25e093..fa005989193a 100644 --- a/Documentation/userspace-api/netlink/genetlink-legacy.rst +++ b/Documentation/userspace-api/netlink/genetlink-legacy.rst @@ -66,9 +66,13 @@ looks like:: [MEMBER1] [MEMBER2] -It wraps the entire array in an extra attribute (hence limiting its size -to 64kB). The ``ENTRY`` nests are special and have the index of the entry -as their type instead of normal attribute type. +Other ``sub-type`` like ``u32`` means there is only one member as described +in ``sub-type`` in the ``ENTRY``. The structure looks like:: + + [SOME-OTHER-ATTR] + [ARRAY-ATTR] + [ENTRY u32] + [ENTRY u32] type-value ~~~~~~~~~~ diff --git a/tools/net/ynl/lib/ynl.py b/tools/net/ynl/lib/ynl.py index d671efea808a..0ba5f6fb8747 100644 --- a/tools/net/ynl/lib/ynl.py +++ b/tools/net/ynl/lib/ynl.py @@ -641,6 +641,16 @@ class YnlFamily(SpecFamily): if attr_spec["sub-type"] == 'nest': subattrs = self._decode(NlAttrs(item.raw), attr_spec['nested-attributes']) decoded.append({ item.type: subattrs }) + elif attr_spec["sub-type"] == 'binary': + subattrs = item.as_bin() + if attr_spec.display_hint: + subattrs = self._formatted_string(subattrs, attr_spec.display_hint) + decoded.append(subattrs) + elif attr_spec["sub-type"] in NlAttr.type_formats: + subattrs = item.as_scalar(attr_spec['sub-type'], attr_spec.byte_order) + if attr_spec.display_hint: + subattrs = self._formatted_string(subattrs, attr_spec.display_hint) + decoded.append(subattrs) else: raise Exception(f'Unknown {attr_spec["sub-type"]} with name {attr_spec["name"]}') return decoded -- cgit v1.2.3-59-g8ed1b From 72ba6cba0a6e9f06c09187ddbbdc9f80ee93ffb3 Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Fri, 12 Apr 2024 07:14:32 -0700 Subject: tools: ynl: don't return None for dumps YNL currently reports None for empty dump: $ cli.py ...netdev.yaml --dump page-pool-get None This doesn't matter for the CLI but when writing YNL based tests having to deal with either list or None is annoying. Limit the None conversion to non-dump ops: $ cli.py ...netdev.yaml --dump page-pool-get [] Reviewed-by: Donald Hunter Link: https://lore.kernel.org/r/20240412141436.828666-3-kuba@kernel.org Signed-off-by: Jakub Kicinski --- tools/net/ynl/lib/ynl.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'tools/net/ynl/lib/ynl.py') diff --git a/tools/net/ynl/lib/ynl.py b/tools/net/ynl/lib/ynl.py index 0ba5f6fb8747..a67f7b6fef92 100644 --- a/tools/net/ynl/lib/ynl.py +++ b/tools/net/ynl/lib/ynl.py @@ -995,9 +995,11 @@ class YnlFamily(SpecFamily): rsp_msg.update(self._decode_struct(decoded.raw, op.fixed_header)) rsp.append(rsp_msg) + if dump: + return rsp if not rsp: return None - if not dump and len(rsp) == 1: + if len(rsp) == 1: return rsp[0] return rsp -- cgit v1.2.3-59-g8ed1b From 0a966d606c681b891fc7ef2e3ace67ac507a221d Mon Sep 17 00:00:00 2001 From: Donald Hunter Date: Thu, 18 Apr 2024 11:47:35 +0100 Subject: tools/net/ynl: Fix extack decoding for directional ops NetlinkProtocol.decode() was looking up ops by response value which breaks when it is used for extack decoding of directional ops. Instead, pass the op to decode(). Signed-off-by: Donald Hunter Link: https://lore.kernel.org/r/20240418104737.77914-3-donald.hunter@gmail.com Signed-off-by: Jakub Kicinski --- tools/net/ynl/lib/ynl.py | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) (limited to 'tools/net/ynl/lib/ynl.py') diff --git a/tools/net/ynl/lib/ynl.py b/tools/net/ynl/lib/ynl.py index a67f7b6fef92..a3ec7a56180a 100644 --- a/tools/net/ynl/lib/ynl.py +++ b/tools/net/ynl/lib/ynl.py @@ -386,12 +386,9 @@ class NetlinkProtocol: def _decode(self, nl_msg): return nl_msg - def decode(self, ynl, nl_msg): + def decode(self, ynl, nl_msg, op): msg = self._decode(nl_msg) - fixed_header_size = 0 - if ynl: - op = ynl.rsp_by_value[msg.cmd()] - fixed_header_size = ynl._struct_size(op.fixed_header) + fixed_header_size = ynl._struct_size(op.fixed_header) msg.raw_attrs = NlAttrs(msg.raw, fixed_header_size) return msg @@ -797,7 +794,7 @@ class YnlFamily(SpecFamily): if 'bad-attr-offs' not in extack: return - msg = self.nlproto.decode(self, NlMsg(request, 0, op.attr_set)) + msg = self.nlproto.decode(self, NlMsg(request, 0, op.attr_set), op) offset = self.nlproto.msghdr_size() + self._struct_size(op.fixed_header) path = self._decode_extack_path(msg.raw_attrs, op.attr_set, offset, extack['bad-attr-offs']) @@ -922,7 +919,8 @@ class YnlFamily(SpecFamily): print("Netlink done while checking for ntf!?") continue - decoded = self.nlproto.decode(self, nl_msg) + op = self.rsp_by_value[nl_msg.cmd()] + decoded = self.nlproto.decode(self, nl_msg, op) if decoded.cmd() not in self.async_msg_ids: print("Unexpected msg id done while checking for ntf", decoded) continue @@ -979,7 +977,7 @@ class YnlFamily(SpecFamily): done = True break - decoded = self.nlproto.decode(self, nl_msg) + decoded = self.nlproto.decode(self, nl_msg, op) # Check if this is a reply to our request if nl_msg.nl_seq != req_seq or decoded.cmd() != op.rsp_value: -- cgit v1.2.3-59-g8ed1b From ba8be00f68f5c70eb1df2193251a579923bd9501 Mon Sep 17 00:00:00 2001 From: Donald Hunter Date: Thu, 18 Apr 2024 11:47:36 +0100 Subject: tools/net/ynl: Add multi message support to ynl Add a "--multi " command line to ynl that makes it possible to add several operations to a single netlink request payload. The --multi command line option is repeated for each operation. This is used by the nftables family for transaction batches. For example: ./tools/net/ynl/cli.py \ --spec Documentation/netlink/specs/nftables.yaml \ --multi batch-begin '{"res-id": 10}' \ --multi newtable '{"name": "test", "nfgen-family": 1}' \ --multi newchain '{"name": "chain", "table": "test", "nfgen-family": 1}' \ --multi batch-end '{"res-id": 10}' [None, None, None, None] It can also be used for bundling get requests: ./tools/net/ynl/cli.py \ --spec Documentation/netlink/specs/nftables.yaml \ --multi gettable '{"name": "test", "nfgen-family": 1}' \ --multi getchain '{"name": "chain", "table": "test", "nfgen-family": 1}' \ --output-json [{"name": "test", "use": 1, "handle": 1, "flags": [], "nfgen-family": 1, "version": 0, "res-id": 2}, {"table": "test", "name": "chain", "handle": 1, "use": 0, "nfgen-family": 1, "version": 0, "res-id": 2}] Signed-off-by: Donald Hunter Link: https://lore.kernel.org/r/20240418104737.77914-4-donald.hunter@gmail.com Signed-off-by: Jakub Kicinski --- tools/net/ynl/cli.py | 25 +++++++++++++++--- tools/net/ynl/lib/ynl.py | 68 ++++++++++++++++++++++++++++++++++-------------- 2 files changed, 71 insertions(+), 22 deletions(-) (limited to 'tools/net/ynl/lib/ynl.py') diff --git a/tools/net/ynl/cli.py b/tools/net/ynl/cli.py index f131e33ac3ee..058926d69ef0 100755 --- a/tools/net/ynl/cli.py +++ b/tools/net/ynl/cli.py @@ -19,13 +19,28 @@ class YnlEncoder(json.JSONEncoder): def main(): - parser = argparse.ArgumentParser(description='YNL CLI sample') + description = """ + YNL CLI utility - a general purpose netlink utility that uses YAML + specs to drive protocol encoding and decoding. + """ + epilog = """ + The --multi option can be repeated to include several do operations + in the same netlink payload. + """ + + parser = argparse.ArgumentParser(description=description, + epilog=epilog) 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) + + group = parser.add_mutually_exclusive_group() + group.add_argument('--do', dest='do', metavar='DO-OPERATION', type=str) + group.add_argument('--multi', dest='multi', nargs=2, action='append', + metavar=('DO-OPERATION', 'JSON_TEXT'), type=str) + group.add_argument('--dump', dest='dump', metavar='DUMP-OPERATION', type=str) + parser.add_argument('--sleep', dest='sleep', type=int) parser.add_argument('--subscribe', dest='ntf', type=str) parser.add_argument('--replace', dest='flags', action='append_const', @@ -73,6 +88,10 @@ def main(): if args.dump: reply = ynl.dump(args.dump, attrs) output(reply) + if args.multi: + ops = [ (item[0], json.loads(item[1]), args.flags or []) for item in args.multi ] + reply = ynl.do_multi(ops) + output(reply) except NlError as e: print(e) exit(1) diff --git a/tools/net/ynl/lib/ynl.py b/tools/net/ynl/lib/ynl.py index a3ec7a56180a..ea48f83c2e84 100644 --- a/tools/net/ynl/lib/ynl.py +++ b/tools/net/ynl/lib/ynl.py @@ -938,16 +938,11 @@ class YnlFamily(SpecFamily): return op['do']['request']['attributes'].copy() - def _op(self, method, vals, flags=None, dump=False): - op = self.ops[method] - + def _encode_message(self, op, vals, flags, req_seq): nl_flags = Netlink.NLM_F_REQUEST | Netlink.NLM_F_ACK for flag in flags or []: nl_flags |= flag - if dump: - nl_flags |= Netlink.NLM_F_DUMP - req_seq = random.randint(1024, 65535) msg = self.nlproto.message(nl_flags, op.req_value, 1, req_seq) if op.fixed_header: msg += self._encode_struct(op.fixed_header, vals) @@ -955,18 +950,36 @@ class YnlFamily(SpecFamily): for name, value in vals.items(): msg += self._add_attr(op.attr_set.name, name, value, search_attrs) msg = _genl_msg_finalize(msg) + return msg - self.sock.send(msg, 0) + def _ops(self, ops): + reqs_by_seq = {} + req_seq = random.randint(1024, 65535) + payload = b'' + for (method, vals, flags) in ops: + op = self.ops[method] + msg = self._encode_message(op, vals, flags, req_seq) + reqs_by_seq[req_seq] = (op, msg, flags) + payload += msg + req_seq += 1 + + self.sock.send(payload, 0) done = False rsp = [] + op_rsp = [] while not done: reply = self.sock.recv(self._recv_size) nms = NlMsgs(reply, attr_space=op.attr_set) self._recv_dbg_print(reply, nms) for nl_msg in nms: - if nl_msg.extack: - self._decode_extack(msg, op, nl_msg.extack) + if nl_msg.nl_seq in reqs_by_seq: + (op, req_msg, req_flags) = reqs_by_seq[nl_msg.nl_seq] + if nl_msg.extack: + self._decode_extack(req_msg, op, nl_msg.extack) + else: + op = self.rsp_by_value[nl_msg.cmd()] + req_flags = [] if nl_msg.error: raise NlError(nl_msg) @@ -974,13 +987,25 @@ class YnlFamily(SpecFamily): if nl_msg.extack: print("Netlink warning:") print(nl_msg) - done = True + + if Netlink.NLM_F_DUMP in req_flags: + rsp.append(op_rsp) + elif not op_rsp: + rsp.append(None) + elif len(op_rsp) == 1: + rsp.append(op_rsp[0]) + else: + rsp.append(op_rsp) + op_rsp = [] + + del reqs_by_seq[nl_msg.nl_seq] + done = len(reqs_by_seq) == 0 break decoded = self.nlproto.decode(self, nl_msg, op) # Check if this is a reply to our request - if nl_msg.nl_seq != req_seq or decoded.cmd() != op.rsp_value: + if nl_msg.nl_seq not in reqs_by_seq or decoded.cmd() != op.rsp_value: if decoded.cmd() in self.async_msg_ids: self.handle_ntf(decoded) continue @@ -991,18 +1016,23 @@ class YnlFamily(SpecFamily): rsp_msg = self._decode(decoded.raw_attrs, op.attr_set.name) if op.fixed_header: rsp_msg.update(self._decode_struct(decoded.raw, op.fixed_header)) - rsp.append(rsp_msg) + op_rsp.append(rsp_msg) - if dump: - return rsp - if not rsp: - return None - if len(rsp) == 1: - return rsp[0] return rsp + def _op(self, method, vals, flags=None, dump=False): + req_flags = flags or [] + if dump: + req_flags.append(Netlink.NLM_F_DUMP) + + ops = [(method, vals, req_flags)] + return self._ops(ops)[0] + def do(self, method, vals, flags=None): return self._op(method, vals, flags) def dump(self, method, vals): - return self._op(method, vals, [], dump=True) + return self._op(method, vals, dump=True) + + def do_multi(self, ops): + return self._ops(ops) -- cgit v1.2.3-59-g8ed1b From a44f2eb106a46f2275a79de54ce0ea63e4f3d8c8 Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Fri, 19 Apr 2024 19:08:26 -0700 Subject: tools: ynl: don't ignore errors in NLMSG_DONE messages NLMSG_DONE contains an error code, it has to be extracted. Prior to this change all dumps will end in success, and in case of failure the result is silently truncated. Fixes: e4b48ed460d3 ("tools: ynl: add a completely generic client") Signed-off-by: Jakub Kicinski Reviewed-by: Donald Hunter Link: https://lore.kernel.org/r/20240420020827.3288615-1-kuba@kernel.org Signed-off-by: Paolo Abeni --- tools/net/ynl/lib/ynl.py | 1 + 1 file changed, 1 insertion(+) (limited to 'tools/net/ynl/lib/ynl.py') diff --git a/tools/net/ynl/lib/ynl.py b/tools/net/ynl/lib/ynl.py index 5fa7957f6e0f..25810e18b0a7 100644 --- a/tools/net/ynl/lib/ynl.py +++ b/tools/net/ynl/lib/ynl.py @@ -182,6 +182,7 @@ class NlMsg: self.done = 1 extack_off = 20 elif self.nl_type == Netlink.NLMSG_DONE: + self.error = struct.unpack("i", self.raw[0:4])[0] self.done = 1 extack_off = 4 -- cgit v1.2.3-59-g8ed1b From 5c4c0edca68a5841a8d53ccd49596fe199c8334c Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Thu, 25 Apr 2024 17:31:11 -0700 Subject: tools: ynl: don't append doc of missing type directly to the type When using YNL in tests appending the doc string to the type name makes it harder to check that we got the correct error. Put the doc under a separate key. Reviewed-by: Donald Hunter Link: https://lore.kernel.org/r/20240426003111.359285-1-kuba@kernel.org Signed-off-by: Jakub Kicinski --- tools/net/ynl/lib/ynl.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) (limited to 'tools/net/ynl/lib/ynl.py') diff --git a/tools/net/ynl/lib/ynl.py b/tools/net/ynl/lib/ynl.py index 35f82a2c2247..35e666928119 100644 --- a/tools/net/ynl/lib/ynl.py +++ b/tools/net/ynl/lib/ynl.py @@ -233,10 +233,9 @@ class NlMsg: miss_type = self.extack['miss-type'] if miss_type in attr_space.attrs_by_val: spec = attr_space.attrs_by_val[miss_type] - desc = spec['name'] + self.extack['miss-type'] = spec['name'] if 'doc' in spec: - desc += f" ({spec['doc']})" - self.extack['miss-type'] = desc + self.extack['miss-type-doc'] = spec['doc'] def _decode_policy(self, raw): policy = {} -- cgit v1.2.3-59-g8ed1b