Re: [PATCH net-next] netlink: specs: rt-route: move fixed-header to operations list

[netdev-bot+sashiko@kernel.org]

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

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

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] The commit message names the root cause: the doc generator only
  renders keys on each `operations.list` entry and ignores operations-…

--

netlink: specs: rt-route: move fixed-header to operations list

This commit moves the operations-level "fixed-header: rtmsg" key in
rt-route.yaml onto each entry in the operations list. This makes the
fixed header show up in the generated HTML documentation.

Closes: https://patch.msgid.link/8610ee11-e1dc-481a-9654-11188a06786a@jacobi.pw

> diff --git a/Documentation/netlink/specs/rt-route.yaml b/Documentation/netlink/specs/rt-route.yaml
> index 48d1659f20f34..b5786b465eeff 100644
> --- a/Documentation/netlink/specs/rt-route.yaml
> +++ b/Documentation/netlink/specs/rt-route.yaml
> @@ -708,13 +708,13 @@ attribute-sets:
>  
>  operations:
>    enum-model: directional
> -  fixed-header: rtmsg
>    name-prefix: rtm-
>    list:
>      -
>        name: getroute
>        doc: Dump route information.
>        attribute-set: route-attrs
> +      fixed-header: rtmsg

[Severity: High]
Should this be fixed in the doc generator rather than in the spec?

The commit message says:

  "The doc generator only renders fields on each entry in the operations
  list, not operations-level keys."

That limitation is still in place after this patch. In
YnlDocGenerator.parse_yaml() in tools/net/ynl/pyynl/lib/doc_generator.py,
only the list is passed down:

    if "operations" in obj:
        lines.append(self.fmt.rst_subtitle("Operations"))
        lines.append(self.parse_operations(obj["operations"]["list"],
                                           family))

parse_operations() then renders only the keys of each list entry:

    for key in operation.keys():
        ...
        value = operation[key]
        if key in linkable:
            value = self.fmt.rst_ref(namespace, key, value)
        lines.append(self.fmt.rst_fields(key, value, 0))

The schema allows an operations-level fixed-header as a family-wide
default (operations.fixed-header in netlink-raw.yaml). The rest of the YNL
tooling uses it as the per-op fallback:

tools/net/ynl/pyynl/lib/nlspec.py:SpecOperation.__init__() {
    ...
    self.fixed_header = self.yaml.get('fixed-header', family.fixed_header)
    ...
}

These specs still set fixed-header at the operations level, so their
generated HTML still leaves out the fixed header:

  rt-addr.yaml       ifaddrmsg (only getmulticast repeats it per op, so
                     newaddr, deladdr and getaddr are still missing it)
  rt-rule.yaml       fib-rule-hdr
  rt-nexthop.yaml    nhmsg
  ovs_datapath.yaml, ovs_flow.yaml, ovs_packet.yaml, ovs_vport.yaml
                     ovs-header

The ovs_* specs are genetlink-legacy, so they can't use the approach in
this patch. If the family-level key is dropped, op.fixed_header no longer
matches family.fixed_header, and ynl_gen_c.py rejects that for
non-classic families:

tools/net/ynl/pyynl/ynl_gen_c.py:RenderInfo.__init__() {
    ...
        if op and op.fixed_header:
            if op.fixed_header != family.fixed_header:
                if family.is_classic():
                    self.fixed_hdr_len = f"sizeof(struct {c_lower(fixed_hdr)})"
                else:
                    raise Exception("Per-op fixed header not supported, yet")
    ...
}

_multi_parse() has the same check.

Could parse_yaml() or parse_operations() render the operations-level
fixed-header as a family default, or apply it to each op that has no
fixed-header of its own? That would cover all of the specs above and
make this rt-route.yaml change unnecessary.

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261112-ynl_fixed_header-v1-1-8475ea19e409%40kylinos.cn