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 6951bcc7efdc..fa4f1c28efc5 100644 --- a/tools/net/ynl/lib/ynl.py +++ b/tools/net/ynl/lib/ynl.py @@ -410,7 +410,12 @@ class YnlFamily(SpecFamily): 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 d7d4ba855f43..ddc1a5209992 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 d8ab50579cd1..a8f860892540 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 bdff8dfc29c9..e27deb199a70 100755 --- a/tools/net/ynl/ynl-gen-c.py +++ b/tools/net/ynl/ynl-gen-c.py @@ -428,6 +428,7 @@ class TypeBinary(Type): 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 e27deb199a70..13d06931c045 100755 --- a/tools/net/ynl/ynl-gen-c.py +++ b/tools/net/ynl/ynl-gen-c.py @@ -978,7 +978,9 @@ class Family(SpecFamily): 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 13d06931c045..9209bdcca9c6 100755 --- a/tools/net/ynl/ynl-gen-c.py +++ b/tools/net/ynl/ynl-gen-c.py @@ -615,7 +615,7 @@ class Struct: 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 3e13826a3fdf..0af5ab842c04 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