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

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

Avinash Duduskar <avinash.duduskar@gmail.com> writes:

> The bpf ci bot flagged the VLAN_FAILURE re-issue advice in the uapi doc
> here, and the finding is real. By the time the lookup fails,
> params->tbid is gone (the h_vlan fields it shares storage with are
> zeroed on entry to bpf_fib_set_fwd_params()) and params->mark is gone
> on the resolved-neighbour path (overwritten by the smac output). A
> program that follows the advice with the same struct and DIRECT|TBID or
> MARK set runs the second lookup with a zero tbid or a garbage mark. The
> selftests did not catch it because every arm re-initializes params,
> which is the safe pattern.
>
> Toke, the re-issue recovery came from
> https://lore.kernel.org/all/87jyrwf9g1.fsf@toke.dk/, so before I
> respin: my preference is to keep the mechanics and fix the sentence,
> "repeat the lookup without the flag, re-initializing *params* first;
> output fields overwrite the inputs they share storage with".
> Overwriting inputs on the way out is the helper's existing behaviour on
> every path (rt_metric lands on top of tos/flowinfo even on NO_NEIGH),
> so one rule, re-initialize before any reuse, seems better than making
> VLAN_FAILURE the only return code that preserves inputs.

Sure, SGTM :)

-Toke