Re: [bpf-next PATCH] xdp: accept that XDP headroom isn't always equal XDP_PACKET_HEADROOM

From: John Fastabend <john.fastabend@gmail.com>
To: Jesper Dangaard Brouer <brouer@redhat.com>,
	John Fastabend <john.fastabend@gmail.com>
Cc: "Alexei Starovoitov" <alexei.starovoitov@gmail.com>,
	bpf@vger.kernel.org, gamemann@gflclan.com, lrizzo@google.com,
	netdev@vger.kernel.org,
	"Daniel Borkmann" <borkmann@iogearbox.net>,
	"Alexander Duyck" <alexander.duyck@gmail.com>,
	"Ilias Apalodimas" <ilias.apalodimas@linaro.org>,
	"Toke Høiland-Jørgensen" <toke@toke.dk>,
	brouer@redhat.com
Subject: Re: [bpf-next PATCH] xdp: accept that XDP headroom isn't always equal XDP_PACKET_HEADROOM
Date: Mon, 09 Mar 2020 22:49:55 -0700	[thread overview]
Message-ID: <5e672a83a5c07_6d9d2ad5365425b4a3@john-XPS-13-9370.notmuch> (raw)
In-Reply-To: <20200309093932.2a738ab1@carbon>

Jesper Dangaard Brouer wrote:
> On Fri, 06 Mar 2020 08:06:35 -0800
> John Fastabend <john.fastabend@gmail.com> wrote:
> 
> > Alexei Starovoitov wrote:
> > > On Tue, Mar 03, 2020 at 12:46:58PM +0100, Jesper Dangaard Brouer wrote:  
> [...]
> > > > 
> > > > Still for generic-XDP if headroom is less, still expand headroom to
> > > > XDP_PACKET_HEADROOM as this is the default in most XDP drivers.
> > > > 
> > > > Tested on ixgbe with xdp_rxq_info --skb-mode and --action XDP_DROP:
> > > > - Before: 4,816,430 pps
> > > > - After : 7,749,678 pps
> > > > (Note that ixgbe in native mode XDP_DROP 14,704,539 pps)
> > > >   
> > 
> > But why do we care about generic-XDP performance? Seems users should
> > just use XDP proper on ixgbe and i40e its supported.
> >
> [...]
> > 
> > Or just let ixgbe/i40e be slow? I guess I'm missing some context?
> 
> The context originates from an email thread[1] on XDP-newbies list, that
> had a production setup (anycast routing of gaming traffic[3]) that used
> XDP and they used XDP-generic (actually without realizing it).  They
> were using Intel igb driver (that don't have native-XDP), and changing
> to e.g. ixgbe (or i40e) is challenging given it requires physical access
> to the PoP (Points of Presence) and upgrading to a 10G port at the PoP
> also have costs associated.

OK maybe igb should get xdp...

I get the wanting to run an XDP program across multiple cards even when
they don't have XDP support.

> 
> Why not simply use TC-BPF (cls_bpf) instead of XDP.  I've actually been
> promoting that more people should use TC-BPF, and also in combination[2].
> The reason it makes sense to stick with XDP here is to allow them to
> deploy the same software on their PoP servers, regardless of which
> NIC driver is available.

Sounds reasonable to me.

> 
> Performance wise, I will admit that I've explicitly chosen not to
> optimize XDP-generic, and I've even seen it as a good thing that we
> have this reallocation penalty.  Given the uniform software deployment
> argument and my measurements in[1] I've changed my mind.  For the igb
> driver I'm not motivated to implement XDP-native, because a newer Intel
> CPU can handle wirespeed even-with the reallocations, but it is just
> wasteful to do these reallocations.  "Allowing" these 1Gbit/s NICs to
> work more optimally with XDP-generic, will allow us to ignore
> converting these drivers to XDP-native, and as HW gets upgraded they
> will transition seamlessly to XDP-native.

OK, might be nice to put the details in the commit message? The original
patch seemed to be mostly about 140e and ixgbe but in those case the
user (from above xdp example) would just use XDP. So more about 1gbps
and missing xdpsupport?

> 
> 
> [1] https://www.spinics.net/lists/xdp-newbies/msg01548.html
> [2] https://github.com/xdp-project/xdp-cpumap-tc
> [3] https://gitlab.com/Dreae/compressor/
> -- 
> Best regards,
>   Jesper Dangaard Brouer
>   MSc.CS, Principal Kernel Engineer at Red Hat
>   LinkedIn: http://www.linkedin.com/in/brouer
> 