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 <[email protected]>
---
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, Mar 21, 2023 at 8:15 AM Faicker Mo <[email protected]> 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 <[email protected]>
> ---
> 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
>
When tx from the net_failover device, the actual tx queue number is the salve device.
The ndo_select_queue of net_failover device returns the txq which is the primary device txq
if the primary device is OK.
This number may be bigger than the default 16 of the net_failover device.
A warning will be reported in netdev_cap_txqueue which device is the net_failover.
From: Pavan Chebbi <[email protected]>
Date: 2023-03-21 13:11:52
To:Faicker Mo <[email protected]>
cc: Sridhar Samudrala <[email protected]>,"David S. Miller" <[email protected]>,Eric Dumazet <[email protected]>,Jakub Kicinski <[email protected]>,Paolo Abeni <[email protected]>,[email protected],[email protected]
Subject: Re: [PATCH] net/net_failover: fix queue exceeding warning>On Tue, Mar 21, 2023 at 8:15 AM Faicker Mo <[email protected]> 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 <[email protected]>
>> ---
>> 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, Mar 21, 2023 at 11:17 AM Faicker Mo <[email protected]> wrote:
>
> When tx from the net_failover device, the actual tx queue number is the salve device.
Then why is primary OK..
> The ndo_select_queue of net_failover device returns the txq which is the primary device txq
> if the primary device is OK.
This is what is done in all the functions. I don't think there is a problem.
Not sure if there is an issue I am not getting, at least with the description.
I will let the maintainer take the call. Thanks.
> This number may be bigger than the default 16 of the net_failover device.
> A warning will be reported in netdev_cap_txqueue which device is the net_failover.
>
>
> From: Pavan Chebbi <[email protected]>
> Date: 2023-03-21 13:11:52
> To:Faicker Mo <[email protected]>
> cc: Sridhar Samudrala <[email protected]>,"David S. Miller" <[email protected]>,Eric Dumazet <[email protected]>,Jakub Kicinski <[email protected]>,Paolo Abeni <[email protected]>,[email protected],[email protected]
> Subject: Re: [PATCH] net/net_failover: fix queue exceeding warning>On Tue, Mar 21, 2023 at 8:15 AM Faicker Mo <[email protected]> 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 <[email protected]>
> >> ---
> >> 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 <[email protected]>
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 <[email protected]>
Date: 2023-03-22 19:40:44
To: Faicker Mo <[email protected]>
Cc: Sridhar Samudrala <[email protected]>,"David S. Miller" <[email protected]>,Eric Dumazet <[email protected]>,Jakub Kicinski <[email protected]>,[email protected],[email protected]
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 <[email protected]>
>
>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
>
>
>