Netdev Archive on lore.kernel.org
help / color / mirror / Atom feed
From: "Íñigo Huguet" <ihuguet@redhat.com>
To: Sasha Levin <sashal@kernel.org>
Cc: linux-kernel@vger.kernel.org, stable@vger.kernel.org,
	"David S . Miller" <davem@davemloft.net>,
	netdev@vger.kernel.org, bpf@vger.kernel.org
Subject: Re: [PATCH AUTOSEL 5.13 09/19] sfc: ensure correct number of XDP queues
Date: Fri, 23 Jul 2021 12:12:30 +0200	[thread overview]
Message-ID: <CACT4oucVa5Lw538M2TEc1ZNU4mUZms+9fiTxw-p5-7J7xcM+kQ@mail.gmail.com> (raw)
In-Reply-To: <20210723035721.531372-9-sashal@kernel.org>

Hi,

On Fri, Jul 23, 2021 at 5:57 AM Sasha Levin <sashal@kernel.org> wrote:
>
> From: Íñigo Huguet <ihuguet@redhat.com>
>
> [ Upstream commit 788bc000d4c2f25232db19ab3a0add0ba4e27671 ]

Applying this commit alone could be problematic, leading even to
kernel crash in certain situations.

The real fix is the previous one of the series:
f43a24f446da sfc: fix lack of XDP TX queues - error XDP TX failed (-22)

This one can be applied too, but not really a must-have.

> Commit 99ba0ea616aa ("sfc: adjust efx->xdp_tx_queue_count with the real
> number of initialized queues") intended to fix a problem caused by a
> round up when calculating the number of XDP channels and queues.
> However, this was not the real problem. The real problem was that the
> number of XDP TX queues had been reduced to half in
> commit e26ca4b53582 ("sfc: reduce the number of requested xdp ev queues"),
> but the variable xdp_tx_queue_count had remained the same.
>
> Once the correct number of XDP TX queues is created again in the
> previous patch of this series, this also can be reverted since the error
> doesn't actually exist.
>
> Only in the case that there is a bug in the code we can have different
> values in xdp_queue_number and efx->xdp_tx_queue_count. Because of this,
> and per Edward Cree's suggestion, I add instead a WARN_ON to catch if it
> happens again in the future.
>
> Note that the number of allocated queues can be higher than the number
> of used ones due to the round up, as explained in the existing comment
> in the code. That's why we also have to stop increasing xdp_queue_number
> beyond efx->xdp_tx_queue_count.
>
> Signed-off-by: Íñigo Huguet <ihuguet@redhat.com>
> Signed-off-by: David S. Miller <davem@davemloft.net>
> Signed-off-by: Sasha Levin <sashal@kernel.org>
> ---
>  drivers/net/ethernet/sfc/efx_channels.c | 15 ++++++++-------
>  1 file changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/net/ethernet/sfc/efx_channels.c b/drivers/net/ethernet/sfc/efx_channels.c
> index a3ca406a3561..bea0b27baf4b 100644
> --- a/drivers/net/ethernet/sfc/efx_channels.c
> +++ b/drivers/net/ethernet/sfc/efx_channels.c
> @@ -891,18 +891,20 @@ int efx_set_channels(struct efx_nic *efx)
>                         if (efx_channel_is_xdp_tx(channel)) {
>                                 efx_for_each_channel_tx_queue(tx_queue, channel) {
>                                         tx_queue->queue = next_queue++;
> -                                       netif_dbg(efx, drv, efx->net_dev, "Channel %u TXQ %u is XDP %u, HW %u\n",
> -                                                 channel->channel, tx_queue->label,
> -                                                 xdp_queue_number, tx_queue->queue);
> +
>                                         /* We may have a few left-over XDP TX
>                                          * queues owing to xdp_tx_queue_count
>                                          * not dividing evenly by EFX_MAX_TXQ_PER_CHANNEL.
>                                          * We still allocate and probe those
>                                          * TXQs, but never use them.
>                                          */
> -                                       if (xdp_queue_number < efx->xdp_tx_queue_count)
> +                                       if (xdp_queue_number < efx->xdp_tx_queue_count) {
> +                                               netif_dbg(efx, drv, efx->net_dev, "Channel %u TXQ %u is XDP %u, HW %u\n",
> +                                                         channel->channel, tx_queue->label,
> +                                                         xdp_queue_number, tx_queue->queue);
>                                                 efx->xdp_tx_queues[xdp_queue_number] = tx_queue;
> -                                       xdp_queue_number++;
> +                                               xdp_queue_number++;
> +                                       }
>                                 }
>                         } else {
>                                 efx_for_each_channel_tx_queue(tx_queue, channel) {
> @@ -914,8 +916,7 @@ int efx_set_channels(struct efx_nic *efx)
>                         }
>                 }
>         }
> -       if (xdp_queue_number)
> -               efx->xdp_tx_queue_count = xdp_queue_number;
> +       WARN_ON(xdp_queue_number != efx->xdp_tx_queue_count);
>
>         rc = netif_set_real_num_tx_queues(efx->net_dev, efx->n_tx_channels);
>         if (rc)
> --
> 2.30.2
>

-- 
Íñigo Huguet


  reply	other threads:[~2021-07-23 10:13 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-23  3:57 [PATCH AUTOSEL 5.13 01/19] net/802/mrp: fix memleak in mrp_request_join() Sasha Levin
2021-07-23  3:57 ` [PATCH AUTOSEL 5.13 02/19] net/802/garp: fix memleak in garp_request_join() Sasha Levin
2021-07-23  3:57 ` [PATCH AUTOSEL 5.13 03/19] net: annotate data race around sk_ll_usec Sasha Levin
2021-07-23  3:57 ` [PATCH AUTOSEL 5.13 04/19] sctp: move 198 addresses from unusable to private scope Sasha Levin
2021-07-23  3:57 ` [PATCH AUTOSEL 5.13 07/19] ipv6: allocate enough headroom in ip6_finish_output2() Sasha Levin
2021-07-23  6:03   ` Vasily Averin
2021-07-28 14:07     ` Sasha Levin
2021-07-23  3:57 ` [PATCH AUTOSEL 5.13 09/19] sfc: ensure correct number of XDP queues Sasha Levin
2021-07-23 10:12   ` Íñigo Huguet [this message]
2021-07-23 10:18     ` Íñigo Huguet

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=CACT4oucVa5Lw538M2TEc1ZNU4mUZms+9fiTxw-p5-7J7xcM+kQ@mail.gmail.com \
    --to=ihuguet@redhat.com \
    --cc=bpf@vger.kernel.org \
    --cc=davem@davemloft.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=sashal@kernel.org \
    --cc=stable@vger.kernel.org \
    --subject='Re: [PATCH AUTOSEL 5.13 09/19] sfc: ensure correct number of XDP queues' \
    /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

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).