From a6471da7745aa16b1534848877ce73739668992b Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Tue, 29 Apr 2025 08:46:53 -0700 Subject: [PATCH 01/12] tools: ynl-gen: fix comment about nested struct dict The dict stores struct objects (of class Struct), not just a trivial set with directions. Reviewed-by: Donald Hunter Reviewed-by: Jacob Keller Signed-off-by: Jakub Kicinski Link: https://patch.msgid.link/20250429154704.2613851-2-kuba@kernel.org Signed-off-by: Paolo Abeni --- tools/net/ynl/pyynl/ynl_gen_c.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/net/ynl/pyynl/ynl_gen_c.py b/tools/net/ynl/pyynl/ynl_gen_c.py index 9613a6135003..077aacd5f33a 100755 --- a/tools/net/ynl/pyynl/ynl_gen_c.py +++ b/tools/net/ynl/pyynl/ynl_gen_c.py @@ -1015,7 +1015,7 @@ class Family(SpecFamily): # dict space-name -> 'request': set(attrs), 'reply': set(attrs) self.root_sets = dict() - # dict space-name -> set('request', 'reply') + # dict space-name -> Struct self.pure_nested_structs = dict() self._mark_notify() From 2286905f1b33cafa25e508f20bfceb47e20e85d8 Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Tue, 29 Apr 2025 08:46:54 -0700 Subject: [PATCH 02/12] tools: ynl-gen: factor out free_needs_iter for a struct Instead of walking the entries in the code gen add a method for the struct class to return if any of the members need an iterator. Reviewed-by: Donald Hunter Reviewed-by: Jacob Keller Signed-off-by: Jakub Kicinski Link: https://patch.msgid.link/20250429154704.2613851-3-kuba@kernel.org Signed-off-by: Paolo Abeni --- tools/net/ynl/pyynl/ynl_gen_c.py | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/tools/net/ynl/pyynl/ynl_gen_c.py b/tools/net/ynl/pyynl/ynl_gen_c.py index 077aacd5f33a..90f7fe6b623b 100755 --- a/tools/net/ynl/pyynl/ynl_gen_c.py +++ b/tools/net/ynl/pyynl/ynl_gen_c.py @@ -809,6 +809,12 @@ class Struct: raise Exception("Inheriting different members not supported") self.inherited = [c_lower(x) for x in sorted(self._inherited)] + def free_needs_iter(self): + for _, attr in self.attr_list: + if attr.free_needs_iter(): + return True + return False + class EnumEntry(SpecEnumEntry): def __init__(self, enum_set, yaml, prev, value_start): @@ -2156,11 +2162,9 @@ def print_wrapped_type(ri): def _free_type_members_iter(ri, struct): - for _, attr in struct.member_list(): - if attr.free_needs_iter(): - ri.cw.p('unsigned int i;') - ri.cw.nl() - break + if struct.free_needs_iter(): + ri.cw.p('unsigned int i;') + ri.cw.nl() def _free_type_members(ri, var, struct, ref=''): From d12a7be02524a6f4d8d5d57920001ea63a93f28d Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Tue, 29 Apr 2025 08:46:55 -0700 Subject: [PATCH 03/12] tools: ynl-gen: fill in missing empty attr lists The C codegen refers to op attribute lists all over the place, without checking if they are present, even tho attribute list is technically an optional property. Add them automatically at init if missing so that we don't have to make specs longer. Reviewed-by: Donald Hunter Reviewed-by: Jacob Keller Signed-off-by: Jakub Kicinski Link: https://patch.msgid.link/20250429154704.2613851-4-kuba@kernel.org Signed-off-by: Paolo Abeni --- tools/net/ynl/pyynl/ynl_gen_c.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tools/net/ynl/pyynl/ynl_gen_c.py b/tools/net/ynl/pyynl/ynl_gen_c.py index 90f7fe6b623b..898c41a7a81f 100755 --- a/tools/net/ynl/pyynl/ynl_gen_c.py +++ b/tools/net/ynl/pyynl/ynl_gen_c.py @@ -938,6 +938,14 @@ class AttrSet(SpecAttrSet): class Operation(SpecOperation): def __init__(self, family, yaml, req_value, rsp_value): + # Fill in missing operation properties (for fixed hdr-only msgs) + for mode in ['do', 'dump', 'event']: + for direction in ['request', 'reply']: + try: + yaml[mode][direction].setdefault('attributes', []) + except KeyError: + pass + super().__init__(family, yaml, req_value, rsp_value) self.render_name = c_lower(family.ident_name + '_' + self.name) From fe7d57e040f7d189e3bd030f311bb2011a0cc35a Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Tue, 29 Apr 2025 08:46:56 -0700 Subject: [PATCH 04/12] tools: ynl: let classic netlink requests specify extra nlflags Classic netlink makes extensive use of flags. Support specifying them the same way as attributes are specified (using a helper), for example: rt_link_newlink_req_set_nlflags(req, NLM_F_CREATE | NLM_F_ECHO); Wrap the code up in a RenderInfo predicate. I think that some genetlink families may want this, too. It should be easy to add a spec property later. Reviewed-by: Donald Hunter Reviewed-by: Jacob Keller Signed-off-by: Jakub Kicinski Link: https://patch.msgid.link/20250429154704.2613851-5-kuba@kernel.org Signed-off-by: Paolo Abeni --- tools/net/ynl/lib/ynl-priv.h | 2 +- tools/net/ynl/lib/ynl.c | 4 ++-- tools/net/ynl/pyynl/ynl_gen_c.py | 21 ++++++++++++++++++++- 3 files changed, 23 insertions(+), 4 deletions(-) diff --git a/tools/net/ynl/lib/ynl-priv.h b/tools/net/ynl/lib/ynl-priv.h index 634eb16548b9..5debb09491e7 100644 --- a/tools/net/ynl/lib/ynl-priv.h +++ b/tools/net/ynl/lib/ynl-priv.h @@ -94,7 +94,7 @@ struct ynl_ntf_base_type { unsigned char data[] __attribute__((aligned(8))); }; -struct nlmsghdr *ynl_msg_start_req(struct ynl_sock *ys, __u32 id); +struct nlmsghdr *ynl_msg_start_req(struct ynl_sock *ys, __u32 id, __u16 flags); struct nlmsghdr *ynl_msg_start_dump(struct ynl_sock *ys, __u32 id); struct nlmsghdr * diff --git a/tools/net/ynl/lib/ynl.c b/tools/net/ynl/lib/ynl.c index 70f899a54007..c16f01372ca3 100644 --- a/tools/net/ynl/lib/ynl.c +++ b/tools/net/ynl/lib/ynl.c @@ -451,9 +451,9 @@ ynl_gemsg_start(struct ynl_sock *ys, __u32 id, __u16 flags, return nlh; } -struct nlmsghdr *ynl_msg_start_req(struct ynl_sock *ys, __u32 id) +struct nlmsghdr *ynl_msg_start_req(struct ynl_sock *ys, __u32 id, __u16 flags) { - return ynl_msg_start(ys, id, NLM_F_REQUEST | NLM_F_ACK); + return ynl_msg_start(ys, id, NLM_F_REQUEST | NLM_F_ACK | flags); } struct nlmsghdr *ynl_msg_start_dump(struct ynl_sock *ys, __u32 id) diff --git a/tools/net/ynl/pyynl/ynl_gen_c.py b/tools/net/ynl/pyynl/ynl_gen_c.py index 898c41a7a81f..c035abb8ae1c 100755 --- a/tools/net/ynl/pyynl/ynl_gen_c.py +++ b/tools/net/ynl/pyynl/ynl_gen_c.py @@ -1294,6 +1294,9 @@ class RenderInfo: def type_empty(self, key): return len(self.struct[key].attr_list) == 0 and self.fixed_hdr is None + def needs_nlflags(self, direction): + return self.op_mode == 'do' and direction == 'request' and self.family.is_classic() + class CodeWriter: def __init__(self, nlib, out_file=None, overwrite=True): @@ -1924,7 +1927,7 @@ def print_req(ri): ri.cw.write_func_lvar(local_vars) if ri.family.is_classic(): - ri.cw.p(f"nlh = ynl_msg_start_req(ys, {ri.op.enum_name});") + ri.cw.p(f"nlh = ynl_msg_start_req(ys, {ri.op.enum_name}, req->_nlmsg_flags);") else: ri.cw.p(f"nlh = ynl_gemsg_start_req(ys, {ri.nl.get_family_id()}, {ri.op.enum_name}, 1);") @@ -2053,6 +2056,16 @@ def print_free_prototype(ri, direction, suffix=';'): ri.cw.write_func_prot('void', f"{name}_free", [f"struct {struct_name} *{arg}"], suffix=suffix) +def print_nlflags_set(ri, direction): + name = op_prefix(ri, direction) + ri.cw.write_func_prot(f'static inline void', f"{name}_set_nlflags", + [f"struct {name} *req", "__u16 nl_flags"]) + ri.cw.block_start() + ri.cw.p('req->_nlmsg_flags = nl_flags;') + ri.cw.block_end() + ri.cw.nl() + + def _print_type(ri, direction, struct): suffix = f'_{ri.type_name}{direction_to_suffix[direction]}' if not direction and ri.type_name_conflict: @@ -2063,6 +2076,9 @@ def _print_type(ri, direction, struct): ri.cw.block_start(line=f"struct {ri.family.c_name}{suffix}") + if ri.needs_nlflags(direction): + ri.cw.p('__u16 _nlmsg_flags;') + ri.cw.nl() if ri.fixed_hdr: ri.cw.p(ri.fixed_hdr + ' _hdr;') ri.cw.nl() @@ -2102,6 +2118,9 @@ def print_type_helpers(ri, direction, deref=False): print_free_prototype(ri, direction) ri.cw.nl() + if ri.needs_nlflags(direction): + print_nlflags_set(ri, direction) + if ri.ku_space == 'user' and direction == 'request': for _, attr in ri.struct[direction].member_list(): attr.setter(ri, ri.attr_set, direction, deref=deref) From bbfb3c557c66899480c679b7958908722b821ac3 Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Tue, 29 Apr 2025 08:46:57 -0700 Subject: [PATCH 05/12] tools: ynl-gen: support using dump types for ntf Classic Netlink has GET callbacks with no doit support, just dumps. Support using their responses in notifications. If notification points at a type which only has a dump - use the dump's type. Reviewed-by: Donald Hunter Reviewed-by: Jacob Keller Signed-off-by: Jakub Kicinski Link: https://patch.msgid.link/20250429154704.2613851-6-kuba@kernel.org Signed-off-by: Paolo Abeni --- tools/net/ynl/pyynl/ynl_gen_c.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/net/ynl/pyynl/ynl_gen_c.py b/tools/net/ynl/pyynl/ynl_gen_c.py index c035abb8ae1c..0febbb3912e3 100755 --- a/tools/net/ynl/pyynl/ynl_gen_c.py +++ b/tools/net/ynl/pyynl/ynl_gen_c.py @@ -1281,7 +1281,7 @@ class RenderInfo: self.struct = dict() if op_mode == 'notify': - op_mode = 'do' + op_mode = 'do' if 'do' in op else 'dump' for op_dir in ['request', 'reply']: if op: type_list = [] From 49398830a4aa650c00b402477042c90db7d6214c Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Tue, 29 Apr 2025 08:46:58 -0700 Subject: [PATCH 06/12] tools: ynl-gen: support CRUD-like notifications for classic Netlink Allow CRUD-style notification where the notification is more like the response to the request, which can optionally be looped back onto the requesting socket. Since the notification and request are different ops in the spec, for example: - name: delrule doc: Remove an existing FIB rule attribute-set: fib-rule-attrs do: request: value: 33 attributes: *fib-rule-all - name: delrule-ntf doc: Notify a rule deletion value: 33 notify: getrule We need to find the request by ID. Ideally we'd detect this model from the spec properties, rather than assume that its what all classic netlink families do. But maybe that'd cause this model to spread and its easy to get wrong. For now assume CRUD == classic. Reviewed-by: Donald Hunter Reviewed-by: Jacob Keller Signed-off-by: Jakub Kicinski Link: https://patch.msgid.link/20250429154704.2613851-7-kuba@kernel.org Signed-off-by: Paolo Abeni --- tools/net/ynl/pyynl/ynl_gen_c.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tools/net/ynl/pyynl/ynl_gen_c.py b/tools/net/ynl/pyynl/ynl_gen_c.py index 0febbb3912e3..31e904f1a2f0 100755 --- a/tools/net/ynl/pyynl/ynl_gen_c.py +++ b/tools/net/ynl/pyynl/ynl_gen_c.py @@ -2787,7 +2787,11 @@ def render_uapi(family, cw): def _render_user_ntf_entry(ri, op): - ri.cw.block_start(line=f"[{op.enum_name}] = ") + if not ri.family.is_classic(): + ri.cw.block_start(line=f"[{op.enum_name}] = ") + else: + crud_op = ri.family.req_by_value[op.rsp_value] + ri.cw.block_start(line=f"[{crud_op.enum_name}] = ") ri.cw.p(f".alloc_sz\t= sizeof({type_name(ri, 'event')}),") ri.cw.p(f".cb\t\t= {op_prefix(ri, 'reply', deref=True)}_parse,") ri.cw.p(f".policy\t\t= &{ri.struct['reply'].render_name}_nest,") From 0ea8cf56cc20ed51cfbef7c6410874ac7fd07642 Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Tue, 29 Apr 2025 08:46:59 -0700 Subject: [PATCH 07/12] tools: ynl-gen: multi-attr: type gen for string Add support for multi attr strings (needed for link alt_names). We record the length individual strings in a len member, to do the same for multi-attr create a struct ynl_string in ynl.h and use it as a layer holding both the string and its length. Since strings may be arbitrary length dynamically allocate each individual one. Adjust arg_member and struct member to avoid spacing the double pointers to get "type **name;" rather than "type * *name;" Reviewed-by: Donald Hunter Reviewed-by: Jacob Keller Signed-off-by: Jakub Kicinski Link: https://patch.msgid.link/20250429154704.2613851-8-kuba@kernel.org Signed-off-by: Paolo Abeni --- tools/net/ynl/lib/ynl.h | 13 +++++++++++++ tools/net/ynl/pyynl/ynl_gen_c.py | 29 +++++++++++++++++++++++++---- 2 files changed, 38 insertions(+), 4 deletions(-) diff --git a/tools/net/ynl/lib/ynl.h b/tools/net/ynl/lib/ynl.h index 59256e258130..6b8a625aaa5f 100644 --- a/tools/net/ynl/lib/ynl.h +++ b/tools/net/ynl/lib/ynl.h @@ -85,6 +85,19 @@ struct ynl_sock { unsigned char raw_buf[]; }; +/** + * struct ynl_string - parsed individual string + * @len: length of the string (excluding terminating character) + * @str: value of the string + * + * Parsed and nul-terminated string. This struct is only used for arrays of + * strings. Non-array string members are placed directly in respective types. + */ +struct ynl_string { + unsigned int len; + char str[]; +}; + struct ynl_sock * ynl_sock_create(const struct ynl_family *yf, struct ynl_error *e); void ynl_sock_destroy(struct ynl_sock *ys); diff --git a/tools/net/ynl/pyynl/ynl_gen_c.py b/tools/net/ynl/pyynl/ynl_gen_c.py index 31e904f1a2f0..895bc1ca9505 100755 --- a/tools/net/ynl/pyynl/ynl_gen_c.py +++ b/tools/net/ynl/pyynl/ynl_gen_c.py @@ -175,7 +175,8 @@ class Type(SpecAttr): def arg_member(self, ri): member = self._complex_member_type(ri) if member: - arg = [member + ' *' + self.c_name] + spc = ' ' if member[-1] != '*' else '' + arg = [member + spc + '*' + self.c_name] if self.presence_type() == 'count': arg += ['unsigned int n_' + self.c_name] return arg @@ -189,7 +190,8 @@ class Type(SpecAttr): ptr = '*' if self.is_multi_val() else '' if self.is_recursive_for_op(ri): ptr = '*' - ri.cw.p(f"{member} {ptr}{self.c_name};") + spc = ' ' if member[-1] != '*' else '' + ri.cw.p(f"{member}{spc}{ptr}{self.c_name};") return members = self.arg_member(ri) for one in members: @@ -638,6 +640,8 @@ class TypeMultiAttr(Type): def _complex_member_type(self, ri): if 'type' not in self.attr or self.attr['type'] == 'nest': return self.nested_struct_type + elif self.attr['type'] == 'string': + return 'struct ynl_string *' elif self.attr['type'] in scalars: scalar_pfx = '__' if ri.ku_space == 'user' else '' return scalar_pfx + self.attr['type'] @@ -645,12 +649,18 @@ class TypeMultiAttr(Type): raise Exception(f"Sub-type {self.attr['type']} not supported yet") def free_needs_iter(self): - return 'type' not in self.attr or self.attr['type'] == 'nest' + return self.attr['type'] in {'nest', 'string'} def _free_lines(self, ri, var, ref): lines = [] if self.attr['type'] in scalars: lines += [f"free({var}->{ref}{self.c_name});"] + elif self.attr['type'] == 'string': + lines += [ + f"for (i = 0; i < {var}->{ref}n_{self.c_name}; i++)", + f"free({var}->{ref}{self.c_name}[i]);", + f"free({var}->{ref}{self.c_name});", + ] elif 'type' not in self.attr or self.attr['type'] == 'nest': lines += [ f"for (i = 0; i < {var}->{ref}n_{self.c_name}; i++)", @@ -675,6 +685,9 @@ class TypeMultiAttr(Type): put_type = self.type ri.cw.p(f"for (i = 0; i < {var}->n_{self.c_name}; i++)") ri.cw.p(f"ynl_attr_put_{put_type}(nlh, {self.enum_name}, {var}->{self.c_name}[i]);") + elif self.attr['type'] == 'string': + ri.cw.p(f"for (i = 0; i < {var}->n_{self.c_name}; i++)") + ri.cw.p(f"ynl_attr_put_str(nlh, {self.enum_name}, {var}->{self.c_name}[i]->str);") elif 'type' not in self.attr or self.attr['type'] == 'nest': ri.cw.p(f"for (i = 0; i < {var}->n_{self.c_name}; i++)") self._attr_put_line(ri, var, f"{self.nested_render_name}_put(nlh, " + @@ -1834,8 +1847,16 @@ def _multi_parse(ri, struct, init_lines, local_vars): ri.cw.p('return YNL_PARSE_CB_ERROR;') elif aspec.type in scalars: ri.cw.p(f"dst->{aspec.c_name}[i] = ynl_attr_get_{aspec.type}(attr);") + elif aspec.type == 'string': + ri.cw.p('unsigned int len;') + ri.cw.nl() + ri.cw.p('len = strnlen(ynl_attr_get_str(attr), ynl_attr_data_len(attr));') + ri.cw.p(f'dst->{aspec.c_name}[i] = malloc(sizeof(struct ynl_string) + len + 1);') + ri.cw.p(f"dst->{aspec.c_name}[i]->len = len;") + ri.cw.p(f"memcpy(dst->{aspec.c_name}[i]->str, ynl_attr_get_str(attr), len);") + ri.cw.p(f"dst->{aspec.c_name}[i]->str[len] = 0;") else: - raise Exception('Nest parsing type not supported yet') + raise Exception(f'Nest parsing of type {aspec.type} not supported yet') ri.cw.p('i++;') ri.cw.block_end() ri.cw.block_end() From 3456084d6361f4c835b73a0bce04fa5894bf9c65 Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Tue, 29 Apr 2025 08:47:00 -0700 Subject: [PATCH 08/12] tools: ynl-gen: mutli-attr: support binary types with struct Binary types with struct are fixed size, relatively easy to handle for multi attr. Declare the member as a pointer. Count the members, allocate an array, copy in the data. Allow the netlink attr to be smaller or larger than our view of the struct in case the build headers are newer or older than the running kernel. Reviewed-by: Donald Hunter Reviewed-by: Jacob Keller Signed-off-by: Jakub Kicinski Link: https://patch.msgid.link/20250429154704.2613851-9-kuba@kernel.org Signed-off-by: Paolo Abeni --- tools/net/ynl/pyynl/ynl_gen_c.py | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/tools/net/ynl/pyynl/ynl_gen_c.py b/tools/net/ynl/pyynl/ynl_gen_c.py index 895bc1ca9505..a969762d557b 100755 --- a/tools/net/ynl/pyynl/ynl_gen_c.py +++ b/tools/net/ynl/pyynl/ynl_gen_c.py @@ -640,6 +640,8 @@ class TypeMultiAttr(Type): def _complex_member_type(self, ri): if 'type' not in self.attr or self.attr['type'] == 'nest': return self.nested_struct_type + elif self.attr['type'] == 'binary' and 'struct' in self.attr: + return None # use arg_member() elif self.attr['type'] == 'string': return 'struct ynl_string *' elif self.attr['type'] in scalars: @@ -648,6 +650,12 @@ class TypeMultiAttr(Type): else: raise Exception(f"Sub-type {self.attr['type']} not supported yet") + def arg_member(self, ri): + if self.type == 'binary' and 'struct' in self.attr: + return [f'struct {c_lower(self.attr["struct"])} *{self.c_name}', + f'unsigned int n_{self.c_name}'] + return super().arg_member(ri) + def free_needs_iter(self): return self.attr['type'] in {'nest', 'string'} @@ -655,6 +663,8 @@ class TypeMultiAttr(Type): lines = [] if self.attr['type'] in scalars: lines += [f"free({var}->{ref}{self.c_name});"] + elif self.attr['type'] == 'binary' and 'struct' in self.attr: + lines += [f"free({var}->{ref}{self.c_name});"] elif self.attr['type'] == 'string': lines += [ f"for (i = 0; i < {var}->{ref}n_{self.c_name}; i++)", @@ -685,6 +695,9 @@ class TypeMultiAttr(Type): put_type = self.type ri.cw.p(f"for (i = 0; i < {var}->n_{self.c_name}; i++)") ri.cw.p(f"ynl_attr_put_{put_type}(nlh, {self.enum_name}, {var}->{self.c_name}[i]);") + elif self.attr['type'] == 'binary' and 'struct' in self.attr: + ri.cw.p(f"for (i = 0; i < {var}->n_{self.c_name}; i++)") + ri.cw.p(f"ynl_attr_put(nlh, {self.enum_name}, &{var}->{self.c_name}[i], sizeof(struct {c_lower(self.attr['struct'])}));") elif self.attr['type'] == 'string': ri.cw.p(f"for (i = 0; i < {var}->n_{self.c_name}; i++)") ri.cw.p(f"ynl_attr_put_str(nlh, {self.enum_name}, {var}->{self.c_name}[i]->str);") @@ -1847,6 +1860,12 @@ def _multi_parse(ri, struct, init_lines, local_vars): ri.cw.p('return YNL_PARSE_CB_ERROR;') elif aspec.type in scalars: ri.cw.p(f"dst->{aspec.c_name}[i] = ynl_attr_get_{aspec.type}(attr);") + elif aspec.type == 'binary' and 'struct' in aspec: + ri.cw.p('size_t len = ynl_attr_data_len(attr);') + ri.cw.nl() + ri.cw.p(f'if (len > sizeof(dst->{aspec.c_name}[0]))') + ri.cw.p(f'len = sizeof(dst->{aspec.c_name}[0]);') + ri.cw.p(f"memcpy(&dst->{aspec.c_name}[i], ynl_attr_data(attr), len);") elif aspec.type == 'string': ri.cw.p('unsigned int len;') ri.cw.nl() From 18b1886447d619fcd7515dcbda55b1903560e266 Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Tue, 29 Apr 2025 08:47:01 -0700 Subject: [PATCH 09/12] tools: ynl-gen: array-nest: support put for scalar C codegen supports ArrayNest AKA indexed-array carrying scalars, but only for the netlink -> struct parsing. Support rendering from struct to netlink. Reviewed-by: Donald Hunter Reviewed-by: Jacob Keller Signed-off-by: Jakub Kicinski Link: https://patch.msgid.link/20250429154704.2613851-10-kuba@kernel.org Signed-off-by: Paolo Abeni --- tools/net/ynl/pyynl/ynl_gen_c.py | 28 +++++++++++++++++++++++++--- 1 file changed, 25 insertions(+), 3 deletions(-) diff --git a/tools/net/ynl/pyynl/ynl_gen_c.py b/tools/net/ynl/pyynl/ynl_gen_c.py index a969762d557b..a4e65da19696 100755 --- a/tools/net/ynl/pyynl/ynl_gen_c.py +++ b/tools/net/ynl/pyynl/ynl_gen_c.py @@ -747,6 +747,23 @@ class TypeArrayNest(Type): '}'] return get_lines, None, local_vars + def attr_put(self, ri, var): + ri.cw.p(f'array = ynl_attr_nest_start(nlh, {self.enum_name});') + if self.sub_type in scalars: + put_type = self.sub_type + ri.cw.block_start(line=f'for (i = 0; i < {var}->n_{self.c_name}; i++)') + ri.cw.p(f"ynl_attr_put_{put_type}(nlh, i, {var}->{self.c_name}[i]);") + ri.cw.block_end() + else: + raise Exception(f"Put for ArrayNest sub-type {self.attr['sub-type']} not supported, yet") + ri.cw.p('ynl_attr_nest_end(nlh, array);') + + def _setter_lines(self, ri, member, presence): + # For multi-attr we have a count, not presence, hack up the presence + presence = presence[:-(len('_present.') + len(self.c_name))] + "n_" + self.c_name + return [f"{member} = {self.c_name};", + f"{presence} = n_{self.c_name};"] + class TypeNestTypeValue(Type): def _complex_member_type(self, ri): @@ -1728,10 +1745,15 @@ def put_req_nested(ri, struct): local_vars.append('struct nlattr *nest;') init_lines.append("nest = ynl_attr_nest_start(nlh, attr_type);") + has_anest = False + has_count = False for _, arg in struct.member_list(): - if arg.presence_type() == 'count': - local_vars.append('unsigned int i;') - break + has_anest |= arg.type == 'indexed-array' + has_count |= arg.presence_type() == 'count' + if has_anest: + local_vars.append('struct nlattr *array;') + if has_count: + local_vars.append('unsigned int i;') put_req_nested_prototype(ri, struct, suffix='') ri.cw.block_start() From 5f7804dd8326d9ffb25553481c33ceaaedb4eb6b Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Tue, 29 Apr 2025 08:47:02 -0700 Subject: [PATCH 10/12] tools: ynl-gen: array-nest: support binary array with exact-len IPv6 addresses are expressed as binary arrays since we don't have u128. Since they are not variable length, however, they are relatively easy to represent as an array of known size. Reviewed-by: Donald Hunter Reviewed-by: Jacob Keller Signed-off-by: Jakub Kicinski Link: https://patch.msgid.link/20250429154704.2613851-11-kuba@kernel.org Signed-off-by: Paolo Abeni --- tools/net/ynl/pyynl/ynl_gen_c.py | 24 ++++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/tools/net/ynl/pyynl/ynl_gen_c.py b/tools/net/ynl/pyynl/ynl_gen_c.py index a4e65da19696..2d185c7ea16c 100755 --- a/tools/net/ynl/pyynl/ynl_gen_c.py +++ b/tools/net/ynl/pyynl/ynl_gen_c.py @@ -183,10 +183,10 @@ class Type(SpecAttr): raise Exception(f"Struct member not implemented for class type {self.type}") def struct_member(self, ri): - if self.is_multi_val(): - ri.cw.p(f"unsigned int n_{self.c_name};") member = self._complex_member_type(ri) if member: + if self.is_multi_val(): + ri.cw.p(f"unsigned int n_{self.c_name};") ptr = '*' if self.is_multi_val() else '' if self.is_recursive_for_op(ri): ptr = '*' @@ -728,12 +728,22 @@ class TypeArrayNest(Type): elif self.attr['sub-type'] in scalars: scalar_pfx = '__' if ri.ku_space == 'user' else '' return scalar_pfx + self.attr['sub-type'] + elif self.attr['sub-type'] == 'binary' and 'exact-len' in self.checks: + return None # use arg_member() else: raise Exception(f"Sub-type {self.attr['sub-type']} not supported yet") + def arg_member(self, ri): + if self.sub_type == 'binary' and 'exact-len' in self.checks: + return [f'unsigned char (*{self.c_name})[{self.checks["exact-len"]}]', + f'unsigned int n_{self.c_name}'] + return super().arg_member(ri) + def _attr_typol(self): if self.attr['sub-type'] in scalars: return f'.type = YNL_PT_U{c_upper(self.sub_type[1:])}, ' + elif self.attr['sub-type'] == 'binary' and 'exact-len' in self.checks: + return f'.type = YNL_PT_BINARY, .len = {self.checks["exact-len"]}, ' else: return f'.type = YNL_PT_NEST, .nest = &{self.nested_render_name}_nest, ' @@ -754,6 +764,9 @@ class TypeArrayNest(Type): ri.cw.block_start(line=f'for (i = 0; i < {var}->n_{self.c_name}; i++)') ri.cw.p(f"ynl_attr_put_{put_type}(nlh, i, {var}->{self.c_name}[i]);") ri.cw.block_end() + elif self.sub_type == 'binary' and 'exact-len' in self.checks: + ri.cw.p(f'for (i = 0; i < {var}->n_{self.c_name}; i++)') + ri.cw.p(f"ynl_attr_put(nlh, i, {var}->{self.c_name}[i], {self.checks['exact-len']});") else: raise Exception(f"Put for ArrayNest sub-type {self.attr['sub-type']} not supported, yet") ri.cw.p('ynl_attr_nest_end(nlh, array);') @@ -964,7 +977,7 @@ class AttrSet(SpecAttrSet): elif elem['type'] == 'nest': t = TypeNest(self.family, self, elem, value) elif elem['type'] == 'indexed-array' and 'sub-type' in elem: - if elem["sub-type"] in ['nest', 'u32']: + if elem["sub-type"] in ['binary', 'nest', 'u32']: t = TypeArrayNest(self.family, self, elem, value) else: raise Exception(f'new_attr: unsupported sub-type {elem["sub-type"]}') @@ -1786,7 +1799,7 @@ def _multi_parse(ri, struct, init_lines, local_vars): needs_parg = False for arg, aspec in struct.member_list(): if aspec['type'] == 'indexed-array' and 'sub-type' in aspec: - if aspec["sub-type"] == 'nest': + if aspec["sub-type"] in {'binary', 'nest'}: local_vars.append(f'const struct nlattr *attr_{aspec.c_name};') array_nests.add(arg) elif aspec['sub-type'] in scalars: @@ -1859,6 +1872,9 @@ def _multi_parse(ri, struct, init_lines, local_vars): ri.cw.p('return YNL_PARSE_CB_ERROR;') elif aspec.sub_type in scalars: ri.cw.p(f"dst->{aspec.c_name}[i] = ynl_attr_get_{aspec.sub_type}(attr);") + elif aspec.sub_type == 'binary' and 'exact-len' in aspec.checks: + # Length is validated by typol + ri.cw.p(f'memcpy(dst->{aspec.c_name}[i], ynl_attr_data(attr), {aspec.checks["exact-len"]});') else: raise Exception(f"Nest parsing type not supported in {aspec['name']}") ri.cw.p('i++;') From 18d574c8dd3e075afb546408fae7edd6a14c9873 Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Tue, 29 Apr 2025 08:47:03 -0700 Subject: [PATCH 11/12] tools: ynl-gen: don't init enum checks for classic netlink rt-link has a vlan-protocols enum with: name: 8021q value: 33024 name: 8021ad value: 34984 It's nice to have, since it converts the values to strings in Python. For C, however, the codegen is trying to use enums to generate strict policy checks. Parsing such sparse enums is not possible via policies. Since for classic netlink we don't support kernel codegen and policy generation - skip the auto-generation of checks from enums. Reviewed-by: Donald Hunter Signed-off-by: Jakub Kicinski Link: https://patch.msgid.link/20250429154704.2613851-12-kuba@kernel.org Signed-off-by: Paolo Abeni --- tools/net/ynl/pyynl/ynl_gen_c.py | 46 ++++++++++++++++++-------------- 1 file changed, 26 insertions(+), 20 deletions(-) diff --git a/tools/net/ynl/pyynl/ynl_gen_c.py b/tools/net/ynl/pyynl/ynl_gen_c.py index 2d185c7ea16c..1dd9580086cd 100755 --- a/tools/net/ynl/pyynl/ynl_gen_c.py +++ b/tools/net/ynl/pyynl/ynl_gen_c.py @@ -357,26 +357,10 @@ class TypeScalar(Type): if 'byte-order' in attr: self.byte_order_comment = f" /* {attr['byte-order']} */" - if 'enum' in self.attr: - enum = self.family.consts[self.attr['enum']] - low, high = enum.value_range() - if 'min' not in self.checks: - if low != 0 or self.type[0] == 's': - self.checks['min'] = low - if 'max' not in self.checks: - self.checks['max'] = high - - if 'min' in self.checks and 'max' in self.checks: - if self.get_limit('min') > self.get_limit('max'): - raise Exception(f'Invalid limit for "{self.name}" min: {self.get_limit("min")} max: {self.get_limit("max")}') - self.checks['range'] = True - - low = min(self.get_limit('min', 0), self.get_limit('max', 0)) - high = max(self.get_limit('min', 0), self.get_limit('max', 0)) - if low < 0 and self.type[0] == 'u': - raise Exception(f'Invalid limit for "{self.name}" negative limit for unsigned type') - if low < -32768 or high > 32767: - self.checks['full-range'] = True + # Classic families have some funny enums, don't bother + # computing checks, since we only need them for kernel policies + if not family.is_classic(): + self._init_checks() # Added by resolve(): self.is_bitfield = None @@ -401,6 +385,28 @@ class TypeScalar(Type): else: self.type_name = '__' + self.type + def _init_checks(self): + if 'enum' in self.attr: + enum = self.family.consts[self.attr['enum']] + low, high = enum.value_range() + if 'min' not in self.checks: + if low != 0 or self.type[0] == 's': + self.checks['min'] = low + if 'max' not in self.checks: + self.checks['max'] = high + + if 'min' in self.checks and 'max' in self.checks: + if self.get_limit('min') > self.get_limit('max'): + raise Exception(f'Invalid limit for "{self.name}" min: {self.get_limit("min")} max: {self.get_limit("max")}') + self.checks['range'] = True + + low = min(self.get_limit('min', 0), self.get_limit('max', 0)) + high = max(self.get_limit('min', 0), self.get_limit('max', 0)) + if low < 0 and self.type[0] == 'u': + raise Exception(f'Invalid limit for "{self.name}" negative limit for unsigned type') + if low < -32768 or high > 32767: + self.checks['full-range'] = True + def _attr_policy(self, policy): if 'flags-mask' in self.checks or self.is_bitfield: if self.is_bitfield: From 777c8029b551caafde89e0440c40127b8b5ba70e Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Tue, 29 Apr 2025 08:47:04 -0700 Subject: [PATCH 12/12] tools: ynl: allow fixed-header to be specified per op rtnetlink has variety of ops with different fixed headers. Detect that op fixed header is not the same as family one, and use sizeof() directly. For reverse parsing we need to pass the fixed header len along the policy (in the socket state). Reviewed-by: Donald Hunter Reviewed-by: Jacob Keller Signed-off-by: Jakub Kicinski Link: https://patch.msgid.link/20250429154704.2613851-13-kuba@kernel.org Signed-off-by: Paolo Abeni --- tools/net/ynl/lib/ynl.c | 8 ++++---- tools/net/ynl/lib/ynl.h | 1 + tools/net/ynl/pyynl/ynl_gen_c.py | 17 ++++++++++++++++- 3 files changed, 21 insertions(+), 5 deletions(-) diff --git a/tools/net/ynl/lib/ynl.c b/tools/net/ynl/lib/ynl.c index c16f01372ca3..d263f6f40ad5 100644 --- a/tools/net/ynl/lib/ynl.c +++ b/tools/net/ynl/lib/ynl.c @@ -191,12 +191,12 @@ ynl_ext_ack_check(struct ynl_sock *ys, const struct nlmsghdr *nlh, n = snprintf(bad_attr, sizeof(bad_attr), "%sbad attribute: ", str ? " (" : ""); - start = ynl_nlmsg_data_offset(ys->nlh, ys->family->hdr_len); + start = ynl_nlmsg_data_offset(ys->nlh, ys->req_hdr_len); end = ynl_nlmsg_end_addr(ys->nlh); off = ys->err.attr_offs; off -= sizeof(struct nlmsghdr); - off -= ys->family->hdr_len; + off -= ys->req_hdr_len; n += ynl_err_walk(ys, start, end, off, ys->req_policy, &bad_attr[n], sizeof(bad_attr) - n, NULL); @@ -216,14 +216,14 @@ ynl_ext_ack_check(struct ynl_sock *ys, const struct nlmsghdr *nlh, n = snprintf(miss_attr, sizeof(miss_attr), "%smissing attribute: ", bad_attr[0] ? ", " : (str ? " (" : "")); - start = ynl_nlmsg_data_offset(ys->nlh, ys->family->hdr_len); + start = ynl_nlmsg_data_offset(ys->nlh, ys->req_hdr_len); end = ynl_nlmsg_end_addr(ys->nlh); nest_pol = ys->req_policy; if (tb[NLMSGERR_ATTR_MISS_NEST]) { off = ynl_attr_get_u32(tb[NLMSGERR_ATTR_MISS_NEST]); off -= sizeof(struct nlmsghdr); - off -= ys->family->hdr_len; + off -= ys->req_hdr_len; n += ynl_err_walk(ys, start, end, off, ys->req_policy, &miss_attr[n], sizeof(miss_attr) - n, diff --git a/tools/net/ynl/lib/ynl.h b/tools/net/ynl/lib/ynl.h index 6b8a625aaa5f..32efeb224829 100644 --- a/tools/net/ynl/lib/ynl.h +++ b/tools/net/ynl/lib/ynl.h @@ -80,6 +80,7 @@ struct ynl_sock { struct nlmsghdr *nlh; const struct ynl_policy_nest *req_policy; + size_t req_hdr_len; unsigned char *tx_buf; unsigned char *rx_buf; unsigned char raw_buf[]; diff --git a/tools/net/ynl/pyynl/ynl_gen_c.py b/tools/net/ynl/pyynl/ynl_gen_c.py index 1dd9580086cd..09b87c9a6908 100755 --- a/tools/net/ynl/pyynl/ynl_gen_c.py +++ b/tools/net/ynl/pyynl/ynl_gen_c.py @@ -1311,8 +1311,15 @@ class RenderInfo: self.op = op self.fixed_hdr = None + self.fixed_hdr_len = 'ys->family->hdr_len' if op and op.fixed_header: self.fixed_hdr = 'struct ' + c_lower(op.fixed_header) + if op.fixed_header != family.fixed_header: + if family.is_classic(): + self.fixed_hdr_len = f"sizeof({self.fixed_hdr})" + else: + raise Exception(f"Per-op fixed header not supported, yet") + # 'do' and 'dump' response parsing is identical self.type_consistent = True @@ -1799,6 +1806,11 @@ def _multi_parse(ri, struct, init_lines, local_vars): if ri.fixed_hdr: local_vars += ['void *hdr;'] iter_line = "ynl_attr_for_each(attr, nlh, yarg->ys->family->hdr_len)" + if ri.op.fixed_header != ri.family.fixed_header: + if ri.family.is_classic(): + iter_line = f"ynl_attr_for_each(attr, nlh, sizeof({ri.fixed_hdr}))" + else: + raise Exception(f"Per-op fixed header not supported, yet") array_nests = set() multi_attrs = set() @@ -2016,6 +2028,7 @@ def print_req(ri): ri.cw.p(f"nlh = ynl_gemsg_start_req(ys, {ri.nl.get_family_id()}, {ri.op.enum_name}, 1);") ri.cw.p(f"ys->req_policy = &{ri.struct['request'].render_name}_nest;") + ri.cw.p(f"ys->req_hdr_len = {ri.fixed_hdr_len};") if 'reply' in ri.op[ri.op_mode]: ri.cw.p(f"yrs.yarg.rsp_policy = &{ri.struct['reply'].render_name}_nest;") ri.cw.nl() @@ -2095,6 +2108,7 @@ def print_dump(ri): if "request" in ri.op[ri.op_mode]: ri.cw.p(f"ys->req_policy = &{ri.struct['request'].render_name}_nest;") + ri.cw.p(f"ys->req_hdr_len = {ri.fixed_hdr_len};") ri.cw.nl() for _, attr in ri.struct["request"].member_list(): attr.attr_put(ri, "req") @@ -2914,7 +2928,8 @@ def render_user_family(family, cw, prototype): cw.p(f'.is_classic\t= true,') cw.p(f'.classic_id\t= {family.get("protonum")},') if family.is_classic(): - cw.p(f'.hdr_len\t= sizeof(struct {c_lower(family.fixed_header)}),') + if family.fixed_header: + cw.p(f'.hdr_len\t= sizeof(struct {c_lower(family.fixed_header)}),') elif family.fixed_header: cw.p(f'.hdr_len\t= sizeof(struct genlmsghdr) + sizeof(struct {c_lower(family.fixed_header)}),') else: