diff mbox series

[net] net: systemport: Fixed queue mapping in internal ring map

Message ID 20200116210859.7376-1-f.fainelli@gmail.com
State Accepted
Delegated to: David Miller
Headers show
Series [net] net: systemport: Fixed queue mapping in internal ring map | expand

Commit Message

Florian Fainelli Jan. 16, 2020, 9:08 p.m. UTC
We would not be transmitting using the correct SYSTEMPORT transmit queue
during ndo_select_queue() which looks up the internal TX ring map
because while establishing the mapping we would be off by 4, so for
instance, when we populate switch port mappings we would be doing:

switch port 0, queue 0 -> ring index #0
switch port 0, queue 1 -> ring index #1
...
switch port 0, queue 3 -> ring index #3
switch port 1, queue 0 -> ring index #8 (4 + 4 * 1)
...

instead of using ring index #4. This would cause our ndo_select_queue()
to use the fallback queue mechanism which would pick up an incorrect
ring for that switch port. Fix this by using the correct switch queue
number instead of SYSTEMPORT queue number.

Fixes: 3ed67ca243b3 ("net: systemport: Simplify queue mapping logic")
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/net/ethernet/broadcom/bcmsysport.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

Comments

David Miller Jan. 17, 2020, 12:32 p.m. UTC | #1
From: Florian Fainelli <f.fainelli@gmail.com>
Date: Thu, 16 Jan 2020 13:08:58 -0800

> We would not be transmitting using the correct SYSTEMPORT transmit queue
> during ndo_select_queue() which looks up the internal TX ring map
> because while establishing the mapping we would be off by 4, so for
> instance, when we populate switch port mappings we would be doing:
> 
> switch port 0, queue 0 -> ring index #0
> switch port 0, queue 1 -> ring index #1
> ...
> switch port 0, queue 3 -> ring index #3
> switch port 1, queue 0 -> ring index #8 (4 + 4 * 1)
> ...
> 
> instead of using ring index #4. This would cause our ndo_select_queue()
> to use the fallback queue mechanism which would pick up an incorrect
> ring for that switch port. Fix this by using the correct switch queue
> number instead of SYSTEMPORT queue number.
> 
> Fixes: 3ed67ca243b3 ("net: systemport: Simplify queue mapping logic")
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>

Applied, but I had to fix the SHA1-ID of the Fixes tag to be:

    Fixes: 25c440704661 ("net: systemport: Simplify queue mapping logic")

Thanks.
Florian Fainelli Jan. 19, 2020, 5:48 p.m. UTC | #2
On 1/17/2020 4:32 AM, David Miller wrote:
> From: Florian Fainelli <f.fainelli@gmail.com>
> Date: Thu, 16 Jan 2020 13:08:58 -0800
> 
>> We would not be transmitting using the correct SYSTEMPORT transmit queue
>> during ndo_select_queue() which looks up the internal TX ring map
>> because while establishing the mapping we would be off by 4, so for
>> instance, when we populate switch port mappings we would be doing:
>>
>> switch port 0, queue 0 -> ring index #0
>> switch port 0, queue 1 -> ring index #1
>> ...
>> switch port 0, queue 3 -> ring index #3
>> switch port 1, queue 0 -> ring index #8 (4 + 4 * 1)
>> ...
>>
>> instead of using ring index #4. This would cause our ndo_select_queue()
>> to use the fallback queue mechanism which would pick up an incorrect
>> ring for that switch port. Fix this by using the correct switch queue
>> number instead of SYSTEMPORT queue number.
>>
>> Fixes: 3ed67ca243b3 ("net: systemport: Simplify queue mapping logic")
>> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> 
> Applied, but I had to fix the SHA1-ID of the Fixes tag to be:
> 
>     Fixes: 25c440704661 ("net: systemport: Simplify queue mapping logic")

Just like the other patch what happened is that the check_fixes script
from Stephen was able to resolve this incorrect SHA1 because I have both
the Broadcom STB downstream remote configured as well as upstream and
the script was not yet updated to make sure this was an object that can
be reached in the upstream remotes (yours, Linus...). This should be
corrected now, sorry about that.
diff mbox series

Patch

diff --git a/drivers/net/ethernet/broadcom/bcmsysport.c b/drivers/net/ethernet/broadcom/bcmsysport.c
index 825af709708e..d6b1a153f9df 100644
--- a/drivers/net/ethernet/broadcom/bcmsysport.c
+++ b/drivers/net/ethernet/broadcom/bcmsysport.c
@@ -2323,7 +2323,7 @@  static int bcm_sysport_map_queues(struct notifier_block *nb,
 		ring->switch_queue = qp;
 		ring->switch_port = port;
 		ring->inspect = true;
-		priv->ring_map[q + port * num_tx_queues] = ring;
+		priv->ring_map[qp + port * num_tx_queues] = ring;
 		qp++;
 	}
 
@@ -2338,7 +2338,7 @@  static int bcm_sysport_unmap_queues(struct notifier_block *nb,
 	struct net_device *slave_dev;
 	unsigned int num_tx_queues;
 	struct net_device *dev;
-	unsigned int q, port;
+	unsigned int q, qp, port;
 
 	priv = container_of(nb, struct bcm_sysport_priv, dsa_notifier);
 	if (priv->netdev != info->master)
@@ -2364,7 +2364,8 @@  static int bcm_sysport_unmap_queues(struct notifier_block *nb,
 			continue;
 
 		ring->inspect = false;
-		priv->ring_map[q + port * num_tx_queues] = NULL;
+		qp = ring->switch_queue;
+		priv->ring_map[qp + port * num_tx_queues] = NULL;
 	}
 
 	return 0;