[patch net 0/2] mlxsw: couple of fixes

[patch net 0/2] mlxsw: couple of fixes

[Jiri Pirko]

From: Jiri Pirko <jiri@mellanox.com>

Couple of fixes from Ido.

Ido Schimmel (2):
  mlxsw: spectrum: Always decrement bridge's ref count
  mlxsw: pci: Correctly determine if descriptor queue is full

 drivers/net/ethernet/mellanox/mlxsw/pci.c      | 2 +-
 drivers/net/ethernet/mellanox/mlxsw/spectrum.c | 4 +---
 2 files changed, 2 insertions(+), 4 deletions(-)

-- 
2.5.0

[patch net 1/2] mlxsw: spectrum: Always decrement bridge's ref count

[Jiri Pirko]

From: Ido Schimmel <idosch@mellanox.com>

Since we only support one VLAN filtering bridge we need to associate a
reference count with it, so that when the last port netdev leaves it, we
would know that a different bridge can be offloaded to hardware.

When a LAG device is memeber in a bridge and port netdevs are leaving
the LAG, we should always decrement the bridge's reference count, as it's
incremented for any port in the LAG.

Fixes: 4dc236c31733 ("mlxsw: spectrum: Handle port leaving LAG while bridged")
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlxsw/spectrum.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
index 09ce451..a94daa8 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
@@ -2358,9 +2358,7 @@ static int mlxsw_sp_port_lag_leave(struct mlxsw_sp_port *mlxsw_sp_port,
 	if (mlxsw_sp_port->bridged) {
 		mlxsw_sp_port_active_vlans_del(mlxsw_sp_port);
 		mlxsw_sp_port_bridge_leave(mlxsw_sp_port, false);
-
-		if (lag->ref_count == 1)
-			mlxsw_sp_master_bridge_dec(mlxsw_sp, NULL);
+		mlxsw_sp_master_bridge_dec(mlxsw_sp, NULL);
 	}
 
 	if (lag->ref_count == 1) {
-- 
2.5.0

[patch net 2/2] mlxsw: pci: Correctly determine if descriptor queue is full

[Jiri Pirko]

From: Ido Schimmel <idosch@mellanox.com>

The descriptor queues for sending (SDQs) and receiving (RDQs) packets
are managed by two counters - producer and consumer - which are both
16-bit in size. A queue is considered full when the difference between
the two equals the queue's maximum number of descriptors.

However, if the producer counter overflows, then it's possible for the
full queue check to fail, as it doesn't take the overflow into account.
In such a case, descriptors already passed to the device - but for which
a completion has yet to be posted - will be overwritten, thereby causing
undefined behavior. The above can be achieved under heavy load (~30
netperf instances).

Fix that by casting the substraction result to u16, preventing it from
being treated as a signed integer.

Fixes: eda6500a987a ("mlxsw: Add PCI bus implementation")
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlxsw/pci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/pci.c b/drivers/net/ethernet/mellanox/mlxsw/pci.c
index c071077..7992c55 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/pci.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/pci.c
@@ -215,7 +215,7 @@ mlxsw_pci_queue_elem_info_producer_get(struct mlxsw_pci_queue *q)
 {
 	int index = q->producer_counter & (q->count - 1);
 
-	if ((q->producer_counter - q->consumer_counter) == q->count)
+	if ((u16) (q->producer_counter - q->consumer_counter) == q->count)
 		return NULL;
 	return mlxsw_pci_queue_elem_info_get(q, index);
 }
-- 
2.5.0

Re: [patch net 2/2] mlxsw: pci: Correctly determine if descriptor queue is full

[Sergei Shtylyov]

Hello.

On 3/7/2016 11:24 AM, Jiri Pirko wrote:

> From: Ido Schimmel <idosch@mellanox.com>
>
> The descriptor queues for sending (SDQs) and receiving (RDQs) packets
> are managed by two counters - producer and consumer - which are both
> 16-bit in size. A queue is considered full when the difference between
> the two equals the queue's maximum number of descriptors.
>
> However, if the producer counter overflows, then it's possible for the
> full queue check to fail, as it doesn't take the overflow into account.
> In such a case, descriptors already passed to the device - but for which
> a completion has yet to be posted - will be overwritten, thereby causing
> undefined behavior. The above can be achieved under heavy load (~30
> netperf instances).
>
> Fix that by casting the substraction result to u16, preventing it from

    Subtraction.

> being treated as a signed integer.
>
> Fixes: eda6500a987a ("mlxsw: Add PCI bus implementation")
> Signed-off-by: Ido Schimmel <idosch@mellanox.com>
> Signed-off-by: Jiri Pirko <jiri@mellanox.com>

[...]

MBR, Sergei

Re: [patch net 2/2] mlxsw: pci: Correctly determine if descriptor queue is full

[Ido Schimmel]

Mon, Mar 07, 2016 at 04:04:59PM IST, sergei.shtylyov@cogentembedded.com wrote:
>Hello.
>
>On 3/7/2016 11:24 AM, Jiri Pirko wrote:
>
>> From: Ido Schimmel <idosch@mellanox.com>
>>
>> The descriptor queues for sending (SDQs) and receiving (RDQs) packets
>> are managed by two counters - producer and consumer - which are both
>> 16-bit in size. A queue is considered full when the difference between
>> the two equals the queue's maximum number of descriptors.
>>
>> However, if the producer counter overflows, then it's possible for the
>> full queue check to fail, as it doesn't take the overflow into account.
>> In such a case, descriptors already passed to the device - but for which
>> a completion has yet to be posted - will be overwritten, thereby causing
>> undefined behavior. The above can be achieved under heavy load (~30
>> netperf instances).
>>
>> Fix that by casting the substraction result to u16, preventing it from
>
>    Subtraction.

Will fix that in v2.

Thanks!

>
>> being treated as a signed integer.
>>
>> Fixes: eda6500a987a ("mlxsw: Add PCI bus implementation")
>> Signed-off-by: Ido Schimmel <idosch@mellanox.com>
>> Signed-off-by: Jiri Pirko <jiri@mellanox.com>
>
>[...]
>
>MBR, Sergei
>