If the primary device queue number is bigger than the default 16,
there is a warning about the queue exceeding when tx from the
net_failover device.

Signed-off-by: Faicker Mo <faicker.mo@ucloud.cn>
---
 drivers/net/net_failover.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/net/net_failover.c b/drivers/net/net_failover.c
index 7a28e082436e..d0c916a53d7c 100644
--- a/drivers/net/net_failover.c
+++ b/drivers/net/net_failover.c
@@ -130,14 +130,10 @@ static u16 net_failover_select_queue(struct net_device *dev,
 			txq = ops->ndo_select_queue(primary_dev, skb, sb_dev);
 		else
 			txq = netdev_pick_tx(primary_dev, skb, NULL);
-
-		qdisc_skb_cb(skb)->slave_dev_queue_mapping = skb->queue_mapping;
-
-		return txq;
+	} else {
+		txq = skb_rx_queue_recorded(skb) ? skb_get_rx_queue(skb) : 0;
 	}
 
-	txq = skb_rx_queue_recorded(skb) ? skb_get_rx_queue(skb) : 0;
-
 	/* Save the original txq to restore before passing to the driver */
 	qdisc_skb_cb(skb)->slave_dev_queue_mapping = skb->queue_mapping;
 
-- 
2.39.1

On Tue, 2023-03-21 at 10:29 +0800, Faicker Mo wrote:
> If the primary device queue number is bigger than the default 16,
> there is a warning about the queue exceeding when tx from the
> net_failover device.
> 
> Signed-off-by: Faicker Mo <faicker.mo@ucloud.cn>

This looks like a fixes, so it should include at least a fixes tag.

More importantly a longer/clearer description of the issue is needed,
including the warning backtrace.

I think this warning:

https://elixir.bootlin.com/linux/latest/source/include/linux/netdevice.h#L3542

should not be ignored/silenced: it's telling that the running
configuration is not using a number of the available tx queues, which
is possibly not the thing you want.

Instead the failover device could use an higher number of tx queues and
eventually set real_num_tx_queues equal to the primary_dev when the
latter is enslaved.

Thanks,

Paolo

Thanks. I will send the v2 fix later.

Yes, the better method is to let the failover device folllows the primary dev
and remove the warning, but more work need to be done.


From: Paolo Abeni <pabeni@redhat.com>
Date: 2023-03-22 19:40:44
To:  Faicker Mo <faicker.mo@ucloud.cn>
Cc:  Sridhar Samudrala <sridhar.samudrala@intel.com>,"David S. Miller" <davem@davemloft.net>,Eric Dumazet <edumazet@google.com>,Jakub Kicinski <kuba@kernel.org>,netdev@vger.kernel.org,linux-kernel@vger.kernel.org
Subject: Re: [PATCH] net/net_failover: fix queue exceeding warning>On Tue, 2023-03-21 at 10:29 +0800, Faicker Mo wrote:
>> If the primary device queue number is bigger than the default 16,
>> there is a warning about the queue exceeding when tx from the
>> net_failover device.
>> 
>> Signed-off-by: Faicker Mo <faicker.mo@ucloud.cn>
>
>This looks like a fixes, so it should include at least a fixes tag.
>
>More importantly a longer/clearer description of the issue is needed,
>including the warning backtrace.
>
>I think this warning:
>
>https://elixir.bootlin.com/linux/latest/source/include/linux/netdevice.h#L3542
>
>should not be ignored/silenced: it's telling that the running
>configuration is not using a number of the available tx queues, which
>is possibly not the thing you want.
>
>Instead the failover device could use an higher number of tx queues and
>eventually set real_num_tx_queues equal to the primary_dev when the
>latter is enslaved.
>
>Thanks,
>
>Paolo
>
>
>

On Tue, Mar 21, 2023 at 8:15 AM Faicker Mo <faicker.mo@ucloud.cn> wrote:
>
> If the primary device queue number is bigger than the default 16,
> there is a warning about the queue exceeding when tx from the
> net_failover device.
>

Can you describe the issue more? If the net device has not implemented
its own selection then netdev_pick_tx should take care of the
real_num_tx_queues.
Is that not happening?

> Signed-off-by: Faicker Mo <faicker.mo@ucloud.cn>
> ---
>  drivers/net/net_failover.c | 8 ++------
>  1 file changed, 2 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/net/net_failover.c b/drivers/net/net_failover.c
> index 7a28e082436e..d0c916a53d7c 100644
> --- a/drivers/net/net_failover.c
> +++ b/drivers/net/net_failover.c
> @@ -130,14 +130,10 @@ static u16 net_failover_select_queue(struct net_device *dev,
>                         txq = ops->ndo_select_queue(primary_dev, skb, sb_dev);
>                 else
>                         txq = netdev_pick_tx(primary_dev, skb, NULL);
> -
> -               qdisc_skb_cb(skb)->slave_dev_queue_mapping = skb->queue_mapping;
> -
> -               return txq;
> +       } else {
> +               txq = skb_rx_queue_recorded(skb) ? skb_get_rx_queue(skb) : 0;
>         }
>
> -       txq = skb_rx_queue_recorded(skb) ? skb_get_rx_queue(skb) : 0;
> -
>         /* Save the original txq to restore before passing to the driver */
>         qdisc_skb_cb(skb)->slave_dev_queue_mapping = skb->queue_mapping;
>
> --
> 2.39.1
>