Further fix for dsa unbinding

PATCH Superseded Neil Armstrong 4 participants 12 messages Oct 29, 2015 – Mar 10, 2016

Subsystems (3)

Networking Network drivers Networking DSA

lore

Share & tools

lore

Neil Armstrong

author

2015-10-29 13:22 UTC #0 lore raw

v2 cover Further fix for dsa unbinding

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(-)

Signature · 1 line

1.9.1

Neil Armstrong

author

2015-12-07 12:57 UTC #1 lore raw

v3 cover Further fix for dsa unbinding

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/[email protected]
v2: http://lkml.kernel.org/r/[email protected]
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(-)

Signature · 1 line

1.9.1

Neil Armstrong

author

2015-12-07 12:57 UTC #2 lore raw

PATCH v3 1/4 net: dsa: remove DSA link polling

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 <[email protected]>

---
 include/net/dsa.h | 12 ------------
 net/dsa/dsa.c     | 43 -------------------------------------------
 2 files changed, 55 deletions(-)

include/net/dsa.h +0 ~~-12~~

index 3f23dd9..26a0e86 100644
@@ -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,

net/dsa/dsa.c +0 ~~-43~~

67 lines not on this page · raw message

Signature · 1 line

1.9.1

Neil Armstrong

author

2015-12-07 12:57 UTC #3 lore raw

PATCH v3 2/4 net: dsa: cleanup resources upon module removal

In reply to Neil Armstrong

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

Signed-off-by Neil Armstrong <[email protected]>

---
 net/dsa/dsa.c | 8 ++++++++
 1 file changed, 8 insertions(+)

net/dsa/dsa.c +8 -0

index 0f41f71..d9e0172 100644
@@ -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];

Signature · 1 line

1.9.1

Neil Armstrong

author

2015-12-07 12:59 UTC #4 lore raw

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

In reply to Neil Armstrong

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

Signed-off-by Neil Armstrong <[email protected]>

---
 net/dsa/dsa.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

net/dsa/dsa.c +5 -1

index d9e0172..d22d303e 100644
@@ -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)

Signature · 1 line

1.9.1

Neil Armstrong

author

2015-12-07 12:59 UTC #5 lore raw

PATCH v3 4/4 net: dsa: move dsa slave destroy code to slave.c

In reply to 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 <[email protected]>

Signed-off-by Neil Armstrong <[email protected]>

---
 net/dsa/dsa.c      |  3 +--
 net/dsa/dsa_priv.h |  1 +
 net/dsa/slave.c    | 11 +++++++++++
 3 files changed, 13 insertions(+), 2 deletions(-)

net/dsa/dsa.c +1 -2

index d22d303e..208d1b2 100644
@@ -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);

net/dsa/dsa_priv.h +1 -0

index 311796c8..1d1a546 100644
@@ -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,

net/dsa/slave.c +11 -0

index 7bc787b..1e9e942 100644
@@ -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;

Signature · 1 line

1.9.1

Andrew Lunn

maintainer Acked-by on the v3 series

2015-12-07 14:28 UTC #6 lore raw

In reply to Neil Armstrong

Quoted 6 lines · 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.

Quoted 7 lines

v1: http://lkml.kernel.org/r/[email protected]
v2: http://lkml.kernel.org/r/[email protected]
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 <[email protected]>

Thanks
Andrew

Florian Fainelli

Reviewed-by on the v3 series

2015-12-07 20:39 UTC #7 lore raw

In reply to Neil Armstrong

Quoted 11 lines · 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/[email protected]
v2: http://lkml.kernel.org/r/[email protected]
remove phy fix and add missing calls in dsa_switch_destroy then add dedicated dsa_slave_destroy

Quoted 8 lines

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 <[email protected]>

Signature · 1 line

Florian

David Miller

maintainer

2015-12-07 21:36 UTC #8 lore raw

In reply to Neil Armstrong

From: Neil Armstrong <[email protected]>
Date: Mon, 7 Dec 2015 13:57:31 +0100

Quoted 6 lines

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.

Quoted 7 lines

v1: http://lkml.kernel.org/r/[email protected]
v2: http://lkml.kernel.org/r/[email protected]
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.

Neil Armstrong

author

2016-03-08 09:36 UTC #9 lore raw

PATCH 1/1 net: dsa: Fix cleanup resources upon module removal

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 <[email protected]>

---
 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

net/dsa/dsa.c +8 -8

index fa4daba..d8fb47f 100644
@@ -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];

Signature · 1 line

1.9.1

Andrew Lunn

maintainer Acked-by on v1 1/1

2016-03-08 14:52 UTC #10 lore raw

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

Quoted 7 lines · 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 <[email protected]>

Acked-by Andrew Lunn <[email protected]>

Thanks

Andrew

David Miller

maintainer

2016-03-10 21:21 UTC #11 lore raw

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

In reply to Neil Armstrong

From: Neil Armstrong <[email protected]>
Date: Tue, 8 Mar 2016 10:36:20 +0100

Quoted 7 lines

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 <[email protected]>

Quoted 7 lines

---
 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.