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

[Avinash Duduskar <avinash.duduskar@gmail.com>]

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. If you would
rather VLAN_FAILURE restore tbid and mark the way it already restores
ifindex, or lose the advice sentence entirely, I will do that instead.

Avi