2015-12-07 12:59:07

by Neil Armstrong

[permalink] [raw]
Subject: [PATCH v3 net-next 0/4] 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(-)

--
1.9.1


2015-12-07 13:00:31

by Neil Armstrong

[permalink] [raw]
Subject: [PATCH v3 net-next 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(-)

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

2015-12-07 12:59:16

by Neil Armstrong

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

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 <[email protected]>
Signed-off-by: Neil Armstrong <[email protected]>
---
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

2015-12-07 12:59:14

by Neil Armstrong

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

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

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

2015-12-07 12:59:55

by Neil Armstrong

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

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

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

2015-12-07 14:28:42

by Andrew Lunn

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

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

2015-12-07 20:39:34

by Florian Fainelli

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

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

Reviewed-by: Florian Fainelli <[email protected]>
--
Florian

2015-12-07 21:36:19

by David Miller

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

From: Neil Armstrong <[email protected]>
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/[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.

2016-03-08 09:36:02

by Neil Armstrong

[permalink] [raw]
Subject: [PATCH] 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

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

2016-03-08 14:52:37

by Andrew Lunn

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

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

Acked-by: Andrew Lunn <[email protected]>

Thanks

Andrew

2016-03-10 21:21:06

by David Miller

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

From: Neil Armstrong <[email protected]>
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 <[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.

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

Thanks.