On 2021-02-01 10:31, Jesper Dangaard Brouer wrote:
On Mon, 1 Feb 2021 07:27:57 +0100
Björn Töpel <bjorn.topel@xxxxxxxxx> wrote:
On 2021-01-29 17:45, Toke Høiland-Jørgensen wrote:
Björn Töpel <bjorn.topel@xxxxxxxxx> writes:
From: Björn Töpel <bjorn.topel@xxxxxxxxx>
Currently the bpf_redirect_map() implementation dispatches to the
correct map-lookup function via a switch-statement. To avoid the
dispatching, this change adds one bpf_redirect_map() implementation per
map. Correct function is automatically selected by the BPF verifier.
Signed-off-by: Björn Töpel <bjorn.topel@xxxxxxxxx>
---
Hi XDP-folks!
This is another take on my bpf_redirect_xsk() patch [1]. I figured I
send it as an RFC for some early input. My plan is to include it as
part of the xdp_do_redirect() optimization of [1].
Assuming the maintainers are OK with the special-casing in the verifier,
this looks like a neat way to avoid the runtime overhead to me. The
macro hackery is not the prettiest; I wonder if the same effect could be
achieved by using inline functions? If not, at least a comment
explaining the reasoning (and that the verifier will substitute the
right function) might be nice? Mostly in relation to this bit:
Yeah, I agree with the macro part. I'll replace it with a
__always_inline function, instead.
Yes, I also prefer __always_inline over the macro.
Ok! Good!
static const struct bpf_func_proto bpf_xdp_redirect_map_proto = {
- .func = bpf_xdp_redirect_map,
+ .func = bpf_xdp_redirect_devmap,
I'll try to clean this up as well.
I do like the optimization of having the verifier call the right map
func directly. Could you please add a descriptive comment that
describe this above "bpf_xdp_redirect_map_proto", that this is
happening in fixup_bpf_calls and use get_xdp_redirect_func (what you
define). It is a cool trick, but people reading the code will have a
hard time following.
Good idea, and makes sense! I'll make sure to do that!
Thanks for the input!
Cheers,
Björn
Surprisingly people do read this code and tries to follow. I've had
discussions on the Cilium Slack channel, where people misunderstood how
our bpf_fib_lookup() calls gets mapped to two different functions
depending on context (SKB vs XDP). And that remapping happens in the
same file (net/core/filter.c).
