Re: [RFC net-next 11/14] tun: run XDP program in tx path

["Toke Høiland-Jørgensen" <toke@redhat.com>]

Jesper Dangaard Brouer <jbrouer@redhat.com> writes:

> On Wed, 18 Dec 2019 17:10:47 +0900
> Prashant Bhole <prashantbhole.linux@gmail.com> wrote:
>
>> +static u32 tun_do_xdp_tx(struct tun_struct *tun, struct tun_file *tfile,
>> +			 struct xdp_frame *frame)
>> +{
>> +	struct bpf_prog *xdp_prog;
>> +	struct tun_page tpage;
>> +	struct xdp_buff xdp;
>> +	u32 act = XDP_PASS;
>> +	int flush = 0;
>> +
>> +	xdp_prog = rcu_dereference(tun->xdp_tx_prog);
>> +	if (xdp_prog) {
>> +		xdp.data_hard_start = frame->data - frame->headroom;
>> +		xdp.data = frame->data;
>> +		xdp.data_end = xdp.data + frame->len;
>> +		xdp.data_meta = xdp.data - frame->metasize;
>
> You have not configured xdp.rxq, thus a BPF-prog accessing this will crash.
>
> For an XDP TX hook, I want us to provide/give BPF-prog access to some
> more information about e.g. the current tx-queue length, or TC-q number.
>
> Question to Daniel or Alexei, can we do this and still keep BPF_PROG_TYPE_XDP?
> Or is it better to introduce a new BPF prog type (enum bpf_prog_type)
> for XDP TX-hook ?

I think a new program type would make the most sense. If/when we
introduce an XDP TX hook[0], it should have different semantics than the
regular XDP hook. I view the XDP TX hook as a hook that executes as the
very last thing before packets leave the interface. It should have
access to different context data as you say, but also I don't think it
makes sense to have XDP_TX and XDP_REDIRECT in an XDP_TX hook. And we
may also want to have a "throttle" return code; or maybe that could be
done via a helper?

In any case, I don't think this "emulated RX hook on the other end of a
virtual device" model that this series introduces is the right semantics
for an XDP TX hook. I can see what you're trying to do, and for virtual
point-to-point links I think it may make sense to emulate the RX hook of
the "other end" on TX. However, form a UAPI perspective, I don't think
we should be calling this a TX hook; logically, it's still an RX hook
on the receive end.

If you guys are up for evolving this design into a "proper" TX hook (as
outlined above an in [0]), that would be awesome, of course. But not
sure what constraints you have on your original problem? Do you
specifically need the "emulated RX hook for unmodified XDP programs"
semantics, or could your problem be solved with a TX hook with different
semantics?

-Toke


[0] We've suggested this in the past, see
https://github.com/xdp-project/xdp-project/blob/master/xdp-project.org#xdp-hook-at-tx