[PATCH net-next 0/8] devlink: netlink spec fixes

[PATCH net-next 0/8] devlink: netlink spec fixes

[Jakub Kicinski]

Number of spec fixes for the devlink family.

Jakub Kicinski (8):
  devlink: fix the enum behind DEVLINK_ATTR_RELOAD_LIMITS
  netlink: specs: devlink: drop the stale port dump reply value
  netlink: specs: devlink: describe DEVLINK_ATTR_NESTED_DEVLINK
  netlink: specs: devlink: complete the port function nest
  devlink: generate the port function policy from the spec
  netlink: specs: devlink: populate multi-attr attrs for region read and
    line card
  netlink: specs: devlink: describe the netns id in the parent-dev nest
  netlink: specs: devlink: add pad to the subsets carrying padded u64s

 Documentation/netlink/specs/devlink.yaml | 153 +++++++++++++++++++++--
 net/devlink/netlink_gen.h                |   2 +-
 net/devlink/netlink_gen.c                |   8 +-
 net/devlink/port.c                       |  19 +--
 4 files changed, 149 insertions(+), 33 deletions(-)

-- 
2.55.0

[PATCH net-next 1/8] devlink: fix the enum behind DEVLINK_ATTR_RELOAD_LIMITS

[Jakub Kicinski]

DEVLINK_ATTR_RELOAD_LIMITS carries enum devlink_reload_limit, the spec
says enum devlink_reload_action. The two are unrelated, and have
a different set of values (bits 1, 2 vs bits 0, 1).

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. User will see either a policy
validation failure or "Requested limit is not supported by the driver".

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 Documentation/netlink/specs/devlink.yaml | 15 ++++++++++++++-
 net/devlink/netlink_gen.c                |  2 +-
 2 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/Documentation/netlink/specs/devlink.yaml b/Documentation/netlink/specs/devlink.yaml
index 38b1190f3d26..d933b205ee86 100644
--- a/Documentation/netlink/specs/devlink.yaml
+++ b/Documentation/netlink/specs/devlink.yaml
@@ -174,6 +174,18 @@ doc: Partial family for Devlink.
         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 @@ doc: Partial family for Devlink.
       -
         name: reload-limits
         type: bitfield32
-        enum: reload-action
+        enum: reload-limit
         enum-as-flags: true
       -
         name: dev-stats
@@ -793,6 +805,7 @@ doc: Partial family for Devlink.
       -
         name: reload-stats-limit
         type: u8
+        enum: reload-limit
       -
         name: reload-stats-value
         type: u32
diff --git a/net/devlink/netlink_gen.c b/net/devlink/netlink_gen.c
index dec00133178d..30f01901b587 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),
 	[DEVLINK_ATTR_NETNS_PID] = { .type = NLA_U32, },
 	[DEVLINK_ATTR_NETNS_FD] = { .type = NLA_U32, },
 	[DEVLINK_ATTR_NETNS_ID] = { .type = NLA_U32, },
-- 
2.55.0

Re: [PATCH net-next 1/8] devlink: fix the enum behind DEVLINK_ATTR_RELOAD_LIMITS

[netdev-bot+sashiko]

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

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

- [Low] Commit message does not describe one of the three spec hunks it
  carries.

--

devlink: fix the enum behind DEVLINK_ATTR_RELOAD_LIMITS

The devlink spec described DEVLINK_ATTR_RELOAD_LIMITS as carrying enum
devlink_reload_action, while the kernel actually puts enum
devlink_reload_limit there.  A new reload-limit enum is added and the
generated policy mask changes from 6 to 3.

> 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

[ ... ]

> @@ -793,6 +805,7 @@ attribute-sets:
>        -
>          name: reload-stats-limit
>          type: u8
> +        enum: reload-limit

[Severity: Low]
This isn't a bug, but should the commit message also mention this hunk?
The subject and body only talk about DEVLINK_ATTR_RELOAD_LIMITS, and the
word "stats" doesn't appear anywhere in the log, yet reload-stats-limit
(DEVLINK_ATTR_RELOAD_STATS_LIMIT) gets annotated here too.

The annotation itself looks correct - devlink_reload_stat_put() does:

	if (nla_put_u8(msg, DEVLINK_ATTR_RELOAD_STATS_LIMIT, limit) ||

with limit iterated over the devlink_reload_limit range, and the attribute
only appears in the dl-reload-stats-entry reply subset, so net/devlink/
netlink_gen.c is untouched for it.

It does change what spec-generated userspace prints, though.  ynl's
_decode_enum() renders any attribute carrying an "enum" key symbolically,
so "devlink dev get" replies now show reload-stats-limit as unspec/no-reset
rather than a plain integer.

Similarly, for reload-limits the symbolic name that generated clients
accept and emit for bit 1 changes from driver-reinit to no-reset via
_encode_enum().

The statement "AFAICT this is a cosmetic change" holds for the kernel side,
but would it be worth spelling out these two tooling-visible output changes
so backporters know what to expect?

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

[PATCH net-next 2/8] netlink: specs: devlink: drop the stale port dump reply value

[Jakub Kicinski]

The bug the comment describes was fixed by
commit 61c43780e944 ("devlink: fix port dump cmd type") two years ago.
devlink_nl_port_get_dump_one() fills DEVLINK_CMD_PORT_NEW (7), not
DEVLINK_CMD_NEW (3) now.

Note that this change is likely a noop. _dictify_ops_directional()
picks the "do" section's value when an op has both "do" and "dump".

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 Documentation/netlink/specs/devlink.yaml | 1 -
 1 file changed, 1 deletion(-)

diff --git a/Documentation/netlink/specs/devlink.yaml b/Documentation/netlink/specs/devlink.yaml
index d933b205ee86..0e0791d4e2c7 100644
--- a/Documentation/netlink/specs/devlink.yaml
+++ b/Documentation/netlink/specs/devlink.yaml
@@ -1401,7 +1401,6 @@ doc: Partial family for Devlink.
         request:
           attributes: *dev-id-attrs
         reply:
-          value: 3  # due to a bug, port dump returns DEVLINK_CMD_NEW
           attributes: *port-id-attrs
 
     -
-- 
2.55.0

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

[Jakub Kicinski]

The "# TODO: fill in the attributes in between" gap before selftests
(176) hides exactly one attribute, DEVLINK_ATTR_NESTED_DEVLINK (175).
devlink_nl_fill() calls devlink_nl_nested_fill() unconditionally, which
emits one of these per entry in devlink->nested_rels (mlx5, ice).

Unlike the other TODO gaps this is a single attribute of an already
modelled type, so just fill it in.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 Documentation/netlink/specs/devlink.yaml | 23 +++++++++++++++++++----
 1 file changed, 19 insertions(+), 4 deletions(-)

diff --git a/Documentation/netlink/specs/devlink.yaml b/Documentation/netlink/specs/devlink.yaml
index 0e0791d4e2c7..7ec52f81c323 100644
--- a/Documentation/netlink/specs/devlink.yaml
+++ b/Documentation/netlink/specs/devlink.yaml
@@ -858,13 +858,14 @@ doc: Partial family for Devlink.
         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
       -
         name: rate-tx-priority
@@ -1351,6 +1352,19 @@ doc: Partial family for Devlink.
       -
         name: index
 
+  -
+    name: dl-nested-devlink
+    subset-of: devlink
+    attributes:
+      -
+        name: bus-name
+      -
+        name: dev-name
+      -
+        name: index
+      -
+        name: netns-id
+
 operations:
   enum-model: directional
   list:
@@ -1376,6 +1390,7 @@ doc: Partial family for Devlink.
             - index
             - reload-failed
             - dev-stats
+            - nested-devlink
       dump:
         reply: *get-reply
 
-- 
2.55.0

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

[netdev-bot+sashiko]

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

[PATCH net-next 4/8] netlink: specs: devlink: complete the port function nest

[Jakub Kicinski]

dl-port-function stops at caps, but the nest also carries
DEVLINK_PORT_FN_ATTR_DEVLINK (5) and DEVLINK_PORT_FN_ATTR_MAX_IO_EQS (6)
both put by devlink_nl_port_function_attrs_put() on every port-get
do and dump. YNL raises

  Space 'dl-port-function' has no attribute with value '6'

for any port reporting max_io_eqs or a nested devlink handle,
i.e. for mlx5 SFs and VFs.

Commit 5af3e3876d56 ("devlink: Support setting max_io_eqs") added the
uAPI value and the hand written policy but never touched the spec.

Add the missing attributes, subsequent commit reworks the code
to use the YNL-generated policy.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 Documentation/netlink/specs/devlink.yaml |  8 ++++++++
 net/devlink/netlink_gen.h                |  3 ++-
 net/devlink/netlink_gen.c                | 11 ++++++++++-
 3 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/Documentation/netlink/specs/devlink.yaml b/Documentation/netlink/specs/devlink.yaml
index 7ec52f81c323..962789dfbfac 100644
--- a/Documentation/netlink/specs/devlink.yaml
+++ b/Documentation/netlink/specs/devlink.yaml
@@ -992,6 +992,14 @@ doc: Partial family for Devlink.
         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
 
   -
     name: dl-dpipe-tables
diff --git a/net/devlink/netlink_gen.h b/net/devlink/netlink_gen.h
index a70e0e4769aa..75572a9a23f6 100644
--- a/net/devlink/netlink_gen.h
+++ b/net/devlink/netlink_gen.h
@@ -13,8 +13,9 @@
 #include <uapi/linux/devlink.h>
 
 /* Common nested types */
+extern const struct nla_policy devlink_dl_nested_devlink_nl_policy[DEVLINK_ATTR_INDEX + 1];
 extern const struct nla_policy devlink_dl_parent_dev_nl_policy[DEVLINK_ATTR_INDEX + 1];
-extern const struct nla_policy devlink_dl_port_function_nl_policy[DEVLINK_PORT_FN_ATTR_CAPS + 1];
+extern const struct nla_policy devlink_dl_port_function_nl_policy[DEVLINK_PORT_FN_ATTR_MAX_IO_EQS + 1];
 extern const struct nla_policy devlink_dl_rate_tc_bws_nl_policy[DEVLINK_RATE_TC_ATTR_BW + 1];
 extern const struct nla_policy devlink_dl_selftest_id_nl_policy[DEVLINK_ATTR_SELFTEST_ID_FLASH + 1];
 
diff --git a/net/devlink/netlink_gen.c b/net/devlink/netlink_gen.c
index 30f01901b587..17d1edcdb935 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, },
+};
+
 const struct nla_policy devlink_dl_parent_dev_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),
 };
 
-const struct nla_policy devlink_dl_port_function_nl_policy[DEVLINK_PORT_FN_ATTR_CAPS + 1] = {
+const struct nla_policy devlink_dl_port_function_nl_policy[DEVLINK_PORT_FN_ATTR_MAX_IO_EQS + 1] = {
 	[DEVLINK_PORT_FUNCTION_ATTR_HW_ADDR] = { .type = NLA_BINARY, },
 	[DEVLINK_PORT_FN_ATTR_STATE] = NLA_POLICY_MAX(NLA_U8, 1),
 	[DEVLINK_PORT_FN_ATTR_OPSTATE] = NLA_POLICY_MAX(NLA_U8, 1),
 	[DEVLINK_PORT_FN_ATTR_CAPS] = NLA_POLICY_BITFIELD32(15),
+	[DEVLINK_PORT_FN_ATTR_DEVLINK] = NLA_POLICY_NESTED(devlink_dl_nested_devlink_nl_policy),
+	[DEVLINK_PORT_FN_ATTR_MAX_IO_EQS] = { .type = NLA_U32, },
 };
 
 const struct nla_policy devlink_dl_rate_tc_bws_nl_policy[DEVLINK_RATE_TC_ATTR_BW + 1] = {
-- 
2.55.0

Re: [PATCH net-next 4/8] netlink: specs: devlink: complete the port function nest

[netdev-bot+sashiko]

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.

> 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?

> 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

[PATCH net-next 5/8] devlink: generate the port function policy from the spec

[Jakub Kicinski]

devlink has a one huge root attribute set for the whole family,
we haven't taken the time to properly define the sub-sets for
each command. Do it for port-set so that we can drop the hand
written policy used by devlink_port_function_set().

We need this subsetting because within the DEVLINK_ATTR_PORT_FUNCTION
nest DEVLINK_PORT_FN_ATTR_OPSTATE and DEVLINK_PORT_FN_ATTR_DEVLINK
are output-only so we have to filter them out of the input set.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 Documentation/netlink/specs/devlink.yaml | 37 +++++++++++++++++++++++-
 net/devlink/netlink_gen.h                |  3 +-
 net/devlink/netlink_gen.c                | 13 ++-------
 net/devlink/port.c                       | 19 ++----------
 4 files changed, 42 insertions(+), 30 deletions(-)

diff --git a/Documentation/netlink/specs/devlink.yaml b/Documentation/netlink/specs/devlink.yaml
index 962789dfbfac..f23466fb27f9 100644
--- a/Documentation/netlink/specs/devlink.yaml
+++ b/Documentation/netlink/specs/devlink.yaml
@@ -1001,6 +1001,41 @@ doc: Partial family for Devlink.
         name: max-io-eqs
         type: u32
 
+  -
+    name: dl-port-function-set
+    subset-of: dl-port-function
+    doc: |
+      Port function attributes that can be configured; opstate and the
+      devlink handle are read-only.
+    attributes:
+      -
+        name: hw-addr
+      -
+        name: state
+      -
+        name: caps
+      -
+        name: max-io-eqs
+
+  -
+    name: dl-port-set
+    subset-of: devlink
+    doc: Attributes accepted by the port-set request.
+    attributes:
+      -
+        name: bus-name
+      -
+        name: dev-name
+      -
+        name: index
+      -
+        name: port-index
+      -
+        name: port-type
+      -
+        name: port-function
+        nested-attributes: dl-port-function-set
+
   -
     name: dl-dpipe-tables
     subset-of: devlink
@@ -1429,7 +1464,7 @@ doc: Partial family for Devlink.
     -
       name: port-set
       doc: Set devlink port instances.
-      attribute-set: devlink
+      attribute-set: dl-port-set
       dont-validate: [strict]
       flags: [admin-perm]
       do:
diff --git a/net/devlink/netlink_gen.h b/net/devlink/netlink_gen.h
index 75572a9a23f6..99ccacc693b7 100644
--- a/net/devlink/netlink_gen.h
+++ b/net/devlink/netlink_gen.h
@@ -13,9 +13,8 @@
 #include <uapi/linux/devlink.h>
 
 /* Common nested types */
-extern const struct nla_policy devlink_dl_nested_devlink_nl_policy[DEVLINK_ATTR_INDEX + 1];
 extern const struct nla_policy devlink_dl_parent_dev_nl_policy[DEVLINK_ATTR_INDEX + 1];
-extern const struct nla_policy devlink_dl_port_function_nl_policy[DEVLINK_PORT_FN_ATTR_MAX_IO_EQS + 1];
+extern const struct nla_policy devlink_dl_port_function_set_nl_policy[DEVLINK_PORT_FN_ATTR_MAX_IO_EQS + 1];
 extern const struct nla_policy devlink_dl_rate_tc_bws_nl_policy[DEVLINK_RATE_TC_ATTR_BW + 1];
 extern const struct nla_policy devlink_dl_selftest_id_nl_policy[DEVLINK_ATTR_SELFTEST_ID_FLASH + 1];
 
diff --git a/net/devlink/netlink_gen.c b/net/devlink/netlink_gen.c
index 17d1edcdb935..43ef6864d462 100644
--- a/net/devlink/netlink_gen.c
+++ b/net/devlink/netlink_gen.c
@@ -46,25 +46,16 @@ 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, },
-};
-
 const struct nla_policy devlink_dl_parent_dev_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),
 };
 
-const struct nla_policy devlink_dl_port_function_nl_policy[DEVLINK_PORT_FN_ATTR_MAX_IO_EQS + 1] = {
+const struct nla_policy devlink_dl_port_function_set_nl_policy[DEVLINK_PORT_FN_ATTR_MAX_IO_EQS + 1] = {
 	[DEVLINK_PORT_FUNCTION_ATTR_HW_ADDR] = { .type = NLA_BINARY, },
 	[DEVLINK_PORT_FN_ATTR_STATE] = NLA_POLICY_MAX(NLA_U8, 1),
-	[DEVLINK_PORT_FN_ATTR_OPSTATE] = NLA_POLICY_MAX(NLA_U8, 1),
 	[DEVLINK_PORT_FN_ATTR_CAPS] = NLA_POLICY_BITFIELD32(15),
-	[DEVLINK_PORT_FN_ATTR_DEVLINK] = NLA_POLICY_NESTED(devlink_dl_nested_devlink_nl_policy),
 	[DEVLINK_PORT_FN_ATTR_MAX_IO_EQS] = { .type = NLA_U32, },
 };
 
@@ -106,7 +97,7 @@ static const struct nla_policy devlink_port_set_nl_policy[DEVLINK_ATTR_INDEX + 1
 	[DEVLINK_ATTR_INDEX] = NLA_POLICY_FULL_RANGE(NLA_UINT, &devlink_attr_index_range),
 	[DEVLINK_ATTR_PORT_INDEX] = { .type = NLA_U32, },
 	[DEVLINK_ATTR_PORT_TYPE] = NLA_POLICY_MAX(NLA_U16, 3),
-	[DEVLINK_ATTR_PORT_FUNCTION] = NLA_POLICY_NESTED(devlink_dl_port_function_nl_policy),
+	[DEVLINK_ATTR_PORT_FUNCTION] = NLA_POLICY_NESTED(devlink_dl_port_function_set_nl_policy),
 };
 
 /* DEVLINK_CMD_PORT_NEW - do */
diff --git a/net/devlink/port.c b/net/devlink/port.c
index 1528f2d148df..803429d9a008 100644
--- a/net/devlink/port.c
+++ b/net/devlink/port.c
@@ -6,19 +6,6 @@
 
 #include "devl_internal.h"
 
-#define DEVLINK_PORT_FN_CAPS_VALID_MASK \
-	(_BITUL(__DEVLINK_PORT_FN_ATTR_CAPS_MAX) - 1)
-
-static const struct nla_policy devlink_function_nl_policy[DEVLINK_PORT_FUNCTION_ATTR_MAX + 1] = {
-	[DEVLINK_PORT_FUNCTION_ATTR_HW_ADDR] = { .type = NLA_BINARY },
-	[DEVLINK_PORT_FN_ATTR_STATE] =
-		NLA_POLICY_RANGE(NLA_U8, DEVLINK_PORT_FN_STATE_INACTIVE,
-				 DEVLINK_PORT_FN_STATE_ACTIVE),
-	[DEVLINK_PORT_FN_ATTR_CAPS] =
-		NLA_POLICY_BITFIELD32(DEVLINK_PORT_FN_CAPS_VALID_MASK),
-	[DEVLINK_PORT_FN_ATTR_MAX_IO_EQS] = { .type = NLA_U32 },
-};
-
 #define ASSERT_DEVLINK_PORT_REGISTERED(devlink_port)				\
 	WARN_ON_ONCE(!(devlink_port)->registered)
 #define ASSERT_DEVLINK_PORT_NOT_REGISTERED(devlink_port)			\
@@ -782,11 +769,11 @@ static int devlink_port_function_set(struct devlink_port *port,
 				     const struct nlattr *attr,
 				     struct netlink_ext_ack *extack)
 {
-	struct nlattr *tb[DEVLINK_PORT_FUNCTION_ATTR_MAX + 1];
+	struct nlattr *tb[ARRAY_SIZE(devlink_dl_port_function_set_nl_policy)];
 	int err;
 
-	err = nla_parse_nested(tb, DEVLINK_PORT_FUNCTION_ATTR_MAX, attr,
-			       devlink_function_nl_policy, extack);
+	err = nla_parse_nested(tb, ARRAY_SIZE(tb) - 1, attr,
+			       devlink_dl_port_function_set_nl_policy, extack);
 	if (err < 0) {
 		NL_SET_ERR_MSG(extack, "Fail to parse port function attributes");
 		return err;
-- 
2.55.0

[PATCH net-next 6/8] netlink: specs: devlink: populate multi-attr attrs for region read and line card

[Jakub Kicinski]

Three attributes are emitted repeatedly inside their nest:

 - devlink_nl_region_read_fill() calls
   devlink_nl_cmd_region_read_chunk_fill() once per 256 byte unit
 - devlink_nl_region_snapshots_id_put() emits one
   DEVLINK_ATTR_REGION_SNAPSHOT sub-nest per entry on the region's
   snapshot list
 - devlink_nl_linecard_fill() emits one DEVLINK_ATTR_LINECARD_TYPE per
   linecard->types_count entry

In all three cases we're missing the multi-attr properties, and the
attributes themselves are missing from the attr list used by C code gen.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 Documentation/netlink/specs/devlink.yaml | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/Documentation/netlink/specs/devlink.yaml b/Documentation/netlink/specs/devlink.yaml
index f23466fb27f9..9d816a0b7da4 100644
--- a/Documentation/netlink/specs/devlink.yaml
+++ b/Documentation/netlink/specs/devlink.yaml
@@ -1264,6 +1264,7 @@ doc: Partial family for Devlink.
     attributes:
       -
         name: region-snapshot
+        multi-attr: true
 
   -
     name: dl-region-snapshot
@@ -1278,6 +1279,7 @@ doc: Partial family for Devlink.
     attributes:
       -
         name: region-chunk
+        multi-attr: true
 
   -
     name: dl-region-chunk
@@ -1360,6 +1362,7 @@ doc: Partial family for Devlink.
     attributes:
       -
         name: linecard-type
+        multi-attr: true
 
   -
     name: dl-selftest-id
@@ -2045,6 +2048,7 @@ doc: Partial family for Devlink.
             - index
             - port-index
             - region-name
+            - region-chunks
 
     -
       name: port-param-get
@@ -2444,14 +2448,22 @@ doc: Partial family for Devlink.
         post: devlink-nl-post-doit
         request:
           value: 78
-          attributes: &linecard-id-attrs
+          attributes:
             - bus-name
             - dev-name
             - index
             - linecard-index
         reply: &linecard-get-reply
           value: 80
-          attributes: *linecard-id-attrs
+          attributes:
+            - bus-name
+            - dev-name
+            - index
+            - linecard-index
+            - linecard-state
+            - linecard-type
+            - linecard-supported-types
+            - nested-devlink
       dump:
         request:
           attributes: *dev-id-attrs
-- 
2.55.0

Re: [PATCH net-next 6/8] netlink: specs: devlink: populate multi-attr attrs for region read and line card

[netdev-bot+sashiko]

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

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

- [High] The commit message enumerates three repeated attributes that
  need both `multi-attr: true` and an entry in the operation attribute…

--

netlink: specs: devlink: populate multi-attr attrs for region read and line card

This adds multi-attr: true to the region-snapshot, region-chunk and
linecard-type attributes, and adds region-chunks to the region-read reply
plus the linecard state/type attributes to the linecard-get reply.  The
stated goal is that all three repeated attributes get both the multi-attr
property and an entry in the attribute list used by C code generation.

> diff --git a/Documentation/netlink/specs/devlink.yaml b/Documentation/netlink/specs/devlink.yaml
> index f23466fb27f97..9d816a0b7da41 100644
> --- a/Documentation/netlink/specs/devlink.yaml
> +++ b/Documentation/netlink/specs/devlink.yaml
> @@ -1264,6 +1264,7 @@ attribute-sets:
>      attributes:
>        -
>          name: region-snapshot
> +        multi-attr: true

[Severity: High]
The commit message says that for all three attributes "the attributes
themselves are missing from the attr list used by C code gen".  For
region-chunks and the linecard attributes the patch adds them to the
respective replies, but is the region-snapshots half missing?

Grepping the spec, region-snapshots only appears in its own definition in
the devlink attribute set and as the dl-region-snapshots subset name; no
operation lists it.  The region-get reply is still:

        reply: &region-get-reply
          value: 42
          attributes: *region-id-attrs

where region-id-attrs is {bus-name, dev-name, index, port-index,
region-name}.

Since ynl-gen-c only generates parsing code for attribute sets reachable
from an operation's attribute list, does the new multi-attr: true on
region-snapshot have any effect?  The generated region-get reply struct
would still have no snapshot list.

There is also a related gap in what the spec describes: for every
REGION_GET do and dump message, devlink_nl_region_fill() in
net/devlink/region.c emits attributes the reply does not declare:

	err = devlink_nl_put_u64(msg, DEVLINK_ATTR_REGION_SIZE, region->size);
	...
	err = nla_put_u32(msg, DEVLINK_ATTR_REGION_MAX_SNAPSHOTS, region->max_snapshots);
	...
	err = devlink_nl_region_snapshots_id_put(msg, devlink, region);

and devlink_nl_region_snapshots_id_put() nests one
DEVLINK_ATTR_REGION_SNAPSHOT per entry of region->snapshot_list:

	snapshots_attr = nla_nest_start_noflag(msg, DEVLINK_ATTR_REGION_SNAPSHOTS);
	...
	list_for_each_entry(snapshot, &region->snapshot_list, list) {
		err = devlink_nl_region_snapshot_id_put(msg, devlink, snapshot);

Would it make sense to add region-snapshots (and region-size /
region-max-snapshots) to the region-get reply, or otherwise to narrow the
commit message to the two operations the patch actually completes?

Checking the tip of the series (59624e85e9fb), the region-get reply is
still attributes: *region-id-attrs, so no later patch in the series
changes this.

> @@ -2045,6 +2048,7 @@ operations:
>              - index
>              - port-index
>              - region-name
> +            - region-chunks
>  

[ ... ]

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

[PATCH net-next 7/8] netlink: specs: devlink: describe the netns id in the parent-dev nest

[Jakub Kicinski]

devlink_nl_rate_parent_fill() emits DEVLINK_ATTR_PARENT_DEV via
devlink_nl_put_nested_handle(), which adds DEVLINK_ATTR_NETNS_ID
whenever the parent rate node lives on a devlink instance in another
netns. Now that we defined dl-nested-devlink subset, which unlike
dl-parent-dev includes netns-id, let's switch to it. The rate APIs
input genuinely does not accept the netns-id, so we need to subset
them to stick with dl-parent-dev.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 Documentation/netlink/specs/devlink.yaml | 39 ++++++++++++++++++++++--
 1 file changed, 36 insertions(+), 3 deletions(-)

diff --git a/Documentation/netlink/specs/devlink.yaml b/Documentation/netlink/specs/devlink.yaml
index 9d816a0b7da4..54de1eeb2234 100644
--- a/Documentation/netlink/specs/devlink.yaml
+++ b/Documentation/netlink/specs/devlink.yaml
@@ -912,7 +912,7 @@ doc: Partial family for Devlink.
       -
         name: parent-dev
         type: nest
-        nested-attributes: dl-parent-dev
+        nested-attributes: dl-nested-devlink
         doc: |
           Identifies the devlink instance which owns the parent rate node.
           Used with rate-set and rate-new to parent a rate object to a node on
@@ -1390,6 +1390,10 @@ doc: Partial family for Devlink.
   -
     name: dl-parent-dev
     subset-of: devlink
+    doc: |
+      Devlink handle accepted as the parent-dev input; the netns id the
+      kernel reports back is not accepted, the parent is always resolved
+      in the caller's netns.
     attributes:
       -
         name: bus-name
@@ -1398,6 +1402,35 @@ doc: Partial family for Devlink.
       -
         name: index
 
+  -
+    name: dl-rate-set
+    subset-of: devlink
+    doc: Attributes accepted by the rate-set and rate-new requests.
+    attributes:
+      -
+        name: bus-name
+      -
+        name: dev-name
+      -
+        name: index
+      -
+        name: rate-node-name
+      -
+        name: rate-tx-share
+      -
+        name: rate-tx-max
+      -
+        name: rate-tx-priority
+      -
+        name: rate-tx-weight
+      -
+        name: rate-parent-node-name
+      -
+        name: rate-tc-bws
+      -
+        name: parent-dev
+        nested-attributes: dl-parent-dev
+
   -
     name: dl-nested-devlink
     subset-of: devlink
@@ -2379,7 +2412,7 @@ doc: Partial family for Devlink.
     -
       name: rate-set
       doc: Set rate instances.
-      attribute-set: devlink
+      attribute-set: dl-rate-set
       dont-validate: [strict]
       flags: [admin-perm]
       do:
@@ -2402,7 +2435,7 @@ doc: Partial family for Devlink.
     -
       name: rate-new
       doc: Create rate instances.
-      attribute-set: devlink
+      attribute-set: dl-rate-set
       dont-validate: [strict]
       flags: [admin-perm]
       do:
-- 
2.55.0

Re: [PATCH net-next 7/8] netlink: specs: devlink: describe the netns id in the parent-dev nest

[netdev-bot+sashiko]

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

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

- [High] The new `dl-rate-set` subset
  (Documentation/netlink/specs/devlink.yaml:1406-1432), which the patch…
- [Low] This patch makes `netns-id` part of the described contents of the
  `parent-dev` nest (parent-dev now nests `dl-nested-devlink`,…

Pre-existing issues:
- [Low] The purpose of the change is to describe the netns id carried
  inside the `parent-dev` nest emitted by…

--

netlink: specs: devlink: describe the netns id in the parent-dev nest

The patch points the devlink parent-dev nest at the dl-nested-devlink
subset so the netns id emitted by devlink_nl_put_nested_handle() is
described, and adds a new dl-rate-set subset used as the attribute-set of
the rate-set and rate-new operations so their input keeps nesting
dl-parent-dev.

> diff --git a/Documentation/netlink/specs/devlink.yaml b/Documentation/netlink/specs/devlink.yaml
> index 9d816a0b7da41..54de1eeb22347 100644
> --- a/Documentation/netlink/specs/devlink.yaml
> +++ b/Documentation/netlink/specs/devlink.yaml
> @@ -912,7 +912,7 @@ attribute-sets:
>        -
>          name: parent-dev
>          type: nest
> -        nested-attributes: dl-parent-dev
> +        nested-attributes: dl-nested-devlink

[Severity: Low]
With parent-dev now nesting dl-nested-devlink, netns-id becomes part of
the described nest contents, but the root devlink attribute set types it
as unsigned:

      -
        name: netns-id
        type: u32

The kernel writes it signed in 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;

and peernet2id_alloc() returns NETNSA_NSID_NOT_ASSIGNED (-1) when
check_net() fails, when maybe_get_net() on the peer fails, or when
alloc_netid() fails.

The width matches so the wire format is unchanged, but should the type be
sint/s32 so spec-driven decoders show the "not assigned" report as -1
rather than 4294967295?

Note the same attribute is consumed as an unsigned input by
devlink_netns_get(), so the output-side typing may need to be expressed
separately from the input.

[Severity: Low]
This is a pre-existing gap rather than something this patch introduces,
but the nest whose netns id is being described here is still not listed in
any reply in the spec. rate-get reuses the request anchor for its reply:

        request:
          value: 74
          attributes: &rate-id-attrs
            - bus-name
            - dev-name
            - index
            - port-index
            - rate-node-name
        reply: &rate-get-reply
          value: 76
          attributes: *rate-id-attrs

while devlink_nl_rate_fill() also emits RATE_TYPE, RATE_TX_SHARE,
RATE_TX_MAX, RATE_TX_PRIORITY, RATE_TX_WEIGHT, RATE_TC_BWS and, through
devlink_nl_rate_parent_fill(), RATE_PARENT_NODE_NAME and PARENT_DEV.

Since parent-dev appears only in the root attribute set, in dl-rate-set
and in the rate-set/rate-new request lists, does the netns id described
here remain unreachable for ynl-gen-c generated consumers, which build the
reply struct from the listed reply attributes?

>          doc: |
>            Identifies the devlink instance which owns the parent rate node.
>            Used with rate-set and rate-new to parent a rate object to a node on

[ ... ]

> @@ -1390,6 +1390,22 @@ attribute-sets:
>    -
>      name: dl-parent-dev
>      subset-of: devlink
> +    doc: |
> +      Devlink handle accepted as the parent-dev input; the netns id the
> +      kernel reports back is not accepted, the parent is always resolved
> +      in the caller's netns.
> +    attributes:
> +      -
> +        name: bus-name
> +      -
> +        name: dev-name
> +      -
> +        name: index
> +
> +  -
> +    name: dl-rate-set
> +    subset-of: devlink
> +    doc: Attributes accepted by the rate-set and rate-new requests.
>      attributes:
>        -
>          name: bus-name
> @@ -1397,6 +1413,23 @@ attribute-sets:
>          name: dev-name
>        -
>          name: index
> +      -
> +        name: rate-node-name
> +      -
> +        name: rate-tx-share
> +      -
> +        name: rate-tx-max
> +      -
> +        name: rate-tx-priority
> +      -
> +        name: rate-tx-weight
> +      -
> +        name: rate-parent-node-name
> +      -
> +        name: rate-tc-bws
> +      -
> +        name: parent-dev
> +        nested-attributes: dl-parent-dev

[Severity: High]
Is port-index missing from dl-rate-set?  It is the only way to address a
leaf (port) rate object, per devlink_rate_get_from_info() in
net/devlink/rate.c:

	if (attrs[DEVLINK_ATTR_PORT_INDEX])
		return devlink_rate_leaf_get_from_info(devlink, info);
	else if (attrs[DEVLINK_ATTR_RATE_NODE_NAME])
		return devlink_rate_node_get_from_info(rate_devlink, devlink, info);
	else
		return ERR_PTR(-EINVAL);

Before this change the two ops used attribute-set: devlink, so pyynl could
encode port-index.  _encode_message() encodes request fields out of the
op's attribute set and _add_attr() raises on an unknown name:

        try:
            attr = self.attr_sets[space][name]
        except KeyError as err:
            raise YnlException(f"Space '{space}' has no attribute '{name}'") from err

Does every leaf rate-set/rate-new now fail client side with "Space
'dl-rate-set' has no attribute 'port-index'" before a message is built?

Two in-tree selftests pass port-index to devnl.rate_set():
tools/testing/selftests/drivers/net/hw/devlink_rate_cross_esw.py
(rate_set_leaf, rate_set_leaf_parent, rate_clear_leaf_parent) and
tools/testing/selftests/drivers/net/hw/devlink_rate_tc_bw.py
(setup_devlink_rate).

The same narrowing also affects extack decoding, which resolves the
offending attribute through op.attr_set via attrs_by_val[attr.type], so a
bad-attr extack naming any devlink attribute outside these eleven members
would now raise a decode exception instead of resolving a name.

Related to this, the doc string says "Attributes accepted by the rate-set
and rate-new requests", but the kernel accepts DEVLINK_ATTR_PORT_INDEX for
these commands, and rate-get's request list does include port-index, so the
two spots describing the same object selector disagree.  Adding port-index
to dl-rate-set (and to the rate-set/rate-new request lists, which also omit
it today and only work because of dont-validate: [strict]) would keep them
consistent.

Checked at the tip of the series (59624e85e9fb): the last patch, "netlink:
specs: devlink: add pad to the subsets carrying padded u64s", only adds pad
members and does not add port-index to dl-rate-set.

>  
>    -
>      name: dl-nested-devlink
> @@ -2379,7 +2412,7 @@ operations:
>      -
>        name: rate-set
>        doc: Set rate instances.
> -      attribute-set: devlink
> +      attribute-set: dl-rate-set
>        dont-validate: [strict]
>        flags: [admin-perm]
>        do:
> @@ -2402,7 +2435,7 @@ operations:
>      -
>        name: rate-new
>        doc: Create rate instances.
> -      attribute-set: devlink
> +      attribute-set: dl-rate-set
>        dont-validate: [strict]
>        flags: [admin-perm]
>        do:

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

[PATCH net-next 8/8] netlink: specs: devlink: add pad to the subsets carrying padded u64s

[Jakub Kicinski]

We are missing the pad attribute in a number of subsets.
Worse, dl-attr-stats which is a separate attr space doesn't
define pad, but reuses the value from the root attr set
(DEVLINK_ATTR_PAD). This would break parsing stats on
an arch without CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 Documentation/netlink/specs/devlink.yaml | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/Documentation/netlink/specs/devlink.yaml b/Documentation/netlink/specs/devlink.yaml
index 54de1eeb2234..acb22856a6a8 100644
--- a/Documentation/netlink/specs/devlink.yaml
+++ b/Documentation/netlink/specs/devlink.yaml
@@ -1065,6 +1065,8 @@ doc: Partial family for Devlink.
         name: dpipe-table-resource-id
       -
         name: dpipe-table-resource-units
+      -
+        name: pad
 
   -
     name: dl-dpipe-table-matches
@@ -1099,6 +1101,8 @@ doc: Partial family for Devlink.
         name: dpipe-entry-action-values
       -
         name: dpipe-entry-counter
+      -
+        name: pad
 
   -
     name: dl-dpipe-entry-match-values
@@ -1237,6 +1241,8 @@ doc: Partial family for Devlink.
         name: resource-unit
       -
         name: resource-occ
+      -
+        name: pad
 
   -
     name: dl-resource-list
@@ -1289,6 +1295,8 @@ doc: Partial family for Devlink.
         name: region-chunk-data
       -
         name: region-chunk-addr
+      -
+        name: pad
 
   -
     name: dl-fmsg
@@ -1329,6 +1337,8 @@ doc: Partial family for Devlink.
         name: health-reporter-auto-dump
       -
         name: health-reporter-burst-period
+      -
+        name: pad
 
   -
     name: dl-attr-stats
@@ -1343,6 +1353,10 @@ doc: Partial family for Devlink.
       -
         name: stats-rx-dropped
         type: u64
+      -
+        name: pad
+        type: pad
+        value: 61
 
   -
     name: dl-trap-metadata
-- 
2.55.0

Re: [PATCH net-next 8/8] netlink: specs: devlink: add pad to the subsets carrying padded u64s

[netdev-bot+sashiko]

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

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

- [Low] Missing `Fixes:` tag on a patch that repairs a concrete, user-
  visible parsing failure.

--

netlink: specs: devlink: add pad to the subsets carrying padded u64s

The devlink netlink spec is missing the pad attribute in several nested
attribute subsets that carry padded u64 values, and the separate
dl-attr-stats space had no pad definition of its own. This patch adds
pad to those subsets and gives dl-attr-stats an explicit pad with value
61.

> diff --git a/Documentation/netlink/specs/devlink.yaml b/Documentation/netlink/specs/devlink.yaml
> index 54de1eeb22347..acb22856a6a85 100644
> --- a/Documentation/netlink/specs/devlink.yaml
> +++ b/Documentation/netlink/specs/devlink.yaml
> @@ -1065,6 +1065,8 @@ attribute-sets:
>          name: dpipe-table-resource-id
>        -
>          name: dpipe-table-resource-units
> +      -
> +        name: pad

[ ... ]

> @@ -1343,6 +1353,10 @@ attribute-sets:
>        -
>          name: stats-rx-dropped
>          type: u64
> +      -
> +        name: pad
> +        type: pad
> +        value: 61

[Severity: Low]
Should this carry a Fixes: tag?  The commit message says the missing pad
"would break parsing stats on an arch without
CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS", which reads like a user-visible
bug fix rather than a spec cleanup.

The kernel really does emit the pad attribute inside these nests:

net/devlink/devl_internal.h:
	static inline int devlink_nl_put_u64(struct sk_buff *msg, int attrtype, u64 val)
	{
		return nla_put_u64_64bit(msg, attrtype, val, DEVLINK_ATTR_PAD);
	}

and all of the touched spaces use it - dpipe table size/resource-id/
resource-units and dpipe entry index/counter in net/devlink/dpipe.c, the
resource attributes in net/devlink/resource.c, region-chunk-addr in
net/devlink/region.c, the health reporter counters in net/devlink/health.c,
and the stats attributes in net/devlink/trap.c.

On the decode side an unknown attribute value in a space is fatal rather
than skipped:

tools/net/ynl/pyynl/lib/ynl.py:YnlFamily._decode() {
	...
		attr_spec = attr_space.attrs_by_val[attr.type]
	except (KeyError, UnboundLocalError) as err:
		if not self.process_unknown:
			raise YnlException(f"Space '{space}' has no attribute "
					   f"with value '{attr.type}'") from err
	...
}

So before this change devlink dpipe/resource/region/health-reporter and
trap stats replies were undecodable by ynl on those architectures.  Would
adding something like:

	Fixes: 74bf6477c18b ("netlink: specs: add partial specification for devlink")

help stable and backport tooling pick this up along with the ynl tooling?

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