summaryrefslogtreecommitdiff
path: root/net/dsa/slave.c
diff options
context:
space:
mode:
authorVladimir Oltean <vladimir.oltean@nxp.com>2022-09-29 09:29:00 +0200
committerJakub Kicinski <kuba@kernel.org>2022-09-30 18:17:16 -0700
commitcf5ca4ddc37a693b17fdb653cb84b920b1185d71 (patch)
treea1421c503e49d6cde71b5bc3bfb260b62c2f5c7a /net/dsa/slave.c
parentd82acd85cc41a8e5d5e0e4c2a3f4b645def29723 (diff)
net: dsa: don't leave dangling pointers in dp->pl when failing
There is a desire to simplify the dsa_port registration path with devlink, and this involves reworking a bit how user ports which fail to connect to their PHY (because it's missing) get reinitialized as UNUSED devlink ports. The desire is for the change to look something like this; basically dsa_port_setup() has failed, we just change dp->type and call dsa_port_setup() again. -/* Destroy the current devlink port, and create a new one which has the UNUSED - * flavour. - */ -static int dsa_port_reinit_as_unused(struct dsa_port *dp) +static int dsa_port_setup_as_unused(struct dsa_port *dp) { - dsa_port_devlink_teardown(dp); dp->type = DSA_PORT_TYPE_UNUSED; - return dsa_port_devlink_setup(dp); + return dsa_port_setup(dp); } For an UNUSED port, dsa_port_setup() mostly only calls dsa_port_devlink_setup() anyway, so we could get away with calling just that. But if we call the full blown dsa_port_setup(dp) (which will be needed to properly set dp->setup = true), the callee will have the tendency to go through this code block too, and call dsa_port_disable(dp): switch (dp->type) { case DSA_PORT_TYPE_UNUSED: dsa_port_disable(dp); break; That is not very good, because dsa_port_disable() has this hidden inside of it: if (dp->pl) phylink_stop(dp->pl); Fact is, we are not prepared to handle a call to dsa_port_disable() with a struct dsa_port that came from a previous (and failed) call to dsa_port_setup(). We do not clean up dp->pl, and this will make the second call to dsa_port_setup() call phylink_stop() on a dangling dp->pl pointer. Solve this by creating an API for phylink destruction which is symmetric to the phylink creation, and never leave dp->pl set to anything except NULL or a valid phylink structure. Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com> Signed-off-by: Jiri Pirko <jiri@nvidia.com> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Diffstat (limited to 'net/dsa/slave.c')
-rw-r--r--net/dsa/slave.c6
1 files changed, 3 insertions, 3 deletions
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index aa47ddc19fdf..1a59918d3b30 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -2304,7 +2304,7 @@ static int dsa_slave_phy_setup(struct net_device *slave_dev)
if (ret) {
netdev_err(slave_dev, "failed to connect to PHY: %pe\n",
ERR_PTR(ret));
- phylink_destroy(dp->pl);
+ dsa_port_phylink_destroy(dp);
}
return ret;
@@ -2476,7 +2476,7 @@ out_phy:
rtnl_lock();
phylink_disconnect_phy(p->dp->pl);
rtnl_unlock();
- phylink_destroy(p->dp->pl);
+ dsa_port_phylink_destroy(p->dp);
out_gcells:
gro_cells_destroy(&p->gcells);
out_free:
@@ -2499,7 +2499,7 @@ void dsa_slave_destroy(struct net_device *slave_dev)
phylink_disconnect_phy(dp->pl);
rtnl_unlock();
- phylink_destroy(dp->pl);
+ dsa_port_phylink_destroy(dp);
gro_cells_destroy(&p->gcells);
free_percpu(slave_dev->tstats);
free_netdev(slave_dev);