| Message ID | 20240221155415.158174-4-jiri@resnulli.us (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | tools: ynl: couple of cmdline enhancements | expand |
On Wed, 21 Feb 2024 at 15:54, Jiri Pirko <jiri@resnulli.us> wrote: > > From: Jiri Pirko <jiri@nvidia.com> > > During decoding of messages coming from kernel, attribute values are > converted to enum names in case the attribute type is enum of bitfield32. > > However, when user constructs json message, he has to pass plain scalar > values. See "state" "selector" and "value" attributes in following > examples: > > $ sudo ./tools/net/ynl/cli.py --spec Documentation/netlink/specs/dpll.yaml --do pin-set --json '{"id": 0, "parent-device": {"parent-id": 0, "state": 1}}' > $ sudo ./tools/net/ynl/cli.py --spec Documentation/netlink/specs/devlink.yaml --do port-set --json '{"bus-name": "pci", "dev-name": "0000:08:00.1", "port-index": 98304, "port-function": {"caps": {"selector": 1, "value": 1 }}}' > > Allow user to pass strings containing enum names, convert them to scalar > values to be encoded into Netlink message: > > $ sudo ./tools/net/ynl/cli.py --spec Documentation/netlink/specs/dpll.yaml --do pin-set --json '{"id": 0, "parent-device": {"parent-id": 0, "state": "connected"}}' > $ sudo ./tools/net/ynl/cli.py --spec Documentation/netlink/specs/devlink.yaml --do port-set --json '{"bus-name": "pci", "dev-name": "0000:08:00.1", "port-index": 98304, "port-function": {"caps": {"selector": ["roce-bit"], "value": ["roce-bit"] }}}' > > Signed-off-by: Jiri Pirko <jiri@nvidia.com> Reviewed-by: Donald Hunter <donald.hunter@gmail.com>
On Wed, 21 Feb 2024 16:54:15 +0100 Jiri Pirko wrote: > + def _encode_enum(self, attr_spec, value): > + try: > + return int(value) > + except (ValueError, TypeError) as e: > + if 'enum' not in attr_spec: > + raise e > + enum = self.consts[attr_spec['enum']] > + if enum.type == 'flags' or attr_spec.get('enum-as-flags', False): > + scalar = 0 > + if isinstance(value, str): > + value = [value] > + for single_value in value: > + scalar += enum.entries[single_value].user_value(as_flags = True) > + return scalar > + else: > + return enum.entries[value].user_value() That's not symmetric with _decode_enum(), I said: vvvvvvvvvvvvvvvvvvvvvv It'd be cleaner to make it more symmetric with _decode_enum(), and call it _encode_enum(). How about you go back to your _get_scalar name and only factor out the parts which encode the enum to be a _encode_enum() ?
Wed, Feb 21, 2024 at 07:49:36PM CET, kuba@kernel.org wrote: >On Wed, 21 Feb 2024 16:54:15 +0100 Jiri Pirko wrote: >> + def _encode_enum(self, attr_spec, value): >> + try: >> + return int(value) >> + except (ValueError, TypeError) as e: >> + if 'enum' not in attr_spec: >> + raise e >> + enum = self.consts[attr_spec['enum']] >> + if enum.type == 'flags' or attr_spec.get('enum-as-flags', False): >> + scalar = 0 >> + if isinstance(value, str): >> + value = [value] >> + for single_value in value: >> + scalar += enum.entries[single_value].user_value(as_flags = True) >> + return scalar >> + else: >> + return enum.entries[value].user_value() > >That's not symmetric with _decode_enum(), I said: > > vvvvvvvvvvvvvvvvvvvvvv > It'd be cleaner to make it more symmetric with _decode_enum(), and > call it _encode_enum(). > >How about you go back to your _get_scalar name and only factor out the >parts which encode the enum to be a _encode_enum() ? Okay.
diff --git a/tools/net/ynl/lib/ynl.py b/tools/net/ynl/lib/ynl.py index 38244aff1ec7..14ae30db984a 100644 --- a/tools/net/ynl/lib/ynl.py +++ b/tools/net/ynl/lib/ynl.py @@ -438,6 +438,23 @@ class YnlFamily(SpecFamily): self.sock.setsockopt(Netlink.SOL_NETLINK, Netlink.NETLINK_ADD_MEMBERSHIP, mcast_id) + def _encode_enum(self, attr_spec, value): + try: + return int(value) + except (ValueError, TypeError) as e: + if 'enum' not in attr_spec: + raise e + enum = self.consts[attr_spec['enum']] + if enum.type == 'flags' or attr_spec.get('enum-as-flags', False): + scalar = 0 + if isinstance(value, str): + value = [value] + for single_value in value: + scalar += enum.entries[single_value].user_value(as_flags = True) + return scalar + else: + return enum.entries[value].user_value() + def _add_attr(self, space, name, value, search_attrs): try: attr = self.attr_sets[space][name] @@ -474,7 +491,7 @@ class YnlFamily(SpecFamily): else: raise Exception(f'Unknown type for binary attribute, value: {value}') elif attr['type'] in NlAttr.type_formats or attr.is_auto_scalar: - scalar = int(value) + scalar = self._encode_enum(attr, value) if attr.is_auto_scalar: attr_type = attr["type"][0] + ('32' if scalar.bit_length() <= 32 else '64') else: @@ -482,7 +499,9 @@ class YnlFamily(SpecFamily): format = NlAttr.get_format(attr_type, attr.byte_order) attr_payload = format.pack(scalar) elif attr['type'] in "bitfield32": - attr_payload = struct.pack("II", int(value["value"]), int(value["selector"])) + scalar_value = self._encode_enum(attr, value["value"]) + scalar_selector = self._encode_enum(attr, value["selector"]) + attr_payload = struct.pack("II", scalar_value, scalar_selector) elif attr['type'] == 'sub-message': msg_format = self._resolve_selector(attr, search_attrs) attr_payload = b''