Re: [PATCH net-next 3/8] netlink: specs: devlink: describe DEVLINK_ATTR_NESTED_DEVLINK

[netdev-bot+sashiko@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider:

Critical: 0 · High: 0 · Medium: 0 · Low: 1

- [Low] Documentation/netlink/specs/devlink.yaml: the new `dl-nested-
  devlink` subset (added by this patch) lists `netns-id` as a member of…

--

netlink: specs: devlink: describe DEVLINK_ATTR_NESTED_DEVLINK

This fills in the single attribute (DEVLINK_ATTR_NESTED_DEVLINK, 175) that
was hidden behind the "# TODO: fill in the attributes in between" gap, adds
a dl-nested-devlink subset for the nest contents, and lists the new
attribute in the dev-get reply.

> diff --git a/Documentation/netlink/specs/devlink.yaml b/Documentation/netlink/specs/devlink.yaml
> index 0e0791d4e2c74..7ec52f81c3230 100644
> --- a/Documentation/netlink/specs/devlink.yaml
> +++ b/Documentation/netlink/specs/devlink.yaml
> @@ -858,13 +858,14 @@ attribute-sets:
>          name: linecard-supported-types
>          type: nest
>          nested-attributes: dl-linecard-supported-types
> -
> -      # TODO: fill in the attributes in between
> -
> +      -
> +        name: nested-devlink
> +        type: nest
> +        multi-attr: true
> +        nested-attributes: dl-nested-devlink
>        -
>          name: selftests
>          type: nest
> -        value: 176
>          nested-attributes: dl-selftest-id

[ ... ]

> @@ -1351,6 +1352,19 @@ attribute-sets:
>        -
>          name: index
>  
> +  -
> +    name: dl-nested-devlink
> +    subset-of: devlink
> +    attributes:
> +      -
> +        name: bus-name
> +      -
> +        name: dev-name
> +      -
> +        name: index
> +      -
> +        name: netns-id

[Severity: Low]
Now that netns-id is described as part of a reply nest, does its declared
type match what the kernel puts on the wire?  In the main devlink
attribute set it is declared as:

      -
        name: netns-id
        type: u32

but devlink_nl_put_nested_handle() in net/devlink/netlink.c emits it as a
signed value:

	int id = peernet2id_alloc(net, devl_net, GFP_ATOMIC);

	rcu_read_unlock();
	if (nla_put_s32(msg, DEVLINK_ATTR_NETNS_ID, id))
		return -EMSGSIZE;

and peernet2id_alloc() in net/core/net_namespace.c can return
NETNSA_NSID_NOT_ASSIGNED (-1) when !check_net(net), when maybe_get_net()
on the peer netns fails, or when alloc_netid() fails under GFP_ATOMIC.

Would a spec-driven decoder (ynl or generated C) then print that -1
sentinel as 4294967295 for devlink dev-get when the nested instance
(mlx5 SF, ice) lives in another netns?

Every other spec in the tree that carries a netns id uses s32:
netdev.yaml (netns-id), psp.yaml (nsid), rt-link.yaml (link-netnsid,
new-netnsid, target-netnsid).

Should the devlink declaration be s32 (or sint) as well?  The request side
parse in devlink_netns_get() uses nla_get_u32(), which is byte-compatible
with an NLA_S32 policy, so the wire format would not change.

I checked the end of the series and netns-id is still declared u32 there,
so this does not look like something a later patch adjusts.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260910200312.2665792-1-kuba%40kernel.org