Sebastian Andrzej Siewior <bigeasy@xxxxxxxxxxxxx> writes:
>> > +static inline struct bpf_redirect_info *bpf_net_ctx_get_ri(void)
>> > +{
>> > + struct bpf_net_context *bpf_net_ctx = bpf_net_ctx_get();
>> > +
>> > + if (!bpf_net_ctx)
>> > + return NULL;
>>
>> ... do we really need all the NULL checks?
>>
>> (not just here, but in the code below as well).
>>
>> I'm a little concerned that we are introducing a bunch of new branches
>> in the XDP hot path. Which is also why I'm asking for benchmarks :)
>
> We could hide the WARN behind CONFIG_DEBUG_NET. The only purpose is to
> see the backtrace where the context is missing. Having just an error
> somewhere will make it difficult to track.
>
> The NULL check is to avoid a crash if the context is missing. You could
> argue that this should be noticed in development and never hit
> production. If so, then we get the backtrace from NULL-pointer
> dereference and don't need the checks and WARN.
Yup, this (relying on the NULL deref) SGTM :)
-Toke
