Re: [PATCH bpf-next 00/12] xsk: clean up ring access functions

[Martin Lau <kafai@fb.com>]

On Mon, Dec 09, 2019 at 08:56:17AM +0100, Magnus Karlsson wrote:
> This patch set cleans up the ring access functions of AF_XDP in hope
> that it will now be easier to understand and maintain. I used to get a
> headache every time I looked at this code in order to really understand it,
> but now I do think it is a lot less painful.
> 
> The code has been simplified a lot and as a bonus we get better
> performance. On my 2.0 GHz Broadwell machine with a standard default
> config plus AF_XDP support and CONFIG_PREEMPT on I get the following
> results in percent performance increases with this patch set compared
> to without it:
> 
> Zero-copy (-N):
>           rxdrop        txpush        l2fwd
> 1 core:     4%            5%            4%
> 2 cores:    1%            0%            2%
> 
> Zero-copy with poll() (-N -p):
>           rxdrop        txpush        l2fwd
> 1 core:     1%            3%            3%
> 2 cores:   22%            0%            5%
> 
> Skb mode (-S):
> Shows a 0% to 1% performance improvement over the same benchmarks as
> above.
> 
> Here 1 core means that we are running the driver processing and the
> application on the same core, while 2 cores means that they execute on
> separate cores. The applications are from the xdpsock sample app.
> 
> When a results says 22% better, as in the case of poll mode with 2
> cores and rxdrop, my first reaction is that it must be a
> bug. Everything else shows between 0% and 5% performance
> improvement. What is giving rise to 22%? A quick bisect indicates that
> it is patches 2, 3, 4, 5, and 6 that are giving rise to most of this
> improvement. So not one patch in particular, but something around 4%
> improvement from each one of them. Note that exactly this benchmark
> has previously had an extraordinary slow down compared to when running
> without poll syscalls. For all the other poll tests above, the
> slowdown has always been around 4% for using poll syscalls. But with
> the bad performing test in question, it was above 25%. Interestingly,
> after this clean up, the slow down is 4%, just like all the other poll
> tests. Please take an extra peek at this so I have not messed up
> something.
It overall makes sense to me.

Jonathan, can you also help to review?