All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Íñigo Huguet" <ihuguet@redhat.com>
To: ecree.xilinx@gmail.com, habetsm.xilinx@gmail.com,
	davem@davemloft.net, kuba@kernel.org, ivan@cloudflare.com
Cc: ast@kernel.org, daniel@iogearbox.net, hawk@kernel.org,
	john.fastabend@gmail.com, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org, ihuguet@redhat.com
Subject: [PATCH 1/3] sfc: revert "reduce the number of requested xdp ev queues"
Date: Wed,  7 Jul 2021 10:16:40 +0200	[thread overview]
Message-ID: <20210707081642.95365-1-ihuguet@redhat.com> (raw)

Fixes: e26ca4b53582 sfc: reduce the number of requested xdp ev queues

The buggy commit intended to allocate less channels for XDP in order to
be more unlikely to reach the limit of 32 channels of the driver.

The idea was to use each IRQ/eventqeue for more XDP TX queues than
before, calculating which is the maximum number of TX queues that one
event queue can handle. For example, in EF10 each event queue could
handle up to 8 queues, better than the 4 they were handling before the
change. This way, it would have to allocate half of channels than before
for XDP TX.

The problem is that the TX queues are also contained inside the channel
structs, and there are only 4 queues per channel. Reducing the number of
channels means also reducing the number of queues, resulting in not
having the desired number of 1 queue per CPU.

This leads to getting errors on XDP_TX and XDP_REDIRECT if they're
executed from a high numbered CPU, because there only exist queues for
the low half of CPUs, actually. If XDP_TX/REDIRECT is executed in a low
numbered CPU, the error doesn't happen. This is the error in the logs
(repeated many times, even rate limited):
sfc 0000:5e:00.0 ens3f0np0: XDP TX failed (-22)

This errors happens in function efx_xdp_tx_buffers, where it expects to
have a dedicated XDP TX queue per CPU.

Reverting the change makes again more likely to reach the limit of 32
channels in machines with many CPUs. If this happen, no XDP_TX/REDIRECT
will be possible at all, and we will have this log error messages:

At interface probe:
sfc 0000:5e:00.0: Insufficient resources for 12 XDP event queues (24 other channels, max 32)

At every subsequent XDP_TX/REDIRECT failure, rate limited:
sfc 0000:5e:00.0 ens3f0np0: XDP TX failed (-22)

However, without reverting the patch, it makes the user to think that
everything is OK at probe time, but later it fails in an unpredictable
way, depending on the CPU that handles the packet.

It is better to restore the predictable behaviour. If the user sees the
error message at probe time, he/she can try to configure the best way it
fits his needs. At least, he/she will have 2 options:
- Accept that XDP_TX/REDIRECT is not available (he/she may not need it)
- Load sfc module with modparam 'rss_cpus' with a lower number, thus
  creating less normal RX queues/channels, letting more free resources
  for XDP, with some performance penalty.

Related mailing list thread:
https://lore.kernel.org/bpf/20201215104327.2be76156@carbon/

Signed-off-by: Íñigo Huguet <ihuguet@redhat.com>
---
 drivers/net/ethernet/sfc/efx_channels.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/sfc/efx_channels.c b/drivers/net/ethernet/sfc/efx_channels.c
index a3ca406a3561..4fa5d675b6d4 100644
--- a/drivers/net/ethernet/sfc/efx_channels.c
+++ b/drivers/net/ethernet/sfc/efx_channels.c
@@ -17,7 +17,6 @@
 #include "rx_common.h"
 #include "nic.h"
 #include "sriov.h"
-#include "workarounds.h"
 
 /* This is the first interrupt mode to try out of:
  * 0 => MSI-X
@@ -138,7 +137,6 @@ static int efx_allocate_msix_channels(struct efx_nic *efx,
 {
 	unsigned int n_channels = parallelism;
 	int vec_count;
-	int tx_per_ev;
 	int n_xdp_tx;
 	int n_xdp_ev;
 
@@ -151,9 +149,9 @@ static int efx_allocate_msix_channels(struct efx_nic *efx,
 	 * multiple tx queues, assuming tx and ev queues are both
 	 * maximum size.
 	 */
-	tx_per_ev = EFX_MAX_EVQ_SIZE / EFX_TXQ_MAX_ENT(efx);
+
 	n_xdp_tx = num_possible_cpus();
-	n_xdp_ev = DIV_ROUND_UP(n_xdp_tx, tx_per_ev);
+	n_xdp_ev = DIV_ROUND_UP(n_xdp_tx, EFX_MAX_TXQ_PER_CHANNEL);
 
 	vec_count = pci_msix_vec_count(efx->pci_dev);
 	if (vec_count < 0)
-- 
2.31.1


             reply	other threads:[~2021-07-07  8:17 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-07  8:16 Íñigo Huguet [this message]
2021-07-07  8:16 ` [PATCH 2/3] sfc: revert "adjust efx->xdp_tx_queue_count with the real number of initialized queues" Íñigo Huguet
2021-07-07  8:16 ` [PATCH 3/3] sfc: add logs explaining XDP_TX/REDIRECT is not available Íñigo Huguet
2021-07-07 11:23 ` [PATCH 1/3] sfc: revert "reduce the number of requested xdp ev queues" Edward Cree
2021-07-07 11:49   ` Íñigo Huguet
2021-07-07 13:01     ` Martin Habets
2021-07-08 12:14       ` Íñigo Huguet
2021-07-09 14:07         ` Edward Cree
2021-07-09 15:06           ` Jesper Dangaard Brouer
2021-07-12 13:40             ` Íñigo Huguet
2021-07-12 14:52               ` Edward Cree
2021-07-13  6:20                 ` Íñigo Huguet
2021-07-09 12:55 ` [PATCH v2 0/3] Fix lack of XDP TX queues Íñigo Huguet
2021-07-09 12:55   ` [PATCH v2 1/3] sfc: fix lack of XDP TX queues - error XDP TX failed (-22) Íñigo Huguet
2021-07-09 12:55   ` [PATCH v2 2/3] sfc: revert "adjust efx->xdp_tx_queue_count with the real number of initialized queues" Íñigo Huguet
2021-07-09 13:53     ` Edward Cree
2021-07-09 12:55   ` [PATCH v2 3/3] sfc: add logs explaining XDP_TX/REDIRECT is not available Íñigo Huguet
2021-07-13 14:21 ` [PATCH v3 0/3] Fix lack of XDP TX queues Íñigo Huguet
2021-07-13 14:21   ` [PATCH v3 1/3] sfc: fix lack of XDP TX queues - error XDP TX failed (-22) Íñigo Huguet
2021-07-13 14:21   ` [PATCH v3 2/3] sfc: ensure correct number of XDP queues Íñigo Huguet
2021-07-13 14:21   ` [PATCH v3 3/3] sfc: add logs explaining XDP_TX/REDIRECT is not available Íñigo Huguet
2021-07-13 14:41   ` [PATCH v3 0/3] Fix lack of XDP TX queues Edward Cree
2021-07-13 20:10   ` patchwork-bot+netdevbpf

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210707081642.95365-1-ihuguet@redhat.com \
    --to=ihuguet@redhat.com \
    --cc=ast@kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=davem@davemloft.net \
    --cc=ecree.xilinx@gmail.com \
    --cc=habetsm.xilinx@gmail.com \
    --cc=hawk@kernel.org \
    --cc=ivan@cloudflare.com \
    --cc=john.fastabend@gmail.com \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.