diff mbox series

[v3,net-next,06/24] net: dsa: Call driver's setup callback after setting up its switchdev notifier

Message ID 20190413012822.30931-7-olteanv@gmail.com
State Changes Requested
Delegated to: David Miller
Headers show
Series NXP SJA1105 DSA driver | expand

Commit Message

Vladimir Oltean April 13, 2019, 1:28 a.m. UTC 
This allows the driver to perform some manipulations of its own during
setup, using generic switchdev calls. Having the notifiers registered at
setup time is important because otherwise any switchdev transaction
emitted during this time would be ignored (dispatched to an empty call
chain).

One current usage scenario is for the driver to request DSA to set up
802.1Q based switch tagging for its ports.

There is no danger for the driver setup code to start racing now with
switchdev events emitted from the network stack (such as bridge core)
even if the notifier is registered earlier. This is because the network
stack needs a net_device as a vehicle to perform switchdev operations,
and the slave net_devices are registered later than the core driver
setup anyway (ds->ops->setup in dsa_switch_setup vs dsa_port_setup).

Luckily DSA doesn't need a net_device to carry out switchdev callbacks,
and therefore drivers shouldn't assume either that net_devices are
available at the time their switchdev callbacks get invoked.

Signed-off-by: Vladimir Oltean <olteanv@gmail.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
---
Changes in v3:
None.

Changes in v2:
More verbiage in commit message.

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

Comments

Andrew Lunn April 13, 2019, 3:05 p.m. UTC | #1 
On Sat, Apr 13, 2019 at 04:28:04AM +0300, Vladimir Oltean wrote:
> This allows the driver to perform some manipulations of its own during
> setup, using generic switchdev calls. Having the notifiers registered at
> setup time is important because otherwise any switchdev transaction
> emitted during this time would be ignored (dispatched to an empty call
> chain).
> 
> One current usage scenario is for the driver to request DSA to set up
> 802.1Q based switch tagging for its ports.
> 
> There is no danger for the driver setup code to start racing now with
> switchdev events emitted from the network stack (such as bridge core)
> even if the notifier is registered earlier. This is because the network
> stack needs a net_device as a vehicle to perform switchdev operations,
> and the slave net_devices are registered later than the core driver
> setup anyway (ds->ops->setup in dsa_switch_setup vs dsa_port_setup).
> 
> Luckily DSA doesn't need a net_device to carry out switchdev callbacks,
> and therefore drivers shouldn't assume either that net_devices are
> available at the time their switchdev callbacks get invoked.

Hi Vladimir

Thanks for adding this explanation to the commit message.

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

    Andrew
diff mbox series

Patch

diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index d122f1bcdab2..17817c1a7fbd 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -369,14 +369,14 @@  static int dsa_switch_setup(struct dsa_switch *ds)
 	if (err)
 		return err;
 
-	err = ds->ops->setup(ds);
-	if (err < 0)
-		return err;
-
 	err = dsa_switch_register_notifier(ds);
 	if (err)
 		return err;
 
+	err = ds->ops->setup(ds);
+	if (err < 0)
+		return err;
+
 	if (!ds->slave_mii_bus && ds->ops->phy_read) {
 		ds->slave_mii_bus = devm_mdiobus_alloc(ds->dev);
 		if (!ds->slave_mii_bus)