Re: [PATCH net-next 5/6] netlink: specs: rt-route: describe lwt BPF program options

[Hangbin Liu]

On Mon, Sep 21, 2026 at 06:38:17AM +0000, netdev-bot+sashiko@xxxxxxxxxx wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential
> issue(s) to consider.
> 
> Critical: 0 · High: 0 · Medium: 1 · Low: 1
> 
> 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).
> 
> - [Medium] Documentation/netlink/specs/rt-route.yaml re-points the
>   seg6-local `bpf` attribute (SEG6_LOCAL_BPF) at the newly added `lwt-…

Thanks, I will update the SEG6_LOCAL_BPF attrs in the seg6 update (patch 06).
in v3.

Hangbin

> - [Low] The new `lwt-bpf-prog` `name` attribute in
>   Documentation/netlink/specs/rt-route.yaml (lines 624-626) is declared…
> 
> --
> 
> netlink: specs: rt-route: describe lwt BPF program options
> 
> This changes the lwt-bpf in/out/xmit attributes and the seg6-local bpf
> attribute in Documentation/netlink/specs/rt-route.yaml from binary to a
> nest, and adds a new shared lwt-bpf-prog attribute set with fd and name
> members.
> 
> > diff --git a/Documentation/netlink/specs/rt-route.yaml b/Documentation/netlink/specs/rt-route.yaml
> > index 6030d7cb8d0d1..5e9537f7dfad4 100644
> > --- a/Documentation/netlink/specs/rt-route.yaml
> > +++ b/Documentation/netlink/specs/rt-route.yaml
> 
> [ ... ]
> 
> > @@ -503,7 +506,8 @@ attribute-sets:
> >          type: u32
> >        -
> >          name: bpf
> > -        type: binary
> > +        type: nest
> > +        nested-attributes: lwt-bpf-prog
> 
> [Severity: Medium]
> Should the seg6-local bpf attribute point at the lwt-bpf-prog set?  This
> attribute lives in the seg6-local set, which carries
> header: linux/seg6_local.h, but lwt-bpf-prog below uses
> name-prefix: lwt-bpf-prog-, so its members document and generate as
> LWT_BPF_PROG_FD / LWT_BPF_PROG_NAME.
> 
> Those identifiers come from linux/lwtunnel.h:
> 
>     enum {
>             LWT_BPF_PROG_UNSPEC,
>             LWT_BPF_PROG_FD,
>             LWT_BPF_PROG_NAME,
>             __LWT_BPF_PROG_MAX,
>     };
> 
> while the seg6-local nest is a separate uAPI namespace in
> linux/seg6_local.h:
> 
>     enum {
>             SEG6_LOCAL_BPF_PROG_UNSPEC,
>             SEG6_LOCAL_BPF_PROG,
>             SEG6_LOCAL_BPF_PROG_NAME,
>             __SEG6_LOCAL_BPF_PROG_MAX,
>     };
> 
> The values (1, 2) and payload types happen to coincide, so the wire format
> still decodes, but does this end up documenting SRv6 attributes with
> lwtunnel.h names?  There also seems to be no header: value that can be
> correct for a set shared between linux/lwtunnel.h and
> linux/seg6_local.h.
> 
> Would a separate seg6-local-bpf-prog set with
> header: linux/seg6_local.h be more accurate here?
> 
> Separately, the commit message says only:
> 
>     Replace binary BPF attributes with a nested lwt-bpf-prog to support
>     lwt bpf prog options.
> 
> Could it also mention that the seg6-local bpf attribute is being re-typed,
> since that is a different attribute space?
> 
> > @@ -610,6 +614,16 @@ attribute-sets:
> >        -
> >          name: hwid
> >          type: u8
> > +  -
> > +    name: lwt-bpf-prog
> > +    name-prefix: lwt-bpf-prog-
> > +    attributes:
> > +      -
> > +        name: fd
> > +        type: u32
> 
> [Severity: Medium]
> Is fd the right name for attribute 1 in both directions?  In requests
> parse_nla_bpf() in net/ipv6/seg6_local.c does treat it as a descriptor:
> 
>     fd = nla_get_u32(tb[SEG6_LOCAL_BPF_PROG]);
>     p = bpf_prog_get_type(fd, BPF_PROG_TYPE_LWT_SEG6LOCAL);
> 
> but on GET/dump put_nla_bpf() emits a program id, not a descriptor:
> 
>     if (nla_put_u32(skb, SEG6_LOCAL_BPF_PROG, slwt->bpf.prog->aux->id))
>             return -EMSGSIZE;
> 
> So a spec-driven decoder would print slwt->bpf.prog->aux->id under a field
> called fd, and feeding that value back into a request would resolve an
> unrelated descriptor or fail.  On the lwt-bpf side bpf_fill_lwt_prog() in
> net/core/lwt_bpf.c never emits attribute 1 at all and only puts
> LWT_BPF_PROG_NAME.
> 
> Would it be worth naming this member after the program reference/id, or at
> least documenting the id-versus-fd asymmetry?
> 
> Note this appears unchanged at the end of the series (30e3ec46f708 still
> has name: bpf with nested-attributes: lwt-bpf-prog and the fd member).
> 
> > +      -
> > +        name: name
> > +        type: string
> 
> [Severity: Low]
> Should this string carry a max-len?  Both kernel consumers of this nest
> bound it to MAX_PROG_NAME (256).
> 
> net/core/lwt_bpf.c:
> 
>     static const struct nla_policy bpf_prog_policy[LWT_BPF_PROG_MAX + 1] = {
>             [LWT_BPF_PROG_FD]   = { .type = NLA_U32, },
>             [LWT_BPF_PROG_NAME] = { .type = NLA_NUL_STRING,
>                                     .len = MAX_PROG_NAME },
>     };
> 
> net/ipv6/seg6_local.c:
> 
>     static const struct nla_policy bpf_prog_policy[SEG6_LOCAL_BPF_PROG_MAX + 1] = {
>             [SEG6_LOCAL_BPF_PROG]      = { .type = NLA_U32, },
>             [SEG6_LOCAL_BPF_PROG_NAME] = { .type = NLA_NUL_STRING,
>                                            .len = MAX_PROG_NAME },
>     };
> 
> As written the spec advertises an unbounded string, so tooling generated
> from it will build a request that the kernel rejects with -EINVAL.
> MAX_PROG_NAME is a private define in those two .c files, so the numeric
> bound would need to be spelled out in the spec, and max-len does not count
> the NUL terminator.
> 
> This is also still missing at the end of the series (30e3ec46f708).
> 
> >  
> >  operations:
> >    enum-model: directional
> 
> -- 
> Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260917-ynl_rt_encap-v1-0-fbbe6e680571%40kylinos.cn