Subject: Re: [PATCH RFC-v5 bpf-next 00/12] Add support for XDP in egress path

From: Toke Høiland-Jørgensen <[email-masked]>
Date: Thu, 16 Apr 2020 15:59:39 +0200

David Ahern <[email-masked]> writes:

> From: David Ahern <[email-masked]>
>
> This series adds support for XDP in the egress path by introducing
> a new XDP attachment type, BPF_XDP_EGRESS, and adding a UAPI to
> if_link.h for attaching the program to a netdevice and reporting
> the program. bpf programs can be run on all packets in the Tx path -
> skbs or redirected xdp frames. The intent is to emulate the current
> RX path for XDP as much as possible to maintain consistency and
> symmetry in the 2 paths with their APIs.
>
> This is a missing primitive for XDP allowing solutions to build small,
> targeted programs properly distributed in the networking path allowing,
> for example, an egress firewall/ACL/traffic verification or packet
> manipulation and encapping an entire ethernet frame whether it is
> locally generated traffic, forwarded via the slow path (ie., full
> stack processing) or xdp redirected frames.
>
> Nothing about running a program in the Tx path requires driver specific
> resources like the Rx path has. Thus, programs can be run in core
> code and attached to the net_device struct similar to skb mode. The
> existing XDP_FLAGS_*_MODE are not relevant at the moment, so none can
> be set in the attach. XDP_FLAGS_HW_MODE can be used in the future
> (e.g., the work on offloading programs from a VM).
>
> The locations chosen to run the egress program - __netdev_start_xmit
> before the call to ndo_start_xmit and bq_xmit_all before invoking
> ndo_xdp_xmit - allow follow on patch sets to handle tx queueing and
> setting the queue index if multi-queue with consistency in handling
> both packet formats.

I like the choice of hook points. It is interesting that it implies that
there will not be not a separate "XDP generic" hook on egress. And it's
certainly a benefit to not have to change all the drivers. So that's
good :)

I also think it'll be possible to get the information we want (such as
TXQ fill level) at the places you put the hooks. For the skb case
through struct netdev_queue and BQL, and for REDIRECT presumably with
Magnus' queue abstraction once that lands. So overall I think we're
getting there :)

I'll add a few more comments for each patch...

-Toke