Re: [PATCH bpf-next v3 6/8] bpf: Add XDP_REDIRECT support to XDP for bpf_prog_run()

On Mon, Dec 13, 2021 at 8:26 AM Toke Høiland-Jørgensen [off-list ref] wrote:

Alexei Starovoitov [off-list ref] writes:

quoted

On Sat, Dec 11, 2021 at 10:43 AM Toke Høiland-Jørgensen [off-list ref] wrote:

quoted

+
+static void bpf_test_run_xdp_teardown(struct bpf_test_timer *t)
+{
+       struct xdp_mem_info mem = {
+               .id = t->xdp.pp->xdp_mem_id,
+               .type = MEM_TYPE_PAGE_POOL,
+       };

pls add a new line.

quoted

+       xdp_unreg_mem_model(&mem);
+}
+
+static bool ctx_was_changed(struct xdp_page_head *head)
+{
+       return (head->orig_ctx.data != head->ctx.data ||
+               head->orig_ctx.data_meta != head->ctx.data_meta ||
+               head->orig_ctx.data_end != head->ctx.data_end);

redundant ()

quoted

        bpf_test_timer_enter(&t);
        old_ctx = bpf_set_run_ctx(&run_ctx.run_ctx);
        do {
                run_ctx.prog_item = &item;
-               if (xdp)
+               if (xdp && xdp_redirect) {
+                       ret = bpf_test_run_xdp_redirect(&t, prog, ctx);
+                       if (unlikely(ret < 0))
+                               break;
+                       *retval = ret;
+               } else if (xdp) {
                        *retval = bpf_prog_run_xdp(prog, ctx);

Can we do this unconditionally without introducing a new uapi flag?
I mean "return bpf_redirect()" was a nop under test_run.
What kind of tests might break if it stops being a nop?

Well, I view the existing mode of bpf_prog_test_run() with XDP as a way
to write XDP unit tests: it allows you to submit a packet, run your XDP
program on it, and check that it returned the right value and did the
right modifications. This means if you XDP program does 'return
bpf_redirect()', userspace will still get the XDP_REDIRECT value and so
it can check correctness of your XDP program.

With this flag the behaviour changes quite drastically, in that it will
actually put packets on the wire instead of getting back the program
return. So I think it makes more sense to make it a separate opt-in
mode; the old behaviour can still be useful for checking XDP program
behaviour.

Ok that all makes sense.
How about using prog_run to feed the data into proper netdev?
XDP prog may or may not attach to it (this detail is tbd) and
prog_run would use prog_fd and ifindex to trigger RX (yes, receive)
in that netdev. XDP prog will execute and will be able to perform
all actions (not only XDP_REDIRECT).
XDP_PASS would pass the packet to the stack, etc.