| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20230726143709.791169dd@kernel.org>
Date: Wed, 26 Jul 2023 14:37:09 -0700
From: Jakub Kicinski <kuba@...nel.org>
To: Donald Hunter <donald.hunter@...il.com>
Cc: netdev@...r.kernel.org, "David S. Miller" <davem@...emloft.net>, Eric
Dumazet <edumazet@...gle.com>, Paolo Abeni <pabeni@...hat.com>,
donald.hunter@...hat.com
Subject: Re: [PATCH net-next v1 2/3] tools/net/ynl: Add support for
netlink-raw families
On Tue, 25 Jul 2023 17:22:04 +0100 Donald Hunter wrote:
> Refactor the ynl code to encapsulate protocol-family specifics into
> NetlinkProtocolFamily and GenlProtocolFamily.
> +class SpecMcastGroup(SpecElement):
> + """Netlink Multicast Group
> +
> + Information about a multicast group.
> +
> + Attributes:
> + id numerical id of this multicast group for netlink-raw
> + yaml raw spec as loaded from the spec file
> + """
> + def __init__(self, family, yaml):
> + super().__init__(family, yaml)
> + self.id = self.yaml.get('id')
name, too?
> class SpecFamily(SpecElement):
> """ Netlink Family Spec class.
>
> @@ -343,6 +357,7 @@ class SpecFamily(SpecElement):
> ntfs dict of all async events
> consts dict of all constants/enums
> fixed_header string, optional name of family default fixed header struct
> + mcast_groups dict of all multicast groups (index by name)
> """
> def __init__(self, spec_path, schema_path=None, exclude_ops=None):
> with open(spec_path, "r") as stream:
> @@ -384,6 +399,7 @@ class SpecFamily(SpecElement):
> self.ops = collections.OrderedDict()
> self.ntfs = collections.OrderedDict()
> self.consts = collections.OrderedDict()
> + self.mcast_groups = collections.OrderedDict()
>
> last_exception = None
> while len(self._resolution_list) > 0:
> @@ -416,6 +432,9 @@ class SpecFamily(SpecElement):
> def new_operation(self, elem, req_val, rsp_val):
> return SpecOperation(self, elem, req_val, rsp_val)
>
> + def new_mcast_group(self, elem):
> + return SpecMcastGroup(self, elem)
> +
> def add_unresolved(self, elem):
> self._resolution_list.append(elem)
>
> @@ -512,3 +531,9 @@ class SpecFamily(SpecElement):
> self.ops[op.name] = op
> elif op.is_async:
> self.ntfs[op.name] = op
> +
> + mcgs = self.yaml.get('mcast-groups')
> + if mcgs:
> + for elem in mcgs['list']:
> + mcg = self.new_mcast_group(elem)
> + self.mcast_groups[elem['name']] = mcg
Could you factor out the mcgroup changes to a separate patch?
> diff --git a/tools/net/ynl/lib/ynl.py b/tools/net/ynl/lib/ynl.py
> index 3e2ade2194cd..7e877c43e10f 100644
> --- a/tools/net/ynl/lib/ynl.py
> +++ b/tools/net/ynl/lib/ynl.py
> @@ -25,6 +25,7 @@ class Netlink:
> NETLINK_ADD_MEMBERSHIP = 1
> NETLINK_CAP_ACK = 10
> NETLINK_EXT_ACK = 11
> + NETLINK_GET_STRICT_CHK = 12
>
> # Netlink message
> NLMSG_ERROR = 2
> @@ -153,6 +154,21 @@ class NlAttr:
> value[m.name] = decoded
> return value
>
> + @classmethod
> + def decode_enum(cls, raw, attr_spec, consts):
> + enum = consts[attr_spec['enum']]
> + if 'enum-as-flags' in attr_spec and attr_spec['enum-as-flags']:
> + i = 0
> + value = set()
> + while raw:
> + if raw & 1:
> + value.add(enum.entries_by_val[i].name)
> + raw >>= 1
> + i += 1
> + else:
> + value = enum.entries_by_val[raw].name
> + return value
This doesn't always operates on netlink attributes, technically,
so how about we make it a standalone function, not a member of NlAttr?
Or should we move it to a parent class, NetlinkProtocolFamily?
> + def decode_fixed_header(self, consts, op):
> + fixed_header_members = consts[op.fixed_header].members
> + self.fixed_header_attrs = dict()
> + offset = 0
> + for m in fixed_header_members:
> + format = NlAttr.get_format(m.type, m.byte_order)
> + [ value ] = format.unpack_from(self.raw, offset)
> + offset += format.size
> +
> + if m.enum:
> + value = NlAttr.decode_enum(value, m, consts)
> +
> + self.fixed_header_attrs[m.name] = value
> + self.raw = self.raw[offset:]
> +
> + def cmd(self):
> + return self.nl_type
And perhaps the pure code moves could be a separate patch for ease
of review?
> def __repr__(self):
> msg = f"nl_len = {self.nl_len} ({len(self.raw)}) nl_flags = 0x{self.nl_flags:x} nl_type = {self.nl_type}\n"
> if self.error:
> @@ -318,23 +353,21 @@ def _genl_load_families():
>
>
> class GenlMsg:
> - def __init__(self, nl_msg, fixed_header_members=[]):
> - self.nl = nl_msg
> + def __init__(self, nl_msg, ynl=None):
> + self.genl_cmd, self.genl_version, _ = struct.unpack_from("BBH", nl_msg.raw, 0)
> + nl_msg.raw = nl_msg.raw[4:]
>
> - self.hdr = nl_msg.raw[0:4]
> - offset = 4
> + if ynl:
> + op = ynl.rsp_by_value[self.genl_cmd]
Took me a while to figure out why ynl gets passed here :S
I'm not sure what the best structure of inheritance is but
I think we should at the very least *not* call genl vs raw-nl
"family".
NetlinkProtocolFamily -> NetlinkProtocol
GenlProtocolFamily -> GenlProtocol
and store them in YnlFamily to self.nlproto or self.protocol
or some such.
> + if op.fixed_header:
> + nl_msg.decode_fixed_header(ynl.consts, op)
> def _decode_binary(self, attr, attr_spec):
> if attr_spec.struct_name:
> members = self.consts[attr_spec.struct_name]
> - decoded = attr.as_struct(members)
> + decoded = attr.as_struct(members, self.consts)
I applied the series on top of Arkadiusz's fixes and this line throws
an "as_struct takes 2 arguments, 3 given" exception.
> for m in members:
> if m.enum:
> decoded[m.name] = self._decode_enum(decoded[m.name], m)
Powered by blists - more mailing lists