This patch adds support for NLA_POLICY_NESTED_ARRAY() policies.
Example spec (from future wireguard.yaml):
-
name: wgpeer
attributes:
-
name: allowedips
type: indexed-array
sub-type: nest
nested-attributes: wgallowedip
yields NLA_POLICY_NESTED_ARRAY(wireguard_wgallowedip_nl_policy).
This doesn't change any currently generated code, as it isn't
used in any specs currently used for generating code.
Signed-off-by: Asbjørn Sloth Tønnesen <ast@fiberby.net>
---
tools/net/ynl/pyynl/ynl_gen_c.py | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/tools/net/ynl/pyynl/ynl_gen_c.py b/tools/net/ynl/pyynl/ynl_gen_c.py
index 1543d4911bf5..b7de7f6b1fc7 100755
--- a/tools/net/ynl/pyynl/ynl_gen_c.py
+++ b/tools/net/ynl/pyynl/ynl_gen_c.py
@@ -816,6 +816,11 @@ class TypeArrayNest(Type):
f'unsigned int n_{self.c_name}']
return super().arg_member(ri)
+ def _attr_policy(self, policy):
+ if self.attr['sub-type'] == 'nest':
+ return f'NLA_POLICY_NESTED_ARRAY({self.nested_render_name}_nl_policy)'
+ return super()._attr_policy(policy)
+
def _attr_typol(self):
if self.attr['sub-type'] in scalars:
return f'.type = YNL_PT_U{c_upper(self.sub_type[1:])}, '
--
2.51.0
On 9/4/2025 3:01 PM, Asbjørn Sloth Tønnesen wrote: > This patch adds support for NLA_POLICY_NESTED_ARRAY() policies. > > Example spec (from future wireguard.yaml): > - > name: wgpeer > attributes: > - > name: allowedips > type: indexed-array > sub-type: nest > nested-attributes: wgallowedip > > yields NLA_POLICY_NESTED_ARRAY(wireguard_wgallowedip_nl_policy). > > This doesn't change any currently generated code, as it isn't > used in any specs currently used for generating code. > > Signed-off-by: Asbjørn Sloth Tønnesen <ast@fiberby.net> > --- Is this keyed of off the sub-type? Does you mean that all the existing uses of 'sub-type: nest' don't generate code today? Or that this _attr_policy implementation is not called yet? I checked and we have quite a number of uses: > $ rg 'sub-type: nest' > Documentation/netlink/specs/nlctrl.yaml > 69: sub-type: nest > 74: sub-type: nest > > Documentation/netlink/specs/tc.yaml > 2045: sub-type: nest > 2065: sub-type: nest > 2190: sub-type: nest > 2304: sub-type: nest > 2494: sub-type: nest > 3021: sub-type: nest > 3181: sub-type: nest > 3567: sub-type: nest > 3799: sub-type: nest > > Documentation/netlink/specs/rt-link.yaml > 2203: sub-type: nest > > Documentation/netlink/specs/nl80211.yaml > 610: sub-type: nest > 1309: sub-type: nest > 1314: sub-type: nest > 1337: sub-type: nest > 1420: sub-type: nest > 1476: sub-type: nest > 1615: sub-type: nest >
CC: Johannes
On 9/6/25 12:19 AM, Jacob Keller wrote:
> On 9/4/2025 3:01 PM, Asbjørn Sloth Tønnesen wrote:
>> This patch adds support for NLA_POLICY_NESTED_ARRAY() policies.
>>
>> Example spec (from future wireguard.yaml):
>> -
>> name: wgpeer
>> attributes:
>> -
>> name: allowedips
>> type: indexed-array
>> sub-type: nest
>> nested-attributes: wgallowedip
>>
>> yields NLA_POLICY_NESTED_ARRAY(wireguard_wgallowedip_nl_policy).
>>
>> This doesn't change any currently generated code, as it isn't
>> used in any specs currently used for generating code.
>>
>> Signed-off-by: Asbjørn Sloth Tønnesen <ast@fiberby.net>
>> ---
>
> Is this keyed of off the sub-type? Does you mean that all the existing
> uses of 'sub-type: nest' don't generate code today? Or that this
> _attr_policy implementation is not called yet?
Thanks for the reviews. Yeah, it is a careful wording, because we have
specs matching it, but there aren't any source files that triggers
ynl-gen to generate code based on those specs.
Therefore this patch, doesn't result in any code changes when running:
$ ./tools/net/ynl/ynl-regen.sh -f
Actually ynl-gen generates a fictive "{ .type = NLA_INDEXED_ARRAY, }"
policy, without this patch, leading to a build failure.
> I checked and we have quite a number of uses:
>
>> $ rg 'sub-type: nest'
>> Documentation/netlink/specs/nlctrl.yaml
>> [..]
>> Documentation/netlink/specs/tc.yaml
>> [..]
>> Documentation/netlink/specs/rt-link.yaml
>> [..]
>> Documentation/netlink/specs/nl80211.yaml
>> [..]
None of those currently have a generated netlink policy.
These are the netlink policies currently generated by ynl-gen:
$ git grep -h -B1 'YNL-GEN kernel source' | grep '^/\*[^ ]'
/* Documentation/netlink/specs/dpll.yaml */
/* Documentation/netlink/specs/ovpn.yaml */
/* Documentation/netlink/specs/team.yaml */
/* Documentation/netlink/specs/lockd.yaml */
/* Documentation/netlink/specs/nfsd.yaml */
/* Documentation/netlink/specs/netdev.yaml */
/* Documentation/netlink/specs/devlink.yaml */
/* Documentation/netlink/specs/handshake.yaml */
/* Documentation/netlink/specs/fou.yaml */
/* Documentation/netlink/specs/mptcp_pm.yaml */
/* Documentation/netlink/specs/net_shaper.yaml */
Johannes introduced NLA_NESTED_ARRAY and the NLA_POLICY_NESTED_ARRAY()
macro in commit 1501d13596b9 for use in nl80211, and it's therefore
used in net/wireless/nl80211.c, but outside of that the macro is
only sparsely adopted (only by mac80211_hwsim.c and nf_tables_api.c).
Wireguard adopts the macro in this RFC patch:
https://lore.kernel.org/netdev/20250904220255.1006675-2-ast@fiberby.net/
On 9/6/2025 7:13 AM, Asbjørn Sloth Tønnesen wrote: > CC: Johannes > > On 9/6/25 12:19 AM, Jacob Keller wrote: >> On 9/4/2025 3:01 PM, Asbjørn Sloth Tønnesen wrote: >>> This patch adds support for NLA_POLICY_NESTED_ARRAY() policies. >>> >>> Example spec (from future wireguard.yaml): >>> - >>> name: wgpeer >>> attributes: >>> - >>> name: allowedips >>> type: indexed-array >>> sub-type: nest >>> nested-attributes: wgallowedip >>> >>> yields NLA_POLICY_NESTED_ARRAY(wireguard_wgallowedip_nl_policy). >>> >>> This doesn't change any currently generated code, as it isn't >>> used in any specs currently used for generating code. >>> >>> Signed-off-by: Asbjørn Sloth Tønnesen <ast@fiberby.net> >>> --- >> >> Is this keyed of off the sub-type? Does you mean that all the existing >> uses of 'sub-type: nest' don't generate code today? Or that this >> _attr_policy implementation is not called yet? > > Thanks for the reviews. Yeah, it is a careful wording, because we have > specs matching it, but there aren't any source files that triggers > ynl-gen to generate code based on those specs. > Ok. That's what I thought and just wanted to be certain, since I saw several uses in the code. Thanks for explaining!
On Sat, 2025-09-06 at 14:13 +0000, Asbjørn Sloth Tønnesen wrote: > > Johannes introduced NLA_NESTED_ARRAY and the NLA_POLICY_NESTED_ARRAY() > macro in commit 1501d13596b9 for use in nl80211, and it's therefore > used in net/wireless/nl80211.c, but outside of that the macro is > only sparsely adopted (only by mac80211_hwsim.c and nf_tables_api.c). > > Wireguard adopts the macro in this RFC patch: > https://lore.kernel.org/netdev/20250904220255.1006675-2-ast@fiberby.net/ I think the general consensus now is that preference should be towards arrays being expressed by giving the attribute holding the array multiple times, i.e. each occurrence of an attribute holds a single entry of the array: [header][type1:a1][type2:b][type1:a2][type1:a3] resulting in an array [a1, a2, a3] and a separate value "b", rather than a nested array: [header][type1:[1:a1][2:a2][3:a3]][type2:b] Of course if each entry has multiple values, then you'd still need nesting: [header][type1:[subtype1:x1][subtype2:x2]][type1:[subtype1:y1][subtype2:y2]] would be an array [[x1, x2], [y1, y2]]. I can't get rid of the nested array types in nl80211 though, of course. I'm not sure the nl80211 ynl code was ever merged, but it wasn't authoritative anyway, just for some limited userspace generation, so I'm not sure the whole ynl handling this is needed at all? johannes
On 9/8/25 7:54 AM, Johannes Berg wrote:
> On Sat, 2025-09-06 at 14:13 +0000, Asbjørn Sloth Tønnesen wrote:
>> Johannes introduced NLA_NESTED_ARRAY and the NLA_POLICY_NESTED_ARRAY()
>> macro in commit 1501d13596b9 for use in nl80211, and it's therefore
>> used in net/wireless/nl80211.c, but outside of that the macro is
>> only sparsely adopted (only by mac80211_hwsim.c and nf_tables_api.c).
>>
>> Wireguard adopts the macro in this RFC patch:
>> https://lore.kernel.org/netdev/20250904220255.1006675-2-ast@fiberby.net/
>
> I think the general consensus now is that preference should be towards
> arrays being expressed by giving the attribute holding the array
> multiple times, i.e. each occurrence of an attribute holds a single
> entry of the array:
>
> [header][type1:a1][type2:b][type1:a2][type1:a3]
>
> resulting in an array
>
> [a1, a2, a3] and a separate value "b",
>
> rather than a nested array:
>
> [header][type1:[1:a1][2:a2][3:a3]][type2:b]
>
>
> Of course if each entry has multiple values, then you'd still need
> nesting:
>
> [header][type1:[subtype1:x1][subtype2:x2]][type1:[subtype1:y1][subtype2:y2]]
>
> would be an array
>
> [[x1, x2], [y1, y2]].
Thank you for the consensus write up. Should we prohibit indexed-array with sub-type
nest for families with a genetlink protocol?
It is currently only used in families with a netlink-raw or genetlink-legacy protocol.
> I can't get rid of the nested array types in nl80211 though, of course.
Wireguard is already in the same boat. It is not using the term, nor the policy,
but it is doing the validation in the handler through, so it can adopt a
NLA_POLICY_NESTED_ARRAY() policy, instead of a plain '{ .type = NLA_NESTED }'.
Comments on the protocol in include/uapi/linux/wireguard.h:
> WGDEVICE_A_PEERS: NLA_NESTED
> 0: NLA_NESTED
> WGPEER_A_PUBLIC_KEY: NLA_EXACT_LEN, len WG_KEY_LEN
> [..]
> 0: NLA_NESTED
> ...
> ...
Given that, as Jacob pointed out, there are more families with nested arrays in
their YNL spec, than those using NLA_NESTED_ARRAY, then it appears that there
are more families already in the boat.
On Mon, 2025-09-08 at 09:08 +0000, Asbjørn Sloth Tønnesen wrote: > > Thank you for the consensus write up. Should we prohibit indexed-array with sub-type > nest for families with a genetlink protocol? > > It is currently only used in families with a netlink-raw or genetlink-legacy protocol. I have no strong opinion on that, but I guess maybe so? At least print out a warning for anyone who's trying to add such a new thing perhaps, so that new stuff that isn't just a port (to ynl) or annotation of existing APIs doesn't add it. > > I can't get rid of the nested array types in nl80211 though, of course. > > Wireguard is already in the same boat. [...] Oh, sorry. I didn't look at the linked patch and thought it was adding such a new thing. Looking now, I see it just makes the policy validate it instead of (only) doing it in the code. (FWIW, in the code you could then also set the policy argument for nla_parse_nested() calls to NULL.) > Given that, as Jacob pointed out, there are more families with nested arrays in > their YNL spec, than those using NLA_NESTED_ARRAY, then it appears that there > are more families already in the boat. Right. johannes
On Thu, 4 Sep 2025 22:01:25 +0000 Asbjørn Sloth Tønnesen wrote: > This patch adds support for NLA_POLICY_NESTED_ARRAY() policies. > > Example spec (from future wireguard.yaml): > - > name: wgpeer > attributes: > - > name: allowedips > type: indexed-array > sub-type: nest > nested-attributes: wgallowedip > > yields NLA_POLICY_NESTED_ARRAY(wireguard_wgallowedip_nl_policy). > > This doesn't change any currently generated code, as it isn't > used in any specs currently used for generating code. Reviewed-by: Jakub Kicinski <kuba@kernel.org>
Asbjørn Sloth Tønnesen <ast@fiberby.net> writes: > This patch adds support for NLA_POLICY_NESTED_ARRAY() policies. > > Example spec (from future wireguard.yaml): > - > name: wgpeer > attributes: > - > name: allowedips > type: indexed-array > sub-type: nest > nested-attributes: wgallowedip > > yields NLA_POLICY_NESTED_ARRAY(wireguard_wgallowedip_nl_policy). > > This doesn't change any currently generated code, as it isn't > used in any specs currently used for generating code. > > Signed-off-by: Asbjørn Sloth Tønnesen <ast@fiberby.net> Reviewed-by: Donald Hunter <donald.hunter@gmail.com>
© 2016 - 2025 Red Hat, Inc.