[PATCH v2 0/6] Further fix for dsa unbinding

[PATCH v2 0/6] Further fix for dsa unbinding

[Neil Armstrong]

This serie fixes further issues for DSA dynamic unbinding.
Frode Isaksen's patches make usage of delayed work and fixes kernel
crashes when dsa is unbind.
The other patches are simple fixes to permit cleanup and avoid netdev
related crashes.

v2: remove phy fix and add missing calls in dsa_switch_destroy
then add dedicated dsa_slave_destroy

Frode Isaksen (4):
  net: dsa: Use delayed work instead of timer+work for polling
  net: dsa: Do not reschedule polling if driver removed
  net: dsa: add missing calls in dsa_switch_destroy
  net: dsa: move dsa slave destroy code to slave.c

Neil Armstrong (2):
  net: dsa: cleanup resources upon module removal
  net: dsa: Add missing master netdev dev_put() calls

 include/net/dsa.h  |  3 +--
 net/dsa/dsa.c      | 45 +++++++++++++++++++++++----------------------
 net/dsa/dsa_priv.h |  1 +
 net/dsa/slave.c    | 10 ++++++++++
 4 files changed, 35 insertions(+), 24 deletions(-)

-- 
1.9.1

[PATCH v3 net-next 0/4] Further fix for dsa unbinding

[Neil Armstrong]

This serie fixes further issues for DSA dynamic unbinding.
The first patch completely removes the PHY link state polling.
The two following cleans up the dsa state upon removal.
The last patch moves slave destroy code as slave function and
adds missing netdev and phy cleanup calls.

v1: http://lkml.kernel.org/r/562F8ECB.6050709@baylibre.com
v2: http://lkml.kernel.org/r/56321D9A.8010109@baylibre.com
remove phy fix and add missing calls in dsa_switch_destroy
then add dedicated dsa_slave_destroy

v3: remove polling instead of fixing it, make single patch for
dsa slave destroy

Neil Armstrong (4):
  net: dsa: remove DSA link polling
  net: dsa: cleanup resources upon module removal
  net: dsa: Add missing master netdev dev_put() calls
  net: dsa: move dsa slave destroy code to slave.c

 include/net/dsa.h  | 12 -----------
 net/dsa/dsa.c      | 60 +++++++++++++-----------------------------------------
 net/dsa/dsa_priv.h |  1 +
 net/dsa/slave.c    | 11 ++++++++++
 4 files changed, 26 insertions(+), 58 deletions(-)

-- 
1.9.1

[PATCH v3 net-next 1/4] net: dsa: remove DSA link polling

[Neil Armstrong]

Since no more DSA driver uses the polling callback, and since
the phylib handles the link detection, remove the link polling
work and timer code.

Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
 include/net/dsa.h | 12 ------------
 net/dsa/dsa.c     | 43 -------------------------------------------
 2 files changed, 55 deletions(-)

diff --git a/include/net/dsa.h b/include/net/dsa.h
index 3f23dd9..26a0e86 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -117,13 +117,6 @@ struct dsa_switch_tree {
 	s8			cpu_port;
 
 	/*
-	 * Link state polling.
-	 */
-	int			link_poll_needed;
-	struct work_struct	link_poll_work;
-	struct timer_list	link_poll_timer;
-
-	/*
 	 * Data for the individual switch chips.
 	 */
 	struct dsa_switch	*ds[DSA_MAX_SWITCHES];
@@ -232,11 +225,6 @@ struct dsa_switch_driver {
 			     int regnum, u16 val);
 
 	/*
-	 * Link state polling and IRQ handling.
-	 */
-	void	(*poll_link)(struct dsa_switch *ds);
-
-	/*
 	 * Link state adjustment (called from libphy)
 	 */
 	void	(*adjust_link)(struct dsa_switch *ds, int port,
diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
index b7448c8..0f41f71 100644
--- a/net/dsa/dsa.c
+++ b/net/dsa/dsa.c
@@ -508,33 +508,6 @@ static int dsa_switch_resume(struct dsa_switch *ds)
 }
 #endif
 
-
-/* link polling *************************************************************/
-static void dsa_link_poll_work(struct work_struct *ugly)
-{
-	struct dsa_switch_tree *dst;
-	int i;
-
-	dst = container_of(ugly, struct dsa_switch_tree, link_poll_work);
-
-	for (i = 0; i < dst->pd->nr_chips; i++) {
-		struct dsa_switch *ds = dst->ds[i];
-
-		if (ds != NULL && ds->drv->poll_link != NULL)
-			ds->drv->poll_link(ds);
-	}
-
-	mod_timer(&dst->link_poll_timer, round_jiffies(jiffies + HZ));
-}
-
-static void dsa_link_poll_timer(unsigned long _dst)
-{
-	struct dsa_switch_tree *dst = (void *)_dst;
-
-	schedule_work(&dst->link_poll_work);
-}
-
-
 /* platform driver init and cleanup *****************************************/
 static int dev_is_class(struct device *dev, void *class)
 {
@@ -877,8 +850,6 @@ static int dsa_setup_dst(struct dsa_switch_tree *dst, struct net_device *dev,
 		}
 
 		dst->ds[i] = ds;
-		if (ds->drv->poll_link != NULL)
-			dst->link_poll_needed = 1;
 
 		++configured;
 	}
@@ -897,15 +868,6 @@ static int dsa_setup_dst(struct dsa_switch_tree *dst, struct net_device *dev,
 	wmb();
 	dev->dsa_ptr = (void *)dst;
 
-	if (dst->link_poll_needed) {
-		INIT_WORK(&dst->link_poll_work, dsa_link_poll_work);
-		init_timer(&dst->link_poll_timer);
-		dst->link_poll_timer.data = (unsigned long)dst;
-		dst->link_poll_timer.function = dsa_link_poll_timer;
-		dst->link_poll_timer.expires = round_jiffies(jiffies + HZ);
-		add_timer(&dst->link_poll_timer);
-	}
-
 	return 0;
 }
 
@@ -972,11 +934,6 @@ static void dsa_remove_dst(struct dsa_switch_tree *dst)
 {
 	int i;
 
-	if (dst->link_poll_needed)
-		del_timer_sync(&dst->link_poll_timer);
-
-	flush_work(&dst->link_poll_work);
-
 	for (i = 0; i < dst->pd->nr_chips; i++) {
 		struct dsa_switch *ds = dst->ds[i];
 
-- 
1.9.1

[PATCH v3 net-next 2/4] net: dsa: cleanup resources upon module removal

[Neil Armstrong]

Make sure that we unassign the master_netdev dsa_ptr to make the packet
processing go through the regular Ethernet receive path.

Suggested-by: Florian Fainelli <f.fainelli@gmail.com>
Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
 net/dsa/dsa.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
index 0f41f71..d9e0172 100644
--- a/net/dsa/dsa.c
+++ b/net/dsa/dsa.c
@@ -985,6 +985,14 @@ static int dsa_suspend(struct device *d)
 	struct dsa_switch_tree *dst = platform_get_drvdata(pdev);
 	int i, ret = 0;
 
+	dst->master_netdev->dsa_ptr = NULL;
+
+	/* If we used a tagging format that doesn't have an ethertype
+	 * field, make sure that all packets from this point get sent
+	 * without the tag and go through the regular receive path.
+	 */
+	wmb();
+
 	for (i = 0; i < dst->pd->nr_chips; i++) {
 		struct dsa_switch *ds = dst->ds[i];
 
-- 
1.9.1

[PATCH v3 net-next 3/4] net: dsa: Add missing master netdev dev_put() calls

[Neil Armstrong]

Upon probe failure or unbinding, add missing dev_put() calls.

Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
 net/dsa/dsa.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
index d9e0172..d22d303e 100644
--- a/net/dsa/dsa.c
+++ b/net/dsa/dsa.c
@@ -919,8 +919,10 @@ static int dsa_probe(struct platform_device *pdev)
 	platform_set_drvdata(pdev, dst);
 
 	ret = dsa_setup_dst(dst, dev, &pdev->dev, pd);
-	if (ret)
+	if (ret) {
+		dev_put(dev);
 		goto out;
+	}
 
 	return 0;
 
@@ -940,6 +942,8 @@ static void dsa_remove_dst(struct dsa_switch_tree *dst)
 		if (ds)
 			dsa_switch_destroy(ds);
 	}
+
+	dev_put(dst->master_netdev);
 }
 
 static int dsa_remove(struct platform_device *pdev)
-- 
1.9.1

[PATCH v3 net-next 4/4] net: dsa: move dsa slave destroy code to slave.c

[Neil Armstrong]

Move dsa slave dedicated code from dsa_switch_destroy to a new
dsa_slave_destroy function in slave.c.
Add the netif_carrier_off and phy_disconnect calls in order to
correctly cleanup the netdev state and PHY state machine.

Signed-off-by: Frode Isaksen <fisaksen@baylibre.com>
Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
 net/dsa/dsa.c      |  3 +--
 net/dsa/dsa_priv.h |  1 +
 net/dsa/slave.c    | 11 +++++++++++
 3 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
index d22d303e..208d1b2 100644
--- a/net/dsa/dsa.c
+++ b/net/dsa/dsa.c
@@ -456,8 +456,7 @@ static void dsa_switch_destroy(struct dsa_switch *ds)
 		if (!ds->ports[port])
 			continue;
 
-		unregister_netdev(ds->ports[port]);
-		free_netdev(ds->ports[port]);
+		dsa_slave_destroy(ds->ports[port]);
 	}
 
 	mdiobus_unregister(ds->slave_mii_bus);
diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
index 311796c8..1d1a546 100644
--- a/net/dsa/dsa_priv.h
+++ b/net/dsa/dsa_priv.h
@@ -61,6 +61,7 @@ extern const struct dsa_device_ops notag_netdev_ops;
 void dsa_slave_mii_bus_init(struct dsa_switch *ds);
 int dsa_slave_create(struct dsa_switch *ds, struct device *parent,
 		     int port, char *name);
+void dsa_slave_destroy(struct net_device *slave_dev);
 int dsa_slave_suspend(struct net_device *slave_dev);
 int dsa_slave_resume(struct net_device *slave_dev);
 int dsa_slave_netdevice_event(struct notifier_block *unused,
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 7bc787b..1e9e942 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -1212,6 +1212,17 @@ int dsa_slave_create(struct dsa_switch *ds, struct device *parent,
 	return 0;
 }
 
+void dsa_slave_destroy(struct net_device *slave_dev)
+{
+	struct dsa_slave_priv *p = netdev_priv(slave_dev);
+
+	netif_carrier_off(slave_dev);
+	if (p->phy)
+		phy_disconnect(p->phy);
+	unregister_netdev(slave_dev);
+	free_netdev(slave_dev);
+}
+
 static bool dsa_slave_dev_check(struct net_device *dev)
 {
 	return dev->netdev_ops == &dsa_slave_netdev_ops;
-- 
1.9.1

Re: [PATCH v3 net-next 0/4] Further fix for dsa unbinding

[Andrew Lunn]

On Mon, Dec 07, 2015 at 01:57:31PM +0100, Neil Armstrong wrote:
> This serie fixes further issues for DSA dynamic unbinding.
> The first patch completely removes the PHY link state polling.
> The two following cleans up the dsa state upon removal.
> The last patch moves slave destroy code as slave function and
> adds missing netdev and phy cleanup calls.
> 
> v1: http://lkml.kernel.org/r/562F8ECB.6050709@baylibre.com
> v2: http://lkml.kernel.org/r/56321D9A.8010109@baylibre.com
> remove phy fix and add missing calls in dsa_switch_destroy
> then add dedicated dsa_slave_destroy
> 
> v3: remove polling instead of fixing it, make single patch for
> dsa slave destroy

Acked-by: Andrew Lunn <andrew@lunn.ch>

Thanks
	Andrew

Re: [PATCH v3 net-next 0/4] Further fix for dsa unbinding

[Florian Fainelli]

On 07/12/15 04:57, Neil Armstrong wrote:
> This serie fixes further issues for DSA dynamic unbinding.
> The first patch completely removes the PHY link state polling.
> The two following cleans up the dsa state upon removal.
> The last patch moves slave destroy code as slave function and
> adds missing netdev and phy cleanup calls.
> 
> v1: http://lkml.kernel.org/r/562F8ECB.6050709@baylibre.com
> v2: http://lkml.kernel.org/r/56321D9A.8010109@baylibre.com
> remove phy fix and add missing calls in dsa_switch_destroy
> then add dedicated dsa_slave_destroy
> 
> v3: remove polling instead of fixing it, make single patch for
> dsa slave destroy
> 
> Neil Armstrong (4):
>   net: dsa: remove DSA link polling
>   net: dsa: cleanup resources upon module removal
>   net: dsa: Add missing master netdev dev_put() calls
>   net: dsa: move dsa slave destroy code to slave.c

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

Re: [PATCH v3 net-next 0/4] Further fix for dsa unbinding

[David Miller]

From: Neil Armstrong <narmstrong@baylibre.com>
Date: Mon,  7 Dec 2015 13:57:31 +0100

> This serie fixes further issues for DSA dynamic unbinding.
> The first patch completely removes the PHY link state polling.
> The two following cleans up the dsa state upon removal.
> The last patch moves slave destroy code as slave function and
> adds missing netdev and phy cleanup calls.
> 
> v1: http://lkml.kernel.org/r/562F8ECB.6050709@baylibre.com
> v2: http://lkml.kernel.org/r/56321D9A.8010109@baylibre.com
> remove phy fix and add missing calls in dsa_switch_destroy
> then add dedicated dsa_slave_destroy
> 
> v3: remove polling instead of fixing it, make single patch for
> dsa slave destroy

Series applied, thanks Neil.

[PATCH] net: dsa: Fix cleanup resources upon module removal

[Neil Armstrong]

The initial commit badly merged into the dsa_resume method instead
of the dsa_remove_dst method.
As consequence, the dst->master_netdev->dsa_ptr is not set to NULL on
removal and re-bind of the dsa device fails with error -17.

Fixes: b0dc635d923c ("net: dsa: cleanup resources upon module removal ")
Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
 net/dsa/dsa.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

David, Florian, Andrew,

This fix is quite urgent since it breaks all the removal cleanup.

Thanks,
Neil

diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
index fa4daba..d8fb47f 100644
--- a/net/dsa/dsa.c
+++ b/net/dsa/dsa.c
@@ -935,6 +935,14 @@ static void dsa_remove_dst(struct dsa_switch_tree *dst)
 {
 	int i;
 
+	dst->master_netdev->dsa_ptr = NULL;
+
+	/* If we used a tagging format that doesn't have an ethertype
+	 * field, make sure that all packets from this point get sent
+	 * without the tag and go through the regular receive path.
+	 */
+	wmb();
+
 	for (i = 0; i < dst->pd->nr_chips; i++) {
 		struct dsa_switch *ds = dst->ds[i];
 
@@ -988,14 +996,6 @@ static int dsa_suspend(struct device *d)
 	struct dsa_switch_tree *dst = platform_get_drvdata(pdev);
 	int i, ret = 0;
 
-	dst->master_netdev->dsa_ptr = NULL;
-
-	/* If we used a tagging format that doesn't have an ethertype
-	 * field, make sure that all packets from this point get sent
-	 * without the tag and go through the regular receive path.
-	 */
-	wmb();
-
 	for (i = 0; i < dst->pd->nr_chips; i++) {
 		struct dsa_switch *ds = dst->ds[i];
 
-- 
1.9.1

Re: [PATCH] net: dsa: Fix cleanup resources upon module removal

[Andrew Lunn]

On Tue, Mar 08, 2016 at 10:36:20AM +0100, Neil Armstrong wrote:
> The initial commit badly merged into the dsa_resume method instead
> of the dsa_remove_dst method.
> As consequence, the dst->master_netdev->dsa_ptr is not set to NULL on
> removal and re-bind of the dsa device fails with error -17.
> 
> Fixes: b0dc635d923c ("net: dsa: cleanup resources upon module removal ")
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>

Acked-by: Andrew Lunn <andrew@lunn.ch>

Thanks

	Andrew

Re: [PATCH] net: dsa: Fix cleanup resources upon module removal

[David Miller]

From: Neil Armstrong <narmstrong@baylibre.com>
Date: Tue,  8 Mar 2016 10:36:20 +0100

> The initial commit badly merged into the dsa_resume method instead
> of the dsa_remove_dst method.
> As consequence, the dst->master_netdev->dsa_ptr is not set to NULL on
> removal and re-bind of the dsa device fails with error -17.
> 
> Fixes: b0dc635d923c ("net: dsa: cleanup resources upon module removal ")
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> ---
>  net/dsa/dsa.c | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> David, Florian, Andrew,
> 
> This fix is quite urgent since it breaks all the removal cleanup.

Since 'net' is closed, I've applied this to 'net-next' and queue it up for
-stable.

Thanks.