Re: [PATCH net-next v2 1/9] devlink: fix the enum behind DEVLINK_ATTR_RELOAD_LIMITS
From: netdev-bot+sashiko@kernel.org
Date: 2026-09-16 19:15:21
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: Critical: 0 · High: 0 · Medium: 1 · Low: 0 - [Medium] Commit message mischaracterizes a user-visible netlink validation change as "cosmetic", and the patch carries no Fixes: tag. -- devlink: fix the enum behind DEVLINK_ATTR_RELOAD_LIMITS The devlink YAML spec annotated DEVLINK_ATTR_RELOAD_LIMITS with enum devlink_reload_action instead of enum devlink_reload_limit, so a new reload-limit enum is added and the attribute (plus reload-stats-limit) is pointed at it. The regenerated policy changes the bitfield32 valid mask for DEVLINK_ATTR_RELOAD_LIMITS from 6 to 3.
quoted hunk ↗ jump to hunk
diff --git a/Documentation/netlink/specs/devlink.yaml b/Documentation/netlink/specs/devlink.yaml index 38b1190f3d269..d933b205ee865 100644 --- a/Documentation/netlink/specs/devlink.yaml +++ b/Documentation/netlink/specs/devlink.yaml@@ -174,6 +174,18 @@ definitions: value: 1 - name: fw-activate + - + type: enum + name: reload-limit + entries: + - + name: unspec + doc: no constraints + - + name: no-reset + doc: >- + No reset allowed, no down time allowed, no link flap and no + configuration is lost. - type: enum name: param-cmode@@ -775,7 +787,7 @@ attribute-sets: - name: reload-limits type: bitfield32 - enum: reload-action + enum: reload-limit enum-as-flags: true
[ ... ]
quoted hunk ↗ jump to hunk
diff --git a/net/devlink/netlink_gen.c b/net/devlink/netlink_gen.c index dec00133178d1..30f01901b5877 100644 --- a/net/devlink/netlink_gen.c +++ b/net/devlink/netlink_gen.c@@ -334,7 +334,7 @@ static const struct nla_policy devlink_reload_nl_policy[DEVLINK_ATTR_INDEX + 1] [DEVLINK_ATTR_DEV_NAME] = { .type = NLA_NUL_STRING, }, [DEVLINK_ATTR_INDEX] = NLA_POLICY_FULL_RANGE(NLA_UINT, &devlink_attr_index_range), [DEVLINK_ATTR_RELOAD_ACTION] = NLA_POLICY_RANGE(NLA_U8, 1, 2), - [DEVLINK_ATTR_RELOAD_LIMITS] = NLA_POLICY_BITFIELD32(6), + [DEVLINK_ATTR_RELOAD_LIMITS] = NLA_POLICY_BITFIELD32(3),
[Severity: Medium]
Is this really only cosmetic? The commit message says:
AFAICT this is a cosmetic change - both DEVLINK_RELOAD_LIMIT_UNSPEC
and the out of bounds bit 2 will be rejected either way because drivers
don't declare them as supported.
That reasoning covers the value bits, since only value bits reach
devlink_reload_limit_is_supported(). But validate_nla_bitfield32() in
lib/nlattr.c checks the selector against the mask on its own:
/*disallow invalid bit selector */
if (bf->selector & ~valid_flags_mask)
return -EINVAL;
and devlink_nl_reload_doit() in net/devlink/dev.c only ever looks at
limits_selected = limits.value & limits.selector;
so selector-only bits never reach any driver check. Doesn't that make the
mask change user-visible in both directions?
Going from 6 to 3 accepts selector bit 0 again. The natural selector for
userspace is the exported DEVLINK_RELOAD_LIMITS_VALID_MASK in
include/uapi/linux/devlink.h:
#define DEVLINK_RELOAD_LIMITS_VALID_MASK (_BITUL(__DEVLINK_RELOAD_LIMIT_MAX) - 1)
which is 3. With mask 6 that request fails early, since 3 & ~6 == 1, so
DEVLINK_CMD_RELOAD with selector DEVLINK_RELOAD_LIMITS_VALID_MASK and
value BIT(DEVLINK_RELOAD_LIMIT_NO_RESET) returns -EINVAL on current
kernels and starts working again with this patch. Should this therefore
carry a Fixes tag for f2f9dd164db079 ("netlink: specs: devlink: add the
remaining command to generate complete split_ops"), which is where mask 6
came from (its parent used DEVLINK_RELOAD_LIMITS_VALID_MASK in
net/devlink/netlink.c)? Without a Fixes tag, and described as cosmetic,
stable backporters will likely skip it and the -EINVAL behaviour stays in
place from v6.7 onwards.
The other direction also changes: a request with selector 6 and value 2
passes policy today, and since limits_selected ends up as
BIT(DEVLINK_RELOAD_LIMIT_NO_RESET) it succeeds on drivers advertising
that limit, for example mlx5:
drivers/net/ethernet/mellanox/mlx5/core/devlink.c:
.reload_limits = BIT(DEVLINK_RELOAD_LIMIT_NO_RESET),
After this patch such a request is rejected with -EINVAL. Could the
changelog mention that narrowing as well?
One more user-visible effect worth noting: ynl decodes bitfield32 flags
via _decode_enum(), so with the reload-action annotation it currently
prints and accepts the unrelated names driver-reinit and fw-activate for
reload-limits rather than unspec and no-reset. That also looks like more
than a documentation-only fix.
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260915161341.1053476-1-kuba%40kernel.org