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
| ||
|
Date: Fri, 4 Sep 2020 17:54:42 +0200 From: Björn Töpel <bjorn.topel@...el.com> To: Jesper Dangaard Brouer <brouer@...hat.com>, Björn Töpel <bjorn.topel@...il.com> Cc: ast@...nel.org, daniel@...earbox.net, netdev@...r.kernel.org, bpf@...r.kernel.org, magnus.karlsson@...el.com, davem@...emloft.net, kuba@...nel.org, john.fastabend@...il.com, intel-wired-lan@...ts.osuosl.org Subject: Re: [PATCH bpf-next 6/6] ixgbe, xsk: finish napi loop if AF_XDP Rx queue is full On 2020-09-04 17:35, Jesper Dangaard Brouer wrote: > On Fri, 4 Sep 2020 15:53:31 +0200 > Björn Töpel <bjorn.topel@...il.com> wrote: > >> From: Björn Töpel <bjorn.topel@...el.com> >> >> Make the AF_XDP zero-copy path aware that the reason for redirect >> failure was due to full Rx queue. If so, exit the napi loop as soon as >> possible (exit the softirq processing), so that the userspace AF_XDP >> process can hopefully empty the Rx queue. This mainly helps the "one >> core scenario", where the userland process and Rx softirq processing >> is on the same core. >> >> Note that the early exit can only be performed if the "need wakeup" >> feature is enabled, because otherwise there is no notification >> mechanism available from the kernel side. >> >> This requires that the driver starts using the newly introduced >> xdp_do_redirect_ext() and xsk_do_redirect_rx_full() functions. >> >> Signed-off-by: Björn Töpel <bjorn.topel@...el.com> >> --- >> drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c | 23 ++++++++++++++------ >> 1 file changed, 16 insertions(+), 7 deletions(-) >> >> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c >> index 3771857cf887..a4aebfd986b3 100644 >> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c >> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c >> @@ -93,9 +93,11 @@ int ixgbe_xsk_pool_setup(struct ixgbe_adapter *adapter, >> >> static int ixgbe_run_xdp_zc(struct ixgbe_adapter *adapter, >> struct ixgbe_ring *rx_ring, >> - struct xdp_buff *xdp) >> + struct xdp_buff *xdp, >> + bool *early_exit) >> { >> int err, result = IXGBE_XDP_PASS; >> + enum bpf_map_type map_type; >> struct bpf_prog *xdp_prog; >> struct xdp_frame *xdpf; >> u32 act; >> @@ -116,8 +118,13 @@ static int ixgbe_run_xdp_zc(struct ixgbe_adapter *adapter, >> result = ixgbe_xmit_xdp_ring(adapter, xdpf); >> break; >> case XDP_REDIRECT: >> - err = xdp_do_redirect(rx_ring->netdev, xdp, xdp_prog); >> - result = !err ? IXGBE_XDP_REDIR : IXGBE_XDP_CONSUMED; >> + err = xdp_do_redirect_ext(rx_ring->netdev, xdp, xdp_prog, &map_type); >> + if (err) { >> + *early_exit = xsk_do_redirect_rx_full(err, map_type); > > Have you tried calling xdp_do_flush (that calls __xsk_map_flush()) and > (I guess) xsk_set_rx_need_wakeup() here, instead of stopping the loop? > (Or doing this in xsk core). > Moving the need_wake logic to the xsk core/flush would be a very nice cleanup. The driver would still need to pass information from the driver though. Still, much cleaner. I'll take a stab at that. Thanks! > Looking at the code, the AF_XDP frames are "published" in the queue > rather late for AF_XDP. Maybe in an orthogonal optimization, have you > considered "publishing" the ring producer when e.g. the queue is > half-full? > Hmm, I haven't. You mean instead of yielding, you publish/submit? I *think* I still prefer stopping the processing. I'll play with this a bit! Very nice suggestions, Jesper! Thanks! Björn
Powered by blists - more mailing lists