Netdev List
 help / color / mirror / Atom feed
* [PATCH net 0/4] Mellanox 100G mlx5 fixes for 4.6-rc
From: Saeed Mahameed @ 2016-05-01 19:59 UTC (permalink / raw)
  To: David S. Miller
  Cc: netdev, Or Gerlitz, Tal Alon, Eran Ben Elisha, Saeed Mahameed

Hi Dave,

This small series provides some bug fixes for mlx5 driver.
 
A small bug fix for iounmap of a null pointer, which dumps a warning on some archs.

One patch to fix the VXLAN/MLX5_EN dependency issue reported by Arnd.

Two patches to fix the scheduling while atomic issue for ndo_add/del_vxlan_port 
NDOs.  The first will add an internal mlx5e workqueue and the second will 
delegate vxlan ports add/del requests to that workqueue.

Note: ('net/mlx5: Kconfig: Fix MLX5_EN/VXLAN build issue') is only needed for net 
and not net-next as the issue was globally fixed for all device drivers by:
b7aade15485a ('vxlan: break dependency with netdev drivers') in net-next.

Applied on top: f27337e16f2d ('ip_tunnel: fix preempt warning in ip tunnel creation/updating')

Gal Pressman (1):
  net/mlx5: Unmap only the relevant IO memory mapping

Matthew Finlay (3):
  net/mlx5: Kconfig: Fix MLX5_EN/VXLAN build issue
  net/mlx5e: Implement a mlx5e workqueue
  net/mlx5e: Use workqueue for vxlan ops

 drivers/net/ethernet/mellanox/mlx5/core/Kconfig   |  1 +
 drivers/net/ethernet/mellanox/mlx5/core/en.h      |  1 +
 drivers/net/ethernet/mellanox/mlx5/core/en_main.c | 34 +++++++++------
 drivers/net/ethernet/mellanox/mlx5/core/uar.c     |  6 ++-
 drivers/net/ethernet/mellanox/mlx5/core/vxlan.c   | 50 +++++++++++++++++------
 drivers/net/ethernet/mellanox/mlx5/core/vxlan.h   | 11 ++++-
 6 files changed, 74 insertions(+), 29 deletions(-)

-- 
2.8.0

^ permalink raw reply

* [PATCH net 1/4] net/mlx5: Unmap only the relevant IO memory mapping
From: Saeed Mahameed @ 2016-05-01 19:59 UTC (permalink / raw)
  To: David S. Miller
  Cc: netdev, Or Gerlitz, Tal Alon, Eran Ben Elisha, Gal Pressman,
	Saeed Mahameed
In-Reply-To: <1462132797-22853-1-git-send-email-saeedm@mellanox.com>

From: Gal Pressman <galp@mellanox.com>

When freeing UAR the driver tries to unmap uar->map and uar->bf_map
which are mutually exclusive thus always unmapping a NULL pointer.
Make sure we only call iounmap() once, for the actual mapping.

Fixes: 0ba422410bbf ('net/mlx5: Fix global UAR mapping')
Signed-off-by: Gal Pressman <galp@mellanox.com>
Reported-by: Doron Tsur <doront@mellanox.com>
Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlx5/core/uar.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/uar.c b/drivers/net/ethernet/mellanox/mlx5/core/uar.c
index 8ba080e..5ff8af4 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/uar.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/uar.c
@@ -269,8 +269,10 @@ EXPORT_SYMBOL(mlx5_alloc_map_uar);
 
 void mlx5_unmap_free_uar(struct mlx5_core_dev *mdev, struct mlx5_uar *uar)
 {
-	iounmap(uar->map);
-	iounmap(uar->bf_map);
+	if (uar->map)
+		iounmap(uar->map);
+	else
+		iounmap(uar->bf_map);
 	mlx5_cmd_free_uar(mdev, uar->index);
 }
 EXPORT_SYMBOL(mlx5_unmap_free_uar);
-- 
2.8.0

^ permalink raw reply related

* [PATCH net 4/4] net/mlx5e: Use workqueue for vxlan ops
From: Saeed Mahameed @ 2016-05-01 19:59 UTC (permalink / raw)
  To: David S. Miller
  Cc: netdev, Or Gerlitz, Tal Alon, Eran Ben Elisha, Matthew Finlay,
	Saeed Mahameed
In-Reply-To: <1462132797-22853-1-git-send-email-saeedm@mellanox.com>

From: Matthew Finlay <matt@mellanox.com>

The vxlan add/delete port NDOs are called under rcu lock.
The current mlx5e implementation can potentially block in these
calls, which is not allowed.  Move to using the mlx5e workqueue
to handle these NDOs.

Fixes: b3f63c3d5e2c ('net/mlx5e: Add netdev support for VXLAN tunneling')
Signed-off-by: Matthew Finlay <matt@mellanox.com>
Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlx5/core/en_main.c |  4 +-
 drivers/net/ethernet/mellanox/mlx5/core/vxlan.c   | 50 +++++++++++++++++------
 drivers/net/ethernet/mellanox/mlx5/core/vxlan.h   | 11 ++++-
 3 files changed, 49 insertions(+), 16 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
index 9ab0841..d4dfc5c 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
@@ -2157,7 +2157,7 @@ static void mlx5e_add_vxlan_port(struct net_device *netdev,
 	if (!mlx5e_vxlan_allowed(priv->mdev))
 		return;
 
-	mlx5e_vxlan_add_port(priv, be16_to_cpu(port));
+	mlx5e_vxlan_queue_work(priv, sa_family, be16_to_cpu(port), 1);
 }
 
 static void mlx5e_del_vxlan_port(struct net_device *netdev,
@@ -2168,7 +2168,7 @@ static void mlx5e_del_vxlan_port(struct net_device *netdev,
 	if (!mlx5e_vxlan_allowed(priv->mdev))
 		return;
 
-	mlx5e_vxlan_del_port(priv, be16_to_cpu(port));
+	mlx5e_vxlan_queue_work(priv, sa_family, be16_to_cpu(port), 0);
 }
 
 static netdev_features_t mlx5e_vxlan_features_check(struct mlx5e_priv *priv,
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/vxlan.c b/drivers/net/ethernet/mellanox/mlx5/core/vxlan.c
index 9f10df2..f2fd1ef 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/vxlan.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/vxlan.c
@@ -95,21 +95,22 @@ struct mlx5e_vxlan *mlx5e_vxlan_lookup_port(struct mlx5e_priv *priv, u16 port)
 	return vxlan;
 }
 
-int mlx5e_vxlan_add_port(struct mlx5e_priv *priv, u16 port)
+static void mlx5e_vxlan_add_port(struct work_struct *work)
 {
+	struct mlx5e_vxlan_work *vxlan_work =
+		container_of(work, struct mlx5e_vxlan_work, work);
+	struct mlx5e_priv *priv = vxlan_work->priv;
 	struct mlx5e_vxlan_db *vxlan_db = &priv->vxlan;
+	u16 port = vxlan_work->port;
 	struct mlx5e_vxlan *vxlan;
 	int err;
 
-	err = mlx5e_vxlan_core_add_port_cmd(priv->mdev, port);
-	if (err)
-		return err;
+	if (mlx5e_vxlan_core_add_port_cmd(priv->mdev, port))
+		goto free_work;
 
 	vxlan = kzalloc(sizeof(*vxlan), GFP_KERNEL);
-	if (!vxlan) {
-		err = -ENOMEM;
+	if (!vxlan)
 		goto err_delete_port;
-	}
 
 	vxlan->udp_port = port;
 
@@ -119,13 +120,14 @@ int mlx5e_vxlan_add_port(struct mlx5e_priv *priv, u16 port)
 	if (err)
 		goto err_free;
 
-	return 0;
+	goto free_work;
 
 err_free:
 	kfree(vxlan);
 err_delete_port:
 	mlx5e_vxlan_core_del_port_cmd(priv->mdev, port);
-	return err;
+free_work:
+	kfree(vxlan_work);
 }
 
 static void __mlx5e_vxlan_core_del_port(struct mlx5e_priv *priv, u16 port)
@@ -145,12 +147,36 @@ static void __mlx5e_vxlan_core_del_port(struct mlx5e_priv *priv, u16 port)
 	kfree(vxlan);
 }
 
-void mlx5e_vxlan_del_port(struct mlx5e_priv *priv, u16 port)
+static void mlx5e_vxlan_del_port(struct work_struct *work)
 {
-	if (!mlx5e_vxlan_lookup_port(priv, port))
-		return;
+	struct mlx5e_vxlan_work *vxlan_work =
+		container_of(work, struct mlx5e_vxlan_work, work);
+	struct mlx5e_priv *priv = vxlan_work->priv;
+	u16 port = vxlan_work->port;
 
 	__mlx5e_vxlan_core_del_port(priv, port);
+
+	kfree(vxlan_work);
+}
+
+void mlx5e_vxlan_queue_work(struct mlx5e_priv *priv, sa_family_t sa_family,
+			    u16 port, int add)
+{
+	struct mlx5e_vxlan_work *vxlan_work;
+
+	vxlan_work = kmalloc(sizeof(*vxlan_work), GFP_ATOMIC);
+	if (!vxlan_work)
+		return;
+
+	if (add)
+		INIT_WORK(&vxlan_work->work, mlx5e_vxlan_add_port);
+	else
+		INIT_WORK(&vxlan_work->work, mlx5e_vxlan_del_port);
+
+	vxlan_work->priv = priv;
+	vxlan_work->port = port;
+	vxlan_work->sa_family = sa_family;
+	queue_work(priv->wq, &vxlan_work->work);
 }
 
 void mlx5e_vxlan_cleanup(struct mlx5e_priv *priv)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/vxlan.h b/drivers/net/ethernet/mellanox/mlx5/core/vxlan.h
index a016850..129f352 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/vxlan.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/vxlan.h
@@ -39,6 +39,13 @@ struct mlx5e_vxlan {
 	u16 udp_port;
 };
 
+struct mlx5e_vxlan_work {
+	struct work_struct	work;
+	struct mlx5e_priv	*priv;
+	sa_family_t		sa_family;
+	u16			port;
+};
+
 static inline bool mlx5e_vxlan_allowed(struct mlx5_core_dev *mdev)
 {
 	return (MLX5_CAP_ETH(mdev, tunnel_stateless_vxlan) &&
@@ -46,8 +53,8 @@ static inline bool mlx5e_vxlan_allowed(struct mlx5_core_dev *mdev)
 }
 
 void mlx5e_vxlan_init(struct mlx5e_priv *priv);
-int  mlx5e_vxlan_add_port(struct mlx5e_priv *priv, u16 port);
-void mlx5e_vxlan_del_port(struct mlx5e_priv *priv, u16 port);
+void mlx5e_vxlan_queue_work(struct mlx5e_priv *priv, sa_family_t sa_family,
+			    u16 port, int add);
 struct mlx5e_vxlan *mlx5e_vxlan_lookup_port(struct mlx5e_priv *priv, u16 port);
 void mlx5e_vxlan_cleanup(struct mlx5e_priv *priv);
 
-- 
2.8.0

^ permalink raw reply related

* [PATCH net 3/4] net/mlx5e: Implement a mlx5e workqueue
From: Saeed Mahameed @ 2016-05-01 19:59 UTC (permalink / raw)
  To: David S. Miller
  Cc: netdev, Or Gerlitz, Tal Alon, Eran Ben Elisha, Matthew Finlay,
	Saeed Mahameed
In-Reply-To: <1462132797-22853-1-git-send-email-saeedm@mellanox.com>

From: Matthew Finlay <matt@mellanox.com>

Implement a mlx5e workqueue to handle all mlx5e specific tasks.  Move
all tasks currently using the system workqueue to the new workqueue.
This is in preparation for vxlan using the mlx5e workqueue in order to
schedule port add/remove operations.

Signed-off-by: Matthew Finlay <matt@mellanox.com>
Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlx5/core/en.h      |  1 +
 drivers/net/ethernet/mellanox/mlx5/core/en_main.c | 30 ++++++++++++++---------
 2 files changed, 20 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en.h b/drivers/net/ethernet/mellanox/mlx5/core/en.h
index e80ce94..3881dce 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en.h
@@ -567,6 +567,7 @@ struct mlx5e_priv {
 	struct mlx5e_vxlan_db      vxlan;
 
 	struct mlx5e_params        params;
+	struct workqueue_struct    *wq;
 	struct work_struct         update_carrier_work;
 	struct work_struct         set_rx_mode_work;
 	struct delayed_work        update_stats_work;
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
index 67d548b..9ab0841 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
@@ -262,9 +262,8 @@ static void mlx5e_update_stats_work(struct work_struct *work)
 	mutex_lock(&priv->state_lock);
 	if (test_bit(MLX5E_STATE_OPENED, &priv->state)) {
 		mlx5e_update_stats(priv);
-		schedule_delayed_work(dwork,
-				      msecs_to_jiffies(
-					      MLX5E_UPDATE_STATS_INTERVAL));
+		queue_delayed_work(priv->wq, dwork,
+				   msecs_to_jiffies(MLX5E_UPDATE_STATS_INTERVAL));
 	}
 	mutex_unlock(&priv->state_lock);
 }
@@ -280,7 +279,7 @@ static void mlx5e_async_event(struct mlx5_core_dev *mdev, void *vpriv,
 	switch (event) {
 	case MLX5_DEV_EVENT_PORT_UP:
 	case MLX5_DEV_EVENT_PORT_DOWN:
-		schedule_work(&priv->update_carrier_work);
+		queue_work(priv->wq, &priv->update_carrier_work);
 		break;
 
 	default:
@@ -1505,7 +1504,7 @@ int mlx5e_open_locked(struct net_device *netdev)
 	mlx5e_update_carrier(priv);
 	mlx5e_timestamp_init(priv);
 
-	schedule_delayed_work(&priv->update_stats_work, 0);
+	queue_delayed_work(priv->wq, &priv->update_stats_work, 0);
 
 	return 0;
 
@@ -1961,7 +1960,7 @@ static void mlx5e_set_rx_mode(struct net_device *dev)
 {
 	struct mlx5e_priv *priv = netdev_priv(dev);
 
-	schedule_work(&priv->set_rx_mode_work);
+	queue_work(priv->wq, &priv->set_rx_mode_work);
 }
 
 static int mlx5e_set_mac(struct net_device *netdev, void *addr)
@@ -1976,7 +1975,7 @@ static int mlx5e_set_mac(struct net_device *netdev, void *addr)
 	ether_addr_copy(netdev->dev_addr, saddr->sa_data);
 	netif_addr_unlock_bh(netdev);
 
-	schedule_work(&priv->set_rx_mode_work);
+	queue_work(priv->wq, &priv->set_rx_mode_work);
 
 	return 0;
 }
@@ -2498,10 +2497,14 @@ static void *mlx5e_create_netdev(struct mlx5_core_dev *mdev)
 
 	priv = netdev_priv(netdev);
 
+	priv->wq = create_singlethread_workqueue("mlx5e");
+	if (!priv->wq)
+		goto err_free_netdev;
+
 	err = mlx5_alloc_map_uar(mdev, &priv->cq_uar, false);
 	if (err) {
 		mlx5_core_err(mdev, "alloc_map uar failed, %d\n", err);
-		goto err_free_netdev;
+		goto err_destroy_wq;
 	}
 
 	err = mlx5_core_alloc_pd(mdev, &priv->pdn);
@@ -2580,7 +2583,7 @@ static void *mlx5e_create_netdev(struct mlx5_core_dev *mdev)
 		vxlan_get_rx_port(netdev);
 
 	mlx5e_enable_async_events(priv);
-	schedule_work(&priv->set_rx_mode_work);
+	queue_work(priv->wq, &priv->set_rx_mode_work);
 
 	return priv;
 
@@ -2617,6 +2620,9 @@ err_dealloc_pd:
 err_unmap_free_uar:
 	mlx5_unmap_free_uar(mdev, &priv->cq_uar);
 
+err_destroy_wq:
+	destroy_workqueue(priv->wq);
+
 err_free_netdev:
 	free_netdev(netdev);
 
@@ -2630,9 +2636,9 @@ static void mlx5e_destroy_netdev(struct mlx5_core_dev *mdev, void *vpriv)
 
 	set_bit(MLX5E_STATE_DESTROYING, &priv->state);
 
-	schedule_work(&priv->set_rx_mode_work);
+	queue_work(priv->wq, &priv->set_rx_mode_work);
 	mlx5e_disable_async_events(priv);
-	flush_scheduled_work();
+	flush_workqueue(priv->wq);
 	if (test_bit(MLX5_INTERFACE_STATE_SHUTDOWN, &mdev->intf_state)) {
 		netif_device_detach(netdev);
 		mutex_lock(&priv->state_lock);
@@ -2655,6 +2661,8 @@ static void mlx5e_destroy_netdev(struct mlx5_core_dev *mdev, void *vpriv)
 	mlx5_core_dealloc_transport_domain(priv->mdev, priv->tdn);
 	mlx5_core_dealloc_pd(priv->mdev, priv->pdn);
 	mlx5_unmap_free_uar(priv->mdev, &priv->cq_uar);
+	cancel_delayed_work_sync(&priv->update_stats_work);
+	destroy_workqueue(priv->wq);
 
 	if (!test_bit(MLX5_INTERFACE_STATE_SHUTDOWN, &mdev->intf_state))
 		free_netdev(netdev);
-- 
2.8.0

^ permalink raw reply related

* [PATCH net 2/4] net/mlx5: Kconfig: Fix MLX5_EN/VXLAN build issue
From: Saeed Mahameed @ 2016-05-01 19:59 UTC (permalink / raw)
  To: David S. Miller
  Cc: netdev, Or Gerlitz, Tal Alon, Eran Ben Elisha, Matthew Finlay,
	Saeed Mahameed
In-Reply-To: <1462132797-22853-1-git-send-email-saeedm@mellanox.com>

From: Matthew Finlay <matt@mellanox.com>

When MLX5_EN=y MLX5_CORE=y and VXLAN=m there is a linker error for
vxlan_get_rx_port() due to the fact that VXLAN is a module. Change Kconfig
to select VXLAN when MLX5_CORE=y. When MLX5_CORE=m there is no dependency
on the value of VXLAN.

Fixes: b3f63c3d5e2c ('net/mlx5e: Add netdev support for VXLAN tunneling')
Signed-off-by: Matthew Finlay <matt@mellanox.com>
Reported-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlx5/core/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/Kconfig b/drivers/net/ethernet/mellanox/mlx5/core/Kconfig
index 1cf722e..559d11a 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/Kconfig
+++ b/drivers/net/ethernet/mellanox/mlx5/core/Kconfig
@@ -14,6 +14,7 @@ config MLX5_CORE_EN
 	bool "Mellanox Technologies ConnectX-4 Ethernet support"
 	depends on NETDEVICES && ETHERNET && PCI && MLX5_CORE
 	select PTP_1588_CLOCK
+	select VXLAN if MLX5_CORE=y
 	default n
 	---help---
 	  Ethernet support in Mellanox Technologies ConnectX-4 NIC.
-- 
2.8.0

^ permalink raw reply related

* Re: [PATCH v1 net] net/mlx4: Avoid wrong virtual mappings
From: David Miller @ 2016-05-01 20:07 UTC (permalink / raw)
  To: okaya-sgV2jX0FEOL9JmXXK+q4OQ
  Cc: hagaya-VPRAkNaXOzVWk0Htik3J/w, dledford-H+wXaHxf7aLQT0dZR+AlfA,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA,
	timur-sgV2jX0FEOL9JmXXK+q4OQ, eli-VPRAkNaXOzVWk0Htik3J/w,
	ogerlitz-VPRAkNaXOzVWk0Htik3J/w, eranbe-VPRAkNaXOzVWk0Htik3J/w,
	yishaih-VPRAkNaXOzVWk0Htik3J/w, talal-VPRAkNaXOzVWk0Htik3J/w,
	saeedm-VPRAkNaXOzVWk0Htik3J/w
In-Reply-To: <5726243A.9070109-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>

From: Sinan Kaya <okaya-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
Date: Sun, 1 May 2016 11:43:54 -0400

> Can we get this queued for 4.7? Mellanox with arm64 has been broken over 1.5 years.

Can you not quote an entire huge patch just to say something like
this?

I'm waiting for the people involved all to give feedback.

If it's been broken for more than a year, waiting for a few more
days to get the fix right is not going to be the end of the world.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [net-next PATCH v2 7/9] mlx5e: Add support for UDP tunnel segmentation with outer checksum offload
From: Saeed Mahameed @ 2016-05-01 20:08 UTC (permalink / raw)
  To: Alexander Duyck
  Cc: Tal Alon, Linux Netdev List, michael.chan, David S. Miller,
	Gal Pressman, Or Gerlitz, Eran Ben Elisha
In-Reply-To: <20160429224345.12418.88876.stgit@ahduyck-xeon-server>

On Sat, Apr 30, 2016 at 1:43 AM, Alexander Duyck <aduyck@mirantis.com> wrote:
> This patch assumes that the mlx5 hardware will ignore existing IPv4/v6
> header fields for length and checksum as well as the length and checksum
> fields for outer UDP headers.
>
> Signed-off-by: Alexander Duyck <aduyck@mirantis.com>

Reviewed-by: Saeed Mahameed <saeedm@mellanox.com>

^ permalink raw reply

* Re: [net-next PATCH v2 8/9] mlx5e: Fix IPv6 tunnel checksum offload
From: Saeed Mahameed @ 2016-05-01 20:09 UTC (permalink / raw)
  To: Alexander Duyck
  Cc: Tal Alon, Linux Netdev List, michael.chan, David S. Miller,
	Gal Pressman, Or Gerlitz, Eran Ben Elisha
In-Reply-To: <20160429224352.12418.68373.stgit@ahduyck-xeon-server>

On Sat, Apr 30, 2016 at 1:43 AM, Alexander Duyck <aduyck@mirantis.com> wrote:
> The mlx5 driver exposes support for TSO6 but not IPv6 csum for hardware
> encapsulated tunnels.  This leads to issues as it triggers warnings in
> skb_checksum_help as it ends up being called as we report supporting the
> segmentation but not the checksumming for IPv6 frames.
>
> This patch corrects that and drops 2 features that don't actually need to
> be supported in hw_enc_features since they are Rx features and don't
> actually impact anything by being present in hw_enc_features.
>
> Signed-off-by: Alexander Duyck <aduyck@mirantis.com>

Reviewed-by: Saeed Mahameed <saeedm@mellanox.com>

^ permalink raw reply

* Re: [PATCH v1 net] net/mlx4: Avoid wrong virtual mappings
From: Sinan Kaya @ 2016-05-01 20:12 UTC (permalink / raw)
  To: David Miller
  Cc: hagaya-VPRAkNaXOzVWk0Htik3J/w, dledford-H+wXaHxf7aLQT0dZR+AlfA,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA,
	timur-sgV2jX0FEOL9JmXXK+q4OQ, eli-VPRAkNaXOzVWk0Htik3J/w,
	ogerlitz-VPRAkNaXOzVWk0Htik3J/w, eranbe-VPRAkNaXOzVWk0Htik3J/w,
	yishaih-VPRAkNaXOzVWk0Htik3J/w, talal-VPRAkNaXOzVWk0Htik3J/w,
	saeedm-VPRAkNaXOzVWk0Htik3J/w
In-Reply-To: <20160501.160720.1356913722642000889.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>

On 5/1/2016 4:07 PM, David Miller wrote:
> From: Sinan Kaya <okaya-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
> Date: Sun, 1 May 2016 11:43:54 -0400
> 
>> Can we get this queued for 4.7? Mellanox with arm64 has been broken over 1.5 years.
> 
> Can you not quote an entire huge patch just to say something like
> this?
> 
> I'm waiting for the people involved all to give feedback.
> 
> If it's been broken for more than a year, waiting for a few more
> days to get the fix right is not going to be the end of the world.
> 

Sure, I thought there is a common consensus. The original patch posted was from Christoph Hellwig.

This patch does the same thing like Christoph Hellwig proposed + a fallback approach for the
infiniband case where fragmented buffers are still supported.

The patch is actually a copy/paste from the MLX5 driver into MLX4.

I'm OK with waiitng for a few more days. I just wanted to make sure that this patch
is moving in the right direction.

If not, let's list the open issues. 

-- 
Sinan Kaya
Qualcomm Technologies, Inc. on behalf of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [net-next PATCH v2 6/9] mlx4: Add support for inner IPv6 checksum offloads and TSO
From: Saeed Mahameed @ 2016-05-01 20:21 UTC (permalink / raw)
  To: Alexander Duyck
  Cc: Tal Alon, Linux Netdev List, michael.chan, David S. Miller,
	Gal Pressman, Or Gerlitz, Eran Ben Elisha
In-Reply-To: <20160429224339.12418.53462.stgit@ahduyck-xeon-server>

On Sat, Apr 30, 2016 at 1:43 AM, Alexander Duyck <aduyck@mirantis.com> wrote:
> >From what I can tell the ConnectX-3 will support an inner IPv6 checksum and
> segmentation offload, however it cannot support outer IPv6 headers.  This
> assumption is based on the fact that I could see the checksum being
> offloaded for inner header on IPv4 tunnels, but not on IPv6 tunnels.
>
> For this reason I am adding the feature to the hw_enc_features and adding
> an extra check to the features_check call that will disable GSO and
> checksum offload in the case that the encapsulated frame has an outer IP
> version of that is not 4.  The check in mlx4_en_features_check could be
> removed if at some point in the future a fix is found that allows the
> hardware to offload segmentation/checksum on tunnels with an outer IPv6
> header.
>
> Signed-off-by: Alexander Duyck <aduyck@mirantis.com>

Reviewed-by: Saeed Mahameed <saeedm@mellanox.com>

^ permalink raw reply

* Re: [PATCH] pxa168_eth: fix mdiobus_scan() error check
From: Sergei Shtylyov @ 2016-05-01 20:25 UTC (permalink / raw)
  To: Florian Fainelli, netdev; +Cc: arnd
In-Reply-To: <57262956.3060206@gmail.com>

Hello.

On 05/01/2016 07:05 PM, Florian Fainelli wrote:

>> Since mdiobus_scan() returns either an error code or NULL on error, the
>> driver should check  for both,  not only for NULL, otherwise a crash is
>> imminent...
>>
>> Reported-by: Arnd Bergmann <arnd@arndb.de>
>> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
>>
>> ---
>> The patch is against DaveM's 'net.git' repo.
>>
>>   drivers/net/ethernet/marvell/pxa168_eth.c |    2 ++
>>   1 file changed, 2 insertions(+)
>>
>> Index: net/drivers/net/ethernet/marvell/pxa168_eth.c
>> ===================================================================
>> --- net.orig/drivers/net/ethernet/marvell/pxa168_eth.c
>> +++ net/drivers/net/ethernet/marvell/pxa168_eth.c
>> @@ -979,6 +979,8 @@ static int pxa168_init_phy(struct net_de
>>   		return 0;
>>
>>   	pep->phy = mdiobus_scan(pep->smi_bus, pep->phy_addr);
>> +	if (IS_ERR(pep->phy))
>> +		return PTR_ERR(pep->phy);
>>   	if (!pep->phy)
>>   		return -ENODEV;
>
> Should not this check be removed too and

    That's my next move -- for now I'm fixing the existing bug in this driver 
only.

> converted to a PTR_ERR(pep->phy) != -ENODEV?

    I don't see what that would achieve, IMO it's enough to just drop this 
*if* altogether.

MBR, Sergei

^ permalink raw reply

* Re: [net-next PATCH v2 5/9] mlx4: Add support for UDP tunnel segmentation with outer checksum offload
From: Saeed Mahameed @ 2016-05-01 20:28 UTC (permalink / raw)
  To: Alexander Duyck
  Cc: Tal Alon, Linux Netdev List, michael.chan, David S. Miller,
	Gal Pressman, Or Gerlitz, Eran Ben Elisha
In-Reply-To: <20160429224333.12418.13862.stgit@ahduyck-xeon-server>

On Sat, Apr 30, 2016 at 1:43 AM, Alexander Duyck <aduyck@mirantis.com> wrote:
> This patch assumes that the mlx4 hardware will ignore existing IPv4/v6
> header fields for length and checksum as well as the length and checksum
> fields for outer UDP headers.
>
> Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
> ---
>  drivers/net/ethernet/mellanox/mlx4/en_netdev.c |   17 +++++++++++++----
>  1 file changed, 13 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
> index 8bd143dda95d..bce37cbfde24 100644
> --- a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
> +++ b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
> @@ -2358,7 +2358,9 @@ out:
>
>         /* set offloads */
>         priv->dev->hw_enc_features |= NETIF_F_IP_CSUM | NETIF_F_RXCSUM |
> -                                     NETIF_F_TSO | NETIF_F_GSO_UDP_TUNNEL;
> +                                     NETIF_F_TSO | NETIF_F_GSO_UDP_TUNNEL |
> +                                     NETIF_F_GSO_UDP_TUNNEL_CSUM |
> +                                     NETIF_F_GSO_PARTIAL;
>  }
>
>  static void mlx4_en_del_vxlan_offloads(struct work_struct *work)
> @@ -2368,7 +2370,9 @@ static void mlx4_en_del_vxlan_offloads(struct work_struct *work)
>                                                  vxlan_del_task);
>         /* unset offloads */
>         priv->dev->hw_enc_features &= ~(NETIF_F_IP_CSUM | NETIF_F_RXCSUM |
> -                                     NETIF_F_TSO | NETIF_F_GSO_UDP_TUNNEL);
> +                                       NETIF_F_TSO | NETIF_F_GSO_UDP_TUNNEL |
> +                                       NETIF_F_GSO_UDP_TUNNEL_CSUM |
> +                                       NETIF_F_GSO_PARTIAL);

I know it is not related to your patch, but is it ok to dynamically
modify priv->dev->hw_enc_features every vxlan add/del_port request ?
especially on a deferred work !
Shouldn't we at least notify the stack ?

^ permalink raw reply

* Re: [PATCH] pxa168_eth: fix mdiobus_scan() error check
From: Sergei Shtylyov @ 2016-05-01 20:30 UTC (permalink / raw)
  To: Florian Fainelli, netdev; +Cc: arnd
In-Reply-To: <57266638.6030704@cogentembedded.com>

On 05/01/2016 11:25 PM, Sergei Shtylyov wrote:

>>> Since mdiobus_scan() returns either an error code or NULL on error, the
>>> driver should check  for both,  not only for NULL, otherwise a crash is
>>> imminent...
>>>
>>> Reported-by: Arnd Bergmann <arnd@arndb.de>
>>> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
>>>
>>> ---
>>> The patch is against DaveM's 'net.git' repo.
>>>
>>>   drivers/net/ethernet/marvell/pxa168_eth.c |    2 ++
>>>   1 file changed, 2 insertions(+)
>>>
>>> Index: net/drivers/net/ethernet/marvell/pxa168_eth.c
>>> ===================================================================
>>> --- net.orig/drivers/net/ethernet/marvell/pxa168_eth.c
>>> +++ net/drivers/net/ethernet/marvell/pxa168_eth.c
>>> @@ -979,6 +979,8 @@ static int pxa168_init_phy(struct net_de
>>>           return 0;
>>>
>>>       pep->phy = mdiobus_scan(pep->smi_bus, pep->phy_addr);
>>> +    if (IS_ERR(pep->phy))
>>> +        return PTR_ERR(pep->phy);
>>>       if (!pep->phy)
>>>           return -ENODEV;
>>
>> Should not this check be removed too and
>
>     That's my next move -- for now I'm fixing the existing bug in this driver
> only.

    In fact, it can't be removed yet as mdiobus_scan() may return NULL on 
other error path. There's certainly a space for improvements yet...
[...]

MBR, Sergei

^ permalink raw reply

* Re: [net-next PATCH v2 1/9] net: Disable segmentation if checksumming is not supported
From: Or Gerlitz @ 2016-05-01 20:30 UTC (permalink / raw)
  To: Alexander Duyck
  Cc: talal@mellanox.com, Linux Netdev List, michael.chan, David Miller,
	Or Gerlitz, Eran Ben Elisha, Tariq Toukan
In-Reply-To: <20160429224308.12418.51424.stgit@ahduyck-xeon-server>

On Sat, Apr 30, 2016 at 1:43 AM, Alexander Duyck <aduyck@mirantis.com> wrote:
> In the case of the mlx4 and mlx5 driver they do not support IPv6 checksum
> offload for tunnels.

Alex,

To clarify, when you say "not support IPv6 checksum for tunnels", you
refer to the offloading of the outer or inner checksum?

Still (me and I think also Tariq from our driver team) catching up on
the series, the primitives and conventions you are introducing using
and how this applies on mlx5. I saw that Saeed acked the the mlx5e
patches (7 and 8).

Specifically, the mlx4 patches are practically fixes so if they don't
land in 4.7 via net-next we can get them there through net, lets give
us the few more days needed to catch up from our side.

Or.

^ permalink raw reply

* Re: [net-next PATCH v2 5/9] mlx4: Add support for UDP tunnel segmentation with outer checksum offload
From: Or Gerlitz @ 2016-05-01 20:35 UTC (permalink / raw)
  To: Alexander Duyck
  Cc: talal@mellanox.com, Linux Netdev List, michael.chan, David Miller,
	Gal Pressman, Or Gerlitz, Eran Ben Elisha
In-Reply-To: <20160429224333.12418.13862.stgit@ahduyck-xeon-server>

On Sat, Apr 30, 2016 at 1:43 AM, Alexander Duyck <aduyck@mirantis.com> wrote:
> This patch assumes that the mlx4 hardware will ignore existing IPv4/v6
> header fields for length and checksum as well as the length and checksum
> fields for outer UDP headers.

Hi Alex,

I see now the above text appearing in bunch of similar commit of
yours, specifically to Intel drivers and mlx5... could you please
elaborate a bit more what you mean here and what are the practical
consequences of that characteristics?

I got that right NETIF_F_GSO_UDP_TUNNEL_CSUM means that the HW can do
segmentation for TCP packets encapsulated by UDP tunnel e.g VXLAN
where the outer checksum is not zero. AFAIK, any other outer checksum
value can't correctly be a constant... are you assuming here  RCO or
LCO?

Or.

^ permalink raw reply

* Re: Q: How to disable vlan strip in Intel igb driver ?
From: Peter Palúch @ 2016-05-01 20:51 UTC (permalink / raw)
  To: Ran Shalit; +Cc: netdev, alexander.duyck
In-Reply-To: <CAJ2oMhJp9M93QujnPYmp5CQkt-tx8P-uRCveiMEkDRLrcZmNhg@mail.gmail.com>

Hi Ran,


> Alex,
> I don't see rx-vlan-offload option in my ethtool. strange, maybe it is
> not available in all ethtool versions ?

According to my manpage for ethtool v4.5, the relevant -K option is 
"rxvlan".

> Hi Peter,
> Yes, I'm using AF_PACKET (I can't validate it now for 100%, but I
> quite sure about it).

Okay - but I assume that you are writing a userspace application, not a 
kernelspace driver or module, right?

> I'm accessing the ethernet header, and it always gives me non extended
> ethernet header (without vlan information).

It seems that this behavior has been around for a long time, and is 
intentional. See the following thread for more information:

http://www.spinics.net/lists/netdev/msg244668.html

The only legit way of accessing VLAN tag info on an AF_PACKET socket I 
know about is by setting the PACKET_AUXDATA socket option, and reading 
the auxiliary data as a struct tpacket_auxdata using recvmsg().

In addition, however, there seems to be a bug in the kernel in that the 
tp_vlan_tci member of the struct tpacket_auxdata is filled in only if 
the AF_PACKET socket is bound to htons(ETH_P_ALL). If the socket is 
bound to any other specific protocol, the tp_vlan_tci is set to 0. I 
have raised this issue recently in the following thread(s) but have not 
received a response yet:

http://www.spinics.net/lists/netdev/msg372830.html
http://www.spinics.net/lists/netdev/msg373112.html


> I can see the vlan tag in vlan_tci field in sk_buff, but this is not
> exactly what I need, I need the header AS-IS with the original
> (extended) ethernet header.

How are you accessing the vlan_tci field from your software?

In any case, with AF_PACKET sockets, I am afraid you don't have much 
choice. Even libpcap reconstructs VLAN-tagged frames by reading the VLAN 
tag from an auxiliary structure and then re-inserting it into the frame, 
because the frame delivered through an AF_PACKET socket does not have 
the VLAN tag present anymore.

Best regards,
Peter

^ permalink raw reply

* [net-next PATCH v2 0/5] stmmac: dwmac-socfpga refactor+cleanup
From: Joachim Eastwood @ 2016-05-01 20:58 UTC (permalink / raw)
  To: davem
  Cc: Joachim Eastwood, marex, dinguyen, peppe.cavallaro,
	alexandre.torgue, netdev

This patch aims to remove the init/exit callbacks from the dwmac-
socfpga driver and instead use standard PM callbacks. Doing this
will also allow us to cleanup the driver.

Eventually the init/exit callbacks will be deprecated and removed
from all drivers dwmac-* except for dwmac-generic. Drivers will be
refactored to use standard PM and remove callbacks.

This patch set should not change the behavior of the driver itself,
it only moves code around. The only exception to this is patch
number 4 which restores the resume callback behavior which was
changed in the "net: stmmac: socfpga: Remove re-registration of
reset controller" patch. I belive calling phy_resume() only
from the resume callback and not probe is the right thing to do.

Changes from v1:
 - Rebase on net-next

One heads-up here:
The first patch changes the prototype of a couple of
functions used in Alexandre's "add Ethernet glue logic for
stm32 chip" patch [1] and will cause build failures for
dwmac-stm32.c if not fixed up!
If Alexandre's patch set is applied first I will gladly
rebase my patch set to account for his driver as well.

[1] https://patchwork.ozlabs.org/patch/614405/

Dave: To avoid the build failure mentioned above you can
      apply the changes below to the stm32 driver or
      Alexandre can use it if he creates a new version
      of his patch set.

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c
@@ -133,7 +133,7 @@ static int stm32_dwmac_remove(struct platform_device *pdev)
 {
 	struct net_device *ndev = platform_get_drvdata(pdev);
 	struct stmmac_priv *priv = netdev_priv(ndev);
-	int ret = stmmac_dvr_remove(ndev);
+	int ret = stmmac_dvr_remove(&pdev->dev);
 
 	stm32_dwmac_clk_disable(priv->plat->bsp_priv);
 
@@ -147,7 +147,7 @@ static int stm32_dwmac_suspend(struct device *dev)
 	struct stmmac_priv *priv = netdev_priv(ndev);
 	int ret;
 
-	ret = stmmac_suspend(ndev);
+	ret = stmmac_suspend(dev);
 	stm32_dwmac_clk_disable(priv->plat->bsp_priv);
 
 	return ret;
@@ -163,7 +163,7 @@ static int stm32_dwmac_resume(struct device *dev)
 	if (ret)
 		return ret;
 
-	ret = stmmac_resume(ndev);
+	ret = stmmac_resume(dev);
 
 	return ret;
 }



Joachim Eastwood (5):
  stmmac: let remove/resume/suspend functions take device pointer
  stmmac: dwmac-socfpga: add PM ops and resume function
  stmmac: dwmac-socfpga: keep a copy of stmmac_rst in driver priv data
  stmmac: dwmac-socfpga: call phy_resume() only in resume callback
  stmmac: dwmac-socfpga: kill init() and rename setup() to set_phy_mode()

 .../net/ethernet/stmicro/stmmac/dwmac-socfpga.c    | 106 +++++++++++----------
 drivers/net/ethernet/stmicro/stmmac/stmmac.h       |   6 +-
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c  |  15 +--
 drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c   |  24 +----
 .../net/ethernet/stmicro/stmmac/stmmac_platform.c  |   6 +-
 5 files changed, 71 insertions(+), 86 deletions(-)

-- 
2.8.0

^ permalink raw reply

* [net-next PATCH v2 1/5] stmmac: let remove/resume/suspend functions take device pointer
From: Joachim Eastwood @ 2016-05-01 20:58 UTC (permalink / raw)
  To: davem
  Cc: Joachim Eastwood, marex, dinguyen, peppe.cavallaro,
	alexandre.torgue, netdev
In-Reply-To: <1462136303-16825-1-git-send-email-manabian@gmail.com>

Change stmmac_remove/resume/suspend to take a device pointer so
they can be used directly by drivers that doesn't need to perform
anything device specific.

This lets us remove the PCI pm functions and later simplifiy the
platform drivers.

Signed-off-by: Joachim Eastwood <manabian@gmail.com>
Tested-by: Marek Vasut <marex@denx.de>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac.h       |  6 +++---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c  | 15 ++++++++------
 drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c   | 24 ++--------------------
 .../net/ethernet/stmicro/stmmac/stmmac_platform.c  |  6 +++---
 4 files changed, 17 insertions(+), 34 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac.h b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
index ff67506..59ae608 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac.h
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
@@ -148,9 +148,9 @@ void stmmac_set_ethtool_ops(struct net_device *netdev);
 
 int stmmac_ptp_register(struct stmmac_priv *priv);
 void stmmac_ptp_unregister(struct stmmac_priv *priv);
-int stmmac_resume(struct net_device *ndev);
-int stmmac_suspend(struct net_device *ndev);
-int stmmac_dvr_remove(struct net_device *ndev);
+int stmmac_resume(struct device *dev);
+int stmmac_suspend(struct device *dev);
+int stmmac_dvr_remove(struct device *dev);
 int stmmac_dvr_probe(struct device *device,
 		     struct plat_stmmacenet_data *plat_dat,
 		     struct stmmac_resources *res);
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index b87edb7..fd5ab7b 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -3350,12 +3350,13 @@ EXPORT_SYMBOL_GPL(stmmac_dvr_probe);
 
 /**
  * stmmac_dvr_remove
- * @ndev: net device pointer
+ * @dev: device pointer
  * Description: this function resets the TX/RX processes, disables the MAC RX/TX
  * changes the link status, releases the DMA descriptor rings.
  */
-int stmmac_dvr_remove(struct net_device *ndev)
+int stmmac_dvr_remove(struct device *dev)
 {
+	struct net_device *ndev = dev_get_drvdata(dev);
 	struct stmmac_priv *priv = netdev_priv(ndev);
 
 	pr_info("%s:\n\tremoving driver", __func__);
@@ -3381,13 +3382,14 @@ EXPORT_SYMBOL_GPL(stmmac_dvr_remove);
 
 /**
  * stmmac_suspend - suspend callback
- * @ndev: net device pointer
+ * @dev: device pointer
  * Description: this is the function to suspend the device and it is called
  * by the platform driver to stop the network queue, release the resources,
  * program the PMT register (for WoL), clean and release driver resources.
  */
-int stmmac_suspend(struct net_device *ndev)
+int stmmac_suspend(struct device *dev)
 {
+	struct net_device *ndev = dev_get_drvdata(dev);
 	struct stmmac_priv *priv = netdev_priv(ndev);
 	unsigned long flags;
 
@@ -3430,12 +3432,13 @@ EXPORT_SYMBOL_GPL(stmmac_suspend);
 
 /**
  * stmmac_resume - resume callback
- * @ndev: net device pointer
+ * @dev: device pointer
  * Description: when resume this function is invoked to setup the DMA and CORE
  * in a usable state.
  */
-int stmmac_resume(struct net_device *ndev)
+int stmmac_resume(struct device *dev)
 {
+	struct net_device *ndev = dev_get_drvdata(dev);
 	struct stmmac_priv *priv = netdev_priv(ndev);
 	unsigned long flags;
 
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
index ae43887..56c8a23 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
@@ -231,30 +231,10 @@ static int stmmac_pci_probe(struct pci_dev *pdev,
  */
 static void stmmac_pci_remove(struct pci_dev *pdev)
 {
-	struct net_device *ndev = pci_get_drvdata(pdev);
-
-	stmmac_dvr_remove(ndev);
-}
-
-#ifdef CONFIG_PM_SLEEP
-static int stmmac_pci_suspend(struct device *dev)
-{
-	struct pci_dev *pdev = to_pci_dev(dev);
-	struct net_device *ndev = pci_get_drvdata(pdev);
-
-	return stmmac_suspend(ndev);
-}
-
-static int stmmac_pci_resume(struct device *dev)
-{
-	struct pci_dev *pdev = to_pci_dev(dev);
-	struct net_device *ndev = pci_get_drvdata(pdev);
-
-	return stmmac_resume(ndev);
+	stmmac_dvr_remove(&pdev->dev);
 }
-#endif
 
-static SIMPLE_DEV_PM_OPS(stmmac_pm_ops, stmmac_pci_suspend, stmmac_pci_resume);
+static SIMPLE_DEV_PM_OPS(stmmac_pm_ops, stmmac_suspend, stmmac_resume);
 
 #define STMMAC_VENDOR_ID 0x700
 #define STMMAC_QUARK_ID  0x0937
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
index effaa4f..409db91 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
@@ -386,7 +386,7 @@ int stmmac_pltfr_remove(struct platform_device *pdev)
 {
 	struct net_device *ndev = platform_get_drvdata(pdev);
 	struct stmmac_priv *priv = netdev_priv(ndev);
-	int ret = stmmac_dvr_remove(ndev);
+	int ret = stmmac_dvr_remove(&pdev->dev);
 
 	if (priv->plat->exit)
 		priv->plat->exit(pdev, priv->plat->bsp_priv);
@@ -410,7 +410,7 @@ static int stmmac_pltfr_suspend(struct device *dev)
 	struct stmmac_priv *priv = netdev_priv(ndev);
 	struct platform_device *pdev = to_platform_device(dev);
 
-	ret = stmmac_suspend(ndev);
+	ret = stmmac_suspend(dev);
 	if (priv->plat->exit)
 		priv->plat->exit(pdev, priv->plat->bsp_priv);
 
@@ -433,7 +433,7 @@ static int stmmac_pltfr_resume(struct device *dev)
 	if (priv->plat->init)
 		priv->plat->init(pdev, priv->plat->bsp_priv);
 
-	return stmmac_resume(ndev);
+	return stmmac_resume(dev);
 }
 #endif /* CONFIG_PM_SLEEP */
 
-- 
2.8.0

^ permalink raw reply related

* [net-next PATCH v2 2/5] stmmac: dwmac-socfpga: add PM ops and resume function
From: Joachim Eastwood @ 2016-05-01 20:58 UTC (permalink / raw)
  To: davem
  Cc: Joachim Eastwood, marex, dinguyen, peppe.cavallaro,
	alexandre.torgue, netdev
In-Reply-To: <1462136303-16825-1-git-send-email-manabian@gmail.com>

Implement the needed PM callbacks in the driver instead of
relying on the init/exit hooks in stmmac_platform. This gives
the driver more flexibility in how the code is organized.

Eventually the init/exit callbacks will be deprecated in favor
of the standard PM callbacks and driver remove function.

Signed-off-by: Joachim Eastwood <manabian@gmail.com>
Tested-by: Marek Vasut <marex@denx.de>
---
 drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
index 784eb53..789013a 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
@@ -261,7 +261,6 @@ static int socfpga_dwmac_probe(struct platform_device *pdev)
 	}
 
 	plat_dat->bsp_priv = dwmac;
-	plat_dat->init = socfpga_dwmac_init;
 	plat_dat->fix_mac_speed = socfpga_dwmac_fix_mac_speed;
 
 	ret = stmmac_dvr_probe(&pdev->dev, plat_dat, &stmmac_res);
@@ -271,6 +270,21 @@ static int socfpga_dwmac_probe(struct platform_device *pdev)
 	return ret;
 }
 
+#ifdef CONFIG_PM_SLEEP
+static int socfpga_dwmac_resume(struct device *dev)
+{
+	struct platform_device *pdev = to_platform_device(dev);
+	struct net_device *ndev = dev_get_drvdata(dev);
+	struct stmmac_priv *priv = netdev_priv(ndev);
+
+	socfpga_dwmac_init(pdev, priv->plat->bsp_priv);
+
+	return stmmac_resume(dev);
+}
+#endif /* CONFIG_PM_SLEEP */
+
+SIMPLE_DEV_PM_OPS(socfpga_dwmac_pm_ops, stmmac_suspend, socfpga_dwmac_resume);
+
 static const struct of_device_id socfpga_dwmac_match[] = {
 	{ .compatible = "altr,socfpga-stmmac" },
 	{ }
@@ -282,7 +296,7 @@ static struct platform_driver socfpga_dwmac_driver = {
 	.remove = stmmac_pltfr_remove,
 	.driver = {
 		.name           = "socfpga-dwmac",
-		.pm		= &stmmac_pltfr_pm_ops,
+		.pm		= &socfpga_dwmac_pm_ops,
 		.of_match_table = socfpga_dwmac_match,
 	},
 };
-- 
2.8.0

^ permalink raw reply related

* [net-next PATCH v2 3/5] stmmac: dwmac-socfpga: keep a copy of stmmac_rst in driver priv data
From: Joachim Eastwood @ 2016-05-01 20:58 UTC (permalink / raw)
  To: davem
  Cc: Joachim Eastwood, marex, dinguyen, peppe.cavallaro,
	alexandre.torgue, netdev
In-Reply-To: <1462136303-16825-1-git-send-email-manabian@gmail.com>

The dwmac-socfpga driver needs to control the reset usually managed
by the core driver to set the PHY mode. Take a copy of the reset
handle from core priv data so it can be used by the driver later.

This also allow us to move reset handling into socfpga_dwmac_setup()
where the code that needs it is located.

Signed-off-by: Joachim Eastwood <manabian@gmail.com>
Tested-by: Marek Vasut <marex@denx.de>
---
 .../net/ethernet/stmicro/stmmac/dwmac-socfpga.c    | 33 ++++++++++++++--------
 1 file changed, 22 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
index 789013a..ba0b793 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
@@ -49,6 +49,7 @@ struct socfpga_dwmac {
 	u32	reg_shift;
 	struct	device *dev;
 	struct regmap *sys_mgr_base_addr;
+	struct reset_control *stmmac_rst;
 	void __iomem *splitter_base;
 	bool f2h_ptp_ref_clk;
 };
@@ -164,6 +165,10 @@ static int socfpga_dwmac_setup(struct socfpga_dwmac *dwmac)
 	if (dwmac->splitter_base)
 		val = SYSMGR_EMACGRP_CTRL_PHYSEL_ENUM_GMII_MII;
 
+	/* Assert reset to the enet controller before changing the phy mode */
+	if (dwmac->stmmac_rst)
+		reset_control_assert(dwmac->stmmac_rst);
+
 	regmap_read(sys_mgr_base_addr, reg_offset, &ctrl);
 	ctrl &= ~(SYSMGR_EMACGRP_CTRL_PHYSEL_MASK << reg_shift);
 	ctrl |= val << reg_shift;
@@ -181,6 +186,12 @@ static int socfpga_dwmac_setup(struct socfpga_dwmac *dwmac)
 
 	regmap_write(sys_mgr_base_addr, reg_offset, ctrl);
 
+	/* Deassert reset for the phy configuration to be sampled by
+	 * the enet controller, and operation to start in requested mode
+	 */
+	if (dwmac->stmmac_rst)
+		reset_control_deassert(dwmac->stmmac_rst);
+
 	return 0;
 }
 
@@ -198,21 +209,11 @@ static int socfpga_dwmac_init(struct platform_device *pdev, void *priv)
 	if (!stpriv)
 		return -EINVAL;
 
-	/* Assert reset to the enet controller before changing the phy mode */
-	if (stpriv->stmmac_rst)
-		reset_control_assert(stpriv->stmmac_rst);
-
 	/* Setup the phy mode in the system manager registers according to
 	 * devicetree configuration
 	 */
 	ret = socfpga_dwmac_setup(dwmac);
 
-	/* Deassert reset for the phy configuration to be sampled by
-	 * the enet controller, and operation to start in requested mode
-	 */
-	if (stpriv->stmmac_rst)
-		reset_control_deassert(stpriv->stmmac_rst);
-
 	/* Before the enet controller is suspended, the phy is suspended.
 	 * This causes the phy clock to be gated. The enet controller is
 	 * resumed before the phy, so the clock is still gated "off" when
@@ -264,8 +265,18 @@ static int socfpga_dwmac_probe(struct platform_device *pdev)
 	plat_dat->fix_mac_speed = socfpga_dwmac_fix_mac_speed;
 
 	ret = stmmac_dvr_probe(&pdev->dev, plat_dat, &stmmac_res);
-	if (!ret)
+	if (!ret) {
+		struct net_device *ndev = platform_get_drvdata(pdev);
+		struct stmmac_priv *stpriv = netdev_priv(ndev);
+
+		/* The socfpga driver needs to control the stmmac reset to
+		 * set the phy mode. Create a copy of the core reset handel
+		 * so it can be used by the driver later.
+		 */
+		dwmac->stmmac_rst = stpriv->stmmac_rst;
+
 		ret = socfpga_dwmac_init(pdev, dwmac);
+	}
 
 	return ret;
 }
-- 
2.8.0

^ permalink raw reply related

* [net-next PATCH v2 4/5] stmmac: dwmac-socfpga: call phy_resume() only in resume callback
From: Joachim Eastwood @ 2016-05-01 20:58 UTC (permalink / raw)
  To: davem
  Cc: Joachim Eastwood, marex, dinguyen, peppe.cavallaro,
	alexandre.torgue, netdev
In-Reply-To: <1462136303-16825-1-git-send-email-manabian@gmail.com>

Calling phy_resume() should only be need during driver resume to
workaround a hardware errata.

Signed-off-by: Joachim Eastwood <manabian@gmail.com>
Tested-by: Marek Vasut <marex@denx.de>
---
 .../net/ethernet/stmicro/stmmac/dwmac-socfpga.c    | 50 ++++++++--------------
 1 file changed, 19 insertions(+), 31 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
index ba0b793..ba49d8c 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
@@ -198,41 +198,11 @@ static int socfpga_dwmac_setup(struct socfpga_dwmac *dwmac)
 static int socfpga_dwmac_init(struct platform_device *pdev, void *priv)
 {
 	struct socfpga_dwmac *dwmac = priv;
-	struct net_device *ndev = platform_get_drvdata(pdev);
-	struct stmmac_priv *stpriv = NULL;
-	int ret = 0;
-
-	if (!ndev)
-		return -EINVAL;
-
-	stpriv = netdev_priv(ndev);
-	if (!stpriv)
-		return -EINVAL;
 
 	/* Setup the phy mode in the system manager registers according to
 	 * devicetree configuration
 	 */
-	ret = socfpga_dwmac_setup(dwmac);
-
-	/* Before the enet controller is suspended, the phy is suspended.
-	 * This causes the phy clock to be gated. The enet controller is
-	 * resumed before the phy, so the clock is still gated "off" when
-	 * the enet controller is resumed. This code makes sure the phy
-	 * is "resumed" before reinitializing the enet controller since
-	 * the enet controller depends on an active phy clock to complete
-	 * a DMA reset. A DMA reset will "time out" if executed
-	 * with no phy clock input on the Synopsys enet controller.
-	 * Verified through Synopsys Case #8000711656.
-	 *
-	 * Note that the phy clock is also gated when the phy is isolated.
-	 * Phy "suspend" and "isolate" controls are located in phy basic
-	 * control register 0, and can be modified by the phy driver
-	 * framework.
-	 */
-	if (stpriv->phydev)
-		phy_resume(stpriv->phydev);
-
-	return ret;
+	return socfpga_dwmac_setup(dwmac);
 }
 
 static int socfpga_dwmac_probe(struct platform_device *pdev)
@@ -290,6 +260,24 @@ static int socfpga_dwmac_resume(struct device *dev)
 
 	socfpga_dwmac_init(pdev, priv->plat->bsp_priv);
 
+	/* Before the enet controller is suspended, the phy is suspended.
+	 * This causes the phy clock to be gated. The enet controller is
+	 * resumed before the phy, so the clock is still gated "off" when
+	 * the enet controller is resumed. This code makes sure the phy
+	 * is "resumed" before reinitializing the enet controller since
+	 * the enet controller depends on an active phy clock to complete
+	 * a DMA reset. A DMA reset will "time out" if executed
+	 * with no phy clock input on the Synopsys enet controller.
+	 * Verified through Synopsys Case #8000711656.
+	 *
+	 * Note that the phy clock is also gated when the phy is isolated.
+	 * Phy "suspend" and "isolate" controls are located in phy basic
+	 * control register 0, and can be modified by the phy driver
+	 * framework.
+	 */
+	if (priv->phydev)
+		phy_resume(priv->phydev);
+
 	return stmmac_resume(dev);
 }
 #endif /* CONFIG_PM_SLEEP */
-- 
2.8.0

^ permalink raw reply related

* [net-next PATCH v2 5/5] stmmac: dwmac-socfpga: kill init() and rename setup() to set_phy_mode()
From: Joachim Eastwood @ 2016-05-01 20:58 UTC (permalink / raw)
  To: davem
  Cc: Joachim Eastwood, marex, dinguyen, peppe.cavallaro,
	alexandre.torgue, netdev
In-Reply-To: <1462136303-16825-1-git-send-email-manabian@gmail.com>

Remove old init callback which now contains only a call to
socfpga_dwmac_setup(). Also rename socfpga_dwmac_setup() to indicate
what the function really does.

Signed-off-by: Joachim Eastwood <manabian@gmail.com>
Tested-by: Marek Vasut <marex@denx.de>
---
 drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c | 17 +++--------------
 1 file changed, 3 insertions(+), 14 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
index ba49d8c..cd9764a 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
@@ -136,7 +136,7 @@ static int socfpga_dwmac_parse_data(struct socfpga_dwmac *dwmac, struct device *
 	return 0;
 }
 
-static int socfpga_dwmac_setup(struct socfpga_dwmac *dwmac)
+static int socfpga_dwmac_set_phy_mode(struct socfpga_dwmac *dwmac)
 {
 	struct regmap *sys_mgr_base_addr = dwmac->sys_mgr_base_addr;
 	int phymode = dwmac->interface;
@@ -195,16 +195,6 @@ static int socfpga_dwmac_setup(struct socfpga_dwmac *dwmac)
 	return 0;
 }
 
-static int socfpga_dwmac_init(struct platform_device *pdev, void *priv)
-{
-	struct socfpga_dwmac *dwmac = priv;
-
-	/* Setup the phy mode in the system manager registers according to
-	 * devicetree configuration
-	 */
-	return socfpga_dwmac_setup(dwmac);
-}
-
 static int socfpga_dwmac_probe(struct platform_device *pdev)
 {
 	struct plat_stmmacenet_data *plat_dat;
@@ -245,7 +235,7 @@ static int socfpga_dwmac_probe(struct platform_device *pdev)
 		 */
 		dwmac->stmmac_rst = stpriv->stmmac_rst;
 
-		ret = socfpga_dwmac_init(pdev, dwmac);
+		ret = socfpga_dwmac_set_phy_mode(dwmac);
 	}
 
 	return ret;
@@ -254,11 +244,10 @@ static int socfpga_dwmac_probe(struct platform_device *pdev)
 #ifdef CONFIG_PM_SLEEP
 static int socfpga_dwmac_resume(struct device *dev)
 {
-	struct platform_device *pdev = to_platform_device(dev);
 	struct net_device *ndev = dev_get_drvdata(dev);
 	struct stmmac_priv *priv = netdev_priv(ndev);
 
-	socfpga_dwmac_init(pdev, priv->plat->bsp_priv);
+	socfpga_dwmac_set_phy_mode(priv->plat->bsp_priv);
 
 	/* Before the enet controller is suspended, the phy is suspended.
 	 * This causes the phy clock to be gated. The enet controller is
-- 
2.8.0

^ permalink raw reply related

* Re: [PATCH] drivers: net: xgene: constify xgene_cle_ops structure
From: Iyappan Subramanian @ 2016-05-01 23:10 UTC (permalink / raw)
  To: Julia Lawall
  Cc: kernel-janitors, Keyur Chudgar, netdev,
	linux-kernel@vger.kernel.org
In-Reply-To: <1462106189-14277-1-git-send-email-Julia.Lawall@lip6.fr>

On Sun, May 1, 2016 at 5:36 AM, Julia Lawall <Julia.Lawall@lip6.fr> wrote:
> The xgene_cle_ops structure is never modified, so declare it as const.
>
> Done with the help of Coccinelle.
>
> Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
>
> ---
>  drivers/net/ethernet/apm/xgene/xgene_enet_cle.c  |    2 +-
>  drivers/net/ethernet/apm/xgene/xgene_enet_cle.h  |    2 +-
>  drivers/net/ethernet/apm/xgene/xgene_enet_main.h |    2 +-
>  3 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/ethernet/apm/xgene/xgene_enet_cle.c b/drivers/net/ethernet/apm/xgene/xgene_enet_cle.c
> index b2124886..6479288 100644
> --- a/drivers/net/ethernet/apm/xgene/xgene_enet_cle.c
> +++ b/drivers/net/ethernet/apm/xgene/xgene_enet_cle.c
> @@ -729,6 +729,6 @@ static int xgene_enet_cle_init(struct xgene_enet_pdata *pdata)
>         return xgene_cle_setup_ptree(pdata, enet_cle);
>  }
>
> -struct xgene_cle_ops xgene_cle3in_ops = {
> +const struct xgene_cle_ops xgene_cle3in_ops = {
>         .cle_init = xgene_enet_cle_init,
>  };
> diff --git a/drivers/net/ethernet/apm/xgene/xgene_enet_cle.h b/drivers/net/ethernet/apm/xgene/xgene_enet_cle.h
> index 29a17ab..13e829a 100644
> --- a/drivers/net/ethernet/apm/xgene/xgene_enet_cle.h
> +++ b/drivers/net/ethernet/apm/xgene/xgene_enet_cle.h
> @@ -290,6 +290,6 @@ struct xgene_enet_cle {
>         u32 jump_bytes;
>  };
>
> -extern struct xgene_cle_ops xgene_cle3in_ops;
> +extern const struct xgene_cle_ops xgene_cle3in_ops;
>
>  #endif /* __XGENE_ENET_CLE_H__ */
> diff --git a/drivers/net/ethernet/apm/xgene/xgene_enet_main.h b/drivers/net/ethernet/apm/xgene/xgene_enet_main.h
> index 175d188..0a2887b 100644
> --- a/drivers/net/ethernet/apm/xgene/xgene_enet_main.h
> +++ b/drivers/net/ethernet/apm/xgene/xgene_enet_main.h
> @@ -191,7 +191,7 @@ struct xgene_enet_pdata {
>         const struct xgene_mac_ops *mac_ops;
>         const struct xgene_port_ops *port_ops;
>         struct xgene_ring_ops *ring_ops;
> -       struct xgene_cle_ops *cle_ops;
> +       const struct xgene_cle_ops *cle_ops;
>         struct delayed_work link_work;
>         u32 port_id;
>         u8 cpu_bufnum;
>

Thanks Julia.

Acked-by: Iyappan Subramanian <isubramanian@apm.com>

^ permalink raw reply

* [PATCH net 1/2] RDS:TCP: Synchronize rds_tcp_accept_one with rds_send_xmit when resetting t_sock
From: Sowmini Varadhan @ 2016-05-01 23:10 UTC (permalink / raw)
  To: sowmini.varadhan, netdev, rds-devel; +Cc: santosh.shilimkar, davem
In-Reply-To: <cover.1462127059.git.sowmini.varadhan@oracle.com>

There is a race condition between rds_send_xmit -> rds_tcp_xmit
and the code that deals with resolution of duelling syns added
by commit 241b271952eb ("RDS-TCP: Reset tcp callbacks if re-using an
outgoing socket in rds_tcp_accept_one()").

Specifically, we may end up derefencing a null pointer in rds_send_xmit
if we have the interleaving sequencee:
           rds_tcp_accept_one                  rds_send_xmit

                                             conn is RDS_CONN_UP, so
    					 invoke rds_tcp_xmit

                                             tc = conn->c_transport_data
        rds_tcp_restore_callbacks
            /* reset t_sock */
    					 null ptr deref from tc->t_sock

The race condition can be avoided without adding the overhead of
additional locking in the xmit path: have rds_tcp_accept_one wait
for rds_tcp_xmit threads to complete before resetting callbacks.
The synchronization can be done in the same manner as rds_conn_shutdown().
First set the rds_conn_state to something other than RDS_CONN_UP
(so that new threads cannot get into rds_tcp_xmit()), then wait for
RDS_IN_XMIT to be cleared in the conn->c_flags indicating that any
threads in rds_tcp_xmit are done.

Fixes: 241b271952eb ("RDS-TCP: Reset tcp callbacks if re-using an
outgoing socket in rds_tcp_accept_one()")

Signed-off-by: Sowmini Varadhan <sowmini.varadhan@oracle.com>
---
 net/rds/tcp.c        |    2 +-
 net/rds/tcp_listen.c |   40 ++++++++++++++++++++++++----------------
 2 files changed, 25 insertions(+), 17 deletions(-)

diff --git a/net/rds/tcp.c b/net/rds/tcp.c
index 61ed2a8..9134544 100644
--- a/net/rds/tcp.c
+++ b/net/rds/tcp.c
@@ -127,7 +127,7 @@ void rds_tcp_restore_callbacks(struct socket *sock,
 
 /*
  * This is the only path that sets tc->t_sock.  Send and receive trust that
- * it is set.  The RDS_CONN_CONNECTED bit protects those paths from being
+ * it is set.  The RDS_CONN_UP bit protects those paths from being
  * called while it isn't set.
  */
 void rds_tcp_set_callbacks(struct socket *sock, struct rds_connection *conn)
diff --git a/net/rds/tcp_listen.c b/net/rds/tcp_listen.c
index 0936a4a..0896187 100644
--- a/net/rds/tcp_listen.c
+++ b/net/rds/tcp_listen.c
@@ -115,24 +115,32 @@ int rds_tcp_accept_one(struct socket *sock)
 	 * rds_tcp_state_change() will do that cleanup
 	 */
 	rs_tcp = (struct rds_tcp_connection *)conn->c_transport_data;
-	if (rs_tcp->t_sock &&
-	    ntohl(inet->inet_saddr) < ntohl(inet->inet_daddr)) {
-		struct sock *nsk = new_sock->sk;
-
-		nsk->sk_user_data = NULL;
-		nsk->sk_prot->disconnect(nsk, 0);
-		tcp_done(nsk);
-		new_sock = NULL;
-		ret = 0;
-		goto out;
-	} else if (rs_tcp->t_sock) {
-		rds_tcp_restore_callbacks(rs_tcp->t_sock, rs_tcp);
-		conn->c_outgoing = 0;
-	}
-
 	rds_conn_transition(conn, RDS_CONN_DOWN, RDS_CONN_CONNECTING);
+	if (rs_tcp->t_sock) {
+		/* Need to resolve a duelling SYN between peers.
+		 * We have an outstanding SYN to this peer, which may
+		 * potentially have transitioned to the RDS_CONN_UP state,
+		 * so we must quiesce any send threads before resetting
+		 * c_transport_data.
+		 */
+		wait_event(conn->c_waitq,
+			   !test_bit(RDS_IN_XMIT, &conn->c_flags));
+		if (ntohl(inet->inet_saddr) < ntohl(inet->inet_daddr)) {
+			struct sock *nsk = new_sock->sk;
+
+			nsk->sk_user_data = NULL;
+			nsk->sk_prot->disconnect(nsk, 0);
+			tcp_done(nsk);
+			new_sock = NULL;
+			ret = 0;
+			goto out;
+		} else if (rs_tcp->t_sock) {
+			rds_tcp_restore_callbacks(rs_tcp->t_sock, rs_tcp);
+			conn->c_outgoing = 0;
+		}
+	}
 	rds_tcp_set_callbacks(new_sock, conn);
-	rds_connect_complete(conn);
+	rds_connect_complete(conn); /* marks RDS_CONN_UP */
 	new_sock = NULL;
 	ret = 0;
 
-- 
1.7.1

^ permalink raw reply related

* [PATCH net 2/2] RDS: TCP: Synchrnozize accept() and connect() paths on t_conn_lock.
From: Sowmini Varadhan @ 2016-05-01 23:10 UTC (permalink / raw)
  To: sowmini.varadhan, netdev, rds-devel; +Cc: santosh.shilimkar, davem
In-Reply-To: <cover.1462127059.git.sowmini.varadhan@oracle.com>

An arbitration scheme for duelling SYNs is implemented as part of
commit 241b271952eb ("RDS-TCP: Reset tcp callbacks if re-using an
outgoing socket in rds_tcp_accept_one()") which ensures that both nodes
involved will arrive at the same arbitration decision. However, this
needs to be synchronized with an outgoing SYN to be generated by
rds_tcp_conn_connect(). This commit achieves the synchronization
through the t_conn_lock mutex in struct rds_tcp_connection.

The rds_conn_state is checked in rds_tcp_conn_connect() after acquiring
the t_conn_lock mutex.  A SYN is sent out only if the RDS connection is
not already UP (an UP would indicate that rds_tcp_accept_one() has
completed 3WH, so no SYN needs to be generated).

Similarly, the rds_conn_state is checked in rds_tcp_accept_one() after
acquiring the t_conn_lock mutex. The only acceptable states (to
allow continuation of the arbitration logic) are UP (i.e., outgoing SYN
was SYN-ACKed by peer after it sent us the SYN) or CONNECTING (we sent
outgoing SYN before we saw incoming SYN).

Signed-off-by: Sowmini Varadhan <sowmini.varadhan@oracle.com>
---
 net/rds/tcp.c         |    1 +
 net/rds/tcp.h         |    4 ++++
 net/rds/tcp_connect.c |    8 ++++++++
 net/rds/tcp_listen.c  |   30 ++++++++++++++++++++----------
 4 files changed, 33 insertions(+), 10 deletions(-)

diff --git a/net/rds/tcp.c b/net/rds/tcp.c
index 9134544..86187da 100644
--- a/net/rds/tcp.c
+++ b/net/rds/tcp.c
@@ -216,6 +216,7 @@ static int rds_tcp_conn_alloc(struct rds_connection *conn, gfp_t gfp)
 	if (!tc)
 		return -ENOMEM;
 
+	mutex_init(&tc->t_conn_lock);
 	tc->t_sock = NULL;
 	tc->t_tinc = NULL;
 	tc->t_tinc_hdr_rem = sizeof(struct rds_header);
diff --git a/net/rds/tcp.h b/net/rds/tcp.h
index 64f873c..41c2283 100644
--- a/net/rds/tcp.h
+++ b/net/rds/tcp.h
@@ -12,6 +12,10 @@ struct rds_tcp_connection {
 
 	struct list_head	t_tcp_node;
 	struct rds_connection   *conn;
+	/* t_conn_lock synchronizes the connection establishment between
+	 * rds_tcp_accept_one and rds_tcp_conn_connect
+	 */
+	struct mutex		t_conn_lock;
 	struct socket		*t_sock;
 	void			*t_orig_write_space;
 	void			*t_orig_data_ready;
diff --git a/net/rds/tcp_connect.c b/net/rds/tcp_connect.c
index 5cb1687..49a3fcf 100644
--- a/net/rds/tcp_connect.c
+++ b/net/rds/tcp_connect.c
@@ -78,7 +78,14 @@ int rds_tcp_conn_connect(struct rds_connection *conn)
 	struct socket *sock = NULL;
 	struct sockaddr_in src, dest;
 	int ret;
+	struct rds_tcp_connection *tc = conn->c_transport_data;
+
+	mutex_lock(&tc->t_conn_lock);
 
+	if (rds_conn_up(conn)) {
+		mutex_unlock(&tc->t_conn_lock);
+		return 0;
+	}
 	ret = sock_create_kern(rds_conn_net(conn), PF_INET,
 			       SOCK_STREAM, IPPROTO_TCP, &sock);
 	if (ret < 0)
@@ -120,6 +127,7 @@ int rds_tcp_conn_connect(struct rds_connection *conn)
 	}
 
 out:
+	mutex_unlock(&tc->t_conn_lock);
 	if (sock)
 		sock_release(sock);
 	return ret;
diff --git a/net/rds/tcp_listen.c b/net/rds/tcp_listen.c
index 0896187..cc8496f 100644
--- a/net/rds/tcp_listen.c
+++ b/net/rds/tcp_listen.c
@@ -76,7 +76,9 @@ int rds_tcp_accept_one(struct socket *sock)
 	struct rds_connection *conn;
 	int ret;
 	struct inet_sock *inet;
-	struct rds_tcp_connection *rs_tcp;
+	struct rds_tcp_connection *rs_tcp = NULL;
+	int conn_state;
+	struct sock *nsk;
 
 	ret = sock_create_kern(sock_net(sock->sk), sock->sk->sk_family,
 			       sock->sk->sk_type, sock->sk->sk_protocol,
@@ -116,6 +118,10 @@ int rds_tcp_accept_one(struct socket *sock)
 	 */
 	rs_tcp = (struct rds_tcp_connection *)conn->c_transport_data;
 	rds_conn_transition(conn, RDS_CONN_DOWN, RDS_CONN_CONNECTING);
+	mutex_lock(&rs_tcp->t_conn_lock);
+	conn_state = rds_conn_state(conn);
+	if (conn_state != RDS_CONN_CONNECTING && conn_state != RDS_CONN_UP)
+		goto rst_nsk;
 	if (rs_tcp->t_sock) {
 		/* Need to resolve a duelling SYN between peers.
 		 * We have an outstanding SYN to this peer, which may
@@ -126,14 +132,7 @@ int rds_tcp_accept_one(struct socket *sock)
 		wait_event(conn->c_waitq,
 			   !test_bit(RDS_IN_XMIT, &conn->c_flags));
 		if (ntohl(inet->inet_saddr) < ntohl(inet->inet_daddr)) {
-			struct sock *nsk = new_sock->sk;
-
-			nsk->sk_user_data = NULL;
-			nsk->sk_prot->disconnect(nsk, 0);
-			tcp_done(nsk);
-			new_sock = NULL;
-			ret = 0;
-			goto out;
+			goto rst_nsk;
 		} else if (rs_tcp->t_sock) {
 			rds_tcp_restore_callbacks(rs_tcp->t_sock, rs_tcp);
 			conn->c_outgoing = 0;
@@ -143,8 +142,19 @@ int rds_tcp_accept_one(struct socket *sock)
 	rds_connect_complete(conn); /* marks RDS_CONN_UP */
 	new_sock = NULL;
 	ret = 0;
-
+	goto out;
+rst_nsk:
+	/* rest the newly returned accept sock and bail */
+	nsk = new_sock->sk;
+	rds_tcp_stats_inc(s_tcp_listen_closed_stale);
+	nsk->sk_user_data = NULL;
+	nsk->sk_prot->disconnect(nsk, 0);
+	tcp_done(nsk);
+	new_sock = NULL;
+	ret = 0;
 out:
+	if (rs_tcp)
+		mutex_unlock(&rs_tcp->t_conn_lock);
 	if (new_sock)
 		sock_release(new_sock);
 	return ret;
-- 
1.7.1

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox