Re: [PATCH RFC v4 bpf-next 09/11] tun: Support xdp in the Tx path for xdp_frames

[Alexei Starovoitov <alexei.starovoitov@gmail.com>]

On Mon, Mar 02, 2020 at 09:27:08PM -0700, David Ahern wrote:
> > 
> > I'm worried that XDP_TX is a silent alias to XDP_PASS.
> > What were the reasons to go with this approach?
> 
> As I stated in the cover letter:
> 
> "XDP_TX on Rx means send the packet out the device it arrived
> on; given that, XDP_Tx for the Tx path is treated as equivalent to
> XDP_PASS - ie., continue on the Tx path."

I saw that, but it states the behavior and doesn't answer my "why" question.

> > imo it's less error prone and extensible to warn on XDP_TX.
> > Which will mean that both XDP_TX and XDP_REDICT are not supported for egress atm.
> 
> I personally don't care either way; I was going with the simplest
> concept from a user perspective.

That's not a good sign when uapi is designed as "dont care either way".

> > 
> > Patches 8 and 9 cover tun only. I'd like to see egress hook to be implemented
> > in at least one physical NIC. Pick any hw. Something that handles real frames.
> > Adding this hook to virtual NIC is easy, but it doesn't demonstrate design
> > trade-offs one would need to think through by adding egress hook to physical
> > nic. That's why I think it's mandatory to have it as part of the patch set.
> > 
> > Patch 11 exposes egress to samples/bpf. It's nice, but without selftests it's
> > no go. All new features must be exercised as part of selftests/bpf.
> 
> Patches that exercise the rtnetlink uapi are fairly easy to do on single
> node; anything traffic related requires multiple nodes or namespace
> level capabilities.  Unless I am missing something that is why all
> current XDP tests ride on top of veth; veth changes are not part of this
> set.
> 
> So to be clear you are saying that all new XDP features require patches
> to a h/w nic, veth and whatever the author really cares about before new
> features like this go in?

I didn't say 'veth'. I really meant 'physical nic'.
The patch set implies that XDP_EGRESS is a generic concept and applicable to
physical and virtual netdevs. There is an implementation for tun. But reading
between the lines I don't see that api was thought through on the physical nic.
Hence I'm requesting to see the patches that implement it. When you'll try to
add xdp_egress to a physical nic I suspect there will be challenges that will
force changes to xdp_egress api and I want that to happen before uapi lands in
the tree and becomes frozen.