While fixing the binary min-len implementaion, I noticed that the only user, should AFAICT be using exact-len instead. In net/ipv4/fou_core.c FOU_ATTR_LOCAL_V6 and FOU_ATTR_PEER_V6 are only used for singular IPv6 addresses, a exact-len policy, therefore seams like a better fit. AFAICT this was caused by lacking support for the exact-len check at the time of the blamed commit, which was later remedied by c63ad379526 ("tools: ynl-gen: add support for exact-len validation"). This patch therefore changes the local-v6/peer-v6 attributes to use an exact-len check, instead of a min-len check. Fixes: 4eb77b4ecd3c ("netlink: add a proto specification for FOU") Signed-off-by: Asbjørn Sloth Tønnesen --- Documentation/netlink/specs/fou.yaml | 4 ++-- net/ipv4/fou_nl.c | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Documentation/netlink/specs/fou.yaml b/Documentation/netlink/specs/fou.yaml index 57735726262ec..8e7974ec453fc 100644 --- a/Documentation/netlink/specs/fou.yaml +++ b/Documentation/netlink/specs/fou.yaml @@ -52,7 +52,7 @@ attribute-sets: name: local-v6 type: binary checks: - min-len: 16 + exact-len: 16 - name: peer-v4 type: u32 @@ -60,7 +60,7 @@ attribute-sets: name: peer-v6 type: binary checks: - min-len: 16 + exact-len: 16 - name: peer-port type: u16 diff --git a/net/ipv4/fou_nl.c b/net/ipv4/fou_nl.c index 3d9614609b2d3..506260b4a4dc2 100644 --- a/net/ipv4/fou_nl.c +++ b/net/ipv4/fou_nl.c @@ -18,9 +18,9 @@ const struct nla_policy fou_nl_policy[FOU_ATTR_IFINDEX + 1] = { [FOU_ATTR_TYPE] = { .type = NLA_U8, }, [FOU_ATTR_REMCSUM_NOPARTIAL] = { .type = NLA_FLAG, }, [FOU_ATTR_LOCAL_V4] = { .type = NLA_U32, }, - [FOU_ATTR_LOCAL_V6] = { .len = 16, }, + [FOU_ATTR_LOCAL_V6] = NLA_POLICY_EXACT_LEN(16), [FOU_ATTR_PEER_V4] = { .type = NLA_U32, }, - [FOU_ATTR_PEER_V6] = { .len = 16, }, + [FOU_ATTR_PEER_V6] = NLA_POLICY_EXACT_LEN(16), [FOU_ATTR_PEER_PORT] = { .type = NLA_BE16, }, [FOU_ATTR_IFINDEX] = { .type = NLA_S32, }, }; -- 2.50.1 This patch changes the generated min-len check for binary attributes to use the NLA_POLICY_MIN_LEN() macro, and thereby ensures that .validation_type is not left at NLA_VALIDATE_NONE. This doesn't change any currently generated code, as it isn't used in any specs currently used for generating code. Fixes: be5bea1cc0bf ("net: add basic C code generators for Netlink") Signed-off-by: Asbjørn Sloth Tønnesen --- 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 ef032e17fec44..52f955ed84a7f 100755 --- a/tools/net/ynl/pyynl/ynl_gen_c.py +++ b/tools/net/ynl/pyynl/ynl_gen_c.py @@ -556,7 +556,7 @@ class TypeBinary(Type): elif 'exact-len' in self.checks: mem = 'NLA_POLICY_EXACT_LEN(' + self.get_limit_str('exact-len') + ')' elif 'min-len' in self.checks: - mem = '{ .len = ' + self.get_limit_str('min-len') + ', }' + mem = 'NLA_POLICY_MIN_LEN(' + self.get_limit_str('min-len') + ')' elif 'max-len' in self.checks: mem = 'NLA_POLICY_MAX_LEN(' + self.get_limit_str('max-len') + ')' -- 2.50.1 The blamed commit introduced the concept of split attribute counting, and later allocating an array to hold them, however TypeArrayNest wasn't updated to use the new counting variable. Abbreviated example from tools/net/ynl/generated/nl80211-user.c: nl80211_if_combination_attributes_parse(...): unsigned int n_limits = 0; [...] ynl_attr_for_each(attr, nlh, yarg->ys->family->hdr_len) if (type == NL80211_IFACE_COMB_LIMITS) ynl_attr_for_each_nested(attr2, attr) dst->_count.limits++; if (n_limits) { dst->_count.limits = n_limits; /* allocate and parse attributes */ } In the above example n_limits is guaranteed to always be 0, hence the conditional is unsatisfiable and is optimized out. This patch changes the attribute counting to use n_limits++ in the attribute counting loop in the above example. Fixes: 58da455b31ba ("tools: ynl-gen: improve unwind on parsing errors") Signed-off-by: Asbjørn Sloth Tønnesen --- 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 52f955ed84a7f..fb7e03805a113 100755 --- a/tools/net/ynl/pyynl/ynl_gen_c.py +++ b/tools/net/ynl/pyynl/ynl_gen_c.py @@ -830,7 +830,7 @@ class TypeArrayNest(Type): 'ynl_attr_for_each_nested(attr2, attr) {', '\tif (ynl_attr_validate(yarg, attr2))', '\t\treturn YNL_PARSE_CB_ERROR;', - f'\t{var}->_count.{self.c_name}++;', + f'\tn_{self.c_name}++;', '}'] return get_lines, None, local_vars -- 2.50.1 In this context "not that ..." should properly be "note that ...". Fixes: b8fd60c36a44 ("genetlink: allow families to use split ops directly") Signed-off-by: Asbjørn Sloth Tønnesen --- include/net/genetlink.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/net/genetlink.h b/include/net/genetlink.h index a03d567658328..7b84f2cef8b1f 100644 --- a/include/net/genetlink.h +++ b/include/net/genetlink.h @@ -62,7 +62,7 @@ struct genl_info; * @small_ops: the small-struct operations supported by this family * @n_small_ops: number of small-struct operations supported by this family * @split_ops: the split do/dump form of operation definition - * @n_split_ops: number of entries in @split_ops, not that with split do/dump + * @n_split_ops: number of entries in @split_ops, note that with split do/dump * ops the number of entries is not the same as number of commands * @sock_priv_size: the size of per-socket private memory * @sock_priv_init: the per-socket private memory initializer -- 2.50.1