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

[netdev-bot+sashiko@kernel.org]

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

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

- [Low] 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