Re: [PATCH bpf-next v5 1/3] bpf: Add BPF_FIB_LOOKUP_VLAN flag to bpf_fib_lookup() helper

["Toke Høiland-Jørgensen" <toke@redhat.com>]

David Ahern <dsahern@kernel.org> writes:

> On 6/29/26 9:08 AM, Toke Høiland-Jørgensen wrote:
>> David Ahern <dsahern@kernel.org> writes:
>> 
>>> On 6/23/26 9:05 PM, Avinash Duduskar wrote:
>>>> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
>>>> index 89b36de5fdbb..e00f0392e728 100644
>>>> --- a/include/uapi/linux/bpf.h
>>>> +++ b/include/uapi/linux/bpf.h
>>>> @@ -3532,6 +3532,29 @@ union bpf_attr {
>>>>   *			Use the mark present in *params*->mark for the fib lookup.
>>>>   *			This option should not be used with BPF_FIB_LOOKUP_DIRECT,
>>>>   *			as it only has meaning for full lookups.
>>>> + *		**BPF_FIB_LOOKUP_VLAN**
>>>
>>> This flag should not be needed. Patches for vlan support were never
>>> submitted (I have them in some old branch). Since the vlan params are
>>> initialized to 0, no new flag should be needed. Besides, these are
>>> output parameters.
>> 
>> There's no enforcement from the kernel side of the parameters being
>> zero, though? So we do need the flag for feature detection; unless we
>> expect applications to do that out of band? But then we'd need a
>> mechanism to do that which could be... the presence of the flag in the
>> ENUM (and thus in BTF)? :)
>> 
>
> This is output direction - return from the fib lookup.

Ah, right, silly me.

> It does not make sense to require a flag to get lookup output. vlan
> proto of 0 is not valid, so it is a clear indication that the vlan
> output parameters were not set during the lookup.

Okay, so we could just unconditionally set the VLAN fields, but if we
start rewriting the ifindex that would be a change of the existing
behaviour that could break existing applications, no?

Specifically, if an XDP application has a table of the interfaces it
forwards between, today they'd get a VLAN interface ifindex, which would
not be in that table, and the application would return XDP_PASS. Whereas
if we change the ifindex and populate the VLAN tag, suddenly the
interface would be in the table, but because the application doesn't
read the returned VLAN tag, it will end up sending packets out without
tagging them, leading to broken forwarding.

So if we don't want the flag, we'd need some other mechanism to resolve
the parent ifindex, AFAICT? Maybe a xdp_get_parent_ifindex() kfunc, say?
That could also be made generic for other stacked interface types, I
suppose.

WDYT?

-Toke