[PATCH net-next 0/7] Mellanox Ethernet driver updates 2013-01-16

[PATCH net-next 0/7] Mellanox Ethernet driver updates 2013-01-16

[Amir Vadai]

Hi Dave,

These are all small bug fixes to the Mellanox mlx4 driver, patches done against
net-next commit 757b8b1 "net_sched: fix qdisc_pkt_len_init()"

Thanks,
Amir

Amir Vadai (2):
  net/mlx4_en: Fix a race when closing TX queue
  net/mlx4_en: Initialize RFS filters lock and list in init_netdev

Aviad Yehezkel (1):
  net/mlx4_en: Fix traffic loss under promiscuous mode

Eugenia Emantayev (3):
  net/mlx4_en: Issue the dump eth statistics command under lock
  net/mlx4_en: Use the correct netif lock on ndo_set_rx_mode
  net/mlx4_en: Set carrier to off when a port is stopped

Jack Morgenstein (1):
  net/mlx4_core: Return proper error code when __mlx4_add_one fails

 drivers/net/ethernet/mellanox/mlx4/en_netdev.c |   56 ++++++++++++++++--------
 drivers/net/ethernet/mellanox/mlx4/en_tx.c     |    9 +++-
 drivers/net/ethernet/mellanox/mlx4/main.c      |    7 ++-
 3 files changed, 51 insertions(+), 21 deletions(-)

-- 
1.7.8.2

[PATCH net-next 1/7] net/mlx4_en: Issue the dump eth statistics command under lock

[Amir Vadai]

From: Eugenia Emantayev <eugenia@mellanox.com>

Performing the DUMP_ETH_STATS firmware command outside the lock leads to kernel
panic when data structures such as RX/TX rings are freed in parallel, e.g when
one changes the mtu or ring sizes.

Signed-off-by: Eugenia Emantayev <eugenia@mellanox.com>
Signed-off-by: Amir Vadai <amirv@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlx4/en_netdev.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
index b467513..b6c645f 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
@@ -977,12 +977,12 @@ static void mlx4_en_do_get_stats(struct work_struct *work)
 	struct mlx4_en_dev *mdev = priv->mdev;
 	int err;
 
-	err = mlx4_en_DUMP_ETH_STATS(mdev, priv->port, 0);
-	if (err)
-		en_dbg(HW, priv, "Could not update stats\n");
-
 	mutex_lock(&mdev->state_lock);
 	if (mdev->device_up) {
+		err = mlx4_en_DUMP_ETH_STATS(mdev, priv->port, 0);
+		if (err)
+			en_dbg(HW, priv, "Could not update stats\n");
+
 		if (priv->port_up)
 			mlx4_en_auto_moderation(priv);
 
-- 
1.7.8.2

[PATCH net-next 2/7] net/mlx4_en: Fix traffic loss under promiscuous mode

[Amir Vadai]

From: Aviad Yehezkel <aviadye@mellanox.com>

When port is stopped and flow steering mode is not device managed: promisc QP
rule wasn't removed from MCG table.
Added code to remove it in all flow steering modes.
In addition, promsic rule removal should be in stop port and not in start
port - moved it accordingly.

Signed-off-by: Aviad Yehezkel <aviadye@mellanox.com>
Signed-off-by: Eugenia Emantayev <eugenia@mellanox.com>
Signed-off-by: Hadar Hen Zion <hadarh@mellanox.com>
Signed-off-by: Amir Vadai <amirv@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlx4/en_netdev.c |   35 +++++++++++++++++------
 1 files changed, 26 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
index b6c645f..bab8cec 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
@@ -1167,15 +1167,6 @@ int mlx4_en_start_port(struct net_device *dev)
 
 	/* Must redo promiscuous mode setup. */
 	priv->flags &= ~(MLX4_EN_FLAG_PROMISC | MLX4_EN_FLAG_MC_PROMISC);
-	if (mdev->dev->caps.steering_mode ==
-	    MLX4_STEERING_MODE_DEVICE_MANAGED) {
-		mlx4_flow_steer_promisc_remove(mdev->dev,
-					       priv->port,
-					       MLX4_FS_PROMISC_UPLINK);
-		mlx4_flow_steer_promisc_remove(mdev->dev,
-					       priv->port,
-					       MLX4_FS_PROMISC_ALL_MULTI);
-	}
 
 	/* Schedule multicast task to populate multicast list */
 	queue_work(mdev->workqueue, &priv->mcast_task);
@@ -1227,6 +1218,32 @@ void mlx4_en_stop_port(struct net_device *dev)
 	/* Set port as not active */
 	priv->port_up = false;
 
+	/* Promsicuous mode */
+	if (mdev->dev->caps.steering_mode ==
+	    MLX4_STEERING_MODE_DEVICE_MANAGED) {
+		priv->flags &= ~(MLX4_EN_FLAG_PROMISC |
+				 MLX4_EN_FLAG_MC_PROMISC);
+		mlx4_flow_steer_promisc_remove(mdev->dev,
+					       priv->port,
+					       MLX4_FS_PROMISC_UPLINK);
+		mlx4_flow_steer_promisc_remove(mdev->dev,
+					       priv->port,
+					       MLX4_FS_PROMISC_ALL_MULTI);
+	} else if (priv->flags & MLX4_EN_FLAG_PROMISC) {
+		priv->flags &= ~MLX4_EN_FLAG_PROMISC;
+
+		/* Disable promiscouos mode */
+		mlx4_unicast_promisc_remove(mdev->dev, priv->base_qpn,
+					    priv->port);
+
+		/* Disable Multicast promisc */
+		if (priv->flags & MLX4_EN_FLAG_MC_PROMISC) {
+			mlx4_multicast_promisc_remove(mdev->dev, priv->base_qpn,
+						      priv->port);
+			priv->flags &= ~MLX4_EN_FLAG_MC_PROMISC;
+		}
+	}
+
 	/* Detach All multicasts */
 	memset(&mc_list[10], 0xff, ETH_ALEN);
 	mc_list[5] = priv->port; /* needed for B0 steering support */
-- 
1.7.8.2

[PATCH net-next 3/7] net/mlx4_en: Use the correct netif lock on ndo_set_rx_mode

[Amir Vadai]

From: Eugenia Emantayev <eugenia@mellanox.com>

The device multicast list is protected by netif_addr_lock_bh in the networking core, we should
use this locking practice in mlx4_en too.

Signed-off-by: Eugenia Emantayev <eugenia@mellanox.com>
Signed-off-by: Amir Vadai <amirv@mellanox.com>
Reviewed-by: Yevgeny Petrilin <yevgenyp@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlx4/en_netdev.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
index bab8cec..805e242 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
@@ -767,9 +767,9 @@ static void mlx4_en_do_set_multicast(struct work_struct *work)
 
 		/* Update multicast list - we cache all addresses so they won't
 		 * change while HW is updated holding the command semaphor */
-		netif_tx_lock_bh(dev);
+		netif_addr_lock_bh(dev);
 		mlx4_en_cache_mclist(dev);
-		netif_tx_unlock_bh(dev);
+		netif_addr_unlock_bh(dev);
 		list_for_each_entry(mclist, &priv->mc_list, list) {
 			mcast_addr = mlx4_en_mac_to_u64(mclist->addr);
 			mlx4_SET_MCAST_FLTR(mdev->dev, priv->port,
-- 
1.7.8.2

[PATCH net-next 4/7] net/mlx4_en: Set carrier to off when a port is stopped

[Amir Vadai]

From: Eugenia Emantayev <eugenia@mellanox.com>

Under heavy CPU load changing ring size/mtu/etc. could result in transmit
timeout, since stop-start port might take more than 10 sec. Set
netif_carrier_off to prevent tx queue transmit timeout.

Signed-off-by: Eugenia Emantayev <eugenia@mellanox.com>
Signed-off-by: Amir Vadai <amirv@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlx4/en_netdev.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
index 805e242..108c4cf 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
@@ -1212,6 +1212,7 @@ void mlx4_en_stop_port(struct net_device *dev)
 
 	/* Synchronize with tx routine */
 	netif_tx_lock_bh(dev);
+	netif_carrier_off(dev);
 	netif_tx_stop_all_queues(dev);
 	netif_tx_unlock_bh(dev);
 
-- 
1.7.8.2

[PATCH net-next 5/7] net/mlx4_core: Return proper error code when __mlx4_add_one fails

[Amir Vadai]

From: Jack Morgenstein <jackm@dev.mellanox.co.il>

Returning 0 (success) when in fact we are aborting the load, leads to kernel
panic when unloading the module. Fix that by returning the actual error code.

Signed-off-by: Jack Morgenstein <jackm@dev.mellanox.co.il>
Signed-off-by: Eugenia Emantayev <eugenia@mellanox.com>
Signed-off-by: Amir Vadai <amirv@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlx4/main.c |    7 +++++--
 1 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx4/main.c b/drivers/net/ethernet/mellanox/mlx4/main.c
index e1bafff..983fd3d 100644
--- a/drivers/net/ethernet/mellanox/mlx4/main.c
+++ b/drivers/net/ethernet/mellanox/mlx4/main.c
@@ -2169,7 +2169,8 @@ slave_start:
 			dev->num_slaves = MLX4_MAX_NUM_SLAVES;
 		else {
 			dev->num_slaves = 0;
-			if (mlx4_multi_func_init(dev)) {
+			err = mlx4_multi_func_init(dev);
+			if (err) {
 				mlx4_err(dev, "Failed to init slave mfunc"
 					 " interface, aborting.\n");
 				goto err_cmd;
@@ -2193,7 +2194,8 @@ slave_start:
 	/* In master functions, the communication channel must be initialized
 	 * after obtaining its address from fw */
 	if (mlx4_is_master(dev)) {
-		if (mlx4_multi_func_init(dev)) {
+		err = mlx4_multi_func_init(dev);
+		if (err) {
 			mlx4_err(dev, "Failed to init master mfunc"
 				 "interface, aborting.\n");
 			goto err_close;
@@ -2210,6 +2212,7 @@ slave_start:
 	mlx4_enable_msi_x(dev);
 	if ((mlx4_is_mfunc(dev)) &&
 	    !(dev->flags & MLX4_FLAG_MSI_X)) {
+		err = -ENOSYS;
 		mlx4_err(dev, "INTx is not supported in multi-function mode."
 			 " aborting.\n");
 		goto err_free_eq;
-- 
1.7.8.2

[PATCH net-next 6/7] net/mlx4_en: Fix a race when closing TX queue

[Amir Vadai]

There is a possible race where the TX completion handler can clean the
entire TX queue between the decision that the queue is full and actually
closing it. To avoid this situation, check again if the queue is really
full, if not, reopen the transmit and continue with sending the packet.

Signed-off-by: Yevgeny Petrilin <yevgenyp@mellanox.com>
Signed-off-by: Eugenia Emantayev <eugenia@mellanox.com>
Signed-off-by: Amir Vadai <amirv@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlx4/en_tx.c |    9 ++++++++-
 1 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx4/en_tx.c b/drivers/net/ethernet/mellanox/mlx4/en_tx.c
index 2b799f4..1d17f5f 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_tx.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_tx.c
@@ -592,7 +592,14 @@ netdev_tx_t mlx4_en_xmit(struct sk_buff *skb, struct net_device *dev)
 		netif_tx_stop_queue(ring->tx_queue);
 		priv->port_stats.queue_stopped++;
 
-		return NETDEV_TX_BUSY;
+		/* Check again whether the queue was cleaned */
+		if (unlikely(((int)(ring->prod - ring->cons)) <=
+				ring->size - HEADROOM - MAX_DESC_TXBBS)) {
+			netif_tx_wake_queue(ring->tx_queue);
+			priv->port_stats.wake_queue++;
+		} else {
+			return NETDEV_TX_BUSY;
+		}
 	}
 
 	/* Track current inflight packets for performance analysis */
-- 
1.7.8.2

[PATCH net-next 7/7] net/mlx4_en: Initialize RFS filters lock and list in init_netdev

[Amir Vadai]

filters_lock might have been used while it was re-initialized.
Moved filters_lock and filters_list initialization to init_netdev instead of
alloc_resources which is called every time the device is configured.

Signed-off-by: Amir Vadai <amirv@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlx4/en_netdev.c |    8 +++++---
 1 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
index 108c4cf..f297d4c 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
@@ -1455,9 +1455,6 @@ int mlx4_en_alloc_resources(struct mlx4_en_priv *priv)
 	priv->dev->rx_cpu_rmap = alloc_irq_cpu_rmap(priv->rx_ring_num);
 	if (!priv->dev->rx_cpu_rmap)
 		goto err;
-
-	INIT_LIST_HEAD(&priv->filters);
-	spin_lock_init(&priv->filters_lock);
 #endif
 
 	return 0;
@@ -1652,6 +1649,11 @@ int mlx4_en_init_netdev(struct mlx4_en_dev *mdev, int port,
 	if (err)
 		goto out;
 
+#ifdef CONFIG_RFS_ACCEL
+	INIT_LIST_HEAD(&priv->filters);
+	spin_lock_init(&priv->filters_lock);
+#endif
+
 	/* Allocate page for receive rings */
 	err = mlx4_alloc_hwq_res(mdev->dev, &priv->res,
 				MLX4_EN_PAGE_SIZE, MLX4_EN_PAGE_SIZE);
-- 
1.7.8.2

Re: [PATCH net-next 4/7] net/mlx4_en: Set carrier to off when a port is stopped

[Ben Hutchings]

On Wed, 2013-01-16 at 17:42 +0200, Amir Vadai wrote:
> From: Eugenia Emantayev <eugenia@mellanox.com>
> 
> Under heavy CPU load changing ring size/mtu/etc. could result in transmit
> timeout, since stop-start port might take more than 10 sec. Set
> netif_carrier_off to prevent tx queue transmit timeout.

A spurious link change can restart L3 auto-configuration (DHCP, SLAAC,
etc.)  netif_device_detach() also inhibits the watchdog and doesn't have
that problem.

Ben.

> Signed-off-by: Eugenia Emantayev <eugenia@mellanox.com>
> Signed-off-by: Amir Vadai <amirv@mellanox.com>
> ---
>  drivers/net/ethernet/mellanox/mlx4/en_netdev.c |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
> index 805e242..108c4cf 100644
> --- a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
> +++ b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
> @@ -1212,6 +1212,7 @@ void mlx4_en_stop_port(struct net_device *dev)
>  
>  	/* Synchronize with tx routine */
>  	netif_tx_lock_bh(dev);
> +	netif_carrier_off(dev);
>  	netif_tx_stop_all_queues(dev);
>  	netif_tx_unlock_bh(dev);
>  

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

Re: [PATCH net-next 4/7] net/mlx4_en: Set carrier to off when a port is stopped

[Amir Vadai]

On 16/01/2013 18:00, Ben Hutchings wrote:
> On Wed, 2013-01-16 at 17:42 +0200, Amir Vadai wrote:
>> >From: Eugenia Emantayev<eugenia@mellanox.com>
>> >
>> >Under heavy CPU load changing ring size/mtu/etc. could result in transmit
>> >timeout, since stop-start port might take more than 10 sec. Set
>> >netif_carrier_off to prevent tx queue transmit timeout.
> A spurious link change can restart L3 auto-configuration (DHCP, SLAAC,
> etc.)  netif_device_detach() also inhibits the watchdog and doesn't have
> that problem.
>
> Ben.
>

Thanks, Ben.

I will change it to netif_device_attach().
Since this will need some testing, will resend the patchset without this 
patch for now.

Amir.

Re: [PATCH net-next 4/7] net/mlx4_en: Set carrier to off when a port is stopped

[Amir Vadai]

On 16/01/2013 18:00, Ben Hutchings wrote:
> On Wed, 2013-01-16 at 17:42 +0200, Amir Vadai wrote:
>> From: Eugenia Emantayev <eugenia@mellanox.com>
>>
>> Under heavy CPU load changing ring size/mtu/etc. could result in transmit
>> timeout, since stop-start port might take more than 10 sec. Set
>> netif_carrier_off to prevent tx queue transmit timeout.
>
> A spurious link change can restart L3 auto-configuration (DHCP, SLAAC,
> etc.)  netif_device_detach() also inhibits the watchdog and doesn't have
> that problem.
>
> Ben.
>

But after calling netif_device_detach(), device present bit is cleared 
and therefore won't be able to access the device. This means, that after 
'ifconfig down', won't be able to do 'ifconfig up'.

Amir


>> Signed-off-by: Eugenia Emantayev <eugenia@mellanox.com>
>> Signed-off-by: Amir Vadai <amirv@mellanox.com>
>> ---
>>   drivers/net/ethernet/mellanox/mlx4/en_netdev.c |    1 +
>>   1 files changed, 1 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
>> index 805e242..108c4cf 100644
>> --- a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
>> +++ b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
>> @@ -1212,6 +1212,7 @@ void mlx4_en_stop_port(struct net_device *dev)
>>
>>   	/* Synchronize with tx routine */
>>   	netif_tx_lock_bh(dev);
>> +	netif_carrier_off(dev);
>>   	netif_tx_stop_all_queues(dev);
>>   	netif_tx_unlock_bh(dev);
>>
>

Re: [PATCH net-next 4/7] net/mlx4_en: Set carrier to off when a port is stopped

[Ben Hutchings]

On Mon, 2013-01-28 at 14:07 +0200, Amir Vadai wrote:
> On 16/01/2013 18:00, Ben Hutchings wrote:
> > On Wed, 2013-01-16 at 17:42 +0200, Amir Vadai wrote:
> >> From: Eugenia Emantayev <eugenia@mellanox.com>
> >>
> >> Under heavy CPU load changing ring size/mtu/etc. could result in transmit
> >> timeout, since stop-start port might take more than 10 sec. Set
> >> netif_carrier_off to prevent tx queue transmit timeout.
> >
> > A spurious link change can restart L3 auto-configuration (DHCP, SLAAC,
> > etc.)  netif_device_detach() also inhibits the watchdog and doesn't have
> > that problem.
> >
> > Ben.
> >
> 
> But after calling netif_device_detach(), device present bit is cleared 
> and therefore won't be able to access the device. This means, that after 
> 'ifconfig down', won't be able to do 'ifconfig up'.

Indeed, so don't do it in ndo_stop!  Your commit message referred to
changing ring size and MTU.

Ben.

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.