netdev - Re: [PATCH v4 bpf-next 09/15] net: Support xdp in the Tx path for packets as an skb

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 for Android: free password hash cracker in your pocket

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

Date:   Wed, 29 Apr 2020 18:17:24 -0600
From:   David Ahern <dsahern@...il.com>
To:     Daniel Borkmann <daniel@...earbox.net>,
        David Ahern <dsahern@...nel.org>, netdev@...r.kernel.org
Cc:     davem@...emloft.net, kuba@...nel.org,
        prashantbhole.linux@...il.com, jasowang@...hat.com,
        brouer@...hat.com, toke@...hat.com, toshiaki.makita1@...il.com,
        john.fastabend@...il.com, ast@...nel.org, kafai@...com,
        songliubraving@...com, yhs@...com, andriin@...com,
        David Ahern <dahern@...italocean.com>
Subject: Re: [PATCH v4 bpf-next 09/15] net: Support xdp in the Tx path for
 packets as an skb

On 4/28/20 9:05 AM, Daniel Borkmann wrote:
> I didn't see anything in the patch series on this (unless I missed it), but
> don't we need to force turning off GSO/TSO for the device where XDP
> egress is
> attached? Otherwise how is this safe? E.g. generic XDP uses
> netif_elide_gro()
> to bypass GRO once enabled. In this case on egress, if helpers like
> bpf_xdp_adjust_head()
> or bpf_xdp_adjust_tail() adapt GSO skbs then drivers will operate on
> wrong GSO
> info. Not sure if this goes into undefined behavior here?

yep, I need to disable gso / tso.

> 
> Overall, for the regular stack, I expect the performance of XDP egress
> to be
> worse than e.g. tc egress, for example, when TSO is disabled but not GSO
> then
> you parse the same packet multiple times given post-GSO whereas with tc
> egress
> it would operate just fine on a GSO skb. Plus all the limitations
> generic XDP
> has with skb_cloned(skb), skb_is_nonlinear(skb), etc, where we need to
> linearize
> so calling it 'XDP egress' might lead to false assumptions. Did you do a
> comparison
> on that as well?

Are suggesting that skb path and xdp_frame paths have different attach
options to make it clearer that attaching to the skb path requires
offloads to be disabled? Or do you think the name 'XDP egress' is wrong?

This option is a building block to let people choose how they want to
deploy a solution. If the overwhelming majority of traffic takes the XDP
redirect path and only a small set (e.g., broadcast, multicast, first
packet of a flow) takes the slow path, the overall solution is solid and
better performing than sending all of the traffic up the stack.

> 
> Also, I presume the XDP egress is intentionally not called when programs
> return
> XDP_TX but only XDP_REDIRECT? Why such design decision?

XDP_REDIRECT sends frame through the networking stack to go from device
1 to device 2 which can be a completely different context (e.g.,
redirect from host NIC to VM tap device).

XDP_TX is within a driver. The driver just ran a program on the packet
and nothing about the context has changed. That said, I did label the
egress attach as "core" to mean it is run in core networking code.
Moving forward if a feature comes up that warrants a change (e.g., Tx
queue selection) there are options - from drivers adding support to run
the attached egress program or using the driver attach mode to
explicitly state that it is run in the driver and covers XDP_TX.