BPF List
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Hangbin Liu" <hangbin.liu@linux.dev>
Cc: bpf@vger.kernel.org
Subject: Re: [PATCH net-next v3 6/6] netlink: specs: rt-route: describe seg6-local attrs
Date: Thu, 01 Oct 2026 01:51:33 +0000	[thread overview]
Message-ID: <20261001015133.E361A1F000FF@smtp.kernel.org> (raw)
In-Reply-To: <20260930-ynl_rt_encap-v3-6-4106c21b9ee7@kylinos.cn>

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] netlink: incorrect type: flags causes uAPI mismatch for seg6-local-flv-ops
- [Low] netlink: yaml naming convention violation in seg6-local-flv-ops name-prefix
- [Low] netlink: missing doc properties on new major yaml spec additions
--

commit 1d8d137d71e90b2ea51b5a3ae23d03831ee06b0b
Author: Hangbin Liu <liuhangbin@kylinos.cn>

netlink: specs: rt-route: describe seg6-local attrs

This commit adds SEG6 local actions enums and seg6-local-flv-ops flags to the
rt-route Netlink specification. It also replaces binary attributes with nested
seg6-local-bpf, seg6-local-cnt, and seg6-local-flv definitions.

> diff --git a/Documentation/netlink/specs/rt-route.yaml b/Documentation/netlink/specs/rt-route.yaml
> index afec0375661f5..0260fca398d2c 100644
> --- a/Documentation/netlink/specs/rt-route.yaml
> +++ b/Documentation/netlink/specs/rt-route.yaml
> @@ -116,6 +116,45 @@ definitions:
>        - rpl
>        - ioam6
>        - xfrm
> +  -
> +    name: seg6-local-actions
> +    type: enum

[Severity: Low]
Does this new major property need a doc field? The Netlink subsystem
guidelines recommend that each major property should carry a doc entry to
explain its purpose.

[ ... ]

> +  -
> +    name: seg6-local-flv-ops
> +    type: flags
> +    name-prefix: seg6_local_flv_op-

[Severity: High]
Can using type: flags here cause a uAPI mismatch with the kernel?

The kernel uAPI defines these operations as sequential values (for example,
SEG6_LOCAL_FLV_OP_USD is 3). When type: flags is used, the YNL code
generator will emit bitmasks (1, 2, 4, 8), resulting in
SEG6_LOCAL_FLV_OP_USD being incorrectly defined as 8 in the generated
headers.

Would it be safer to use type: enum to ensure the generated values match
the existing kernel constants?

[Severity: Low]
Also, could the name-prefix use dashes instead of underscores? The naming
convention for YAML specs specifies that names should contain dashes, as
the code generator handles converting them to underscores for the C code.

[Severity: Low]
Additionally, should seg6-local-flv-ops include a doc property?

[ ... ]

> @@ -634,6 +677,50 @@ attribute-sets:
>        -
>          name: name
>          type: string
> +  -
> +    name: seg6-local-bpf
> +    name-prefix: seg6-local-bpf-

[Severity: Low]
Would it be helpful to include a doc property for this attribute set? The
guidelines mention that new major properties, such as seg6-local-bpf,
seg6-local-cnt, and seg6-local-flv, should carry documentation.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260930-ynl_rt_encap-v3-0-4106c21b9ee7@kylinos.cn?part=6

  reply	other threads:[~2026-10-01  1:51 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-30  1:50 [PATCH net-next v3 0/6] netlink: add lwtunnel encap sub-message support to rt-route Hangbin Liu
2026-09-30  1:50 ` [PATCH net-next v3 1/6] net: lwtunnel: change encap fill order Hangbin Liu
2026-10-01 14:09   ` Ido Schimmel
2026-09-30  1:50 ` [PATCH net-next v3 2/6] tools: ynl: convert enum selector to string for sub-message parsing Hangbin Liu
2026-10-01  1:51   ` sashiko-bot
2026-10-03  6:23     ` Hangbin Liu
2026-10-05 22:33       ` Jakub Kicinski
2026-09-30  1:50 ` [PATCH net-next v3 3/6] netlink: specs: rt-route: add lwtunnel encap sub-message support Hangbin Liu
2026-09-30  1:50 ` [PATCH net-next v3 4/6] netlink: specs: rt-route: describe lwtunnel IP options Hangbin Liu
2026-09-30  1:50 ` [PATCH net-next v3 5/6] netlink: specs: rt-route: describe lwt BPF program options Hangbin Liu
2026-10-01  1:51   ` sashiko-bot
2026-10-03  6:09     ` Hangbin Liu
2026-09-30  1:50 ` [PATCH net-next v3 6/6] netlink: specs: rt-route: describe seg6-local attrs Hangbin Liu
2026-10-01  1:51   ` sashiko-bot [this message]
2026-10-03  6:17     ` Hangbin Liu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20261001015133.E361A1F000FF@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=hangbin.liu@linux.dev \
    --cc=sashiko-reviews@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox