On Thu, Jan 6, 2022 at 10:21 AM Toke Høiland-Jørgensen <toke@xxxxxxxxxx> wrote:
>
> Alexei Starovoitov <alexei.starovoitov@xxxxxxxxx> writes:
>
> > On Thu, Jan 6, 2022 at 6:34 AM Toke Høiland-Jørgensen <toke@xxxxxxxxxx> wrote:
> >>
> >> Alexei Starovoitov <alexei.starovoitov@xxxxxxxxx> writes:
> >>
> >> > On Mon, Jan 03, 2022 at 04:08:12PM +0100, Toke Høiland-Jørgensen wrote:
> >> >> +
> >> >> +#define NUM_PKTS 3
> >> >
> >> > May be send a bit more than 3 packets?
> >> > Just to test skb_list logic for XDP_PASS.
> >>
> >> OK, can do.
> >>
> >> >> +
> >> >> + /* We setup a veth pair that we can not only XDP_REDIRECT packets
> >> >> + * between, but also route them. The test packet (defined above) has
> >> >> + * address information so it will be routed back out the same interface
> >> >> + * after it has been received, which will allow it to be picked up by
> >> >> + * the XDP program on the destination interface.
> >> >> + *
> >> >> + * The XDP program we run with bpf_prog_run() will cycle through all
> >> >> + * four return codes (DROP/PASS/TX/REDIRECT), so we should end up with
> >> >> + * NUM_PKTS - 1 packets seen on the dst iface. We match the packets on
> >> >> + * the UDP payload.
> >> >> + */
> >> >> + SYS("ip link add veth_src type veth peer name veth_dst");
> >> >> + SYS("ip link set dev veth_src address 00:11:22:33:44:55");
> >> >> + SYS("ip link set dev veth_dst address 66:77:88:99:aa:bb");
> >> >> + SYS("ip link set dev veth_src up");
> >> >> + SYS("ip link set dev veth_dst up");
> >> >> + SYS("ip addr add dev veth_src fc00::1/64");
> >> >> + SYS("ip addr add dev veth_dst fc00::2/64");
> >> >> + SYS("ip neigh add fc00::2 dev veth_src lladdr 66:77:88:99:aa:bb");
> >> >> + SYS("sysctl -w net.ipv6.conf.all.forwarding=1");
> >> >
> >> > These commands pollute current netns. The test has to create its own netns
> >> > like other tests do.
> >>
> >> Right, will fix.
> >>
> >> > The forwarding=1 is odd. Nothing in the comments or commit logs
> >> > talks about it.
> >>
> >> Hmm, yeah, should probably have added an explanation, sorry about that :)
> >>
> >> > I'm guessing it's due to patch 6 limitation of picking loopback
> >> > for XDP_PASS and XDP_TX, right?
> >> > There is ingress_ifindex field in struct xdp_md.
> >> > May be use that to setup dev and rxq in test_run in patch 6?
> >> > Then there will be no need to hack through forwarding=1 ?
> >>
> >> No, as you note there's already ingress_ifindex to set the device, and
> >> the test does use that:
> >>
> >> + memcpy(skel->rodata->expect_dst, &pkt_udp.eth.h_dest, ETH_ALEN);
> >> + skel->rodata->ifindex_out = ifindex_src;
> >> + ctx_in.ingress_ifindex = ifindex_src;
> >
> > My point is that this ingress_ifindex should be used instead of loopback.
> > Otherwise the test_run infra is lying to the xdp program.
>
> But it is already using that! There is just no explicit code in patch 6
> to do that because that was already part of the XDP prog_run
> functionality.
>
> Specifically, the existing bpf_prog_test_run_xdp() will pass the context
> through xdp_convert_md_to_buff() which will resolve the ifindex and get
> a dev reference. So the xdp_buff object being passed to the new
> bpf_test_run_xdp_live() function already has the right device in
> ctx->rxq.
Got it. Please make it clear in the commit log.
> No the problem of XDP_PASS going in the opposite direction of XDP_TX and
> XDP_REDIRECT remains. This is just like on a physical interface: if you
> XDP_TX a packet it goes back out, if you XDP_PASS it, it goes up the
> stack. To intercept both after the fact, you need to look in two
> different places.
>
> Anyhow, just using a TC hook for XDP_PASS works fine and gets rid of the
> forwarding hack; I'll send a v6 with that just as soon as I verify that
> I didn't break anything when running the traffic generator on bare metal :)
Got it. You mean a tc ingress prog attached to veth_src ? That should work.
