Message-ID: <20180711234857.axj62u3ux7nsmbj5@ast-mbp.dhcp.thefacebook.com>
Date:   Wed, 11 Jul 2018 16:48:59 -0700
From:   Alexei Starovoitov <alexei.starovoitov@...il.com>
To:     Magnus Karlsson <magnus.karlsson@...el.com>
Cc:     bjorn.topel@...el.com, ast@...com, daniel@...earbox.net,
        netdev@...r.kernel.org, eric.dumazet@...il.com,
        qi.z.zhang@...el.com, pavel@...tnetmon.com
Subject: Re: [PATCH bpf 0/4] Consistent sendmsg error reporting in AF_XDP

On Wed, Jul 11, 2018 at 10:12:48AM +0200, Magnus Karlsson wrote:
> This patch set adjusts the AF_XDP TX error reporting so that it becomes
> consistent between copy mode and zero-copy. First some background:
> 
> Copy-mode for TX uses the SKB path in which the action of sending the
> packet is performed from process context using the sendmsg
> syscall. Completions are usually done asynchronously from NAPI mode by
> using a TX interrupt. In this mode, send errors can be returned back
> through the syscall.
> 
> In zero-copy mode both the sending of the packet and the completions
> are done asynchronously from NAPI mode for performance reasons. In
> this mode, the sendmsg syscall only makes sure that the TX NAPI loop
> will be run that performs both the actions of sending and
> completing. In this mode it is therefore not possible to return errors
> through the sendmsg syscall as the sending is done from the NAPI
> loop. Note that it is possible to implement a synchronous send with
> our API, but in our benchmarks that made the TX performance drop by
> nearly half due to synchronization requirements and cache line
> bouncing. But for some netdevs this might be preferable so let us
> leave it up to the implementation to decide.
> 
> The problem is that the current code base returns some errors in
> copy-mode that are not possible to return in zero-copy mode. This
> patch set aligns them so that the two modes always return the same
> error code. We achieve this by removing some of the errors returned by
> sendmsg in copy-mode (and in one case adding an error message for
> zero-copy mode) and offering alternative error detection methods that
> are consistent between the two modes.
> 
> The structure of the patch set is as follows:
> 
> Patch 1: removes the ENXIO return code from copy-mode when someone has
> forcefully changed the number of queues on the device so that the
> queue bound to the socket is no longer available. Just silently stop
> sending anything as in zero-copy mode.
> 
> Patch 2: stop returning EAGAIN in copy mode when the completion queue
> is full as zero-copy does not do this. Instead this situation can be
> detected by comparing the head and tail pointers of the completion
> queue in both modes. In any case, EAGAIN was not the correct error code
> here since no amount of calling sendmsg will solve the problem. Only
> consuming one or more messages on the completion queue will fix this.
> 
> Patch 3: Always return ENOBUFS from sendmsg if there is no TX queue
> configured. This was not the case for zero-copy mode.
> 
> Patch 4: stop returning EMSGSIZE when the size of the packet is larger
> than the MTU. Just send it to the device so that it will drop it as in
> zero-copy mode.
> 
> Note that copy-mode can still return EAGAIN in certain circumstances,
> but as these conditions cannot occur in zero-copy mode it is fine for
> copy-mode to return them.
> 
> Question: For patch 4, is it fine to let the device drop a packet
> that is greater than its MTU, or should I have a check for this in
> both zero-copy and copy-mode and drop the packet up in the AF_XDP
> code? The drawback of this is that it will have performance
> implications for zero-copy mode as we will touch one more cache line
> with dev->mtu.
> 
> Thanks: Magnus

for the set:
Acked-by: Alexei Starovoitov <ast@...nel.org>

Powered by blists - more mailing lists