* [PATCH net-next] sfc: reduce the number of requested xdp ev queues
@ 2020-12-15 1:29 Ivan Babrou
2020-12-15 9:43 ` Jesper Dangaard Brouer
` (2 more replies)
0 siblings, 3 replies; 15+ messages in thread
From: Ivan Babrou @ 2020-12-15 1:29 UTC (permalink / raw)
To: netdev
Cc: linux-kernel, bpf, kernel-team, Ivan Babrou, Edward Cree,
Martin Habets, David S. Miller, Jakub Kicinski,
Alexei Starovoitov, Daniel Borkmann, Jesper Dangaard Brouer,
John Fastabend
Without this change the driver tries to allocate too many queues,
breaching the number of available msi-x interrupts on machines
with many logical cpus and default adapter settings:
Insufficient resources for 12 XDP event queues (24 other channels, max 32)
Which in turn triggers EINVAL on XDP processing:
sfc 0000:86:00.0 ext0: XDP TX failed (-22)
Signed-off-by: Ivan Babrou <ivan@cloudflare.com>
---
drivers/net/ethernet/sfc/efx_channels.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/sfc/efx_channels.c b/drivers/net/ethernet/sfc/efx_channels.c
index a4a626e9cd9a..1bfeee283ea9 100644
--- a/drivers/net/ethernet/sfc/efx_channels.c
+++ b/drivers/net/ethernet/sfc/efx_channels.c
@@ -17,6 +17,7 @@
#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
@@ -137,6 +138,7 @@ 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;
@@ -149,9 +151,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, EFX_MAX_TXQ_PER_CHANNEL);
+ n_xdp_ev = DIV_ROUND_UP(n_xdp_tx, tx_per_ev);
vec_count = pci_msix_vec_count(efx->pci_dev);
if (vec_count < 0)
--
2.29.2
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH net-next] sfc: reduce the number of requested xdp ev queues
2020-12-15 1:29 [PATCH net-next] sfc: reduce the number of requested xdp ev queues Ivan Babrou
@ 2020-12-15 9:43 ` Jesper Dangaard Brouer
2020-12-15 18:49 ` Edward Cree
2020-12-16 8:18 ` Martin Habets
2020-12-17 18:14 ` Jakub Kicinski
2 siblings, 1 reply; 15+ messages in thread
From: Jesper Dangaard Brouer @ 2020-12-15 9:43 UTC (permalink / raw)
To: Ivan Babrou
Cc: netdev, linux-kernel, bpf, kernel-team, Edward Cree,
Martin Habets, David S. Miller, Jakub Kicinski,
Alexei Starovoitov, Daniel Borkmann, Jesper Dangaard Brouer,
John Fastabend
On Mon, 14 Dec 2020 17:29:06 -0800
Ivan Babrou <ivan@cloudflare.com> wrote:
> Without this change the driver tries to allocate too many queues,
> breaching the number of available msi-x interrupts on machines
> with many logical cpus and default adapter settings:
>
> Insufficient resources for 12 XDP event queues (24 other channels, max 32)
>
> Which in turn triggers EINVAL on XDP processing:
>
> sfc 0000:86:00.0 ext0: XDP TX failed (-22)
I have a similar QA report with XDP_REDIRECT:
sfc 0000:05:00.0 ens1f0np0: XDP redirect failed (-22)
Here we are back to the issue we discussed with ixgbe, that NIC / msi-x
interrupts hardware resources are not enough on machines with many
logical cpus.
After this fix, what will happen if (cpu >= efx->xdp_tx_queue_count) ?
(Copied efx_xdp_tx_buffers code below signature)
The question leads to, does this driver need a fallback mechanism when
HW resource or systems logical cpus exceed the one TX-queue per CPU
assumption?
> Signed-off-by: Ivan Babrou <ivan@cloudflare.com>
> ---
> drivers/net/ethernet/sfc/efx_channels.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/sfc/efx_channels.c
> b/drivers/net/ethernet/sfc/efx_channels.c index
> a4a626e9cd9a..1bfeee283ea9 100644 ---
> a/drivers/net/ethernet/sfc/efx_channels.c +++
> b/drivers/net/ethernet/sfc/efx_channels.c @@ -17,6 +17,7 @@
> #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
> @@ -137,6 +138,7 @@ 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;
>
> @@ -149,9 +151,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, EFX_MAX_TXQ_PER_CHANNEL);
> + n_xdp_ev = DIV_ROUND_UP(n_xdp_tx, tx_per_ev);
>
> vec_count = pci_msix_vec_count(efx->pci_dev);
> if (vec_count < 0)
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Principal Kernel Engineer at Red Hat
LinkedIn: http://www.linkedin.com/in/brouer
/* Transmit a packet from an XDP buffer
*
* Returns number of packets sent on success, error code otherwise.
* Runs in NAPI context, either in our poll (for XDP TX) or a different NIC
* (for XDP redirect).
*/
int efx_xdp_tx_buffers(struct efx_nic *efx, int n, struct xdp_frame **xdpfs,
bool flush)
{
struct efx_tx_buffer *tx_buffer;
struct efx_tx_queue *tx_queue;
struct xdp_frame *xdpf;
dma_addr_t dma_addr;
unsigned int len;
int space;
int cpu;
int i;
cpu = raw_smp_processor_id();
if (!efx->xdp_tx_queue_count ||
unlikely(cpu >= efx->xdp_tx_queue_count))
return -EINVAL;
tx_queue = efx->xdp_tx_queues[cpu];
if (unlikely(!tx_queue))
return -EINVAL;
if (unlikely(n && !xdpfs))
return -EINVAL;
if (!n)
return 0;
/* Check for available space. We should never need multiple
* descriptors per frame.
*/
space = efx->txq_entries +
tx_queue->read_count - tx_queue->insert_count;
for (i = 0; i < n; i++) {
xdpf = xdpfs[i];
if (i >= space)
break;
/* We'll want a descriptor for this tx. */
prefetchw(__efx_tx_queue_get_insert_buffer(tx_queue));
len = xdpf->len;
/* Map for DMA. */
dma_addr = dma_map_single(&efx->pci_dev->dev,
xdpf->data, len,
DMA_TO_DEVICE);
if (dma_mapping_error(&efx->pci_dev->dev, dma_addr))
break;
/* Create descriptor and set up for unmapping DMA. */
tx_buffer = efx_tx_map_chunk(tx_queue, dma_addr, len);
tx_buffer->xdpf = xdpf;
tx_buffer->flags = EFX_TX_BUF_XDP |
EFX_TX_BUF_MAP_SINGLE;
tx_buffer->dma_offset = 0;
tx_buffer->unmap_len = len;
tx_queue->tx_packets++;
}
/* Pass mapped frames to hardware. */
if (flush && i > 0)
efx_nic_push_buffers(tx_queue);
if (i == 0)
return -EIO;
efx_xdp_return_frames(n - i, xdpfs + i);
return i;
}
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH net-next] sfc: reduce the number of requested xdp ev queues
2020-12-15 9:43 ` Jesper Dangaard Brouer
@ 2020-12-15 18:49 ` Edward Cree
2020-12-16 8:45 ` Jesper Dangaard Brouer
0 siblings, 1 reply; 15+ messages in thread
From: Edward Cree @ 2020-12-15 18:49 UTC (permalink / raw)
To: Jesper Dangaard Brouer, Ivan Babrou
Cc: netdev, linux-kernel, bpf, kernel-team, Martin Habets,
David S. Miller, Jakub Kicinski, Alexei Starovoitov,
Daniel Borkmann, Jesper Dangaard Brouer, John Fastabend
On 15/12/2020 09:43, Jesper Dangaard Brouer wrote:
> On Mon, 14 Dec 2020 17:29:06 -0800
> Ivan Babrou <ivan@cloudflare.com> wrote:
>
>> Without this change the driver tries to allocate too many queues,
>> breaching the number of available msi-x interrupts on machines
>> with many logical cpus and default adapter settings:
>>
>> Insufficient resources for 12 XDP event queues (24 other channels, max 32)
>>
>> Which in turn triggers EINVAL on XDP processing:
>>
>> sfc 0000:86:00.0 ext0: XDP TX failed (-22)
>
> I have a similar QA report with XDP_REDIRECT:
> sfc 0000:05:00.0 ens1f0np0: XDP redirect failed (-22)
>
> Here we are back to the issue we discussed with ixgbe, that NIC / msi-x
> interrupts hardware resources are not enough on machines with many
> logical cpus.
>
> After this fix, what will happen if (cpu >= efx->xdp_tx_queue_count) ?
Same as happened before: the "failed -22". But this fix will make that
less likely to happen, because it ties more TXQs to each EVQ, and it's
the EVQs that are in short supply.
(Strictly speaking, I believe the limitation is a software one, that
comes from the driver's channel structures having been designed a
decade ago when 32 cpus ought to be enough for anybody... AFAIR the
hardware is capable of giving us something like 1024 evqs if we ask
for them, it just might not have that many msi-x vectors for us.)
Anyway, the patch looks correct, so
Acked-by: Edward Cree <ecree.xilinx@gmail.com>
-ed
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH net-next] sfc: reduce the number of requested xdp ev queues
2020-12-15 18:49 ` Edward Cree
@ 2020-12-16 8:45 ` Jesper Dangaard Brouer
2020-12-16 23:12 ` Edward Cree
0 siblings, 1 reply; 15+ messages in thread
From: Jesper Dangaard Brouer @ 2020-12-16 8:45 UTC (permalink / raw)
To: Edward Cree
Cc: brouer, Ivan Babrou, netdev, linux-kernel, bpf, kernel-team,
Martin Habets, David S. Miller, Jakub Kicinski,
Alexei Starovoitov, Daniel Borkmann, Jesper Dangaard Brouer,
John Fastabend
On Tue, 15 Dec 2020 18:49:55 +0000
Edward Cree <ecree.xilinx@gmail.com> wrote:
> On 15/12/2020 09:43, Jesper Dangaard Brouer wrote:
> > On Mon, 14 Dec 2020 17:29:06 -0800
> > Ivan Babrou <ivan@cloudflare.com> wrote:
> >
> >> Without this change the driver tries to allocate too many queues,
> >> breaching the number of available msi-x interrupts on machines
> >> with many logical cpus and default adapter settings:
> >>
> >> Insufficient resources for 12 XDP event queues (24 other channels, max 32)
> >>
> >> Which in turn triggers EINVAL on XDP processing:
> >>
> >> sfc 0000:86:00.0 ext0: XDP TX failed (-22)
> >
> > I have a similar QA report with XDP_REDIRECT:
> > sfc 0000:05:00.0 ens1f0np0: XDP redirect failed (-22)
> >
> > Here we are back to the issue we discussed with ixgbe, that NIC / msi-x
> > interrupts hardware resources are not enough on machines with many
> > logical cpus.
> >
> > After this fix, what will happen if (cpu >= efx->xdp_tx_queue_count) ?
>
> Same as happened before: the "failed -22". But this fix will make that
> less likely to happen, because it ties more TXQs to each EVQ, and it's
> the EVQs that are in short supply.
>
So, what I hear is that this fix is just pampering over the real issue.
I suggest that you/we detect the situation, and have a code path that
will take a lock (per 16 packets bulk) and solve the issue.
If you care about maximum performance you can implement this via
changing the ndo_xdp_xmit pointer to the fallback function when needed,
to avoid having a to check for the fallback mode in the fast-path.
>
> (Strictly speaking, I believe the limitation is a software one, that
> comes from the driver's channel structures having been designed a
> decade ago when 32 cpus ought to be enough for anybody... AFAIR the
> hardware is capable of giving us something like 1024 evqs if we ask
> for them, it just might not have that many msi-x vectors for us.)
> Anyway, the patch looks correct, so
> Acked-by: Edward Cree <ecree.xilinx@gmail.com>
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Principal Kernel Engineer at Red Hat
LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH net-next] sfc: reduce the number of requested xdp ev queues
2020-12-16 8:45 ` Jesper Dangaard Brouer
@ 2020-12-16 23:12 ` Edward Cree
0 siblings, 0 replies; 15+ messages in thread
From: Edward Cree @ 2020-12-16 23:12 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: Ivan Babrou, netdev, linux-kernel, bpf, kernel-team,
Martin Habets, David S. Miller, Jakub Kicinski,
Alexei Starovoitov, Daniel Borkmann, Jesper Dangaard Brouer,
John Fastabend, Marek Majtyka
On 16/12/2020 08:45, Jesper Dangaard Brouer wrote:
> So, what I hear is that this fix is just pampering over the real issue.
Yes, it is, but it's better than nothing in the meantime while we work
out the complete fix.
> I suggest that you/we detect the situation, and have a code path that
> will take a lock (per 16 packets bulk) and solve the issue.
Imho that would _also_ paper over the issue, because it would mean the
system degraded to a lower performance mode of operation, while still
appearing to support XDP_TX. I think that that in general should not
happen unless there is a way for the user to determine at runtime
whether it has/should happen. Perhaps Marek's new XDP feature flags
could include a "tx-lockless" flag to indicate this?
-ed
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH net-next] sfc: reduce the number of requested xdp ev queues
2020-12-15 1:29 [PATCH net-next] sfc: reduce the number of requested xdp ev queues Ivan Babrou
2020-12-15 9:43 ` Jesper Dangaard Brouer
@ 2020-12-16 8:18 ` Martin Habets
2020-12-17 18:14 ` Jakub Kicinski
2 siblings, 0 replies; 15+ messages in thread
From: Martin Habets @ 2020-12-16 8:18 UTC (permalink / raw)
To: Ivan Babrou
Cc: netdev, linux-kernel, bpf, kernel-team, Edward Cree,
David S. Miller, Jakub Kicinski, Alexei Starovoitov,
Daniel Borkmann, Jesper Dangaard Brouer, John Fastabend
On Mon, Dec 14, 2020 at 05:29:06PM -0800, Ivan Babrou wrote:
> Without this change the driver tries to allocate too many queues,
> breaching the number of available msi-x interrupts on machines
> with many logical cpus and default adapter settings:
>
> Insufficient resources for 12 XDP event queues (24 other channels, max 32)
>
> Which in turn triggers EINVAL on XDP processing:
>
> sfc 0000:86:00.0 ext0: XDP TX failed (-22)
>
> Signed-off-by: Ivan Babrou <ivan@cloudflare.com>
Acked-by: Martin Habets <habetsm.xilinx@gmail.com>
> ---
> drivers/net/ethernet/sfc/efx_channels.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/sfc/efx_channels.c b/drivers/net/ethernet/sfc/efx_channels.c
> index a4a626e9cd9a..1bfeee283ea9 100644
> --- a/drivers/net/ethernet/sfc/efx_channels.c
> +++ b/drivers/net/ethernet/sfc/efx_channels.c
> @@ -17,6 +17,7 @@
> #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
> @@ -137,6 +138,7 @@ 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;
>
> @@ -149,9 +151,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, EFX_MAX_TXQ_PER_CHANNEL);
> + n_xdp_ev = DIV_ROUND_UP(n_xdp_tx, tx_per_ev);
>
> vec_count = pci_msix_vec_count(efx->pci_dev);
> if (vec_count < 0)
> --
> 2.29.2
--
Martin Habets <habetsm.xilinx@gmail.com>
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH net-next] sfc: reduce the number of requested xdp ev queues
2020-12-15 1:29 [PATCH net-next] sfc: reduce the number of requested xdp ev queues Ivan Babrou
2020-12-15 9:43 ` Jesper Dangaard Brouer
2020-12-16 8:18 ` Martin Habets
@ 2020-12-17 18:14 ` Jakub Kicinski
2021-01-19 23:43 ` Ivan Babrou
2 siblings, 1 reply; 15+ messages in thread
From: Jakub Kicinski @ 2020-12-17 18:14 UTC (permalink / raw)
To: Ivan Babrou
Cc: netdev, linux-kernel, bpf, kernel-team, Edward Cree,
Martin Habets, David S. Miller, Alexei Starovoitov,
Daniel Borkmann, Jesper Dangaard Brouer, John Fastabend
On Mon, 14 Dec 2020 17:29:06 -0800 Ivan Babrou wrote:
> Without this change the driver tries to allocate too many queues,
> breaching the number of available msi-x interrupts on machines
> with many logical cpus and default adapter settings:
>
> Insufficient resources for 12 XDP event queues (24 other channels, max 32)
>
> Which in turn triggers EINVAL on XDP processing:
>
> sfc 0000:86:00.0 ext0: XDP TX failed (-22)
>
> Signed-off-by: Ivan Babrou <ivan@cloudflare.com>
Looks like the discussion may have concluded, but we don't take -next
patches during the merge window, so please repost when net-next reopens.
Thanks!
--
# Form letter - net-next is closed
We have already sent the networking pull request for 5.11 and therefore
net-next is closed for new drivers, features, code refactoring and
optimizations. We are currently accepting bug fixes only.
Please repost when net-next reopens after 5.11-rc1 is cut.
Look out for the announcement on the mailing list or check:
http://vger.kernel.org/~davem/net-next.html
RFC patches sent for review only are obviously welcome at any time.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH net-next] sfc: reduce the number of requested xdp ev queues
2020-12-17 18:14 ` Jakub Kicinski
@ 2021-01-19 23:43 ` Ivan Babrou
2021-01-20 0:31 ` Jakub Kicinski
0 siblings, 1 reply; 15+ messages in thread
From: Ivan Babrou @ 2021-01-19 23:43 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Linux Kernel Network Developers, linux-kernel, bpf, kernel-team,
Edward Cree, Martin Habets, David S. Miller, Alexei Starovoitov,
Daniel Borkmann, Jesper Dangaard Brouer, John Fastabend
On Thu, Dec 17, 2020 at 10:14 AM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Mon, 14 Dec 2020 17:29:06 -0800 Ivan Babrou wrote:
> > Without this change the driver tries to allocate too many queues,
> > breaching the number of available msi-x interrupts on machines
> > with many logical cpus and default adapter settings:
> >
> > Insufficient resources for 12 XDP event queues (24 other channels, max 32)
> >
> > Which in turn triggers EINVAL on XDP processing:
> >
> > sfc 0000:86:00.0 ext0: XDP TX failed (-22)
> >
> > Signed-off-by: Ivan Babrou <ivan@cloudflare.com>
>
> Looks like the discussion may have concluded, but we don't take -next
> patches during the merge window, so please repost when net-next reopens.
>
> Thanks!
> --
> # Form letter - net-next is closed
>
> We have already sent the networking pull request for 5.11 and therefore
> net-next is closed for new drivers, features, code refactoring and
> optimizations. We are currently accepting bug fixes only.
>
> Please repost when net-next reopens after 5.11-rc1 is cut.
Should I resend my patch now that the window is open or is bumping
this thread enough?
> Look out for the announcement on the mailing list or check:
> http://vger.kernel.org/~davem/net-next.html
>
> RFC patches sent for review only are obviously welcome at any time.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH net-next] sfc: reduce the number of requested xdp ev queues
2021-01-19 23:43 ` Ivan Babrou
@ 2021-01-20 0:31 ` Jakub Kicinski
0 siblings, 0 replies; 15+ messages in thread
From: Jakub Kicinski @ 2021-01-20 0:31 UTC (permalink / raw)
To: Ivan Babrou
Cc: Linux Kernel Network Developers, linux-kernel, bpf, kernel-team,
Edward Cree, Martin Habets, David S. Miller, Alexei Starovoitov,
Daniel Borkmann, Jesper Dangaard Brouer, John Fastabend
On Tue, 19 Jan 2021 15:43:43 -0800 Ivan Babrou wrote:
> On Thu, Dec 17, 2020 at 10:14 AM Jakub Kicinski <kuba@kernel.org> wrote:
> > On Mon, 14 Dec 2020 17:29:06 -0800 Ivan Babrou wrote:
> > > Without this change the driver tries to allocate too many queues,
> > > breaching the number of available msi-x interrupts on machines
> > > with many logical cpus and default adapter settings:
> > >
> > > Insufficient resources for 12 XDP event queues (24 other channels, max 32)
> > >
> > > Which in turn triggers EINVAL on XDP processing:
> > >
> > > sfc 0000:86:00.0 ext0: XDP TX failed (-22)
> > >
> > > Signed-off-by: Ivan Babrou <ivan@cloudflare.com>
> >
> > Looks like the discussion may have concluded, but we don't take -next
> > patches during the merge window, so please repost when net-next reopens.
> >
> > Thanks!
> > --
> > # Form letter - net-next is closed
> >
> > We have already sent the networking pull request for 5.11 and therefore
> > net-next is closed for new drivers, features, code refactoring and
> > optimizations. We are currently accepting bug fixes only.
> >
> > Please repost when net-next reopens after 5.11-rc1 is cut.
>
> Should I resend my patch now that the window is open or is bumping
> this thread enough?
You need to resend, thanks!
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH net-next] sfc: reduce the number of requested xdp ev queues
@ 2021-01-20 21:27 Ivan Babrou
2021-01-21 16:10 ` Edward Cree
` (3 more replies)
0 siblings, 4 replies; 15+ messages in thread
From: Ivan Babrou @ 2021-01-20 21:27 UTC (permalink / raw)
To: netdev
Cc: linux-kernel, bpf, kernel-team, Ivan Babrou, Edward Cree,
Martin Habets, David S. Miller, Jakub Kicinski,
Alexei Starovoitov, Daniel Borkmann, Jesper Dangaard Brouer,
John Fastabend
Without this change the driver tries to allocate too many queues,
breaching the number of available msi-x interrupts on machines
with many logical cpus and default adapter settings:
Insufficient resources for 12 XDP event queues (24 other channels, max 32)
Which in turn triggers EINVAL on XDP processing:
sfc 0000:86:00.0 ext0: XDP TX failed (-22)
Signed-off-by: Ivan Babrou <ivan@cloudflare.com>
---
drivers/net/ethernet/sfc/efx_channels.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/sfc/efx_channels.c b/drivers/net/ethernet/sfc/efx_channels.c
index a4a626e9cd9a..1bfeee283ea9 100644
--- a/drivers/net/ethernet/sfc/efx_channels.c
+++ b/drivers/net/ethernet/sfc/efx_channels.c
@@ -17,6 +17,7 @@
#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
@@ -137,6 +138,7 @@ 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;
@@ -149,9 +151,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, EFX_MAX_TXQ_PER_CHANNEL);
+ n_xdp_ev = DIV_ROUND_UP(n_xdp_tx, tx_per_ev);
vec_count = pci_msix_vec_count(efx->pci_dev);
if (vec_count < 0)
--
2.29.2
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH net-next] sfc: reduce the number of requested xdp ev queues
2021-01-20 21:27 Ivan Babrou
@ 2021-01-21 16:10 ` Edward Cree
2021-01-21 17:11 ` Jesper Dangaard Brouer
` (2 subsequent siblings)
3 siblings, 0 replies; 15+ messages in thread
From: Edward Cree @ 2021-01-21 16:10 UTC (permalink / raw)
To: Ivan Babrou, netdev
Cc: linux-kernel, bpf, kernel-team, Martin Habets, David S. Miller,
Jakub Kicinski, Alexei Starovoitov, Daniel Borkmann,
Jesper Dangaard Brouer, John Fastabend
On 20/01/2021 21:27, Ivan Babrou wrote:
> Without this change the driver tries to allocate too many queues,
> breaching the number of available msi-x interrupts on machines
> with many logical cpus and default adapter settings:
>
> Insufficient resources for 12 XDP event queues (24 other channels, max 32)
>
> Which in turn triggers EINVAL on XDP processing:
>
> sfc 0000:86:00.0 ext0: XDP TX failed (-22)
>
> Signed-off-by: Ivan Babrou <ivan@cloudflare.com>
Acked-by: Edward Cree <ecree.xilinx@gmail.com>
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH net-next] sfc: reduce the number of requested xdp ev queues
2021-01-20 21:27 Ivan Babrou
2021-01-21 16:10 ` Edward Cree
@ 2021-01-21 17:11 ` Jesper Dangaard Brouer
2021-01-21 17:14 ` Maciej Fijalkowski
2021-01-23 3:30 ` patchwork-bot+netdevbpf
2021-02-06 10:43 ` Martin Habets
3 siblings, 1 reply; 15+ messages in thread
From: Jesper Dangaard Brouer @ 2021-01-21 17:11 UTC (permalink / raw)
To: Ivan Babrou
Cc: netdev, linux-kernel, bpf, kernel-team, Edward Cree,
Martin Habets, David S. Miller, Jakub Kicinski,
Alexei Starovoitov, Daniel Borkmann, Jesper Dangaard Brouer,
John Fastabend
On Wed, 20 Jan 2021 13:27:59 -0800
Ivan Babrou <ivan@cloudflare.com> wrote:
> Without this change the driver tries to allocate too many queues,
> breaching the number of available msi-x interrupts on machines
> with many logical cpus and default adapter settings:
>
> Insufficient resources for 12 XDP event queues (24 other channels, max 32)
>
> Which in turn triggers EINVAL on XDP processing:
>
> sfc 0000:86:00.0 ext0: XDP TX failed (-22)
>
> Signed-off-by: Ivan Babrou <ivan@cloudflare.com>
> ---
I guess the patch is good in itself due to available msi-x interrupts.
Per earlier discussion: What will happen if a CPU with an ID higher
than available XDP TX-queues redirect a packet out this driver?
> drivers/net/ethernet/sfc/efx_channels.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/sfc/efx_channels.c b/drivers/net/ethernet/sfc/efx_channels.c
> index a4a626e9cd9a..1bfeee283ea9 100644
> --- a/drivers/net/ethernet/sfc/efx_channels.c
> +++ b/drivers/net/ethernet/sfc/efx_channels.c
> @@ -17,6 +17,7 @@
> #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
> @@ -137,6 +138,7 @@ 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;
>
> @@ -149,9 +151,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, EFX_MAX_TXQ_PER_CHANNEL);
> + n_xdp_ev = DIV_ROUND_UP(n_xdp_tx, tx_per_ev);
>
> vec_count = pci_msix_vec_count(efx->pci_dev);
> if (vec_count < 0)
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Principal Kernel Engineer at Red Hat
LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH net-next] sfc: reduce the number of requested xdp ev queues
2021-01-21 17:11 ` Jesper Dangaard Brouer
@ 2021-01-21 17:14 ` Maciej Fijalkowski
0 siblings, 0 replies; 15+ messages in thread
From: Maciej Fijalkowski @ 2021-01-21 17:14 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: Ivan Babrou, netdev, linux-kernel, bpf, kernel-team, Edward Cree,
Martin Habets, David S. Miller, Jakub Kicinski,
Alexei Starovoitov, Daniel Borkmann, Jesper Dangaard Brouer,
John Fastabend
On Thu, Jan 21, 2021 at 06:11:30PM +0100, Jesper Dangaard Brouer wrote:
> On Wed, 20 Jan 2021 13:27:59 -0800
> Ivan Babrou <ivan@cloudflare.com> wrote:
>
> > Without this change the driver tries to allocate too many queues,
> > breaching the number of available msi-x interrupts on machines
> > with many logical cpus and default adapter settings:
> >
> > Insufficient resources for 12 XDP event queues (24 other channels, max 32)
> >
> > Which in turn triggers EINVAL on XDP processing:
> >
> > sfc 0000:86:00.0 ext0: XDP TX failed (-22)
Please mention in commit message *how* you are addressing/fixing this
issue.
> >
> > Signed-off-by: Ivan Babrou <ivan@cloudflare.com>
> > ---
>
> I guess the patch is good in itself due to available msi-x interrupts.
>
> Per earlier discussion: What will happen if a CPU with an ID higher
> than available XDP TX-queues redirect a packet out this driver?
+1 on that question
>
>
> > drivers/net/ethernet/sfc/efx_channels.c | 6 ++++--
> > 1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/sfc/efx_channels.c b/drivers/net/ethernet/sfc/efx_channels.c
> > index a4a626e9cd9a..1bfeee283ea9 100644
> > --- a/drivers/net/ethernet/sfc/efx_channels.c
> > +++ b/drivers/net/ethernet/sfc/efx_channels.c
> > @@ -17,6 +17,7 @@
> > #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
> > @@ -137,6 +138,7 @@ 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;
> >
> > @@ -149,9 +151,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, EFX_MAX_TXQ_PER_CHANNEL);
> > + n_xdp_ev = DIV_ROUND_UP(n_xdp_tx, tx_per_ev);
> >
> > vec_count = pci_msix_vec_count(efx->pci_dev);
> > if (vec_count < 0)
>
>
>
> --
> Best regards,
> Jesper Dangaard Brouer
> MSc.CS, Principal Kernel Engineer at Red Hat
> LinkedIn: http://www.linkedin.com/in/brouer
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH net-next] sfc: reduce the number of requested xdp ev queues
2021-01-20 21:27 Ivan Babrou
2021-01-21 16:10 ` Edward Cree
2021-01-21 17:11 ` Jesper Dangaard Brouer
@ 2021-01-23 3:30 ` patchwork-bot+netdevbpf
2021-02-06 10:43 ` Martin Habets
3 siblings, 0 replies; 15+ messages in thread
From: patchwork-bot+netdevbpf @ 2021-01-23 3:30 UTC (permalink / raw)
To: Ivan Babrou
Cc: netdev, linux-kernel, bpf, kernel-team, ecree.xilinx,
habetsm.xilinx, davem, kuba, ast, daniel, hawk, john.fastabend
Hello:
This patch was applied to netdev/net-next.git (refs/heads/master):
On Wed, 20 Jan 2021 13:27:59 -0800 you wrote:
> Without this change the driver tries to allocate too many queues,
> breaching the number of available msi-x interrupts on machines
> with many logical cpus and default adapter settings:
>
> Insufficient resources for 12 XDP event queues (24 other channels, max 32)
>
> Which in turn triggers EINVAL on XDP processing:
>
> [...]
Here is the summary with links:
- [net-next] sfc: reduce the number of requested xdp ev queues
https://git.kernel.org/netdev/net-next/c/e26ca4b53582
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH net-next] sfc: reduce the number of requested xdp ev queues
2021-01-20 21:27 Ivan Babrou
` (2 preceding siblings ...)
2021-01-23 3:30 ` patchwork-bot+netdevbpf
@ 2021-02-06 10:43 ` Martin Habets
3 siblings, 0 replies; 15+ messages in thread
From: Martin Habets @ 2021-02-06 10:43 UTC (permalink / raw)
To: Ivan Babrou
Cc: netdev, linux-kernel, bpf, kernel-team, Edward Cree,
David S. Miller, Jakub Kicinski, Alexei Starovoitov,
Daniel Borkmann, Jesper Dangaard Brouer, John Fastabend
On Wed, Jan 20, 2021 at 01:27:59PM -0800, Ivan Babrou wrote:
> Without this change the driver tries to allocate too many queues,
> breaching the number of available msi-x interrupts on machines
> with many logical cpus and default adapter settings:
>
> Insufficient resources for 12 XDP event queues (24 other channels, max 32)
>
> Which in turn triggers EINVAL on XDP processing:
>
> sfc 0000:86:00.0 ext0: XDP TX failed (-22)
>
> Signed-off-by: Ivan Babrou <ivan@cloudflare.com>
Acked-by: Martin Habets <habetsm.xilinx@gmail.com>
> ---
> drivers/net/ethernet/sfc/efx_channels.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/sfc/efx_channels.c b/drivers/net/ethernet/sfc/efx_channels.c
> index a4a626e9cd9a..1bfeee283ea9 100644
> --- a/drivers/net/ethernet/sfc/efx_channels.c
> +++ b/drivers/net/ethernet/sfc/efx_channels.c
> @@ -17,6 +17,7 @@
> #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
> @@ -137,6 +138,7 @@ 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;
>
> @@ -149,9 +151,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, EFX_MAX_TXQ_PER_CHANNEL);
> + n_xdp_ev = DIV_ROUND_UP(n_xdp_tx, tx_per_ev);
>
> vec_count = pci_msix_vec_count(efx->pci_dev);
> if (vec_count < 0)
> --
> 2.29.2
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2021-02-06 10:45 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-15 1:29 [PATCH net-next] sfc: reduce the number of requested xdp ev queues Ivan Babrou
2020-12-15 9:43 ` Jesper Dangaard Brouer
2020-12-15 18:49 ` Edward Cree
2020-12-16 8:45 ` Jesper Dangaard Brouer
2020-12-16 23:12 ` Edward Cree
2020-12-16 8:18 ` Martin Habets
2020-12-17 18:14 ` Jakub Kicinski
2021-01-19 23:43 ` Ivan Babrou
2021-01-20 0:31 ` Jakub Kicinski
2021-01-20 21:27 Ivan Babrou
2021-01-21 16:10 ` Edward Cree
2021-01-21 17:11 ` Jesper Dangaard Brouer
2021-01-21 17:14 ` Maciej Fijalkowski
2021-01-23 3:30 ` patchwork-bot+netdevbpf
2021-02-06 10:43 ` Martin Habets
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).