* [PATCH net] net: dpaa2-switch: disable the control interface on error path
@ 2021-08-19 14:17 Vladimir Oltean
2021-08-19 15:29 ` Ioana Ciornei
0 siblings, 1 reply; 3+ messages in thread
From: Vladimir Oltean @ 2021-08-19 14:17 UTC (permalink / raw)
To: netdev, Jakub Kicinski, David S. Miller; +Cc: Ioana Ciornei
Currently dpaa2_switch_takedown has a funny name and does not do the
opposite of dpaa2_switch_init, which makes probing fail when we need to
handle an -EPROBE_DEFER.
A sketch of what dpaa2_switch_init does:
dpsw_open
dpaa2_switch_detect_features
dpsw_reset
for (i = 0; i < ethsw->sw_attr.num_ifs; i++) {
dpsw_if_disable
dpsw_if_set_stp
dpsw_vlan_remove_if_untagged
dpsw_if_set_tci
dpsw_vlan_remove_if
}
dpsw_vlan_remove
alloc_ordered_workqueue
dpsw_fdb_remove
dpaa2_switch_ctrl_if_setup
When dpaa2_switch_takedown is called from the error path of
dpaa2_switch_probe(), the control interface, enabled by
dpaa2_switch_ctrl_if_setup from dpaa2_switch_init, remains enabled,
because dpaa2_switch_takedown does not call
dpaa2_switch_ctrl_if_teardown.
Since dpaa2_switch_probe might fail due to EPROBE_DEFER of a PHY, this
means that a second probe of the driver will happen with the control
interface directly enabled.
This will trigger a second error:
[ 93.273528] fsl_dpaa2_switch dpsw.0: dpsw_ctrl_if_set_pools() failed
[ 93.281966] fsl_dpaa2_switch dpsw.0: fsl_mc_driver_probe failed: -13
[ 93.288323] fsl_dpaa2_switch: probe of dpsw.0 failed with error -13
Which if we investigate the /dev/dpaa2_mc_console log, we find out is
caused by:
[E, ctrl_if_set_pools:2211, DPMNG] ctrl_if must be disabled
So make dpaa2_switch_takedown do the opposite of dpaa2_switch_init (in
reasonable limits, no reason to change STP state, re-add VLANs etc), and
rename it to something more conventional, like dpaa2_switch_teardown.
Fixes: 613c0a5810b7 ("staging: dpaa2-switch: enable the control interface")
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
.../ethernet/freescale/dpaa2/dpaa2-switch.c | 36 +++++++++----------
1 file changed, 18 insertions(+), 18 deletions(-)
diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c b/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c
index 68b78642c045..98cc0133c343 100644
--- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c
+++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c
@@ -3038,26 +3038,30 @@ static int dpaa2_switch_port_init(struct ethsw_port_priv *port_priv, u16 port)
return err;
}
-static void dpaa2_switch_takedown(struct fsl_mc_device *sw_dev)
+static void dpaa2_switch_ctrl_if_teardown(struct ethsw_core *ethsw)
+{
+ dpsw_ctrl_if_disable(ethsw->mc_io, 0, ethsw->dpsw_handle);
+ dpaa2_switch_free_dpio(ethsw);
+ dpaa2_switch_destroy_rings(ethsw);
+ dpaa2_switch_drain_bp(ethsw);
+ dpaa2_switch_free_dpbp(ethsw);
+}
+
+static void dpaa2_switch_teardown(struct fsl_mc_device *sw_dev)
{
struct device *dev = &sw_dev->dev;
struct ethsw_core *ethsw = dev_get_drvdata(dev);
int err;
+ dpaa2_switch_ctrl_if_teardown(ethsw);
+
+ destroy_workqueue(ethsw->workqueue);
+
err = dpsw_close(ethsw->mc_io, 0, ethsw->dpsw_handle);
if (err)
dev_warn(dev, "dpsw_close err %d\n", err);
}
-static void dpaa2_switch_ctrl_if_teardown(struct ethsw_core *ethsw)
-{
- dpsw_ctrl_if_disable(ethsw->mc_io, 0, ethsw->dpsw_handle);
- dpaa2_switch_free_dpio(ethsw);
- dpaa2_switch_destroy_rings(ethsw);
- dpaa2_switch_drain_bp(ethsw);
- dpaa2_switch_free_dpbp(ethsw);
-}
-
static int dpaa2_switch_remove(struct fsl_mc_device *sw_dev)
{
struct ethsw_port_priv *port_priv;
@@ -3068,8 +3072,6 @@ static int dpaa2_switch_remove(struct fsl_mc_device *sw_dev)
dev = &sw_dev->dev;
ethsw = dev_get_drvdata(dev);
- dpaa2_switch_ctrl_if_teardown(ethsw);
-
dpaa2_switch_teardown_irqs(sw_dev);
dpsw_disable(ethsw->mc_io, 0, ethsw->dpsw_handle);
@@ -3084,9 +3086,7 @@ static int dpaa2_switch_remove(struct fsl_mc_device *sw_dev)
kfree(ethsw->acls);
kfree(ethsw->ports);
- dpaa2_switch_takedown(sw_dev);
-
- destroy_workqueue(ethsw->workqueue);
+ dpaa2_switch_teardown(sw_dev);
fsl_mc_portal_free(ethsw->mc_io);
@@ -3199,7 +3199,7 @@ static int dpaa2_switch_probe(struct fsl_mc_device *sw_dev)
GFP_KERNEL);
if (!(ethsw->ports)) {
err = -ENOMEM;
- goto err_takedown;
+ goto err_teardown;
}
ethsw->fdbs = kcalloc(ethsw->sw_attr.num_ifs, sizeof(*ethsw->fdbs),
@@ -3270,8 +3270,8 @@ static int dpaa2_switch_probe(struct fsl_mc_device *sw_dev)
err_free_ports:
kfree(ethsw->ports);
-err_takedown:
- dpaa2_switch_takedown(sw_dev);
+err_teardown:
+ dpaa2_switch_teardown(sw_dev);
err_free_cmdport:
fsl_mc_portal_free(ethsw->mc_io);
--
2.25.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH net] net: dpaa2-switch: disable the control interface on error path
2021-08-19 14:17 [PATCH net] net: dpaa2-switch: disable the control interface on error path Vladimir Oltean
@ 2021-08-19 15:29 ` Ioana Ciornei
2021-08-19 18:15 ` Jakub Kicinski
0 siblings, 1 reply; 3+ messages in thread
From: Ioana Ciornei @ 2021-08-19 15:29 UTC (permalink / raw)
To: Vladimir Oltean; +Cc: netdev, Jakub Kicinski, David S. Miller
On Thu, Aug 19, 2021 at 05:17:55PM +0300, Vladimir Oltean wrote:
> Currently dpaa2_switch_takedown has a funny name and does not do the
> opposite of dpaa2_switch_init, which makes probing fail when we need to
> handle an -EPROBE_DEFER.
>
> A sketch of what dpaa2_switch_init does:
>
> dpsw_open
>
> dpaa2_switch_detect_features
>
> dpsw_reset
>
> for (i = 0; i < ethsw->sw_attr.num_ifs; i++) {
> dpsw_if_disable
>
> dpsw_if_set_stp
>
> dpsw_vlan_remove_if_untagged
>
> dpsw_if_set_tci
>
> dpsw_vlan_remove_if
> }
>
> dpsw_vlan_remove
>
> alloc_ordered_workqueue
>
> dpsw_fdb_remove
>
> dpaa2_switch_ctrl_if_setup
>
> When dpaa2_switch_takedown is called from the error path of
> dpaa2_switch_probe(), the control interface, enabled by
> dpaa2_switch_ctrl_if_setup from dpaa2_switch_init, remains enabled,
> because dpaa2_switch_takedown does not call
> dpaa2_switch_ctrl_if_teardown.
>
> Since dpaa2_switch_probe might fail due to EPROBE_DEFER of a PHY, this
> means that a second probe of the driver will happen with the control
> interface directly enabled.
>
> This will trigger a second error:
>
> [ 93.273528] fsl_dpaa2_switch dpsw.0: dpsw_ctrl_if_set_pools() failed
> [ 93.281966] fsl_dpaa2_switch dpsw.0: fsl_mc_driver_probe failed: -13
> [ 93.288323] fsl_dpaa2_switch: probe of dpsw.0 failed with error -13
>
> Which if we investigate the /dev/dpaa2_mc_console log, we find out is
> caused by:
>
> [E, ctrl_if_set_pools:2211, DPMNG] ctrl_if must be disabled
>
> So make dpaa2_switch_takedown do the opposite of dpaa2_switch_init (in
> reasonable limits, no reason to change STP state, re-add VLANs etc), and
> rename it to something more conventional, like dpaa2_switch_teardown.
>
> Fixes: 613c0a5810b7 ("staging: dpaa2-switch: enable the control interface")
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
Reviewed-by: Ioana Ciornei <ioana.ciornei@nxp.com>
Thanks!
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH net] net: dpaa2-switch: disable the control interface on error path
2021-08-19 15:29 ` Ioana Ciornei
@ 2021-08-19 18:15 ` Jakub Kicinski
0 siblings, 0 replies; 3+ messages in thread
From: Jakub Kicinski @ 2021-08-19 18:15 UTC (permalink / raw)
To: Ioana Ciornei, Vladimir Oltean; +Cc: netdev, David S. Miller
On Thu, 19 Aug 2021 15:29:49 +0000 Ioana Ciornei wrote:
> On Thu, Aug 19, 2021 at 05:17:55PM +0300, Vladimir Oltean wrote:
> > Currently dpaa2_switch_takedown has a funny name and does not do the
> > opposite of dpaa2_switch_init, which makes probing fail when we need to
> > handle an -EPROBE_DEFER.
> > [E, ctrl_if_set_pools:2211, DPMNG] ctrl_if must be disabled
> >
> > So make dpaa2_switch_takedown do the opposite of dpaa2_switch_init (in
> > reasonable limits, no reason to change STP state, re-add VLANs etc), and
> > rename it to something more conventional, like dpaa2_switch_teardown.
> >
> > Fixes: 613c0a5810b7 ("staging: dpaa2-switch: enable the control interface")
> > Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
>
> Reviewed-by: Ioana Ciornei <ioana.ciornei@nxp.com>
Applied, thanks!
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-08-19 18:16 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-19 14:17 [PATCH net] net: dpaa2-switch: disable the control interface on error path Vladimir Oltean
2021-08-19 15:29 ` Ioana Ciornei
2021-08-19 18:15 ` Jakub Kicinski
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.