Re: [PATCH net-next 4/8] netlink: specs: devlink: complete the port function nest
From: netdev-bot+sashiko@kernel.org
Date: 2026-09-11 20:11:17
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: Critical: 0 · High: 0 · Medium: 0 · Low: 1 (1) - [Low] The patch fixes a user-visible tooling breakage (any ynl/spec- driven decoder aborts on a devlink port-get do/dump reply with "Space… Pre-existing issues: - [Low] The nest newly referenced by the added `devlink` attribute (`dl- nested-devlink`, subset-of devlink) declares `netns-id` as `type: u32`… -- netlink: specs: devlink: complete the port function nest The dl-port-function attribute set in the devlink spec stopped at caps, while the kernel also emits DEVLINK_PORT_FN_ATTR_DEVLINK and DEVLINK_PORT_FN_ATTR_MAX_IO_EQS on port-get do and dump. The patch adds both attributes to the spec and regenerates the netlink policy.
quoted hunk ↗ jump to hunk
diff --git a/Documentation/netlink/specs/devlink.yaml b/Documentation/netlink/specs/devlink.yaml index 7ec52f81c3230..962789dfbfacf 100644 --- a/Documentation/netlink/specs/devlink.yaml +++ b/Documentation/netlink/specs/devlink.yaml@@ -992,6 +992,14 @@ attribute-sets: type: bitfield32 enum: port-fn-attr-cap enum-as-flags: true + - + name: devlink + type: nest + nested-attributes: dl-nested-devlink + doc: Handle of the peer devlink instance instantiated for this function. + - + name: max-io-eqs + type: u32
[Severity: Low]
Two attributes are added here, but the commit message attributes the spec
gap to a single commit:
Commit 5af3e3876d56 ("devlink: Support setting max_io_eqs") added the
uAPI value and the hand written policy but never touched the spec.
git blame on enum devlink_port_function_attr in
include/uapi/linux/devlink.h shows the two values come from different
commits:
0b7a2721e36c11 (Jiri Pirko 2023-09-13) DEVLINK_PORT_FN_ATTR_DEVLINK, /* nested */
5af3e3876d567f (Parav Pandit 2024-04-06) DEVLINK_PORT_FN_ATTR_MAX_IO_EQS, /* u32 */
Should the message also mention 0b7a2721e36c1 as the origin of the
missing devlink nest attribute?
Since devlink_nl_port_function_attrs_put() emits both attributes on every
port fill:
net/devlink/port.c:devlink_nl_port_function_attrs_put() {
err = devlink_port_fn_max_io_eqs_fill(port, msg, extack, &msg_updated);
if (err)
goto out;
err = devlink_rel_devlink_handle_put(msg, port->devlink,
port->rel_index,
DEVLINK_PORT_FN_ATTR_DEVLINK,
&msg_updated);
}
any spec-driven decoder fails on such a reply. Would Fixes: tags for both
commits be appropriate so this gets picked up for the affected stable
trees?
quoted hunk ↗ jump to hunk
diff --git a/net/devlink/netlink_gen.c b/net/devlink/netlink_gen.c index 30f01901b5877..17d1edcdb935e 100644 --- a/net/devlink/netlink_gen.c +++ b/net/devlink/netlink_gen.c@@ -46,17 +46,26 @@ devlink_attr_param_type_validate(const struct nlattr *attr, } /* Common nested types */ +const struct nla_policy devlink_dl_nested_devlink_nl_policy[DEVLINK_ATTR_INDEX + 1] = { + [DEVLINK_ATTR_BUS_NAME] = { .type = NLA_NUL_STRING, }, + [DEVLINK_ATTR_DEV_NAME] = { .type = NLA_NUL_STRING, }, + [DEVLINK_ATTR_INDEX] = NLA_POLICY_FULL_RANGE(NLA_UINT, &devlink_attr_index_range), + [DEVLINK_ATTR_NETNS_ID] = { .type = NLA_U32, }, +};
[Severity: Low]
This is a pre-existing issue and not introduced by this patch, but the new
devlink attribute points at dl-nested-devlink, which makes the netns-id
member reachable from the port-function reply, and the spec declares it
unsigned:
Documentation/netlink/specs/devlink.yaml:
-
name: netns-id
type: u32
The kernel writes that member as signed, and the value can be the
negative sentinel:
net/devlink/netlink.c:devlink_nl_put_nested_handle() {
int id = peernet2id_alloc(net, devl_net, GFP_ATOMIC);
rcu_read_unlock();
if (nla_put_s32(msg, DEVLINK_ATTR_NETNS_ID, id))
return -EMSGSIZE;
}
net/core/net_namespace.c:peernet2id_alloc() {
if (!check_net(net))
return NETNSA_NSID_NOT_ASSIGNED;
}
So for a peer devlink instance in another namespace whose nsid is not
assigned, a spec-driven decoder prints 4294967295 instead of -1 and cannot
recognise the not-assigned sentinel. Length validation is the same for
NLA_U32 and NLA_S32, so kernel side behaviour is unaffected.
Documentation/netlink/specs/netdev.yaml declares the equivalent attribute
as s32:
-
name: netns-id
doc: The network namespace id of the netdev.
type: s32
Would it make sense to change netns-id in devlink.yaml to s32 (or sint)
here as well? It is still type: u32 at the end of this series.
[ ... ]
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260910200312.2665792-1-kuba%40kernel.org