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

[David Ahern <dahern@digitalocean.com>]

On 3/2/20 11:30 AM, Alexei Starovoitov wrote:
> On Wed, Feb 26, 2020 at 08:20:11PM -0700, David Ahern wrote:
>> +
>> +		act = bpf_prog_run_xdp(xdp_prog, &xdp);
>> +		switch (act) {
>> +		case XDP_TX:    /* for Tx path, XDP_TX == XDP_PASS */
>> +			act = XDP_PASS;
>> +			break;
>> +		case XDP_PASS:
>> +			break;
>> +		case XDP_REDIRECT:
>> +			/* fall through */
>> +		default:
>> +			bpf_warn_invalid_xdp_action(act);
>> +			/* fall through */
>> +		case XDP_ABORTED:
>> +			trace_xdp_exception(tun->dev, xdp_prog, act);
>> +			/* fall through */
>> +		case XDP_DROP:
>> +			break;
>> +		}
> 
> patch 8 has very similar switch. Can you share the code?

Most likely; I'll take a look.

> 
> 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."

> 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.

> 
> 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?