Hi Taehee,
On Tue, Apr 05, 2022 at 05:00:19AM +0000, Taehee Yoo wrote:
> In some cases, xdp tx_queue can get used before initialization.
> 1. interface up/down
> 2. ring buffer size change
>
> When CPU cores are lower than maximum number of channels of sfc driver,
> it creates new channels only for XDP.
>
> When an interface is up or ring buffer size is changed, all channels
> are initialized.
> But xdp channels are always initialized later.
> So, the below scenario is possible.
> Packets are received to rx queue of normal channels and it is acted
> XDP_TX and tx_queue of xdp channels get used.
> But these tx_queues are not initialized yet.
> If so, TX DMA or queue error occurs.
>
> In order to avoid this problem
> 1. initializes xdp tx_queues earlier than other rx_queue in
> efx_start_channels().
> 2. checks whether tx_queue is initialized or not in efx_xdp_tx_buffers().
>
> Splat looks like:
> sfc 0000:08:00.1 enp8s0f1np1: TX queue 10 spurious TX completion id 250
> sfc 0000:08:00.1 enp8s0f1np1: resetting (RECOVER_OR_ALL)
> sfc 0000:08:00.1 enp8s0f1np1: MC command 0x80 inlen 100 failed rc=-22
> (raw=22) arg=789
> sfc 0000:08:00.1 enp8s0f1np1: has been disabled
>
> Fixes: dfe44c1f52ee ("sfc: handle XDP_TX outcomes of XDP eBPF programs")
A better fixes tag for this might be
f28100cb9c96 ("sfc: fix lack of XDP TX queues - error XDP TX failed (-22)")
as it enabled XDP in more situations.
> Signed-off-by: Taehee Yoo <ap420073@xxxxxxxxx>
Acked-by: Martin Habets <habetsm.xilinx@xxxxxxxxx>
> ---
> drivers/net/ethernet/sfc/efx_channels.c | 2 +-
> drivers/net/ethernet/sfc/tx.c | 3 +++
> drivers/net/ethernet/sfc/tx_common.c | 2 ++
> 3 files changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/sfc/efx_channels.c b/drivers/net/ethernet/sfc/efx_channels.c
> index 83e27231fbe6..377df8b7f015 100644
> --- a/drivers/net/ethernet/sfc/efx_channels.c
> +++ b/drivers/net/ethernet/sfc/efx_channels.c
> @@ -1140,7 +1140,7 @@ void efx_start_channels(struct efx_nic *efx)
> struct efx_rx_queue *rx_queue;
> struct efx_channel *channel;
>
> - efx_for_each_channel(channel, efx) {
> + efx_for_each_channel_rev(channel, efx) {
> efx_for_each_channel_tx_queue(tx_queue, channel) {
> efx_init_tx_queue(tx_queue);
> atomic_inc(&efx->active_queues);
> diff --git a/drivers/net/ethernet/sfc/tx.c b/drivers/net/ethernet/sfc/tx.c
> index d16e031e95f4..6983799e1c05 100644
> --- a/drivers/net/ethernet/sfc/tx.c
> +++ b/drivers/net/ethernet/sfc/tx.c
> @@ -443,6 +443,9 @@ int efx_xdp_tx_buffers(struct efx_nic *efx, int n, struct xdp_frame **xdpfs,
> if (unlikely(!tx_queue))
> return -EINVAL;
>
> + if (!tx_queue->initialised)
> + return -EINVAL;
> +
> if (efx->xdp_txq_queues_mode != EFX_XDP_TX_QUEUES_DEDICATED)
> HARD_TX_LOCK(efx->net_dev, tx_queue->core_txq, cpu);
>
> diff --git a/drivers/net/ethernet/sfc/tx_common.c b/drivers/net/ethernet/sfc/tx_common.c
> index d530cde2b864..9bc8281b7f5b 100644
> --- a/drivers/net/ethernet/sfc/tx_common.c
> +++ b/drivers/net/ethernet/sfc/tx_common.c
> @@ -101,6 +101,8 @@ void efx_fini_tx_queue(struct efx_tx_queue *tx_queue)
> netif_dbg(tx_queue->efx, drv, tx_queue->efx->net_dev,
> "shutting down TX queue %d\n", tx_queue->queue);
>
> + tx_queue->initialised = false;
> +
> if (!tx_queue->buffer)
> return;
>
> --
> 2.17.1
