Skip to content

Commit

Permalink
Merge branch 'bpf-af-xdp-wakeup'
Browse files Browse the repository at this point in the history
Magnus Karlsson says:

====================
This patch set adds support for a new flag called need_wakeup in the
AF_XDP Tx and fill rings. When this flag is set by the driver, it
means that the application has to explicitly wake up the kernel Rx
(for the bit in the fill ring) or kernel Tx (for bit in the Tx ring)
processing by issuing a syscall. Poll() can wake up both and sendto()
will wake up Tx processing only.

The main reason for introducing this new flag is to be able to
efficiently support the case when application and driver is executing
on the same core. Previously, the driver was just busy-spinning on the
fill ring if it ran out of buffers in the HW and there were none to
get from the fill ring. This approach works when the application and
driver is running on different cores as the application can replenish
the fill ring while the driver is busy-spinning. Though, this is a
lousy approach if both of them are running on the same core as the
probability of the fill ring getting more entries when the driver is
busy-spinning is zero. With this new feature the driver now sets the
need_wakeup flag and returns to the application. The application can
then replenish the fill queue and then explicitly wake up the Rx
processing in the kernel using the syscall poll(). For Tx, the flag is
only set to one if the driver has no outstanding Tx completion
interrupts. If it has some, the flag is zero as it will be woken up by
a completion interrupt anyway. This flag can also be used in other
situations where the driver needs to be woken up explicitly.

As a nice side effect, this new flag also improves the Tx performance
of the case where application and driver are running on two different
cores as it reduces the number of syscalls to the kernel. The kernel
tells user space if it needs to be woken up by a syscall, and this
eliminates many of the syscalls. The Rx performance of the 2-core case
is on the other hand slightly worse, since there is a need to use a
syscall now to wake up the driver, instead of the driver
busy-spinning. It does waste less CPU cycles though, which might lead
to better overall system performance.

This new flag needs some simple driver support. If the driver does not
support it, the Rx flag is always zero and the Tx flag is always
one. This makes any application relying on this feature default to the
old behavior of not requiring any syscalls in the Rx path and always
having to call sendto() in the Tx path.

For backwards compatibility reasons, this feature has to be explicitly
turned on using a new bind flag (XDP_USE_NEED_WAKEUP). I recommend
that you always turn it on as it has a large positive performance
impact for the one core case and does not degrade 2 core performance
and actually improves it for Tx heavy workloads.

Here are some performance numbers measured on my local,
non-performance optimized development system. That is why you are
seeing numbers lower than the ones from Björn and Jesper. 64 byte
packets at 40Gbit/s line rate. All results in Mpps. Cores == 1 means
that both application and driver is executing on the same core. Cores
== 2 that they are on different cores.

                              Applications
need_wakeup  cores    txpush    rxdrop      l2fwd
---------------------------------------------------------------
     n         1       0.07      0.06        0.03
     y         1       21.6      8.2         6.5
     n         2       32.3      11.7        8.7
     y         2       33.1      11.7        8.7

Overall, the need_wakeup flag provides the same or better performance
in all the micro-benchmarks. The reduction of sendto() calls in txpush
is large. Only a few per second is needed. For l2fwd, the drop is 50%
for the 1 core case and more than 99.9% for the 2 core case. Do not
know why I am not seeing the same drop for the 1 core case yet.

The name and inspiration of the flag has been taken from io_uring by
Jens Axboe. Details about this feature in io_uring can be found in
http://kernel.dk/io_uring.pdf, section 8.3. It also addresses most of
the denial of service and sendto() concerns raised by Maxim
Mikityanskiy in https://www.spinics.net/lists/netdev/msg554657.html.

The typical Tx part of an application will have to change from:

ret = sendto(fd,....)

to:

if (xsk_ring_prod__needs_wakeup(&xsk->tx))
       ret = sendto(fd,....)

and th Rx part from:

rcvd = xsk_ring_cons__peek(&xsk->rx, BATCH_SIZE, &idx_rx);
if (!rcvd)
       return;

to:

rcvd = xsk_ring_cons__peek(&xsk->rx, BATCH_SIZE, &idx_rx);
if (!rcvd) {
       if (xsk_ring_prod__needs_wakeup(&xsk->umem->fq))
              ret = poll(fd,.....);
       return;
}

v3 -> v4:
* Maxim found a possible race in the Tx part of the driver. The
  setting of the flag needs to happen before the sending, otherwise it
  might trigger this race. Fixed in ixgbe and i40e driver.
* Mellanox support contributed by Maxim
* Removed the XSK_DRV_CAN_SLEEP flag as it was not used
  anymore. Thanks to Sridhar for discovering this.
* For consistency the feature is now always called need_wakeup. There
  were some places where it was referred to as might_sleep, but they
  have been removed. Thanks to Sridhar for spotting.
* Fixed some typos in the commit messages

v2 -> v3:
* Converted the Mellanox driver to the new ndo in patch 1 as pointed
  out by Maxim
* Fixed the compatibility code of XDP_MMAP_OFFSETS so it now works.

v1 -> v2:
* Fixed bisectability problem pointed out by Jakub
* Added missing initiliztion of the Tx need_wakeup flag to 1

This patch has been applied against commit b753c5a ("Merge branch 'r8152-RX-improve'")

Structure of the patch set:

Patch 1: Replaces the ndo_xsk_async_xmit with ndo_xsk_wakeup to
         support waking up both Rx and Tx processing
Patch 2: Implements the need_wakeup functionality in common code
Patch 3-4: Add need_wakeup support to the i40e and ixgbe drivers
Patch 5: Add need_wakeup support to libbpf
Patch 6: Add need_wakeup support to the xdpsock sample application
Patch 7-8: Add need_wakeup support to the Mellanox mlx5 driver
====================

Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
  • Loading branch information
Daniel Borkmann committed Aug 17, 2019
2 parents e032500 + a7bd401 commit c8186c8
Show file tree
Hide file tree
Showing 23 changed files with 462 additions and 115 deletions.
5 changes: 3 additions & 2 deletions drivers/net/ethernet/intel/i40e/i40e_main.c
Original file line number Diff line number Diff line change
Expand Up @@ -12570,7 +12570,8 @@ static int i40e_xdp_setup(struct i40e_vsi *vsi,
if (need_reset && prog)
for (i = 0; i < vsi->num_queue_pairs; i++)
if (vsi->xdp_rings[i]->xsk_umem)
(void)i40e_xsk_async_xmit(vsi->netdev, i);
(void)i40e_xsk_wakeup(vsi->netdev, i,
XDP_WAKEUP_RX);

return 0;
}
Expand Down Expand Up @@ -12892,7 +12893,7 @@ static const struct net_device_ops i40e_netdev_ops = {
.ndo_bridge_setlink = i40e_ndo_bridge_setlink,
.ndo_bpf = i40e_xdp,
.ndo_xdp_xmit = i40e_xdp_xmit,
.ndo_xsk_async_xmit = i40e_xsk_async_xmit,
.ndo_xsk_wakeup = i40e_xsk_wakeup,
.ndo_dfwd_add_station = i40e_fwd_add,
.ndo_dfwd_del_station = i40e_fwd_del,
};
Expand Down
25 changes: 22 additions & 3 deletions drivers/net/ethernet/intel/i40e/i40e_xsk.c
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ static int i40e_xsk_umem_enable(struct i40e_vsi *vsi, struct xdp_umem *umem,
return err;

/* Kick start the NAPI context so that receiving will start */
err = i40e_xsk_async_xmit(vsi->netdev, qid);
err = i40e_xsk_wakeup(vsi->netdev, qid, XDP_WAKEUP_RX);
if (err)
return err;
}
Expand Down Expand Up @@ -626,6 +626,15 @@ int i40e_clean_rx_irq_zc(struct i40e_ring *rx_ring, int budget)

i40e_finalize_xdp_rx(rx_ring, xdp_xmit);
i40e_update_rx_stats(rx_ring, total_rx_bytes, total_rx_packets);

if (xsk_umem_uses_need_wakeup(rx_ring->xsk_umem)) {
if (failure || rx_ring->next_to_clean == rx_ring->next_to_use)
xsk_set_rx_need_wakeup(rx_ring->xsk_umem);
else
xsk_clear_rx_need_wakeup(rx_ring->xsk_umem);

return (int)total_rx_packets;
}
return failure ? budget : (int)total_rx_packets;
}

Expand Down Expand Up @@ -681,6 +690,8 @@ static bool i40e_xmit_zc(struct i40e_ring *xdp_ring, unsigned int budget)
i40e_xdp_ring_update_tail(xdp_ring);

xsk_umem_consume_tx_done(xdp_ring->xsk_umem);
if (xsk_umem_uses_need_wakeup(xdp_ring->xsk_umem))
xsk_clear_tx_need_wakeup(xdp_ring->xsk_umem);
}

return !!budget && work_done;
Expand Down Expand Up @@ -759,19 +770,27 @@ bool i40e_clean_xdp_tx_irq(struct i40e_vsi *vsi,
i40e_update_tx_stats(tx_ring, completed_frames, total_bytes);

out_xmit:
if (xsk_umem_uses_need_wakeup(tx_ring->xsk_umem)) {
if (tx_ring->next_to_clean == tx_ring->next_to_use)
xsk_set_tx_need_wakeup(tx_ring->xsk_umem);
else
xsk_clear_tx_need_wakeup(tx_ring->xsk_umem);
}

xmit_done = i40e_xmit_zc(tx_ring, budget);

return work_done && xmit_done;
}

/**
* i40e_xsk_async_xmit - Implements the ndo_xsk_async_xmit
* i40e_xsk_wakeup - Implements the ndo_xsk_wakeup
* @dev: the netdevice
* @queue_id: queue id to wake up
* @flags: ignored in our case since we have Rx and Tx in the same NAPI.
*
* Returns <0 for errors, 0 otherwise.
**/
int i40e_xsk_async_xmit(struct net_device *dev, u32 queue_id)
int i40e_xsk_wakeup(struct net_device *dev, u32 queue_id, u32 flags)
{
struct i40e_netdev_priv *np = netdev_priv(dev);
struct i40e_vsi *vsi = np->vsi;
Expand Down
2 changes: 1 addition & 1 deletion drivers/net/ethernet/intel/i40e/i40e_xsk.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,6 @@ int i40e_clean_rx_irq_zc(struct i40e_ring *rx_ring, int budget);

bool i40e_clean_xdp_tx_irq(struct i40e_vsi *vsi,
struct i40e_ring *tx_ring, int napi_budget);
int i40e_xsk_async_xmit(struct net_device *dev, u32 queue_id);
int i40e_xsk_wakeup(struct net_device *dev, u32 queue_id, u32 flags);

#endif /* _I40E_XSK_H_ */
5 changes: 3 additions & 2 deletions drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
Original file line number Diff line number Diff line change
Expand Up @@ -10263,7 +10263,8 @@ static int ixgbe_xdp_setup(struct net_device *dev, struct bpf_prog *prog)
if (need_reset && prog)
for (i = 0; i < adapter->num_rx_queues; i++)
if (adapter->xdp_ring[i]->xsk_umem)
(void)ixgbe_xsk_async_xmit(adapter->netdev, i);
(void)ixgbe_xsk_wakeup(adapter->netdev, i,
XDP_WAKEUP_RX);

return 0;
}
Expand Down Expand Up @@ -10382,7 +10383,7 @@ static const struct net_device_ops ixgbe_netdev_ops = {
.ndo_features_check = ixgbe_features_check,
.ndo_bpf = ixgbe_xdp,
.ndo_xdp_xmit = ixgbe_xdp_xmit,
.ndo_xsk_async_xmit = ixgbe_xsk_async_xmit,
.ndo_xsk_wakeup = ixgbe_xsk_wakeup,
};

static void ixgbe_disable_txr_hw(struct ixgbe_adapter *adapter,
Expand Down
2 changes: 1 addition & 1 deletion drivers/net/ethernet/intel/ixgbe/ixgbe_txrx_common.h
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ int ixgbe_clean_rx_irq_zc(struct ixgbe_q_vector *q_vector,
void ixgbe_xsk_clean_rx_ring(struct ixgbe_ring *rx_ring);
bool ixgbe_clean_xdp_tx_irq(struct ixgbe_q_vector *q_vector,
struct ixgbe_ring *tx_ring, int napi_budget);
int ixgbe_xsk_async_xmit(struct net_device *dev, u32 queue_id);
int ixgbe_xsk_wakeup(struct net_device *dev, u32 queue_id, u32 flags);
void ixgbe_xsk_clean_tx_ring(struct ixgbe_ring *tx_ring);

#endif /* #define _IXGBE_TXRX_COMMON_H_ */
22 changes: 20 additions & 2 deletions drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ static int ixgbe_xsk_umem_enable(struct ixgbe_adapter *adapter,
ixgbe_txrx_ring_enable(adapter, qid);

/* Kick start the NAPI context so that receiving will start */
err = ixgbe_xsk_async_xmit(adapter->netdev, qid);
err = ixgbe_xsk_wakeup(adapter->netdev, qid, XDP_WAKEUP_RX);
if (err)
return err;
}
Expand Down Expand Up @@ -547,6 +547,14 @@ int ixgbe_clean_rx_irq_zc(struct ixgbe_q_vector *q_vector,
q_vector->rx.total_packets += total_rx_packets;
q_vector->rx.total_bytes += total_rx_bytes;

if (xsk_umem_uses_need_wakeup(rx_ring->xsk_umem)) {
if (failure || rx_ring->next_to_clean == rx_ring->next_to_use)
xsk_set_rx_need_wakeup(rx_ring->xsk_umem);
else
xsk_clear_rx_need_wakeup(rx_ring->xsk_umem);

return (int)total_rx_packets;
}
return failure ? budget : (int)total_rx_packets;
}

Expand Down Expand Up @@ -615,6 +623,8 @@ static bool ixgbe_xmit_zc(struct ixgbe_ring *xdp_ring, unsigned int budget)
if (tx_desc) {
ixgbe_xdp_ring_update_tail(xdp_ring);
xsk_umem_consume_tx_done(xdp_ring->xsk_umem);
if (xsk_umem_uses_need_wakeup(xdp_ring->xsk_umem))
xsk_clear_tx_need_wakeup(xdp_ring->xsk_umem);
}

return !!budget && work_done;
Expand Down Expand Up @@ -688,11 +698,19 @@ bool ixgbe_clean_xdp_tx_irq(struct ixgbe_q_vector *q_vector,
if (xsk_frames)
xsk_umem_complete_tx(umem, xsk_frames);

if (xsk_umem_uses_need_wakeup(tx_ring->xsk_umem)) {
if (tx_ring->next_to_clean == tx_ring->next_to_use)
xsk_set_tx_need_wakeup(tx_ring->xsk_umem);
else
xsk_clear_tx_need_wakeup(tx_ring->xsk_umem);
}

xmit_done = ixgbe_xmit_zc(tx_ring, q_vector->tx.work_limit);

return budget > 0 && xmit_done;
}

int ixgbe_xsk_async_xmit(struct net_device *dev, u32 qid)
int ixgbe_xsk_wakeup(struct net_device *dev, u32 qid, u32 flags)
{
struct ixgbe_adapter *adapter = netdev_priv(dev);
struct ixgbe_ring *ring;
Expand Down
14 changes: 14 additions & 0 deletions drivers/net/ethernet/mellanox/mlx5/core/en/xsk/rx.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#define __MLX5_EN_XSK_RX_H__

#include "en.h"
#include <net/xdp_sock.h>

/* RX data path */

Expand All @@ -24,4 +25,17 @@ struct sk_buff *mlx5e_xsk_skb_from_cqe_linear(struct mlx5e_rq *rq,
struct mlx5e_wqe_frag_info *wi,
u32 cqe_bcnt);

static inline bool mlx5e_xsk_update_rx_wakeup(struct mlx5e_rq *rq, bool alloc_err)
{
if (!xsk_umem_uses_need_wakeup(rq->umem))
return alloc_err;

if (unlikely(alloc_err))
xsk_set_rx_need_wakeup(rq->umem);
else
xsk_clear_rx_need_wakeup(rq->umem);

return false;
}

#endif /* __MLX5_EN_XSK_RX_H__ */
2 changes: 1 addition & 1 deletion drivers/net/ethernet/mellanox/mlx5/core/en/xsk/tx.c
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
#include "en/params.h"
#include <net/xdp_sock.h>

int mlx5e_xsk_async_xmit(struct net_device *dev, u32 qid)
int mlx5e_xsk_wakeup(struct net_device *dev, u32 qid, u32 flags)
{
struct mlx5e_priv *priv = netdev_priv(dev);
struct mlx5e_params *params = &priv->channels.params;
Expand Down
14 changes: 13 additions & 1 deletion drivers/net/ethernet/mellanox/mlx5/core/en/xsk/tx.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,23 @@
#define __MLX5_EN_XSK_TX_H__

#include "en.h"
#include <net/xdp_sock.h>

/* TX data path */

int mlx5e_xsk_async_xmit(struct net_device *dev, u32 qid);
int mlx5e_xsk_wakeup(struct net_device *dev, u32 qid, u32 flags);

bool mlx5e_xsk_tx(struct mlx5e_xdpsq *sq, unsigned int budget);

static inline void mlx5e_xsk_update_tx_wakeup(struct mlx5e_xdpsq *sq)
{
if (!xsk_umem_uses_need_wakeup(sq->umem))
return;

if (sq->pc != sq->cc)
xsk_clear_tx_need_wakeup(sq->umem);
else
xsk_set_tx_need_wakeup(sq->umem);
}

#endif /* __MLX5_EN_XSK_TX_H__ */
2 changes: 1 addition & 1 deletion drivers/net/ethernet/mellanox/mlx5/core/en_main.c
Original file line number Diff line number Diff line change
Expand Up @@ -4540,7 +4540,7 @@ const struct net_device_ops mlx5e_netdev_ops = {
.ndo_tx_timeout = mlx5e_tx_timeout,
.ndo_bpf = mlx5e_xdp,
.ndo_xdp_xmit = mlx5e_xdp_xmit,
.ndo_xsk_async_xmit = mlx5e_xsk_async_xmit,
.ndo_xsk_wakeup = mlx5e_xsk_wakeup,
#ifdef CONFIG_MLX5_EN_ARFS
.ndo_rx_flow_steer = mlx5e_rx_flow_steer,
#endif
Expand Down
7 changes: 5 additions & 2 deletions drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
Original file line number Diff line number Diff line change
Expand Up @@ -692,8 +692,11 @@ bool mlx5e_post_rx_mpwqes(struct mlx5e_rq *rq)
rq->mpwqe.umr_in_progress += rq->mpwqe.umr_last_bulk;
rq->mpwqe.actual_wq_head = head;

/* If XSK Fill Ring doesn't have enough frames, busy poll by
* rescheduling the NAPI poll.
/* If XSK Fill Ring doesn't have enough frames, report the error, so
* that one of the actions can be performed:
* 1. If need_wakeup is used, signal that the application has to kick
* the driver when it refills the Fill Ring.
* 2. Otherwise, busy poll by rescheduling the NAPI poll.
*/
if (unlikely(alloc_err == -ENOMEM && rq->umem))
return true;
Expand Down
27 changes: 25 additions & 2 deletions drivers/net/ethernet/mellanox/mlx5/core/en_txrx.c
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
#include <linux/irq.h>
#include "en.h"
#include "en/xdp.h"
#include "en/xsk/rx.h"
#include "en/xsk/tx.h"

static inline bool mlx5e_channel_no_affinity_change(struct mlx5e_channel *c)
Expand Down Expand Up @@ -81,6 +82,29 @@ void mlx5e_trigger_irq(struct mlx5e_icosq *sq)
mlx5e_notify_hw(wq, sq->pc, sq->uar_map, &nopwqe->ctrl);
}

static bool mlx5e_napi_xsk_post(struct mlx5e_xdpsq *xsksq, struct mlx5e_rq *xskrq)
{
bool busy_xsk = false, xsk_rx_alloc_err;

/* Handle the race between the application querying need_wakeup and the
* driver setting it:
* 1. Update need_wakeup both before and after the TX. If it goes to
* "yes", it can only happen with the first update.
* 2. If the application queried need_wakeup before we set it, the
* packets will be transmitted anyway, even w/o a wakeup.
* 3. Give a chance to clear need_wakeup after new packets were queued
* for TX.
*/
mlx5e_xsk_update_tx_wakeup(xsksq);
busy_xsk |= mlx5e_xsk_tx(xsksq, MLX5E_TX_XSK_POLL_BUDGET);
mlx5e_xsk_update_tx_wakeup(xsksq);

xsk_rx_alloc_err = xskrq->post_wqes(xskrq);
busy_xsk |= mlx5e_xsk_update_rx_wakeup(xskrq, xsk_rx_alloc_err);

return busy_xsk;
}

int mlx5e_napi_poll(struct napi_struct *napi, int budget)
{
struct mlx5e_channel *c = container_of(napi, struct mlx5e_channel,
Expand Down Expand Up @@ -122,8 +146,7 @@ int mlx5e_napi_poll(struct napi_struct *napi, int budget)
if (xsk_open) {
mlx5e_poll_ico_cq(&c->xskicosq.cq);
busy |= mlx5e_poll_xdpsq_cq(&xsksq->cq);
busy_xsk |= mlx5e_xsk_tx(xsksq, MLX5E_TX_XSK_POLL_BUDGET);
busy_xsk |= xskrq->post_wqes(xskrq);
busy_xsk |= mlx5e_napi_xsk_post(xsksq, xskrq);
}

busy |= busy_xsk;
Expand Down
14 changes: 12 additions & 2 deletions include/linux/netdevice.h
Original file line number Diff line number Diff line change
Expand Up @@ -901,6 +901,10 @@ struct netdev_bpf {
};
};

/* Flags for ndo_xsk_wakeup. */
#define XDP_WAKEUP_RX (1 << 0)
#define XDP_WAKEUP_TX (1 << 1)

#ifdef CONFIG_XFRM_OFFLOAD
struct xfrmdev_ops {
int (*xdo_dev_state_add) (struct xfrm_state *x);
Expand Down Expand Up @@ -1227,6 +1231,12 @@ struct tlsdev_ops;
* that got dropped are freed/returned via xdp_return_frame().
* Returns negative number, means general error invoking ndo, meaning
* no frames were xmit'ed and core-caller will free all frames.
* int (*ndo_xsk_wakeup)(struct net_device *dev, u32 queue_id, u32 flags);
* This function is used to wake up the softirq, ksoftirqd or kthread
* responsible for sending and/or receiving packets on a specific
* queue id bound to an AF_XDP socket. The flags field specifies if
* only RX, only Tx, or both should be woken up using the flags
* XDP_WAKEUP_RX and XDP_WAKEUP_TX.
* struct devlink_port *(*ndo_get_devlink_port)(struct net_device *dev);
* Get devlink port instance associated with a given netdev.
* Called with a reference on the netdevice and devlink locks only,
Expand Down Expand Up @@ -1426,8 +1436,8 @@ struct net_device_ops {
int (*ndo_xdp_xmit)(struct net_device *dev, int n,
struct xdp_frame **xdp,
u32 flags);
int (*ndo_xsk_async_xmit)(struct net_device *dev,
u32 queue_id);
int (*ndo_xsk_wakeup)(struct net_device *dev,
u32 queue_id, u32 flags);
struct devlink_port * (*ndo_get_devlink_port)(struct net_device *dev);
};

Expand Down
Loading

0 comments on commit c8186c8

Please sign in to comment.