From 649bde9004ac7e034383dcd810cb52f3f5d9e577 Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Wed, 23 Aug 2023 17:30:52 -0700 Subject: [PATCH 1/5] tools: ynl: allow passing binary data Recent changes made us assume that input for binary data is in hex. When using YNL as a Python library it's possible to pass in raw bytes. Bring the ability to do that back. Reviewed-by: Donald Hunter Link: https://lore.kernel.org/r/20230824003056.1436637-2-kuba@kernel.org Signed-off-by: Jakub Kicinski --- tools/net/ynl/lib/ynl.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/tools/net/ynl/lib/ynl.py b/tools/net/ynl/lib/ynl.py index 6951bcc7efdcb..fa4f1c28efc55 100644 --- a/tools/net/ynl/lib/ynl.py +++ b/tools/net/ynl/lib/ynl.py @@ -410,7 +410,12 @@ def _add_attr(self, space, name, value): elif attr["type"] == 'string': attr_payload = str(value).encode('ascii') + b'\x00' elif attr["type"] == 'binary': - attr_payload = bytes.fromhex(value) + if isinstance(value, bytes): + attr_payload = value + elif isinstance(value, str): + attr_payload = bytes.fromhex(value) + else: + raise Exception(f'Unknown type for binary attribute, value: {value}') elif attr['type'] in NlAttr.type_formats: format = NlAttr.get_format(attr['type'], attr.byte_order) attr_payload = format.pack(int(value)) From a149a3a13bbcc5b87ade9073b6f1c9584f85ab18 Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Wed, 23 Aug 2023 17:30:53 -0700 Subject: [PATCH 2/5] tools: ynl-gen: set length of binary fields Remember to set the length field in the request setters. Reviewed-by: Donald Hunter Link: https://lore.kernel.org/r/20230824003056.1436637-3-kuba@kernel.org Signed-off-by: Jakub Kicinski --- tools/net/ynl/generated/ethtool-user.h | 4 ++++ tools/net/ynl/generated/fou-user.h | 6 ++++++ tools/net/ynl/ynl-gen-c.py | 1 + 3 files changed, 11 insertions(+) diff --git a/tools/net/ynl/generated/ethtool-user.h b/tools/net/ynl/generated/ethtool-user.h index d7d4ba855f439..ddc1a52099929 100644 --- a/tools/net/ynl/generated/ethtool-user.h +++ b/tools/net/ynl/generated/ethtool-user.h @@ -1422,6 +1422,7 @@ ethtool_wol_set_req_set_sopass(struct ethtool_wol_set_req *req, const void *sopass, size_t len) { free(req->sopass); + req->_present.sopass_len = len; req->sopass = malloc(req->_present.sopass_len); memcpy(req->sopass, sopass, req->_present.sopass_len); } @@ -4071,6 +4072,7 @@ ethtool_fec_set_req_set_stats_corrected(struct ethtool_fec_set_req *req, const void *corrected, size_t len) { free(req->stats.corrected); + req->stats._present.corrected_len = len; req->stats.corrected = malloc(req->stats._present.corrected_len); memcpy(req->stats.corrected, corrected, req->stats._present.corrected_len); } @@ -4079,6 +4081,7 @@ ethtool_fec_set_req_set_stats_uncorr(struct ethtool_fec_set_req *req, const void *uncorr, size_t len) { free(req->stats.uncorr); + req->stats._present.uncorr_len = len; req->stats.uncorr = malloc(req->stats._present.uncorr_len); memcpy(req->stats.uncorr, uncorr, req->stats._present.uncorr_len); } @@ -4087,6 +4090,7 @@ ethtool_fec_set_req_set_stats_corr_bits(struct ethtool_fec_set_req *req, const void *corr_bits, size_t len) { free(req->stats.corr_bits); + req->stats._present.corr_bits_len = len; req->stats.corr_bits = malloc(req->stats._present.corr_bits_len); memcpy(req->stats.corr_bits, corr_bits, req->stats._present.corr_bits_len); } diff --git a/tools/net/ynl/generated/fou-user.h b/tools/net/ynl/generated/fou-user.h index d8ab50579cd13..a8f860892540b 100644 --- a/tools/net/ynl/generated/fou-user.h +++ b/tools/net/ynl/generated/fou-user.h @@ -91,6 +91,7 @@ fou_add_req_set_local_v6(struct fou_add_req *req, const void *local_v6, size_t len) { free(req->local_v6); + req->_present.local_v6_len = len; req->local_v6 = malloc(req->_present.local_v6_len); memcpy(req->local_v6, local_v6, req->_present.local_v6_len); } @@ -99,6 +100,7 @@ fou_add_req_set_peer_v6(struct fou_add_req *req, const void *peer_v6, size_t len) { free(req->peer_v6); + req->_present.peer_v6_len = len; req->peer_v6 = malloc(req->_present.peer_v6_len); memcpy(req->peer_v6, peer_v6, req->_present.peer_v6_len); } @@ -192,6 +194,7 @@ fou_del_req_set_local_v6(struct fou_del_req *req, const void *local_v6, size_t len) { free(req->local_v6); + req->_present.local_v6_len = len; req->local_v6 = malloc(req->_present.local_v6_len); memcpy(req->local_v6, local_v6, req->_present.local_v6_len); } @@ -200,6 +203,7 @@ fou_del_req_set_peer_v6(struct fou_del_req *req, const void *peer_v6, size_t len) { free(req->peer_v6); + req->_present.peer_v6_len = len; req->peer_v6 = malloc(req->_present.peer_v6_len); memcpy(req->peer_v6, peer_v6, req->_present.peer_v6_len); } @@ -280,6 +284,7 @@ fou_get_req_set_local_v6(struct fou_get_req *req, const void *local_v6, size_t len) { free(req->local_v6); + req->_present.local_v6_len = len; req->local_v6 = malloc(req->_present.local_v6_len); memcpy(req->local_v6, local_v6, req->_present.local_v6_len); } @@ -288,6 +293,7 @@ fou_get_req_set_peer_v6(struct fou_get_req *req, const void *peer_v6, size_t len) { free(req->peer_v6); + req->_present.peer_v6_len = len; req->peer_v6 = malloc(req->_present.peer_v6_len); memcpy(req->peer_v6, peer_v6, req->_present.peer_v6_len); } diff --git a/tools/net/ynl/ynl-gen-c.py b/tools/net/ynl/ynl-gen-c.py index bdff8dfc29c94..e27deb199a707 100755 --- a/tools/net/ynl/ynl-gen-c.py +++ b/tools/net/ynl/ynl-gen-c.py @@ -428,6 +428,7 @@ def _attr_get(self, ri, var): def _setter_lines(self, ri, member, presence): return [f"free({member});", + f"{presence}_len = len;", f"{member} = malloc({presence}_len);", f'memcpy({member}, {self.c_name}, {presence}_len);'] From dc2ef94d892634fba912e4fdeb01cd1c19346c9a Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Wed, 23 Aug 2023 17:30:54 -0700 Subject: [PATCH 3/5] tools: ynl-gen: fix collecting global policy attrs We look for attributes inside do.request, but there's another layer of nesting in the spec, look inside do.request.attributes. This bug had no effect as all global policies we generate (fou) seem to be full, anyway, and we treat full and empty the same. Next patch will change the treatment of empty policies. Reviewed-by: Donald Hunter Link: https://lore.kernel.org/r/20230824003056.1436637-4-kuba@kernel.org Signed-off-by: Jakub Kicinski --- tools/net/ynl/ynl-gen-c.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tools/net/ynl/ynl-gen-c.py b/tools/net/ynl/ynl-gen-c.py index e27deb199a707..13d06931c045e 100755 --- a/tools/net/ynl/ynl-gen-c.py +++ b/tools/net/ynl/ynl-gen-c.py @@ -978,7 +978,9 @@ def _load_global_policy(self): for op_mode in ['do', 'dump']: if op_mode in op: - global_set.update(op[op_mode].get('request', [])) + req = op[op_mode].get('request') + if req: + global_set.update(req.get('attributes', [])) self.global_policy = [] self.global_policy_set = attr_set_name From 4c8c24e801e60a2b4a6f78401c9b7cb6cbf47cbf Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Wed, 23 Aug 2023 17:30:55 -0700 Subject: [PATCH 4/5] tools: ynl-gen: support empty attribute lists Differentiate between empty list and None for member lists. New families may want to create request responses with no attribute. If we treat those the same as None we end up rendering a full parsing policy in user space, instead of an empty one. Reviewed-by: Donald Hunter Link: https://lore.kernel.org/r/20230824003056.1436637-5-kuba@kernel.org Signed-off-by: Jakub Kicinski --- tools/net/ynl/ynl-gen-c.py | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/tools/net/ynl/ynl-gen-c.py b/tools/net/ynl/ynl-gen-c.py index 13d06931c045e..9209bdcca9c61 100755 --- a/tools/net/ynl/ynl-gen-c.py +++ b/tools/net/ynl/ynl-gen-c.py @@ -615,7 +615,7 @@ def __init__(self, family, space_name, type_list=None, inherited=None): self.attr_list = [] self.attrs = dict() - if type_list: + if type_list is not None: for t in type_list: self.attr_list.append((t, self.attr_set[t]),) else: @@ -1543,7 +1543,14 @@ def parse_rsp_msg(ri, deref=False): ri.cw.write_func_prot('int', f'{op_prefix(ri, "reply", deref=deref)}_parse', func_args) - _multi_parse(ri, ri.struct["reply"], init_lines, local_vars) + if ri.struct["reply"].member_list(): + _multi_parse(ri, ri.struct["reply"], init_lines, local_vars) + else: + # Empty reply + ri.cw.block_start() + ri.cw.p('return MNL_CB_OK;') + ri.cw.block_end() + ri.cw.nl() def print_req(ri): From e83d4e9b2d0ff12f02f85fbef909c46c6447d41e Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Wed, 23 Aug 2023 17:30:56 -0700 Subject: [PATCH 5/5] netlink: specs: fix indent in fou Fix up the indentation. This has no functional effect, AFAICT. Reviewed-by: Donald Hunter Link: https://lore.kernel.org/r/20230824003056.1436637-6-kuba@kernel.org Signed-off-by: Jakub Kicinski --- Documentation/netlink/specs/fou.yaml | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/Documentation/netlink/specs/fou.yaml b/Documentation/netlink/specs/fou.yaml index 3e13826a3fdf1..0af5ab842c04d 100644 --- a/Documentation/netlink/specs/fou.yaml +++ b/Documentation/netlink/specs/fou.yaml @@ -107,16 +107,16 @@ operations: flags: [ admin-perm ] do: - request: &select_attrs + request: &select_attrs attributes: - - af - - ifindex - - port - - peer_port - - local_v4 - - peer_v4 - - local_v6 - - peer_v6 + - af + - ifindex + - port + - peer_port + - local_v4 + - peer_v4 + - local_v6 + - peer_v6 - name: get