Netdev List
 help / color / mirror / Atom feed
* [PATCH net-next v2 0/9] devlink: netlink spec fixes
@ 2026-09-15 16:13 Jakub Kicinski
  2026-09-15 16:13 ` [PATCH net-next v2 1/9] devlink: fix the enum behind DEVLINK_ATTR_RELOAD_LIMITS Jakub Kicinski
                   ` (9 more replies)
  0 siblings, 10 replies; 21+ messages in thread
From: Jakub Kicinski @ 2026-09-15 16:13 UTC (permalink / raw)
  To: davem
  Cc: netdev, edumazet, pabeni, andrew+netdev, horms, jiri, tariqt,
	moshe, donald.hunter, Jakub Kicinski

Number of spec fixes for the devlink family.

v2:
 - [patch 1] reword commit msg
 - [patch 6] also update the attr list in the spec for region get
 - [patch 7] include port-index in the subset attr list
 - [patch 9] new patch
v1: https://lore.kernel.org/20260910200312.2665792-1-kuba@kernel.org

Jakub Kicinski (9):
  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
  devlink: validate the port index in the rate set request

 Documentation/netlink/specs/devlink.yaml | 168 +++++++++++++++++++++--
 net/devlink/netlink_gen.h                |   2 +-
 net/devlink/netlink_gen.c                |   9 +-
 net/devlink/port.c                       |  19 +--
 4 files changed, 163 insertions(+), 35 deletions(-)

-- 
2.55.0


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH net-next v2 1/9] devlink: fix the enum behind DEVLINK_ATTR_RELOAD_LIMITS
  2026-09-15 16:13 [PATCH net-next v2 0/9] devlink: netlink spec fixes Jakub Kicinski
@ 2026-09-15 16:13 ` Jakub Kicinski
  2026-09-16 19:15   ` netdev-bot+sashiko
  2026-09-15 16:13 ` [PATCH net-next v2 2/9] netlink: specs: devlink: drop the stale port dump reply value Jakub Kicinski
                   ` (8 subsequent siblings)
  9 siblings, 1 reply; 21+ messages in thread
From: Jakub Kicinski @ 2026-09-15 16:13 UTC (permalink / raw)
  To: davem
  Cc: netdev, edumazet, pabeni, andrew+netdev, horms, jiri, tariqt,
	moshe, donald.hunter, 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".

While at it annotate the right enum for reload-stats-limit

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
v2: add the last sentence to the commit msg
---
 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


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH net-next v2 2/9] netlink: specs: devlink: drop the stale port dump reply value
  2026-09-15 16:13 [PATCH net-next v2 0/9] devlink: netlink spec fixes Jakub Kicinski
  2026-09-15 16:13 ` [PATCH net-next v2 1/9] devlink: fix the enum behind DEVLINK_ATTR_RELOAD_LIMITS Jakub Kicinski
@ 2026-09-15 16:13 ` Jakub Kicinski
  2026-09-15 16:13 ` [PATCH net-next v2 3/9] netlink: specs: devlink: describe DEVLINK_ATTR_NESTED_DEVLINK Jakub Kicinski
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 21+ messages in thread
From: Jakub Kicinski @ 2026-09-15 16:13 UTC (permalink / raw)
  To: davem
  Cc: netdev, edumazet, pabeni, andrew+netdev, horms, jiri, tariqt,
	moshe, donald.hunter, 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>
---
v2: no change
---
 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


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH net-next v2 3/9] netlink: specs: devlink: describe DEVLINK_ATTR_NESTED_DEVLINK
  2026-09-15 16:13 [PATCH net-next v2 0/9] devlink: netlink spec fixes Jakub Kicinski
  2026-09-15 16:13 ` [PATCH net-next v2 1/9] devlink: fix the enum behind DEVLINK_ATTR_RELOAD_LIMITS Jakub Kicinski
  2026-09-15 16:13 ` [PATCH net-next v2 2/9] netlink: specs: devlink: drop the stale port dump reply value Jakub Kicinski
@ 2026-09-15 16:13 ` Jakub Kicinski
  2026-09-16 19:15   ` netdev-bot+sashiko
  2026-09-15 16:13 ` [PATCH net-next v2 4/9] netlink: specs: devlink: complete the port function nest Jakub Kicinski
                   ` (6 subsequent siblings)
  9 siblings, 1 reply; 21+ messages in thread
From: Jakub Kicinski @ 2026-09-15 16:13 UTC (permalink / raw)
  To: davem
  Cc: netdev, edumazet, pabeni, andrew+netdev, horms, jiri, tariqt,
	moshe, donald.hunter, 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>
---
v2: no change
---
 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


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH net-next v2 4/9] netlink: specs: devlink: complete the port function nest
  2026-09-15 16:13 [PATCH net-next v2 0/9] devlink: netlink spec fixes Jakub Kicinski
                   ` (2 preceding siblings ...)
  2026-09-15 16:13 ` [PATCH net-next v2 3/9] netlink: specs: devlink: describe DEVLINK_ATTR_NESTED_DEVLINK Jakub Kicinski
@ 2026-09-15 16:13 ` Jakub Kicinski
  2026-09-16 19:15   ` netdev-bot+sashiko
  2026-09-15 16:13 ` [PATCH net-next v2 5/9] devlink: generate the port function policy from the spec Jakub Kicinski
                   ` (5 subsequent siblings)
  9 siblings, 1 reply; 21+ messages in thread
From: Jakub Kicinski @ 2026-09-15 16:13 UTC (permalink / raw)
  To: davem
  Cc: netdev, edumazet, pabeni, andrew+netdev, horms, jiri, tariqt,
	moshe, donald.hunter, 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>
---
v2: no change
---
 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


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH net-next v2 5/9] devlink: generate the port function policy from the spec
  2026-09-15 16:13 [PATCH net-next v2 0/9] devlink: netlink spec fixes Jakub Kicinski
                   ` (3 preceding siblings ...)
  2026-09-15 16:13 ` [PATCH net-next v2 4/9] netlink: specs: devlink: complete the port function nest Jakub Kicinski
@ 2026-09-15 16:13 ` Jakub Kicinski
  2026-09-15 16:13 ` [PATCH net-next v2 6/9] netlink: specs: devlink: populate multi-attr attrs for region read and line card Jakub Kicinski
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 21+ messages in thread
From: Jakub Kicinski @ 2026-09-15 16:13 UTC (permalink / raw)
  To: davem
  Cc: netdev, edumazet, pabeni, andrew+netdev, horms, jiri, tariqt,
	moshe, donald.hunter, 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>
---
v2: no change
---
 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


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH net-next v2 6/9] netlink: specs: devlink: populate multi-attr attrs for region read and line card
  2026-09-15 16:13 [PATCH net-next v2 0/9] devlink: netlink spec fixes Jakub Kicinski
                   ` (4 preceding siblings ...)
  2026-09-15 16:13 ` [PATCH net-next v2 5/9] devlink: generate the port function policy from the spec Jakub Kicinski
@ 2026-09-15 16:13 ` Jakub Kicinski
  2026-09-16 19:15   ` netdev-bot+sashiko
  2026-09-15 16:13 ` [PATCH net-next v2 7/9] netlink: specs: devlink: describe the netns id in the parent-dev nest Jakub Kicinski
                   ` (3 subsequent siblings)
  9 siblings, 1 reply; 21+ messages in thread
From: Jakub Kicinski @ 2026-09-15 16:13 UTC (permalink / raw)
  To: davem
  Cc: netdev, edumazet, pabeni, andrew+netdev, horms, jiri, tariqt,
	moshe, donald.hunter, 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>
---
v2: also update the attr list in the spec for region get
---
 Documentation/netlink/specs/devlink.yaml | 28 ++++++++++++++++++++----
 1 file changed, 24 insertions(+), 4 deletions(-)

diff --git a/Documentation/netlink/specs/devlink.yaml b/Documentation/netlink/specs/devlink.yaml
index f23466fb27f9..5c9c672497d4 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
@@ -1971,7 +1974,7 @@ doc: Partial family for Devlink.
         post: devlink-nl-post-doit
         request:
           value: 42
-          attributes: &region-id-attrs
+          attributes:
             - bus-name
             - dev-name
             - index
@@ -1979,7 +1982,15 @@ doc: Partial family for Devlink.
             - region-name
         reply: &region-get-reply
           value: 42
-          attributes: *region-id-attrs
+          attributes:
+            - bus-name
+            - dev-name
+            - index
+            - port-index
+            - region-name
+            - region-size
+            - region-max-snapshots
+            - region-snapshots
       dump:
         request:
           attributes: *dev-id-attrs
@@ -2045,6 +2056,7 @@ doc: Partial family for Devlink.
             - index
             - port-index
             - region-name
+            - region-chunks
 
     -
       name: port-param-get
@@ -2444,14 +2456,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


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH net-next v2 7/9] netlink: specs: devlink: describe the netns id in the parent-dev nest
  2026-09-15 16:13 [PATCH net-next v2 0/9] devlink: netlink spec fixes Jakub Kicinski
                   ` (5 preceding siblings ...)
  2026-09-15 16:13 ` [PATCH net-next v2 6/9] netlink: specs: devlink: populate multi-attr attrs for region read and line card Jakub Kicinski
@ 2026-09-15 16:13 ` Jakub Kicinski
  2026-09-16 19:15   ` netdev-bot+sashiko
  2026-09-15 16:13 ` [PATCH net-next v2 8/9] netlink: specs: devlink: add pad to the subsets carrying padded u64s Jakub Kicinski
                   ` (2 subsequent siblings)
  9 siblings, 1 reply; 21+ messages in thread
From: Jakub Kicinski @ 2026-09-15 16:13 UTC (permalink / raw)
  To: davem
  Cc: netdev, edumazet, pabeni, andrew+netdev, horms, jiri, tariqt,
	moshe, donald.hunter, 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>
---
v2: include port-index in the subset attr list
---
 Documentation/netlink/specs/devlink.yaml | 41 ++++++++++++++++++++++--
 1 file changed, 38 insertions(+), 3 deletions(-)

diff --git a/Documentation/netlink/specs/devlink.yaml b/Documentation/netlink/specs/devlink.yaml
index 5c9c672497d4..5535247faf14 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,37 @@ 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: port-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
@@ -2387,7 +2422,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:
@@ -2410,7 +2445,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


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH net-next v2 8/9] netlink: specs: devlink: add pad to the subsets carrying padded u64s
  2026-09-15 16:13 [PATCH net-next v2 0/9] devlink: netlink spec fixes Jakub Kicinski
                   ` (6 preceding siblings ...)
  2026-09-15 16:13 ` [PATCH net-next v2 7/9] netlink: specs: devlink: describe the netns id in the parent-dev nest Jakub Kicinski
@ 2026-09-15 16:13 ` Jakub Kicinski
  2026-09-16 19:15   ` netdev-bot+sashiko
  2026-09-15 16:13 ` [PATCH net-next v2 9/9] devlink: validate the port index in the rate set request Jakub Kicinski
  2026-09-18  1:40 ` [PATCH net-next v2 0/9] devlink: netlink spec fixes patchwork-bot+netdevbpf
  9 siblings, 1 reply; 21+ messages in thread
From: Jakub Kicinski @ 2026-09-15 16:13 UTC (permalink / raw)
  To: davem
  Cc: netdev, edumazet, pabeni, andrew+netdev, horms, jiri, tariqt,
	moshe, donald.hunter, 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>
---
v2: no change
---
 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 5535247faf14..4fb64e71063d 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


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH net-next v2 9/9] devlink: validate the port index in the rate set request
  2026-09-15 16:13 [PATCH net-next v2 0/9] devlink: netlink spec fixes Jakub Kicinski
                   ` (7 preceding siblings ...)
  2026-09-15 16:13 ` [PATCH net-next v2 8/9] netlink: specs: devlink: add pad to the subsets carrying padded u64s Jakub Kicinski
@ 2026-09-15 16:13 ` Jakub Kicinski
  2026-09-16 19:15   ` netdev-bot+sashiko
  2026-09-18  1:40 ` [PATCH net-next v2 0/9] devlink: netlink spec fixes patchwork-bot+netdevbpf
  9 siblings, 1 reply; 21+ messages in thread
From: Jakub Kicinski @ 2026-09-15 16:13 UTC (permalink / raw)
  To: davem
  Cc: netdev, edumazet, pabeni, andrew+netdev, horms, jiri, tariqt,
	moshe, donald.hunter, Jakub Kicinski

port-index is the only way rate-set can address a leaf (port) rate object,
but it was never listed in the request, so the generated policy has no
entry for it. The op declares .maxattr = DEVLINK_ATTR_PARENT_DEV, so the
attribute still reaches info->attrs[], validated against a zeroed slot -
NLA_UNSPEC, length 0 - which GENL_DONT_VALIDATE_STRICT accepts at any
length.

devlink_port_get_from_attrs() then runs nla_get_u32() on it. Handed a
zero-length port-index the kernel reads the four bytes past the payload,
which are the next attribute's header, and acts on the port index those
spell out. Since we're reading a linear skb the OOB read is still
within the same memory allocation, it's just garbage. We also do not
echo the garbage back to the user so it's not an info leak either.
Hence not treating this is a real bug fix.

rate-new is left alone on purpose. It creates rate nodes, resolved by
name through devlink_rate_node_get_from_attrs(), and never looks at
port-index.

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

diff --git a/Documentation/netlink/specs/devlink.yaml b/Documentation/netlink/specs/devlink.yaml
index 4fb64e71063d..1de0daa0f921 100644
--- a/Documentation/netlink/specs/devlink.yaml
+++ b/Documentation/netlink/specs/devlink.yaml
@@ -2447,6 +2447,7 @@ doc: Partial family for Devlink.
             - bus-name
             - dev-name
             - index
+            - port-index
             - rate-node-name
             - rate-tx-share
             - rate-tx-max
diff --git a/net/devlink/netlink_gen.c b/net/devlink/netlink_gen.c
index 43ef6864d462..9e1b4c081a7b 100644
--- a/net/devlink/netlink_gen.c
+++ b/net/devlink/netlink_gen.c
@@ -618,6 +618,7 @@ static const struct nla_policy devlink_rate_set_nl_policy[DEVLINK_ATTR_PARENT_DE
 	[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_PORT_INDEX] = { .type = NLA_U32, },
 	[DEVLINK_ATTR_RATE_NODE_NAME] = { .type = NLA_NUL_STRING, },
 	[DEVLINK_ATTR_RATE_TX_SHARE] = { .type = NLA_U64, },
 	[DEVLINK_ATTR_RATE_TX_MAX] = { .type = NLA_U64, },
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [PATCH net-next v2 1/9] devlink: fix the enum behind DEVLINK_ATTR_RELOAD_LIMITS
  2026-09-15 16:13 ` [PATCH net-next v2 1/9] devlink: fix the enum behind DEVLINK_ATTR_RELOAD_LIMITS Jakub Kicinski
@ 2026-09-16 19:15   ` netdev-bot+sashiko
  2026-09-18  1:19     ` Jakub Kicinski
  0 siblings, 1 reply; 21+ messages in thread
From: netdev-bot+sashiko @ 2026-09-16 19:15 UTC (permalink / raw)
  To: kuba
  Cc: davem, netdev, edumazet, pabeni, andrew+netdev, horms, jiri,
	tariqt, moshe, donald.hunter

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.

> 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

[ ... ]

> 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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH net-next v2 3/9] netlink: specs: devlink: describe DEVLINK_ATTR_NESTED_DEVLINK
  2026-09-15 16:13 ` [PATCH net-next v2 3/9] netlink: specs: devlink: describe DEVLINK_ATTR_NESTED_DEVLINK Jakub Kicinski
@ 2026-09-16 19:15   ` netdev-bot+sashiko
  2026-09-18  1:21     ` Jakub Kicinski
  0 siblings, 1 reply; 21+ messages in thread
From: netdev-bot+sashiko @ 2026-09-16 19:15 UTC (permalink / raw)
  To: kuba
  Cc: davem, netdev, edumazet, pabeni, andrew+netdev, horms, jiri,
	tariqt, moshe, donald.hunter

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

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

- [Medium] The new `dl-nested-devlink` fractional set added by "netlink:
  specs: devlink: describe DEVLINK_ATTR_NESTED_DEVLINK"…

--

netlink: specs: devlink: describe DEVLINK_ATTR_NESTED_DEVLINK

This fills in the single attribute hidden by the "# TODO: fill in the
attributes in between" gap before selftests (176), adding nested-devlink
(175) as a multi-attr nest, plus a new dl-nested-devlink fractional set
and the 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

[ ... ]

> @@ -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: Medium]
Is the inherited type right for netns-id here?  Since this is a
subset-of set, it picks up the main devlink set declaration:

      -
        name: netns-id
        type: u32

But the kernel emits this attribute signed, in
devlink_nl_put_nested_handle() (net/devlink/netlink.c), which is the
function reached by devlink_nl_fill() -> devlink_nl_nested_fill() for
each nested_rels entry:

	if (!net_eq(net, devl_net)) {
		int id = peernet2id_alloc(net, devl_net, GFP_ATOMIC);

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

peernet2id_alloc() in net/core/net_namespace.c returns the negative
sentinel NETNSA_NSID_NOT_ASSIGNED (-1, include/uapi/linux/net_namespace.h)
in three cases:

	if (!check_net(net))
		return NETNSA_NSID_NOT_ASSIGNED;
	...
	if (!maybe_get_net(peer)) {
		spin_unlock(&net->nsid_lock);
		return NETNSA_NSID_NOT_ASSIGNED;
	}
	...
	if (id < 0)
		return NETNSA_NSID_NOT_ASSIGNED;

The wire width is 4 bytes either way, so nothing breaks in the kernel,
but would ynl-generated decoders and C structs now render the "nsid not
assigned" case as 4294967295, with no way to tell it apart from a valid
id?

The request side does read the attribute unsigned, in devlink_netns_get()
(net/devlink/dev.c):

	} else if (netns_id_attr) {
		net = get_net_ns_by_id(sock_net(skb->sk),
				       nla_get_u32(netns_id_attr));

so flipping the shared declaration would change the request typing too.
Would it be better to declare the reply-side attribute as s32, or
otherwise narrow the type per-use, so the documented type matches
nla_put_s32()?

One more note on scope: a later patch in this series, "netlink: specs:
devlink: describe the netns id in the parent-dev nest", reuses this same
subset for DEVLINK_ATTR_PARENT_DEV replies, and at 0fe71b34ba2e
("devlink: validate the port index in the rate set request") netns-id is
still type: u32 while the emitter still uses nla_put_s32(), so the
mismatch persists through the end of the series.

> +
>  operations:
>    enum-model: directional
>    list:
> @@ -1376,6 +1390,7 @@ operations:
>              - index
>              - reload-failed
>              - dev-stats
> +            - nested-devlink
>        dump:
>          reply: *get-reply
>

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH net-next v2 4/9] netlink: specs: devlink: complete the port function nest
  2026-09-15 16:13 ` [PATCH net-next v2 4/9] netlink: specs: devlink: complete the port function nest Jakub Kicinski
@ 2026-09-16 19:15   ` netdev-bot+sashiko
  0 siblings, 0 replies; 21+ messages in thread
From: netdev-bot+sashiko @ 2026-09-16 19:15 UTC (permalink / raw)
  To: kuba
  Cc: davem, netdev, edumazet, pabeni, andrew+netdev, horms, jiri,
	tariqt, moshe, donald.hunter

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

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

- [Low] Missing `Fixes:` tag on a user-visible uAPI-description fix.
- [Low] The newly added `max-io-eqs` attribute in the `dl-port-function`
  attribute set (Documentation/netlink/specs/devlink.yaml) carries no…

Pre-existing issues:
- [Low] Concern (assessed as pre-existing and low impact): the `dl-
  nested-devlink` nest now referenced by the new port-function `devlink`…

--

netlink: specs: devlink: complete the port function nest

The devlink spec's dl-port-function nest stopped at caps even though the
kernel also emits DEVLINK_PORT_FN_ATTR_DEVLINK and
DEVLINK_PORT_FN_ATTR_MAX_IO_EQS, so ynl could not decode port replies on
mlx5 SFs and VFs.  This adds the two missing attributes to the spec and
regenerates the devlink netlink policy.

A couple of questions below.

The commit message says:

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

[Severity: Low]
Since the offending commit is already identified, and the symptom is a hard
decode failure in userspace rather than a cosmetic gap, would a Fixes: tag
be appropriate here?

  Fixes: 5af3e3876d56 ("devlink: Support setting max_io_eqs")

devlink_nl_port_function_attrs_put() puts DEVLINK_PORT_FN_ATTR_MAX_IO_EQS
on every port-get do and dump for affected hardware:

net/devlink/port.c:devlink_port_fn_max_io_eqs_fill() {
	...
	err = nla_put_u32(msg, DEVLINK_PORT_FN_ATTR_MAX_IO_EQS, max_io_eqs);
	...
}

so ynl-based tooling aborts with the quoted "Space 'dl-port-function' has
no attribute with value '6'" until this spec change lands.  Without a tag,
backport tooling has nothing structured to key off.

> 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]
This isn't a bug, but the sibling devlink attribute added in the same hunk
carries a doc: while max-io-eqs does not.  Could a doc: line be added
stating what the value means, e.g. the maximum number of IO event queues
configurable for the function?

>  
>    -
>      name: dl-dpipe-tables
> 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 typing mismatch and not something this patch
introduces, but pulling dl-nested-devlink into the port function nest makes
it visible to more spec-driven decoders.

The spec declares netns-id as u32 in the root devlink attribute set:

      -
        name: netns-id
        type: u32

while the kernel emits it signed:

net/devlink/netlink.c:devlink_nl_put_nested_handle() {
	...
		if (nla_put_s32(msg, DEVLINK_ATTR_NETNS_ID, id))
	...
}

and the value can be the negative sentinel:

net/core/net_namespace.c:peernet2id_alloc() {
	...
	if (!check_net(net))
		return NETNSA_NSID_NOT_ASSIGNED;
	...
}

Would ynl then render an unassigned namespace as 4294967295 rather than -1?
Wire width and kernel-side length validation are identical for NLA_U32 and
NLA_S32, so this looks like a display-only difference, but should the spec
(and hence the generated policy) say sint/s32 here?

>  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),
>  };

[ ... ]

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH net-next v2 6/9] netlink: specs: devlink: populate multi-attr attrs for region read and line card
  2026-09-15 16:13 ` [PATCH net-next v2 6/9] netlink: specs: devlink: populate multi-attr attrs for region read and line card Jakub Kicinski
@ 2026-09-16 19:15   ` netdev-bot+sashiko
  0 siblings, 0 replies; 21+ messages in thread
From: netdev-bot+sashiko @ 2026-09-16 19:15 UTC (permalink / raw)
  To: kuba
  Cc: davem, netdev, edumazet, pabeni, andrew+netdev, horms, jiri,
	tariqt, moshe, donald.hunter

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

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

- [Low] The patch fixes a user-visible decoding defect but carries no
  Fixes: tag.

Pre-existing issues:
- [Medium] DEVLINK_ATTR_INDEX is specified as `type: uint`
  (Documentation/netlink/specs/devlink.yaml:896-901) and the generated…
- [Low] Residual instance of the same spec/codegen defect class in the
  same file: the `dl-fmsg` fractional attribute set…

--

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 entries in their fractional attribute sets, and expands the
region-get and linecard-get request/reply attribute lists so the attributes
the kernel actually emits are visible to the C code generator.

A few questions below.

> netlink: specs: devlink: populate multi-attr attrs for region read and line card
>
> Three attributes are emitted repeatedly inside their nest:
> [ ... ]
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>

[Severity: Low]
Would a Fixes: tag be appropriate here?  The change looks like it repairs
a userspace-visible decoding defect rather than only improving the spec
text.  SpecAttr.is_multi defaults to False in
tools/net/ynl/pyynl/lib/nlspec.py, and _rsp_add() in
tools/net/ynl/pyynl/lib/ynl.py then overwrites earlier values:

    if not is_multi:
        rsp[name] = decoded

So before this patch ynl reported only the last DEVLINK_ATTR_REGION_CHUNK,
the last DEVLINK_ATTR_REGION_SNAPSHOT and the last
DEVLINK_ATTR_LINECARD_TYPE, meaning region reads and linecard
supported-type lists came out truncated.

git blame points at f2f9dd164db0 for the region-read op attribute list and
68c84289bcc0 for the dl-region-snapshots / dl-region-chunks /
dl-linecard-supported-types sets.  Without Fixes: tags, stable tooling has
no way to work out which releases carry the broken spec.

> diff --git a/Documentation/netlink/specs/devlink.yaml b/Documentation/netlink/specs/devlink.yaml
> index f23466fb27f97..5c9c672497d44 100644
> --- a/Documentation/netlink/specs/devlink.yaml
> +++ b/Documentation/netlink/specs/devlink.yaml

[ ... ]

> @@ -1278,6 +1279,7 @@ attribute-sets:
>      attributes:
>        -
>          name: region-chunk
> +        multi-attr: true
>  
>    -
>      name: dl-region-chunk

[Severity: Low]
This isn't a bug introduced by this patch, but is the dl-fmsg set a few
lines further down in the same file affected by the same mismatch?  It
declares fmsg-obj-nest-start, fmsg-pair-nest-start, fmsg-arr-nest-start,
fmsg-nest-end and fmsg-obj-name with no multi-attr, while
devlink_fmsg_prepare_skb() in net/devlink/health.c emits each of those
repeatedly inside one DEVLINK_ATTR_FMSG nest:

    fmsg_nlattr = nla_nest_start_noflag(skb, DEVLINK_ATTR_FMSG);
    ...
    list_for_each_entry(item, &fmsg->item_list, list) {
    ...
            case DEVLINK_ATTR_FMSG_OBJ_NAME:
                    err = nla_put_string(skb, item->attrtype, (char *)&item->value);

dl-fmsg is reachable from the health-reporter-diagnose reply, so the same
last-value-wins decoding applies there.

Note that the fix for fmsg cannot be the same one-line annotation:
TypeMultiAttr._complex_member_type() in tools/net/ynl/pyynl/ynl_gen_c.py
raises

    raise Exception(f"Sub-type {self.attr['type']} not supported yet")

for flag sub-types, so marking the four flag markers multi-attr would break
C code generation.  Is a separate, larger modelling change for fmsg planned,
or is it intentionally left out of scope given the commit message limits
itself to "region read and line card"?

[ ... ]

> @@ -1971,7 +1974,7 @@ operations:
>          post: devlink-nl-post-doit
>          request:
>            value: 42
> -          attributes: &region-id-attrs
> +          attributes:
>              - bus-name
>              - dev-name
>              - index
                 ^^^^^

[Severity: Medium]
This is a pre-existing issue and not something this patch introduces - the
patch only re-enumerates index in the region-get and linecard-get lists -
but while the index attribute is being written out explicitly here, is the
declared width consistent with how the kernel reads it?

The spec declares it as variable width:

      -
        name: index
        type: uint
        doc: Unique devlink instance index.
        checks:
          max: u32-max

and the generated policy is NLA_POLICY_FULL_RANGE(NLA_UINT, ...) for these
ops.  validate_nla() accepts either width for NLA_UINT:

    case NLA_SINT:
    case NLA_UINT:
            if (attrlen != sizeof(u32) && attrlen != sizeof(u64)) {

so an 8-byte payload holding a value <= U32_MAX passes validation, since the
range check itself uses nla_get_uint().

The two readers in net/devlink/netlink.c then disagree.
devlink_nl_filter_alloc() does:

    flt->devlink_index = nla_get_uint(attrs[DEVLINK_ATTR_INDEX]);

while devlink_get_from_attrs_lock(), the pre_doit resolver used by these
ops, does:

    index = nla_get_u32(attrs[DEVLINK_ATTR_INDEX]);
    devlink = devlinks_xa_lookup_get(net, index);

On a big-endian kernel, wouldn't nla_get_u32() return the high half of an
8-byte payload, i.e. 0 for any in-range value, so the request resolves
devlink index 0 or returns -ENODEV instead of addressing the instance the
caller asked for?

> @@ -1979,7 +1982,15 @@ operations:
>          reply: &region-get-reply
>            value: 42
> -          attributes: *region-id-attrs
> +          attributes:
> +            - bus-name
> +            - dev-name
> +            - index
> +            - port-index
> +            - region-name
> +            - region-size
> +            - region-max-snapshots
> +            - region-snapshots

[ ... ]

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH net-next v2 7/9] netlink: specs: devlink: describe the netns id in the parent-dev nest
  2026-09-15 16:13 ` [PATCH net-next v2 7/9] netlink: specs: devlink: describe the netns id in the parent-dev nest Jakub Kicinski
@ 2026-09-16 19:15   ` netdev-bot+sashiko
  2026-09-18  1:23     ` Jakub Kicinski
  0 siblings, 1 reply; 21+ messages in thread
From: netdev-bot+sashiko @ 2026-09-16 19:15 UTC (permalink / raw)
  To: kuba
  Cc: davem, netdev, edumazet, pabeni, andrew+netdev, horms, jiri,
	tariqt, moshe, donald.hunter

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

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

- [Medium] Understated impact and missing Fixes: tag.
- [Medium] Spec/implementation signedness mismatch for the very field
  this patch sets out to describe: devlink.yaml declares `netns-id` as…
- [Low] Baseline decision: FALSE POSITIVE at the end of the series
  (retained only because the concern is flagged requires_validation).

Pre-existing issues:
- [Low] Baseline decision: DISCARDED as a low-severity pre-existing gap
  (retained only because the concern is flagged requires_validation).

--

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

Points the parent-dev nest at the dl-nested-devlink subset so the netns id
that the kernel emits inside that nest is described, and adds a new
dl-rate-set subset so the rate-set and rate-new request attribute sets keep
using dl-parent-dev, which has no netns id.

> netlink: specs: devlink: describe the netns id in the parent-dev nest
>
> 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.

[Severity: Medium]
Is this description understating what the change fixes?  The wording reads
like the switch is descriptive polish ("Now that we defined
dl-nested-devlink subset ... let's switch to it"), but before this patch
python ynl seems to fail decoding real kernel messages, not just describe
them incompletely.

devlink_nl_rate_parent_fill() puts the nest through
devlink_nl_put_nested_handle(), which adds the netns id:

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))
	...
}

With parent-dev nesting dl-parent-dev (bus-name, dev-name, index only),
YnlFamily._decode() raises when it cannot find the attribute in the nested
space:

tools/net/ynl/pyynl/lib/ynl.py:YnlFamily._decode() {
	...
                if not self.process_unknown:
                    raise YnlException(f"Space '{space}' has no attribute "
                                       f"with value '{attr.type}'") from err
	...
}

so rate-get do/dump and the rate notifications abort for a cross-netns
parent, the same failure class quoted in the sibling patch "netlink: specs:
devlink: complete the port function nest".

Would it make sense to say that in the commit message and add:

Fixes: b5f90fd4580ce7 ("devlink: Add parent dev to devlink API")

which is the commit that added both parent-dev and the dl-parent-dev
subset?  Without it this looks cosmetic and stable trees keep the spec that
breaks decoding.

> diff --git a/Documentation/netlink/specs/devlink.yaml b/Documentation/netlink/specs/devlink.yaml
> index 5c9c672497d44..5535247faf14f 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: Medium]
Now that the netns id is described in this nest, is its declared type
right?  In the devlink attribute set netns-id is declared unsigned:

        name: netns-id
        type: u32

but the emitter uses the signed helper on a value that can be negative:

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;
	...
}

peernet2id_alloc() returns NETNSA_NSID_NOT_ASSIGNED (-1) in several cases:

net/core/net_namespace.c:peernet2id_alloc() {
	...
	if (!check_net(net))
		return NETNSA_NSID_NOT_ASSIGNED;
	...
	if (!maybe_get_net(peer)) {
		spin_unlock(&net->nsid_lock);
		return NETNSA_NSID_NOT_ASSIGNED;
	}
	...
}

So when the peer netns is going away, or no nsid can be allocated, does a
ynl based tool render the not-assigned marker as 4294967295 for this nest?
Should netns-id be sint/s32 instead?  Note the same declaration is reused
by the reload input path in net/devlink/dev.c, and the uAPI header comment
repeats it:

include/uapi/linux/devlink.h:
	DEVLINK_ATTR_NETNS_ID,			/* u32 */

so separating the input and output descriptions may be needed.

>          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

[ ... ]

> @@ -1398,6 +1402,37 @@ attribute-sets:
>        -
>          name: index
>  
> +  -
> +    name: dl-rate-set
> +    subset-of: devlink
> +    doc: Attributes accepted by the rate-set and rate-new requests.

[Severity: Low]
This isn't a bug, but is the doc line accurate for rate-new?  The subset
lists port-index, while rate-new's do: request: attributes: omits it, and
devlink_nl_rate_new_doit() never looks at DEVLINK_ATTR_PORT_INDEX
(devlink_rate_node_get_from_attrs() resolves the node by name), so
port-index is silently ignored there.  Could the doc say that port-index
applies to rate-set only?

For the record, the related concern about DEVLINK_ATTR_PORT_INDEX having
no policy entry in devlink_rate_set_nl_policy, which let
devlink_port_get_from_attrs() run nla_get_u32() on a short attribute under
dont-validate: [strict], is resolved later in this same series by
"devlink: validate the port index in the rate set request", which adds
port-index to the rate-set request list and regenerates the NLA_U32 policy
entry.  So there is nothing left here beyond the doc wording.

> +    attributes:
> +      -
> +        name: bus-name
> +      -
> +        name: dev-name
> +      -
> +        name: index
> +      -
> +        name: port-index
> +      -
> +        name: rate-node-name

[ ... ]

> +      -
> +        name: parent-dev
> +        nested-attributes: dl-parent-dev
> +
>    -
>      name: dl-nested-devlink
>      subset-of: devlink
> @@ -2387,7 +2422,7 @@ operations:
>      -
>        name: rate-set
>        doc: Set rate instances.
> -      attribute-set: devlink
> +      attribute-set: dl-rate-set
>        dont-validate: [strict]
>        flags: [admin-perm]
>        do:

[ ... ]

> @@ -2410,7 +2445,7 @@ operations:
>      -
>        name: rate-new
>        doc: Create rate instances.
> -      attribute-set: devlink
> +      attribute-set: dl-rate-set
>        dont-validate: [strict]
>        flags: [admin-perm]
>        do:

[Severity: Low]
This is a pre-existing gap and not something this patch introduces, but
while the output side of parent-dev is being described here, parent-dev
still does not appear in any reply or notification attribute list.

The rate-get reply anchor (&rate-get-reply) lists only *rate-id-attrs
(bus-name, dev-name, index, port-index, rate-node-name), and that same
anchor is reused by the rate dump and by the rate-new/rate-del
notifications, while devlink_nl_rate_fill() emits more than that:

net/devlink/rate.c:devlink_nl_rate_fill() {
	...
	if (nla_put_u16(msg, DEVLINK_ATTR_RATE_TYPE, devlink_rate->type))
	...
	if (devlink_rate->parent &&
	    devlink_nl_rate_parent_fill(msg, devlink_rate))
	...
	if (devlink_rate_put_tc_bws(msg, devlink_rate->tc_bw))
	...
}

and the nest itself comes from:

net/devlink/rate.c:devlink_nl_rate_parent_fill() {
	...
	if (devlink != devlink_rate->devlink &&
	    devlink_nl_put_nested_handle(msg,
					 devlink_net(devlink_rate->devlink),
					 devlink, DEVLINK_ATTR_PARENT_DEV))
	...
}

Does this mean generated C ynl reply structures for rate-get still have no
parent_dev (and no rate tx share/max/priority/weight or tc-bws) member, so
C consumers cannot see the netns id this patch documents?  Would adding the
emitted attributes to the rate-get reply be a reasonable follow-up?

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH net-next v2 8/9] netlink: specs: devlink: add pad to the subsets carrying padded u64s
  2026-09-15 16:13 ` [PATCH net-next v2 8/9] netlink: specs: devlink: add pad to the subsets carrying padded u64s Jakub Kicinski
@ 2026-09-16 19:15   ` netdev-bot+sashiko
  0 siblings, 0 replies; 21+ messages in thread
From: netdev-bot+sashiko @ 2026-09-16 19:15 UTC (permalink / raw)
  To: kuba
  Cc: davem, netdev, edumazet, pabeni, andrew+netdev, horms, jiri,
	tariqt, moshe, donald.hunter

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 spec change that repairs a real, user-
  visible parsing failure.

--

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

This adds the missing pad attribute to several devlink attribute subsets
that carry padded u64 values, and defines pad explicitly in the separate
dl-attr-stats space with value 61.

> diff --git a/Documentation/netlink/specs/devlink.yaml b/Documentation/netlink/specs/devlink.yaml
> index 5535247faf14f..4fb64e71063dc 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 describes a real user-visible failure rather than a
cosmetic spec cleanup:

    This would break parsing stats on
    an arch without CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS.

The kernel does emit that attribute type 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 this helper feeds the u64s in every nest touched here, e.g.
DEVLINK_ATTR_DPIPE_TABLE_SIZE / _RESOURCE_ID / _RESOURCE_UNITS in
net/devlink/dpipe.c, the resource attributes in net/devlink/resource.c,
DEVLINK_ATTR_REGION_CHUNK_ADDR in net/devlink/region.c, the health reporter
attributes in net/devlink/health.c, and STATS_RX_PACKETS / _BYTES / _DROPPED
inside the DEVLINK_ATTR_STATS nest in net/devlink/trap.c.

On an arch where nla_need_padding_for_64bit() is true, ynl aborts rather
than skipping the unknown attribute, in YnlFamily._decode() of
tools/net/ynl/pyynl/lib/ynl.py:

            try:
                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 "

Since the defective spec text came in via several different historical
commits (the individual subsets, and separately the dl-attr-stats space),
would it help backporters to name at least the commit that introduced
dl-attr-stats in a Fixes: trailer? Other patches in this series reference
culprit commits only in prose, for example the port function nest patch
mentions 5af3e3876d56 in the body, and nothing here states that leaving out
the trailer is intentional.

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH net-next v2 9/9] devlink: validate the port index in the rate set request
  2026-09-15 16:13 ` [PATCH net-next v2 9/9] devlink: validate the port index in the rate set request Jakub Kicinski
@ 2026-09-16 19:15   ` netdev-bot+sashiko
  0 siblings, 0 replies; 21+ messages in thread
From: netdev-bot+sashiko @ 2026-09-16 19:15 UTC (permalink / raw)
  To: kuba
  Cc: davem, netdev, edumazet, pabeni, andrew+netdev, horms, jiri,
	tariqt, moshe, donald.hunter

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 characterization / missing `Fixes:` tag.

--

devlink: validate the port index in the rate set request

This adds port-index to the rate-set request attribute list in the devlink
YAML spec and adds the matching NLA_U32 entry to the generated
devlink_rate_set_nl_policy. rate-new is deliberately left unchanged since
it resolves rate nodes by name.

> diff --git a/net/devlink/netlink_gen.c b/net/devlink/netlink_gen.c
> index 43ef6864d462f..9e1b4c081a7b3 100644
> --- a/net/devlink/netlink_gen.c
> +++ b/net/devlink/netlink_gen.c
> @@ -618,6 +618,7 @@ static const struct nla_policy devlink_rate_set_nl_policy[DEVLINK_ATTR_PARENT_DE
>  	[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_PORT_INDEX] = { .type = NLA_U32, },

[Severity: Low]
This isn't a bug in the change itself, but should this carry a Fixes tag?

The missing policy slot looks bisectable to f2f9dd164db0 ("netlink: specs:
devlink: add the remaining command to generate complete split_ops"), which
moved DEVLINK_CMD_RATE_SET out of small_ops into split_ops. The old
small_ops path used the global devlink_nl_policy in net/devlink/netlink.c,
which does declare:

	[DEVLINK_ATTR_PORT_INDEX] = { .type = NLA_U32 },

so before that conversion a short port-index was rejected, and afterwards
it is not.

The commit message says:

	Since we're reading a linear skb the OOB read is still
	within the same memory allocation, it's just garbage. We also do not
	echo the garbage back to the user so it's not an info leak either.
	Hence not treating this is a real bug fix.

That reasoning checks out, and the op is GENL_ADMIN_PERM so only a caller
that can already reconfigure any port reaches it. The one remaining effect
is behavioural rather than memory-safety related: with the zeroed policy
slot plus GENL_DONT_VALIDATE_STRICT, a zero-length port-index passes
validation, devlink_rate_get_from_info() picks the leaf path purely on
attribute presence:

net/devlink/rate.c:devlink_rate_get_from_info() {
	if (attrs[DEVLINK_ATTR_PORT_INDEX])
		return devlink_rate_leaf_get_from_info(devlink, info);
	...
}

and devlink_port_get_from_attrs() then reads past the payload:

net/devlink/port.c:devlink_port_get_from_attrs() {
	u32 port_index = nla_get_u32(attrs[DEVLINK_ATTR_PORT_INDEX]);
	...
}

If those four bytes happen to name a registered port,
devlink_nl_rate_set_doit() goes on to change that leaf's rate settings
instead of returning -EINVAL. Would a Fixes tag help stable/backport
tooling identify the trees (v6.7 onward) where a malformed rate-set request
is acted upon rather than rejected?

>  	[DEVLINK_ATTR_RATE_NODE_NAME] = { .type = NLA_NUL_STRING, },
>  	[DEVLINK_ATTR_RATE_TX_SHARE] = { .type = NLA_U64, },
>  	[DEVLINK_ATTR_RATE_TX_MAX] = { .type = NLA_U64, },

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH net-next v2 1/9] devlink: fix the enum behind DEVLINK_ATTR_RELOAD_LIMITS
  2026-09-16 19:15   ` netdev-bot+sashiko
@ 2026-09-18  1:19     ` Jakub Kicinski
  0 siblings, 0 replies; 21+ messages in thread
From: Jakub Kicinski @ 2026-09-18  1:19 UTC (permalink / raw)
  To: netdev-bot+sashiko
  Cc: davem, netdev, edumazet, pabeni, andrew+netdev, horms, jiri,
	tariqt, moshe, donald.hunter

On Wed, 16 Sep 2026 19:15:18 +0000 netdev-bot+sashiko@kernel.org wrote:
> 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.

My point was that bit 1 / mask 0x2 is the only one that has practical
use, and it so happens that both old 0x6 and new 0x3 covers it. Which
is why I think it's is cosmetic.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH net-next v2 3/9] netlink: specs: devlink: describe DEVLINK_ATTR_NESTED_DEVLINK
  2026-09-16 19:15   ` netdev-bot+sashiko
@ 2026-09-18  1:21     ` Jakub Kicinski
  0 siblings, 0 replies; 21+ messages in thread
From: Jakub Kicinski @ 2026-09-18  1:21 UTC (permalink / raw)
  To: netdev-bot+sashiko
  Cc: davem, netdev, edumazet, pabeni, andrew+netdev, horms, jiri,
	tariqt, moshe, donald.hunter

On Wed, 16 Sep 2026 19:15:19 +0000 netdev-bot+sashiko@kernel.org wrote:
> > +      -
> > +        name: netns-id  
>                 ^^^^^^^^^
> 
> [Severity: Medium]
> Is the inherited type right for netns-id here?  Since this is a
> subset-of set, it picks up the main devlink set declaration:
> 
>       -
>         name: netns-id
>         type: u32

As previously mentioned, negative netns-ids is something for a separate
discussion. In practice since only negative value that can be output is
-1 == "invalid", this has limited impact even on the convenience. 
The bigger question is whether we should be outputting error ids in 
the first place. I don't want to switch to signed in case we decided
that we don't.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH net-next v2 7/9] netlink: specs: devlink: describe the netns id in the parent-dev nest
  2026-09-16 19:15   ` netdev-bot+sashiko
@ 2026-09-18  1:23     ` Jakub Kicinski
  0 siblings, 0 replies; 21+ messages in thread
From: Jakub Kicinski @ 2026-09-18  1:23 UTC (permalink / raw)
  To: netdev-bot+sashiko
  Cc: davem, netdev, edumazet, pabeni, andrew+netdev, horms, jiri,
	tariqt, moshe, donald.hunter

On Wed, 16 Sep 2026 19:15:23 +0000 netdev-bot+sashiko@kernel.org wrote:
> > 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.  
> 
> [Severity: Medium]
> Is this description understating what the change fixes?  The wording reads
> like the switch is descriptive polish ("Now that we defined
> dl-nested-devlink subset ... let's switch to it"), but before this patch
> python ynl seems to fail decoding real kernel messages, not just describe
> them incompletely.

Well.. sure. It makes YNL work for the requests, it's sort of 
the overarching point of the spec fixes.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH net-next v2 0/9] devlink: netlink spec fixes
  2026-09-15 16:13 [PATCH net-next v2 0/9] devlink: netlink spec fixes Jakub Kicinski
                   ` (8 preceding siblings ...)
  2026-09-15 16:13 ` [PATCH net-next v2 9/9] devlink: validate the port index in the rate set request Jakub Kicinski
@ 2026-09-18  1:40 ` patchwork-bot+netdevbpf
  9 siblings, 0 replies; 21+ messages in thread
From: patchwork-bot+netdevbpf @ 2026-09-18  1:40 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: davem, netdev, edumazet, pabeni, andrew+netdev, horms, jiri,
	tariqt, moshe, donald.hunter

Hello:

This series was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Tue, 15 Sep 2026 09:13:32 -0700 you wrote:
> Number of spec fixes for the devlink family.
> 
> v2:
>  - [patch 1] reword commit msg
>  - [patch 6] also update the attr list in the spec for region get
>  - [patch 7] include port-index in the subset attr list
>  - [patch 9] new patch
> v1: https://lore.kernel.org/20260910200312.2665792-1-kuba@kernel.org
> 
> [...]

Here is the summary with links:
  - [net-next,v2,1/9] devlink: fix the enum behind DEVLINK_ATTR_RELOAD_LIMITS
    https://git.kernel.org/netdev/net-next/c/6c53effab88d
  - [net-next,v2,2/9] netlink: specs: devlink: drop the stale port dump reply value
    https://git.kernel.org/netdev/net-next/c/b8ffb7e5b9a1
  - [net-next,v2,3/9] netlink: specs: devlink: describe DEVLINK_ATTR_NESTED_DEVLINK
    https://git.kernel.org/netdev/net-next/c/6338e31e6855
  - [net-next,v2,4/9] netlink: specs: devlink: complete the port function nest
    https://git.kernel.org/netdev/net-next/c/4ced66fc97bf
  - [net-next,v2,5/9] devlink: generate the port function policy from the spec
    https://git.kernel.org/netdev/net-next/c/3ec610a6bb12
  - [net-next,v2,6/9] netlink: specs: devlink: populate multi-attr attrs for region read and line card
    https://git.kernel.org/netdev/net-next/c/b1380f907cb0
  - [net-next,v2,7/9] netlink: specs: devlink: describe the netns id in the parent-dev nest
    https://git.kernel.org/netdev/net-next/c/150079380e4b
  - [net-next,v2,8/9] netlink: specs: devlink: add pad to the subsets carrying padded u64s
    https://git.kernel.org/netdev/net-next/c/826e2a736e33
  - [net-next,v2,9/9] devlink: validate the port index in the rate set request
    https://git.kernel.org/netdev/net-next/c/2a22788ba265

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2026-09-18  1:41 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-15 16:13 [PATCH net-next v2 0/9] devlink: netlink spec fixes Jakub Kicinski
2026-09-15 16:13 ` [PATCH net-next v2 1/9] devlink: fix the enum behind DEVLINK_ATTR_RELOAD_LIMITS Jakub Kicinski
2026-09-16 19:15   ` netdev-bot+sashiko
2026-09-18  1:19     ` Jakub Kicinski
2026-09-15 16:13 ` [PATCH net-next v2 2/9] netlink: specs: devlink: drop the stale port dump reply value Jakub Kicinski
2026-09-15 16:13 ` [PATCH net-next v2 3/9] netlink: specs: devlink: describe DEVLINK_ATTR_NESTED_DEVLINK Jakub Kicinski
2026-09-16 19:15   ` netdev-bot+sashiko
2026-09-18  1:21     ` Jakub Kicinski
2026-09-15 16:13 ` [PATCH net-next v2 4/9] netlink: specs: devlink: complete the port function nest Jakub Kicinski
2026-09-16 19:15   ` netdev-bot+sashiko
2026-09-15 16:13 ` [PATCH net-next v2 5/9] devlink: generate the port function policy from the spec Jakub Kicinski
2026-09-15 16:13 ` [PATCH net-next v2 6/9] netlink: specs: devlink: populate multi-attr attrs for region read and line card Jakub Kicinski
2026-09-16 19:15   ` netdev-bot+sashiko
2026-09-15 16:13 ` [PATCH net-next v2 7/9] netlink: specs: devlink: describe the netns id in the parent-dev nest Jakub Kicinski
2026-09-16 19:15   ` netdev-bot+sashiko
2026-09-18  1:23     ` Jakub Kicinski
2026-09-15 16:13 ` [PATCH net-next v2 8/9] netlink: specs: devlink: add pad to the subsets carrying padded u64s Jakub Kicinski
2026-09-16 19:15   ` netdev-bot+sashiko
2026-09-15 16:13 ` [PATCH net-next v2 9/9] devlink: validate the port index in the rate set request Jakub Kicinski
2026-09-16 19:15   ` netdev-bot+sashiko
2026-09-18  1:40 ` [PATCH net-next v2 0/9] devlink: netlink spec fixes patchwork-bot+netdevbpf

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox