On Thu, 7 May 2020 at 15:44, Toke Høiland-Jørgensen <toke@xxxxxxxxxx> wrote: > > Björn Töpel <bjorn.topel@xxxxxxxxx> writes: > > > Before I start hacking on this, I might as well check with the XDP > > folks if this considered a crappy idea or not. :-) > > > > The XDP redirect flow for a packet is typical a dance of > > bpf_redirect_map() that updates the bpf_redirect_info structure with > > maps type/items, which is then followed by an xdp_do_redirect(). That > > function takes an action based on the bpf_redirect_info content. > > > > I'd like to get rid of the xdp_do_redirect() call, and the > > bpf_redirect_info (per-cpu) lookup. The idea is to introduce a new > > (oh-no!) XDP action, say, XDP_CONSUMED and a built-in helper with > > tail-call semantics. > > > > Something across the lines of: > > > > --8<-- > > > > struct { > > __uint(type, BPF_MAP_TYPE_XSKMAP); > > __uint(max_entries, MAX_SOCKS); > > __uint(key_size, sizeof(int)); > > __uint(value_size, sizeof(int)); > > } xsks_map SEC(".maps"); > > > > SEC("xdp1") > > int xdp_prog1(struct xdp_md *ctx) > > { > > bpf_tail_call_redirect(ctx, &xsks_map, 0); > > // Redirect the packet to an AF_XDP socket at entry 0 of the > > // map. > > // > > // After a successful call, ctx is said to be > > // consumed. XDP_CONSUMED will be returned by the program. > > // Note that if the call is not successful, the buffer is > > // still valid. > > // > > // XDP_CONSUMED in the driver means that the driver should not > > // issue an xdp_do_direct() call, but only xdp_flush(). > > // > > // The verifier need to be taught that XDP_CONSUMED can only > > // be returned "indirectly", meaning a bpf_tail_call_XXX() > > // call. An explicit "return XDP_CONSUMED" should be > > // rejected. Can that be implemented? > > return XDP_PASS; // or any other valid action. > > } > > > > -->8-- > > > > The bpf_tail_call_redirect() would work with all redirectable maps. > > > > Thoughts? Tomatoes? Pitchforks? > > The above answers the 'what'. Might be easier to evaluate if you also > included the 'why'? :) > Ah! Sorry! Performance, performance, performance. Getting rid of a bunch of calls/instructions per packet, which helps my (AF_XDP) case. This would be faster than the regular REDIRECT path. Today, in bpf_redirect_map(), instead of actually performing the action, we populate the bpf_redirect_info structure, just to look up the action again in xdp_do_redirect(). I'm pretty certain this would be a gain for AF_XDP (quite easy to do a quick hack, and measure). It would also shave off the same amount of instructions for "vanilla" XDP_REDIRECT cases. The bigger issue; Is this new semantic something people would be comfortable being added to XDP. Cheers, Björn