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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 3 Mar 2020 10:16:01 -0800
From:   Alexei Starovoitov <alexei.starovoitov@...il.com>
To:     David Ahern <dahern@...italocean.com>
Cc:     David Ahern <dsahern@...nel.org>, netdev@...r.kernel.org,
        davem@...emloft.net, kuba@...nel.org,
        prashantbhole.linux@...il.com, jasowang@...hat.com,
        brouer@...hat.com, toke@...hat.com, mst@...hat.com,
        toshiaki.makita1@...il.com, daniel@...earbox.net,
        john.fastabend@...il.com, ast@...nel.org, kafai@...com,
        songliubraving@...com, yhs@...com, andriin@...com,
        dsahern@...il.com
Subject: Re: [PATCH RFC v4 bpf-next 09/11] tun: Support xdp in the Tx path
 for xdp_frames

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.