Thread (15 messages) flat view 15 messages, 2 authors, 9d ago

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

From: netdev-bot+sashiko@kernel.org
Date: 2026-09-11 20:11:15

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.
quoted hunk ↗ jump to hunk
diff --git a/Documentation/netlink/specs/devlink.yaml b/Documentation/netlink/specs/devlink.yaml
index 38b1190f3d269..d933b205ee865 100644
--- a/Documentation/netlink/specs/devlink.yaml
+++ b/Documentation/netlink/specs/devlink.yaml
@@ -174,6 +174,18 @@ definitions:
         value: 1
       -
         name: fw-activate
+  -
+    type: enum
+    name: reload-limit
+    entries:
+      -
+        name: unspec
+        doc: no constraints
+      -
+        name: no-reset
+        doc: >-
+          No reset allowed, no down time allowed, no link flap and no
+          configuration is lost.
   -
     type: enum
     name: param-cmode
@@ -775,7 +787,7 @@ attribute-sets:
       -
         name: reload-limits
         type: bitfield32
-        enum: reload-action
+        enum: reload-limit
         enum-as-flags: true
[ ... ]
quoted hunk ↗ jump to hunk
@@ -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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help