From da2c398e59d6b47260e4198559b9f284e3b557e0 Mon Sep 17 00:00:00 2001 From: Vladimir Oltean Date: Thu, 18 Aug 2022 14:54:58 +0300 Subject: net: dsa: avoid dsa_port_link_{,un}register_of() calls with platform data dsa_port_link_register_of() and dsa_port_link_unregister_of() are not written with the fact in mind that they can be called with a dp->dn that is NULL (as evidenced even by the _of suffix in their name), but this is exactly what happens. How this behaves will differ depending on whether the backing driver implements ->adjust_link() or not. If it doesn't, the "if (of_phy_is_fixed_link(dp->dn) || phy_np)" condition will return false, and dsa_port_link_register_of() will do nothing and return 0. If the driver does implement ->adjust_link(), the "if (of_phy_is_fixed_link(dp->dn))" condition will return false (dp->dn is NULL) and we will call dsa_port_setup_phy_of(). This will call dsa_port_get_phy_device(), which will also return NULL, and we will also do nothing and return 0. It is hard to maintain this code and make future changes to it in this state, so just suppress calls to these 2 functions if dp->dn is NULL. The only functional effect is that if the driver does implement ->adjust_link(), we'll stop printing this to the console: Using legacy PHYLIB callbacks. Please migrate to PHYLINK! but instead we'll always print: [ 8.539848] dsa-loop fixed-0:1f: skipping link registration for CPU port 5 This is for the better anyway, since "using legacy phylib callbacks" was misleading information - we weren't issuing _any_ callbacks due to dsa_port_get_phy_device() returning NULL. Signed-off-by: Vladimir Oltean Signed-off-by: Jakub Kicinski --- net/dsa/dsa2.c | 34 ++++++++++++++++++++++++---------- 1 file changed, 24 insertions(+), 10 deletions(-) (limited to 'net/dsa') diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c index cac48a741f27..12479707bf96 100644 --- a/net/dsa/dsa2.c +++ b/net/dsa/dsa2.c @@ -469,10 +469,16 @@ static int dsa_port_setup(struct dsa_port *dp) dsa_port_disable(dp); break; case DSA_PORT_TYPE_CPU: - err = dsa_port_link_register_of(dp); - if (err) - break; - dsa_port_link_registered = true; + if (dp->dn) { + err = dsa_port_link_register_of(dp); + if (err) + break; + dsa_port_link_registered = true; + } else { + dev_warn(ds->dev, + "skipping link registration for CPU port %d\n", + dp->index); + } err = dsa_port_enable(dp, NULL); if (err) @@ -481,10 +487,16 @@ static int dsa_port_setup(struct dsa_port *dp) break; case DSA_PORT_TYPE_DSA: - err = dsa_port_link_register_of(dp); - if (err) - break; - dsa_port_link_registered = true; + if (dp->dn) { + err = dsa_port_link_register_of(dp); + if (err) + break; + dsa_port_link_registered = true; + } else { + dev_warn(ds->dev, + "skipping link registration for DSA port %d\n", + dp->index); + } err = dsa_port_enable(dp, NULL); if (err) @@ -577,11 +589,13 @@ static void dsa_port_teardown(struct dsa_port *dp) break; case DSA_PORT_TYPE_CPU: dsa_port_disable(dp); - dsa_port_link_unregister_of(dp); + if (dp->dn) + dsa_port_link_unregister_of(dp); break; case DSA_PORT_TYPE_DSA: dsa_port_disable(dp); - dsa_port_link_unregister_of(dp); + if (dp->dn) + dsa_port_link_unregister_of(dp); break; case DSA_PORT_TYPE_USER: if (dp->slave) { -- cgit v1.2.3 From 770375ff331111b8495e7d7099395659616f1025 Mon Sep 17 00:00:00 2001 From: Vladimir Oltean Date: Thu, 18 Aug 2022 14:54:59 +0300 Subject: net: dsa: rename dsa_port_link_{,un}register_of There is a subset of functions that applies only to shared (DSA and CPU) ports, yet this is difficult to comprehend by looking at their code alone. These are dsa_port_link_register_of(), dsa_port_link_unregister_of(), and the functions that only these 2 call. Rename this class of functions to dsa_shared_port_* to make this fact more evident, even if this goes against the apparent convention that function names in port.c must start with dsa_port_. Signed-off-by: Vladimir Oltean Signed-off-by: Jakub Kicinski --- net/dsa/dsa2.c | 10 +++++----- net/dsa/dsa_priv.h | 4 ++-- net/dsa/port.c | 18 +++++++++--------- 3 files changed, 16 insertions(+), 16 deletions(-) (limited to 'net/dsa') diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c index 12479707bf96..055a6d1d4372 100644 --- a/net/dsa/dsa2.c +++ b/net/dsa/dsa2.c @@ -470,7 +470,7 @@ static int dsa_port_setup(struct dsa_port *dp) break; case DSA_PORT_TYPE_CPU: if (dp->dn) { - err = dsa_port_link_register_of(dp); + err = dsa_shared_port_link_register_of(dp); if (err) break; dsa_port_link_registered = true; @@ -488,7 +488,7 @@ static int dsa_port_setup(struct dsa_port *dp) break; case DSA_PORT_TYPE_DSA: if (dp->dn) { - err = dsa_port_link_register_of(dp); + err = dsa_shared_port_link_register_of(dp); if (err) break; dsa_port_link_registered = true; @@ -517,7 +517,7 @@ static int dsa_port_setup(struct dsa_port *dp) if (err && dsa_port_enabled) dsa_port_disable(dp); if (err && dsa_port_link_registered) - dsa_port_link_unregister_of(dp); + dsa_shared_port_link_unregister_of(dp); if (err) { if (ds->ops->port_teardown) ds->ops->port_teardown(ds, dp->index); @@ -590,12 +590,12 @@ static void dsa_port_teardown(struct dsa_port *dp) case DSA_PORT_TYPE_CPU: dsa_port_disable(dp); if (dp->dn) - dsa_port_link_unregister_of(dp); + dsa_shared_port_link_unregister_of(dp); break; case DSA_PORT_TYPE_DSA: dsa_port_disable(dp); if (dp->dn) - dsa_port_link_unregister_of(dp); + dsa_shared_port_link_unregister_of(dp); break; case DSA_PORT_TYPE_USER: if (dp->slave) { diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h index d9722e49864b..8924366467e0 100644 --- a/net/dsa/dsa_priv.h +++ b/net/dsa/dsa_priv.h @@ -285,8 +285,8 @@ int dsa_port_mrp_add_ring_role(const struct dsa_port *dp, int dsa_port_mrp_del_ring_role(const struct dsa_port *dp, const struct switchdev_obj_ring_role_mrp *mrp); int dsa_port_phylink_create(struct dsa_port *dp); -int dsa_port_link_register_of(struct dsa_port *dp); -void dsa_port_link_unregister_of(struct dsa_port *dp); +int dsa_shared_port_link_register_of(struct dsa_port *dp); +void dsa_shared_port_link_unregister_of(struct dsa_port *dp); int dsa_port_hsr_join(struct dsa_port *dp, struct net_device *hsr); void dsa_port_hsr_leave(struct dsa_port *dp, struct net_device *hsr); int dsa_port_tag_8021q_vlan_add(struct dsa_port *dp, u16 vid, bool broadcast); diff --git a/net/dsa/port.c b/net/dsa/port.c index a8895ee3cd60..bd0cb38a63f6 100644 --- a/net/dsa/port.c +++ b/net/dsa/port.c @@ -1555,7 +1555,7 @@ int dsa_port_phylink_create(struct dsa_port *dp) return 0; } -static int dsa_port_setup_phy_of(struct dsa_port *dp, bool enable) +static int dsa_shared_port_setup_phy_of(struct dsa_port *dp, bool enable) { struct dsa_switch *ds = dp->ds; struct phy_device *phydev; @@ -1593,7 +1593,7 @@ err_put_dev: return err; } -static int dsa_port_fixed_link_register_of(struct dsa_port *dp) +static int dsa_shared_port_fixed_link_register_of(struct dsa_port *dp) { struct device_node *dn = dp->dn; struct dsa_switch *ds = dp->ds; @@ -1627,7 +1627,7 @@ static int dsa_port_fixed_link_register_of(struct dsa_port *dp) return 0; } -static int dsa_port_phylink_register(struct dsa_port *dp) +static int dsa_shared_port_phylink_register(struct dsa_port *dp) { struct dsa_switch *ds = dp->ds; struct device_node *port_dn = dp->dn; @@ -1653,7 +1653,7 @@ err_phy_connect: return err; } -int dsa_port_link_register_of(struct dsa_port *dp) +int dsa_shared_port_link_register_of(struct dsa_port *dp) { struct dsa_switch *ds = dp->ds; struct device_node *phy_np; @@ -1666,7 +1666,7 @@ int dsa_port_link_register_of(struct dsa_port *dp) ds->ops->phylink_mac_link_down(ds, port, MLO_AN_FIXED, PHY_INTERFACE_MODE_NA); of_node_put(phy_np); - return dsa_port_phylink_register(dp); + return dsa_shared_port_phylink_register(dp); } of_node_put(phy_np); return 0; @@ -1676,12 +1676,12 @@ int dsa_port_link_register_of(struct dsa_port *dp) "Using legacy PHYLIB callbacks. Please migrate to PHYLINK!\n"); if (of_phy_is_fixed_link(dp->dn)) - return dsa_port_fixed_link_register_of(dp); + return dsa_shared_port_fixed_link_register_of(dp); else - return dsa_port_setup_phy_of(dp, true); + return dsa_shared_port_setup_phy_of(dp, true); } -void dsa_port_link_unregister_of(struct dsa_port *dp) +void dsa_shared_port_link_unregister_of(struct dsa_port *dp) { struct dsa_switch *ds = dp->ds; @@ -1697,7 +1697,7 @@ void dsa_port_link_unregister_of(struct dsa_port *dp) if (of_phy_is_fixed_link(dp->dn)) of_phy_deregister_fixed_link(dp->dn); else - dsa_port_setup_phy_of(dp, false); + dsa_shared_port_setup_phy_of(dp, false); } int dsa_port_hsr_join(struct dsa_port *dp, struct net_device *hsr) -- cgit v1.2.3 From e09e9873152e3f804dd704eb3e772538f8447149 Mon Sep 17 00:00:00 2001 From: Vladimir Oltean Date: Thu, 18 Aug 2022 14:55:00 +0300 Subject: net: dsa: make phylink-related OF properties mandatory on DSA and CPU ports MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Early DSA drivers were kind of simplistic in that they assumed a fairly narrow hardware layout. User ports would have integrated PHYs at an internal MDIO address that is derivable from the port number, and shared (DSA and CPU) ports would have an MII-style (serial or parallel) connection to another MAC. Phylib and then phylink were used to drive the internal PHYs, and this needed little to no description through the platform data structures. Bringing up the shared ports at the maximum supported link speed was the responsibility of the drivers. As a result of this, when these early drivers were converted from platform data to the new DSA OF bindings, there was no link information translated into the first DT bindings. https://lore.kernel.org/all/YtXFtTsf++AeDm1l@lunn.ch/ Later, phylink was adopted for shared ports as well, and today we have a workaround in place, introduced by commit a20f997010c4 ("net: dsa: Don't instantiate phylink for CPU/DSA ports unless needed"). There, DSA checks for the presence of phy-handle/fixed-link/managed OF properties, and if missing, phylink registration would be skipped. This is because phylink is optional for some drivers (the shared ports already work without it), but the process of starting to register a port with phylink is irreversible: if phylink_create() fails to find the fwnode properties it needs, it bails out and it leaves the ports inoperational (because phylink expects ports to be initially down, so DSA necessarily takes them down, and doesn't know how to put them back up again). DSA being a common framework, new drivers opt into this workaround willy-nilly, but the ideal behavior from the DSA core's side would have been to not interfere with phylink's process of failing at all. This isn't possible because of regression concerns with pre-phylink DT blobs, but at least DSA should put a stop to the proliferation of more of such cases that rely on the workaround to skip phylink registration, and sanitize the environment that new drivers work in. To that end, create a list of compatible strings for which the workaround is preserved, and don't apply the workaround for any drivers outside that list (this includes new drivers). In some cases, we make the assumption that even existing drivers don't rely on DSA's workaround, and we do this by looking at the device trees in which they appear. We can't fully know what is the situation with downstream DT blobs, but we can guess the overall trend by studying the DT blobs that were submitted upstream. If there are upstream blobs that have lacking descriptions, we take it as very likely that there are many more downstream blobs that do so too. If all upstream blobs have complete descriptions, we take that as a hint that the driver is a candidate for enforcing strict DT bindings (considering that most bindings are copy-pasted). If there are no upstream DT blobs, we take the conservative route of allowing the workaround, unless the driver maintainer instructs us otherwise. The driver situation is as follows: ar9331 ~~~~~~ compatible strings: - qca,ar9331-switch 1 occurrence in mainline device trees, part of SoC dtsi (arch/mips/boot/dts/qca/ar9331.dtsi), description is not problematic. Verdict: opt into strict DT bindings and out of workarounds. b53 ~~~ compatible strings: - brcm,bcm5325 - brcm,bcm53115 - brcm,bcm53125 - brcm,bcm53128 - brcm,bcm5365 - brcm,bcm5389 - brcm,bcm5395 - brcm,bcm5397 - brcm,bcm5398 - brcm,bcm53010-srab - brcm,bcm53011-srab - brcm,bcm53012-srab - brcm,bcm53018-srab - brcm,bcm53019-srab - brcm,bcm5301x-srab - brcm,bcm11360-srab - brcm,bcm58522-srab - brcm,bcm58525-srab - brcm,bcm58535-srab - brcm,bcm58622-srab - brcm,bcm58623-srab - brcm,bcm58625-srab - brcm,bcm88312-srab - brcm,cygnus-srab - brcm,nsp-srab - brcm,omega-srab - brcm,bcm3384-switch - brcm,bcm6328-switch - brcm,bcm6368-switch - brcm,bcm63xx-switch I've found at least these mainline DT blobs with problems: arch/arm/boot/dts/bcm47094-linksys-panamera.dts - lacks phy-mode arch/arm/boot/dts/bcm47189-tenda-ac9.dts - lacks phy-mode and fixed-link arch/arm/boot/dts/bcm47081-luxul-xap-1410.dts arch/arm/boot/dts/bcm47081-luxul-xwr-1200.dts arch/arm/boot/dts/bcm47081-buffalo-wzr-600dhp2.dts - lacks phy-mode and fixed-link arch/arm/boot/dts/bcm47094-luxul-xbr-4500.dts arch/arm/boot/dts/bcm4708-smartrg-sr400ac.dts arch/arm/boot/dts/bcm4708-luxul-xap-1510.dts arch/arm/boot/dts/bcm953012er.dts arch/arm/boot/dts/bcm4708-netgear-r6250.dts arch/arm/boot/dts/bcm4708-buffalo-wzr-1166dhp-common.dtsi arch/arm/boot/dts/bcm4708-luxul-xwc-1000.dts arch/arm/boot/dts/bcm47094-luxul-abr-4500.dts - lacks phy-mode and fixed-link arch/arm/boot/dts/bcm53016-meraki-mr32.dts - lacks phy-mode Verdict: opt into DSA workarounds. bcm_sf2 ~~~~~~~ compatible strings: - brcm,bcm4908-switch - brcm,bcm7445-switch-v4.0 - brcm,bcm7278-switch-v4.0 - brcm,bcm7278-switch-v4.8 A single occurrence in mainline (arch/arm64/boot/dts/broadcom/bcm4908/bcm4908.dtsi), part of a SoC dtsi, valid description. Florian Fainelli explains that most of the bcm_sf2 device trees lack a full description for the internal IMP ports. Verdict: opt the BCM4908 into strict DT bindings, and opt the rest into the workarounds. Note that even though BCM4908 has strict DT bindings, it still does not register with phylink on the IMP port due to it implementing ->adjust_link(). hellcreek ~~~~~~~~~ compatible strings: - hirschmann,hellcreek-de1soc-r1 No occurrence in mainline device trees. Kurt Kanzenbach explains that the downstream device trees lacked phy-mode and fixed link, and needed work, but were fixed in the meantime. Verdict: opt into strict DT bindings and out of workarounds. lan9303 ~~~~~~~ compatible strings: - smsc,lan9303-mdio - smsc,lan9303-i2c 1 occurrence in mainline device trees: arch/arm/boot/dts/imx53-kp-hsc.dts - no phy-mode, no fixed-link Verdict: opt out of strict DT bindings and into workarounds. lantiq_gswip ~~~~~~~~~~~~ compatible strings: - lantiq,xrx200-gswip - lantiq,xrx300-gswip - lantiq,xrx330-gswip No occurrences in mainline device trees. Martin Blumenstingl confirms that the downstream OpenWrt device trees lack a proper fixed-link and need work, and that the incomplete description can even be seen in the example from Documentation/devicetree/bindings/net/dsa/lantiq-gswip.txt. Verdict: opt out of strict DT bindings and into workarounds. microchip ksz ~~~~~~~~~~~~~ compatible strings: - microchip,ksz8765 - microchip,ksz8794 - microchip,ksz8795 - microchip,ksz8863 - microchip,ksz8873 - microchip,ksz9477 - microchip,ksz9897 - microchip,ksz9893 - microchip,ksz9563 - microchip,ksz8563 - microchip,ksz9567 - microchip,lan9370 - microchip,lan9371 - microchip,lan9372 - microchip,lan9373 - microchip,lan9374 5 occurrences in mainline device trees, all descriptions are valid. But we had a snafu for the ksz8795 and ksz9477 drivers where the phy-mode property would be expected to be located directly under the 'switch' node rather than under a port OF node. It was fixed by commit edecfa98f602 ("net: dsa: microchip: look for phy-mode in port nodes"). The driver still has compatibility with the old DT blobs. The lan937x support was added later than the above snafu was fixed, and even though it has support for the broken DT blobs by virtue of sharing a common probing function, I'll take it that its DT blobs are correct. Verdict: opt lan937x into strict DT bindings, and the others out. mt7530 ~~~~~~ compatible strings - mediatek,mt7621 - mediatek,mt7530 - mediatek,mt7531 Multiple occurrences in mainline device trees, one is part of an SoC dtsi (arch/mips/boot/dts/ralink/mt7621.dtsi), all descriptions are fine. Verdict: opt into strict DT bindings and out of workarounds. mv88e6060 ~~~~~~~~~ compatible string: - marvell,mv88e6060 no occurrences in mainline, nobody knows anybody who uses it. Verdict: opt out of strict DT bindings and into workarounds. mv88e6xxx ~~~~~~~~~ compatible strings: - marvell,mv88e6085 - marvell,mv88e6190 - marvell,mv88e6250 Device trees that have incomplete descriptions of CPU or DSA ports: arch/arm64/boot/dts/freescale/imx8mq-zii-ultra.dtsi - lacks phy-mode arch/arm64/boot/dts/marvell/cn9130-crb.dtsi - lacks phy-mode and fixed-link arch/arm/boot/dts/vf610-zii-ssmb-spu3.dts - lacks phy-mode arch/arm/boot/dts/kirkwood-mv88f6281gtw-ge.dts - lacks phy-mode arch/arm/boot/dts/vf610-zii-spb4.dts - lacks phy-mode arch/arm/boot/dts/vf610-zii-cfu1.dts - lacks phy-mode arch/arm/boot/dts/vf610-zii-dev-rev-c.dts - lacks phy-mode on CPU port, fixed-link on DSA ports arch/arm/boot/dts/vf610-zii-dev-rev-b.dts - lacks phy-mode on CPU port arch/arm/boot/dts/armada-381-netgear-gs110emx.dts - lacks phy-mode arch/arm/boot/dts/vf610-zii-scu4-aib.dts - lacks fixed-link on xgmii DSA ports and/or in-band-status on 2500base-x DSA ports, and phy-mode on CPU port arch/arm/boot/dts/imx6qdl-gw5904.dtsi - lacks phy-mode and fixed-link arch/arm/boot/dts/armada-385-clearfog-gtr-l8.dts - lacks phy-mode and fixed-link arch/arm/boot/dts/vf610-zii-ssmb-dtu.dts - lacks phy-mode arch/arm/boot/dts/kirkwood-dir665.dts - lacks phy-mode arch/arm/boot/dts/kirkwood-rd88f6281.dtsi - lacks phy-mode arch/arm/boot/dts/orion5x-netgear-wnr854t.dts - lacks phy-mode and fixed-link arch/arm/boot/dts/armada-388-clearfog.dts - lacks phy-mode arch/arm/boot/dts/armada-xp-linksys-mamba.dts - lacks phy-mode arch/arm/boot/dts/armada-385-linksys.dtsi - lacks phy-mode arch/arm/boot/dts/imx6q-b450v3.dts arch/arm/boot/dts/imx6q-b850v3.dts - has a phy-handle but not a phy-mode? arch/arm/boot/dts/armada-370-rd.dts - lacks phy-mode arch/arm/boot/dts/kirkwood-linksys-viper.dts - lacks phy-mode arch/arm/boot/dts/imx51-zii-rdu1.dts - lacks phy-mode arch/arm/boot/dts/imx51-zii-scu2-mezz.dts - lacks phy-mode arch/arm/boot/dts/imx6qdl-zii-rdu2.dtsi - lacks phy-mode arch/arm/boot/dts/armada-385-clearfog-gtr-s4.dts - lacks phy-mode and fixed-link Verdict: opt out of strict DT bindings and into workarounds. ocelot ~~~~~~ compatible strings: - mscc,vsc9953-switch - felix (arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi) is a PCI device, has no compatible string 2 occurrences in mainline, both are part of SoC dtsi and complete. Verdict: opt into strict DT bindings and out of workarounds. qca8k ~~~~~ compatible strings: - qca,qca8327 - qca,qca8328 - qca,qca8334 - qca,qca8337 5 occurrences in mainline device trees, none of the descriptions are problematic. Verdict: opt into strict DT bindings and out of workarounds. realtek ~~~~~~~ compatible strings: - realtek,rtl8366rb - realtek,rtl8365mb 2 occurrences in mainline, both descriptions are fine, additionally rtl8365mb.c has a comment "The device tree firmware should also specify the link partner of the extension port - either via a fixed-link or other phy-handle." Verdict: opt into strict DT bindings and out of workarounds. rzn1_a5psw ~~~~~~~~~~ compatible strings: - renesas,rzn1-a5psw One single occurrence, part of SoC dtsi (arch/arm/boot/dts/r9a06g032.dtsi), description is fine. Verdict: opt into strict DT bindings and out of workarounds. sja1105 ~~~~~~~ Driver already validates its port OF nodes in sja1105_parse_ports_node(). Verdict: opt into strict DT bindings and out of workarounds. vsc73xx ~~~~~~~ compatible strings: - vitesse,vsc7385 - vitesse,vsc7388 - vitesse,vsc7395 - vitesse,vsc7398 2 occurrences in mainline device trees, both descriptions are fine. Verdict: opt into strict DT bindings and out of workarounds. xrs700x ~~~~~~~ compatible strings: - arrow,xrs7003e - arrow,xrs7003f - arrow,xrs7004e - arrow,xrs7004f no occurrences in mainline, we don't know. Verdict: opt out of strict DT bindings and into workarounds. Because there is a pattern where newly added switches reuse existing drivers more often than introducing new ones, I've opted for deciding who gets to opt into the workaround based on an OF compatible match table in the DSA core. The alternative would have been to add another boolean property to struct dsa_switch, like configure_vlan_while_not_filtering. But this avoids situations where sometimes driver maintainers obfuscate what goes on by sharing a common probing function, and therefore making new switches inherit old quirks. Side note, we also warn about missing properties for drivers that rely on the workaround. This isn't an indication that we'll break compatibility with those DT blobs any time soon, but is rather done to raise awareness about the change, for future DT blob authors. Cc: Rob Herring Cc: Frank Rowand Acked-by: Alvin Šipraga # realtek Signed-off-by: Vladimir Oltean Signed-off-by: Jakub Kicinski --- net/dsa/port.c | 172 +++++++++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 167 insertions(+), 5 deletions(-) (limited to 'net/dsa') diff --git a/net/dsa/port.c b/net/dsa/port.c index bd0cb38a63f6..7afc35db0c29 100644 --- a/net/dsa/port.c +++ b/net/dsa/port.c @@ -1653,22 +1653,184 @@ err_phy_connect: return err; } +/* During the initial DSA driver migration to OF, port nodes were sometimes + * added to device trees with no indication of how they should operate from a + * link management perspective (phy-handle, fixed-link, etc). Additionally, the + * phy-mode may be absent. The interpretation of these port OF nodes depends on + * their type. + * + * User ports with no phy-handle or fixed-link are expected to connect to an + * internal PHY located on the ds->slave_mii_bus at an MDIO address equal to + * the port number. This description is still actively supported. + * + * Shared (CPU and DSA) ports with no phy-handle or fixed-link are expected to + * operate at the maximum speed that their phy-mode is capable of. If the + * phy-mode is absent, they are expected to operate using the phy-mode + * supported by the port that gives the highest link speed. It is unspecified + * if the port should use flow control or not, half duplex or full duplex, or + * if the phy-mode is a SERDES link, whether in-band autoneg is expected to be + * enabled or not. + * + * In the latter case of shared ports, omitting the link management description + * from the firmware node is deprecated and strongly discouraged. DSA uses + * phylink, which rejects the firmware nodes of these ports for lacking + * required properties. + * + * For switches in this table, DSA will skip enforcing validation and will + * later omit registering a phylink instance for the shared ports, if they lack + * a fixed-link, a phy-handle, or a managed = "in-band-status" property. + * It becomes the responsibility of the driver to ensure that these ports + * operate at the maximum speed (whatever this means) and will interoperate + * with the DSA master or other cascade port, since phylink methods will not be + * invoked for them. + * + * If you are considering expanding this table for newly introduced switches, + * think again. It is OK to remove switches from this table if there aren't DT + * blobs in circulation which rely on defaulting the shared ports. + */ +static const char * const dsa_switches_apply_workarounds[] = { +#if IS_ENABLED(CONFIG_NET_DSA_XRS700X) + "arrow,xrs7003e", + "arrow,xrs7003f", + "arrow,xrs7004e", + "arrow,xrs7004f", +#endif +#if IS_ENABLED(CONFIG_B53) + "brcm,bcm5325", + "brcm,bcm53115", + "brcm,bcm53125", + "brcm,bcm53128", + "brcm,bcm5365", + "brcm,bcm5389", + "brcm,bcm5395", + "brcm,bcm5397", + "brcm,bcm5398", + "brcm,bcm53010-srab", + "brcm,bcm53011-srab", + "brcm,bcm53012-srab", + "brcm,bcm53018-srab", + "brcm,bcm53019-srab", + "brcm,bcm5301x-srab", + "brcm,bcm11360-srab", + "brcm,bcm58522-srab", + "brcm,bcm58525-srab", + "brcm,bcm58535-srab", + "brcm,bcm58622-srab", + "brcm,bcm58623-srab", + "brcm,bcm58625-srab", + "brcm,bcm88312-srab", + "brcm,cygnus-srab", + "brcm,nsp-srab", + "brcm,omega-srab", + "brcm,bcm3384-switch", + "brcm,bcm6328-switch", + "brcm,bcm6368-switch", + "brcm,bcm63xx-switch", +#endif +#if IS_ENABLED(CONFIG_NET_DSA_BCM_SF2) + "brcm,bcm7445-switch-v4.0", + "brcm,bcm7278-switch-v4.0", + "brcm,bcm7278-switch-v4.8", +#endif +#if IS_ENABLED(CONFIG_NET_DSA_LANTIQ_GSWIP) + "lantiq,xrx200-gswip", + "lantiq,xrx300-gswip", + "lantiq,xrx330-gswip", +#endif +#if IS_ENABLED(CONFIG_NET_DSA_MV88E6060) + "marvell,mv88e6060", +#endif +#if IS_ENABLED(CONFIG_NET_DSA_MV88E6XXX) + "marvell,mv88e6085", + "marvell,mv88e6190", + "marvell,mv88e6250", +#endif +#if IS_ENABLED(CONFIG_NET_DSA_MICROCHIP_KSZ_COMMON) + "microchip,ksz8765", + "microchip,ksz8794", + "microchip,ksz8795", + "microchip,ksz8863", + "microchip,ksz8873", + "microchip,ksz9477", + "microchip,ksz9897", + "microchip,ksz9893", + "microchip,ksz9563", + "microchip,ksz8563", + "microchip,ksz9567", +#endif +#if IS_ENABLED(CONFIG_NET_DSA_SMSC_LAN9303_MDIO) + "smsc,lan9303-mdio", +#endif +#if IS_ENABLED(CONFIG_NET_DSA_SMSC_LAN9303_I2C) + "smsc,lan9303-i2c", +#endif + NULL, +}; + +static void dsa_shared_port_validate_of(struct dsa_port *dp, + bool *missing_phy_mode, + bool *missing_link_description) +{ + struct device_node *dn = dp->dn, *phy_np; + struct dsa_switch *ds = dp->ds; + phy_interface_t mode; + + *missing_phy_mode = false; + *missing_link_description = false; + + if (of_get_phy_mode(dn, &mode)) { + *missing_phy_mode = true; + dev_err(ds->dev, + "OF node %pOF of %s port %d lacks the required \"phy-mode\" property\n", + dn, dsa_port_is_cpu(dp) ? "CPU" : "DSA", dp->index); + } + + /* Note: of_phy_is_fixed_link() also returns true for + * managed = "in-band-status" + */ + if (of_phy_is_fixed_link(dn)) + return; + + phy_np = of_parse_phandle(dn, "phy-handle", 0); + if (phy_np) { + of_node_put(phy_np); + return; + } + + *missing_link_description = true; + + dev_err(ds->dev, + "OF node %pOF of %s port %d lacks the required \"phy-handle\", \"fixed-link\" or \"managed\" properties\n", + dn, dsa_port_is_cpu(dp) ? "CPU" : "DSA", dp->index); +} + int dsa_shared_port_link_register_of(struct dsa_port *dp) { struct dsa_switch *ds = dp->ds; - struct device_node *phy_np; + bool missing_link_description; + bool missing_phy_mode; int port = dp->index; + dsa_shared_port_validate_of(dp, &missing_phy_mode, + &missing_link_description); + + if ((missing_phy_mode || missing_link_description) && + !of_device_compatible_match(ds->dev->of_node, + dsa_switches_apply_workarounds)) + return -EINVAL; + if (!ds->ops->adjust_link) { - phy_np = of_parse_phandle(dp->dn, "phy-handle", 0); - if (of_phy_is_fixed_link(dp->dn) || phy_np) { + if (missing_link_description) { + dev_warn(ds->dev, + "Skipping phylink registration for %s port %d\n", + dsa_port_is_cpu(dp) ? "CPU" : "DSA", dp->index); + } else { if (ds->ops->phylink_mac_link_down) ds->ops->phylink_mac_link_down(ds, port, MLO_AN_FIXED, PHY_INTERFACE_MODE_NA); - of_node_put(phy_np); + return dsa_shared_port_phylink_register(dp); } - of_node_put(phy_np); return 0; } -- cgit v1.2.3 From e4d44b3d278d71dbe711752131fef6c981fc6fc8 Mon Sep 17 00:00:00 2001 From: Wolfram Sang Date: Thu, 18 Aug 2022 23:02:16 +0200 Subject: dsa: move from strlcpy with unused retval to strscpy Follow the advice of the below link and prefer 'strscpy' in this subsystem. Conversion is 1:1 because the return value is not used. Generated by a coccinelle script. Link: https://lore.kernel.org/r/CAHk-=wgfRnXz0W3D37d01q3JFkr_i_uTL=V6A6G1oUZcprmknw@mail.gmail.com/ Signed-off-by: Wolfram Sang Reviewed-by: Andrew Lunn Link: https://lore.kernel.org/r/20220818210216.8419-1-wsa+renesas@sang-engineering.com Signed-off-by: Jakub Kicinski --- net/dsa/master.c | 2 +- net/dsa/slave.c | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) (limited to 'net/dsa') diff --git a/net/dsa/master.c b/net/dsa/master.c index 2851e44c4cf0..9e5dabc751d9 100644 --- a/net/dsa/master.c +++ b/net/dsa/master.c @@ -58,7 +58,7 @@ static void dsa_master_get_regs(struct net_device *dev, } cpu_info = (struct ethtool_drvinfo *)data; - strlcpy(cpu_info->driver, "dsa", sizeof(cpu_info->driver)); + strscpy(cpu_info->driver, "dsa", sizeof(cpu_info->driver)); data += sizeof(*cpu_info); cpu_regs = (struct ethtool_regs *)data; data += sizeof(*cpu_regs); diff --git a/net/dsa/slave.c b/net/dsa/slave.c index ad6a6663feeb..ce12c3427bad 100644 --- a/net/dsa/slave.c +++ b/net/dsa/slave.c @@ -826,9 +826,9 @@ static netdev_tx_t dsa_slave_xmit(struct sk_buff *skb, struct net_device *dev) static void dsa_slave_get_drvinfo(struct net_device *dev, struct ethtool_drvinfo *drvinfo) { - strlcpy(drvinfo->driver, "dsa", sizeof(drvinfo->driver)); - strlcpy(drvinfo->fw_version, "N/A", sizeof(drvinfo->fw_version)); - strlcpy(drvinfo->bus_info, "platform", sizeof(drvinfo->bus_info)); + strscpy(drvinfo->driver, "dsa", sizeof(drvinfo->driver)); + strscpy(drvinfo->fw_version, "N/A", sizeof(drvinfo->fw_version)); + strscpy(drvinfo->bus_info, "platform", sizeof(drvinfo->bus_info)); } static int dsa_slave_get_regs_len(struct net_device *dev) -- cgit v1.2.3 From b18e04e362c0dce625d00b13f7b29c7c1c07e852 Mon Sep 17 00:00:00 2001 From: Vladimir Oltean Date: Thu, 18 Aug 2022 17:38:08 +0300 Subject: net: dsa: tag_8021q: remove old comment regarding dsa_8021q_netdev_ops Since commit 129bd7ca8ac0 ("net: dsa: Prevent usage of NET_DSA_TAG_8021Q as tagging protocol"), dsa_8021q_netdev_ops no longer exists, so remove the comment that talks about it. Signed-off-by: Vladimir Oltean Link: https://lore.kernel.org/r/20220818143808.2808393-1-vladimir.oltean@nxp.com Signed-off-by: Jakub Kicinski --- net/dsa/tag_8021q.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) (limited to 'net/dsa') diff --git a/net/dsa/tag_8021q.c b/net/dsa/tag_8021q.c index 01a427800797..b5f80bc45ceb 100644 --- a/net/dsa/tag_8021q.c +++ b/net/dsa/tag_8021q.c @@ -2,9 +2,7 @@ /* Copyright (c) 2019, Vladimir Oltean * * This module is not a complete tagger implementation. It only provides - * primitives for taggers that rely on 802.1Q VLAN tags to use. The - * dsa_8021q_netdev_ops is registered for API compliance and not used - * directly by callers. + * primitives for taggers that rely on 802.1Q VLAN tags to use. */ #include #include -- cgit v1.2.3 From 4c3f80d22b2eca911143ce656fa45c4699ff5bf4 Mon Sep 17 00:00:00 2001 From: Vladimir Oltean Date: Fri, 19 Aug 2022 20:48:12 +0300 Subject: net: dsa: walk through all changeupper notifier functions Traditionally, DSA has had a single netdev notifier handling function for each device type. For the sake of code cleanliness, we would like to introduce more handling functions which do one thing, but the conditions for entering these functions start to overlap. Example: a handling function which tracks whether any bridges contain both DSA and non-DSA interfaces. Either this is placed before dsa_slave_changeupper(), case in which it will prevent that function from executing, or we place it after dsa_slave_changeupper(), case in which we will prevent it from executing. The other alternative is to ignore errors from the new handling function (not ideal). To support this usage, we need to change the pattern. In the new model, we enter all notifier handling sub-functions, and exit with NOTIFY_DONE if there is nothing to do. This allows the sub-functions to be relatively free-form and independent from each other. Signed-off-by: Vladimir Oltean Reviewed-by: Florian Fainelli Signed-off-by: Paolo Abeni --- net/dsa/slave.c | 37 ++++++++++++++++++++++++++++--------- 1 file changed, 28 insertions(+), 9 deletions(-) (limited to 'net/dsa') diff --git a/net/dsa/slave.c b/net/dsa/slave.c index ce12c3427bad..3157da2adf6b 100644 --- a/net/dsa/slave.c +++ b/net/dsa/slave.c @@ -2476,6 +2476,9 @@ static int dsa_slave_changeupper(struct net_device *dev, struct netlink_ext_ack *extack; int err = NOTIFY_DONE; + if (!dsa_slave_dev_check(dev)) + return err; + extack = netdev_notifier_info_to_extack(&info->info); if (netif_is_bridge_master(info->upper_dev)) { @@ -2531,6 +2534,9 @@ static int dsa_slave_prechangeupper(struct net_device *dev, { struct dsa_port *dp = dsa_slave_to_port(dev); + if (!dsa_slave_dev_check(dev)) + return NOTIFY_DONE; + if (netif_is_bridge_master(info->upper_dev) && !info->linking) dsa_port_pre_bridge_leave(dp, info->upper_dev); else if (netif_is_lag_master(info->upper_dev) && !info->linking) @@ -2551,6 +2557,9 @@ dsa_slave_lag_changeupper(struct net_device *dev, int err = NOTIFY_DONE; struct dsa_port *dp; + if (!netif_is_lag_master(dev)) + return err; + netdev_for_each_lower_dev(dev, lower, iter) { if (!dsa_slave_dev_check(lower)) continue; @@ -2580,6 +2589,9 @@ dsa_slave_lag_prechangeupper(struct net_device *dev, int err = NOTIFY_DONE; struct dsa_port *dp; + if (!netif_is_lag_master(dev)) + return err; + netdev_for_each_lower_dev(dev, lower, iter) { if (!dsa_slave_dev_check(lower)) continue; @@ -2701,22 +2713,29 @@ static int dsa_slave_netdevice_event(struct notifier_block *nb, if (err != NOTIFY_DONE) return err; - if (dsa_slave_dev_check(dev)) - return dsa_slave_prechangeupper(dev, ptr); + err = dsa_slave_prechangeupper(dev, ptr); + if (notifier_to_errno(err)) + return err; - if (netif_is_lag_master(dev)) - return dsa_slave_lag_prechangeupper(dev, ptr); + err = dsa_slave_lag_prechangeupper(dev, ptr); + if (notifier_to_errno(err)) + return err; break; } - case NETDEV_CHANGEUPPER: - if (dsa_slave_dev_check(dev)) - return dsa_slave_changeupper(dev, ptr); + case NETDEV_CHANGEUPPER: { + int err; + + err = dsa_slave_changeupper(dev, ptr); + if (notifier_to_errno(err)) + return err; - if (netif_is_lag_master(dev)) - return dsa_slave_lag_changeupper(dev, ptr); + err = dsa_slave_lag_changeupper(dev, ptr); + if (notifier_to_errno(err)) + return err; break; + } case NETDEV_CHANGELOWERSTATE: { struct netdev_notifier_changelowerstate_info *info = ptr; struct dsa_port *dp; -- cgit v1.2.3 From 0498277ee17bb40cef43e0c9064b06c25f9bda29 Mon Sep 17 00:00:00 2001 From: Vladimir Oltean Date: Fri, 19 Aug 2022 20:48:13 +0300 Subject: net: dsa: don't stop at NOTIFY_OK when calling ds->ops->port_prechangeupper dsa_slave_prechangeupper_sanity_check() is supposed to enforce some adjacency restrictions, and calls ds->ops->port_prechangeupper if the driver implements it. We convert the error code from the port_prechangeupper() call to a notifier code, and 0 is converted to NOTIFY_OK, but the caller of dsa_slave_prechangeupper_sanity_check() stops at any notifier code different from NOTIFY_DONE. Avoid this by converting back the notifier code to an error code, so that both NOTIFY_OK and NOTIFY_DONE will be seen as 0. This allows more parallel sanity check functions to be added. Signed-off-by: Vladimir Oltean Reviewed-by: Florian Fainelli Signed-off-by: Paolo Abeni --- net/dsa/slave.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'net/dsa') diff --git a/net/dsa/slave.c b/net/dsa/slave.c index 3157da2adf6b..85cffd844364 100644 --- a/net/dsa/slave.c +++ b/net/dsa/slave.c @@ -2710,7 +2710,7 @@ static int dsa_slave_netdevice_event(struct notifier_block *nb, int err; err = dsa_slave_prechangeupper_sanity_check(dev, info); - if (err != NOTIFY_DONE) + if (notifier_to_errno(err)) return err; err = dsa_slave_prechangeupper(dev, ptr); -- cgit v1.2.3 From 920a33cd72314f4ea96b5224fa8c7d4472d81880 Mon Sep 17 00:00:00 2001 From: Vladimir Oltean Date: Fri, 19 Aug 2022 20:48:14 +0300 Subject: net: bridge: move DSA master bridging restriction to DSA When DSA gains support for multiple CPU ports in a LAG, it will become mandatory to monitor the changeupper events for the DSA master. In fact, there are already some restrictions to be imposed in that area, namely that a DSA master cannot be a bridge port except in some special circumstances. Centralize the restrictions at the level of the DSA layer as a preliminary step. Signed-off-by: Vladimir Oltean Reviewed-by: Florian Fainelli Acked-by: Nikolay Aleksandrov Signed-off-by: Paolo Abeni --- net/bridge/br_if.c | 20 -------------------- net/dsa/slave.c | 44 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+), 20 deletions(-) (limited to 'net/dsa') diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c index a84a7cfb9d6d..efbd93e92ce2 100644 --- a/net/bridge/br_if.c +++ b/net/bridge/br_if.c @@ -568,26 +568,6 @@ int br_add_if(struct net_bridge *br, struct net_device *dev, !is_valid_ether_addr(dev->dev_addr)) return -EINVAL; - /* Also don't allow bridging of net devices that are DSA masters, since - * the bridge layer rx_handler prevents the DSA fake ethertype handler - * to be invoked, so we don't get the chance to strip off and parse the - * DSA switch tag protocol header (the bridge layer just returns - * RX_HANDLER_CONSUMED, stopping RX processing for these frames). - * The only case where that would not be an issue is when bridging can - * already be offloaded, such as when the DSA master is itself a DSA - * or plain switchdev port, and is bridged only with other ports from - * the same hardware device. - */ - if (netdev_uses_dsa(dev)) { - list_for_each_entry(p, &br->port_list, list) { - if (!netdev_port_same_parent_id(dev, p->dev)) { - NL_SET_ERR_MSG(extack, - "Cannot do software bridging with a DSA master"); - return -EINVAL; - } - } - } - /* No bridging of bridges */ if (dev->netdev_ops->ndo_start_xmit == br_dev_xmit) { NL_SET_ERR_MSG(extack, diff --git a/net/dsa/slave.c b/net/dsa/slave.c index 85cffd844364..aa8bf89ec127 100644 --- a/net/dsa/slave.c +++ b/net/dsa/slave.c @@ -2699,6 +2699,46 @@ dsa_slave_prechangeupper_sanity_check(struct net_device *dev, return NOTIFY_DONE; } +/* Don't allow bridging of DSA masters, since the bridge layer rx_handler + * prevents the DSA fake ethertype handler to be invoked, so we don't get the + * chance to strip off and parse the DSA switch tag protocol header (the bridge + * layer just returns RX_HANDLER_CONSUMED, stopping RX processing for these + * frames). + * The only case where that would not be an issue is when bridging can already + * be offloaded, such as when the DSA master is itself a DSA or plain switchdev + * port, and is bridged only with other ports from the same hardware device. + */ +static int +dsa_bridge_prechangelower_sanity_check(struct net_device *new_lower, + struct netdev_notifier_changeupper_info *info) +{ + struct net_device *br = info->upper_dev; + struct netlink_ext_ack *extack; + struct net_device *lower; + struct list_head *iter; + + if (!netif_is_bridge_master(br)) + return NOTIFY_DONE; + + if (!info->linking) + return NOTIFY_DONE; + + extack = netdev_notifier_info_to_extack(&info->info); + + netdev_for_each_lower_dev(br, lower, iter) { + if (!netdev_uses_dsa(new_lower) && !netdev_uses_dsa(lower)) + continue; + + if (!netdev_port_same_parent_id(lower, new_lower)) { + NL_SET_ERR_MSG(extack, + "Cannot do software bridging with a DSA master"); + return notifier_from_errno(-EINVAL); + } + } + + return NOTIFY_DONE; +} + static int dsa_slave_netdevice_event(struct notifier_block *nb, unsigned long event, void *ptr) { @@ -2713,6 +2753,10 @@ static int dsa_slave_netdevice_event(struct notifier_block *nb, if (notifier_to_errno(err)) return err; + err = dsa_bridge_prechangelower_sanity_check(dev, info); + if (notifier_to_errno(err)) + return err; + err = dsa_slave_prechangeupper(dev, ptr); if (notifier_to_errno(err)) return err; -- cgit v1.2.3 From 4f03dcc6b9a0734d755601002e33adbd42469b47 Mon Sep 17 00:00:00 2001 From: Vladimir Oltean Date: Fri, 19 Aug 2022 20:48:15 +0300 Subject: net: dsa: existing DSA masters cannot join upper interfaces All the traffic to/from a DSA master is supposed to be distributed among its DSA switch upper interfaces, so we should not allow other upper device kinds. An exception to this is DSA_TAG_PROTO_NONE (switches with no DSA tags), and in that case it is actually expected to create e.g. VLAN interfaces on the master. But for those, netdev_uses_dsa(master) returns false, so the restriction doesn't apply. The motivation for this change is to allow LAG interfaces of DSA masters to be DSA masters themselves. We want to restrict the user's degrees of freedom by 1: the LAG should already have all DSA masters as lowers, and while lower ports of the LAG can be removed, none can be added after the fact. Signed-off-by: Vladimir Oltean Reviewed-by: Florian Fainelli Signed-off-by: Paolo Abeni --- net/dsa/slave.c | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) (limited to 'net/dsa') diff --git a/net/dsa/slave.c b/net/dsa/slave.c index aa8bf89ec127..d0ffa642106f 100644 --- a/net/dsa/slave.c +++ b/net/dsa/slave.c @@ -2699,6 +2699,35 @@ dsa_slave_prechangeupper_sanity_check(struct net_device *dev, return NOTIFY_DONE; } +static int +dsa_master_prechangeupper_sanity_check(struct net_device *master, + struct netdev_notifier_changeupper_info *info) +{ + struct netlink_ext_ack *extack; + + if (!netdev_uses_dsa(master)) + return NOTIFY_DONE; + + if (!info->linking) + return NOTIFY_DONE; + + /* Allow DSA switch uppers */ + if (dsa_slave_dev_check(info->upper_dev)) + return NOTIFY_DONE; + + /* Allow bridge uppers of DSA masters, subject to further + * restrictions in dsa_bridge_prechangelower_sanity_check() + */ + if (netif_is_bridge_master(info->upper_dev)) + return NOTIFY_DONE; + + extack = netdev_notifier_info_to_extack(&info->info); + + NL_SET_ERR_MSG_MOD(extack, + "DSA master cannot join unknown upper interfaces"); + return notifier_from_errno(-EBUSY); +} + /* Don't allow bridging of DSA masters, since the bridge layer rx_handler * prevents the DSA fake ethertype handler to be invoked, so we don't get the * chance to strip off and parse the DSA switch tag protocol header (the bridge @@ -2753,6 +2782,10 @@ static int dsa_slave_netdevice_event(struct notifier_block *nb, if (notifier_to_errno(err)) return err; + err = dsa_master_prechangeupper_sanity_check(dev, info); + if (notifier_to_errno(err)) + return err; + err = dsa_bridge_prechangelower_sanity_check(dev, info); if (notifier_to_errno(err)) return err; -- cgit v1.2.3 From 7136097e11996417da59c67682694d8a5a7f3f4b Mon Sep 17 00:00:00 2001 From: Vladimir Oltean Date: Fri, 19 Aug 2022 20:48:16 +0300 Subject: net: dsa: only bring down user ports assigned to a given DSA master This is an adaptation of commit c0a8a9c27493 ("net: dsa: automatically bring user ports down when master goes down") for multiple DSA masters. When a DSA master goes down, only the user ports under its control should go down too, the others can still send/receive traffic. Signed-off-by: Vladimir Oltean Reviewed-by: Florian Fainelli Signed-off-by: Paolo Abeni --- net/dsa/slave.c | 3 +++ 1 file changed, 3 insertions(+) (limited to 'net/dsa') diff --git a/net/dsa/slave.c b/net/dsa/slave.c index d0ffa642106f..b24f1131af90 100644 --- a/net/dsa/slave.c +++ b/net/dsa/slave.c @@ -2873,6 +2873,9 @@ static int dsa_slave_netdevice_event(struct notifier_block *nb, if (!dsa_port_is_user(dp)) continue; + if (dp->cpu_dp != cpu_dp) + continue; + list_add(&dp->slave->close_list, &close_list); } -- cgit v1.2.3 From f41ec1fd1c20e2a4e60a4ab8490b3e63423c0a8a Mon Sep 17 00:00:00 2001 From: Vladimir Oltean Date: Fri, 19 Aug 2022 20:48:17 +0300 Subject: net: dsa: all DSA masters must be down when changing the tagging protocol The fact that the tagging protocol is set and queried from the /sys/class/net//dsa/tagging file is a bit of a quirk from the single CPU port days which isn't aging very well now that DSA can have more than a single CPU port. This is because the tagging protocol is a switch property, yet in the presence of multiple CPU ports it can be queried and set from multiple sysfs files, all of which are handled by the same implementation. The current logic ensures that the net device whose sysfs file we're changing the tagging protocol through must be down. That net device is the DSA master, and this is fine for single DSA master / CPU port setups. But exactly because the tagging protocol is per switch [ tree, in fact ] and not per DSA master, this isn't fine any longer with multiple CPU ports, and we must iterate through the tree and find all DSA masters, and make sure that all of them are down. Signed-off-by: Vladimir Oltean Reviewed-by: Florian Fainelli Signed-off-by: Paolo Abeni --- net/dsa/dsa2.c | 10 +++------- net/dsa/dsa_priv.h | 1 - net/dsa/master.c | 2 +- 3 files changed, 4 insertions(+), 9 deletions(-) (limited to 'net/dsa') diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c index 055a6d1d4372..c7bdf7106d23 100644 --- a/net/dsa/dsa2.c +++ b/net/dsa/dsa2.c @@ -1252,7 +1252,6 @@ out_disconnect: * they would have formed disjoint trees (different "dsa,member" values). */ int dsa_tree_change_tag_proto(struct dsa_switch_tree *dst, - struct net_device *master, const struct dsa_device_ops *tag_ops, const struct dsa_device_ops *old_tag_ops) { @@ -1268,14 +1267,11 @@ int dsa_tree_change_tag_proto(struct dsa_switch_tree *dst, * attempts to change the tagging protocol. If we ever lift the IFF_UP * restriction, there needs to be another mutex which serializes this. */ - if (master->flags & IFF_UP) - goto out_unlock; - list_for_each_entry(dp, &dst->ports, list) { - if (!dsa_port_is_user(dp)) - continue; + if (dsa_port_is_cpu(dp) && (dp->master->flags & IFF_UP)) + goto out_unlock; - if (dp->slave->flags & IFF_UP) + if (dsa_port_is_user(dp) && (dp->slave->flags & IFF_UP)) goto out_unlock; } diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h index 8924366467e0..614fbba8fe39 100644 --- a/net/dsa/dsa_priv.h +++ b/net/dsa/dsa_priv.h @@ -545,7 +545,6 @@ struct dsa_lag *dsa_tree_lag_find(struct dsa_switch_tree *dst, int dsa_tree_notify(struct dsa_switch_tree *dst, unsigned long e, void *v); int dsa_broadcast(unsigned long e, void *v); int dsa_tree_change_tag_proto(struct dsa_switch_tree *dst, - struct net_device *master, const struct dsa_device_ops *tag_ops, const struct dsa_device_ops *old_tag_ops); void dsa_tree_master_admin_state_change(struct dsa_switch_tree *dst, diff --git a/net/dsa/master.c b/net/dsa/master.c index 9e5dabc751d9..fb810edc8281 100644 --- a/net/dsa/master.c +++ b/net/dsa/master.c @@ -307,7 +307,7 @@ static ssize_t tagging_store(struct device *d, struct device_attribute *attr, */ goto out; - err = dsa_tree_change_tag_proto(cpu_dp->ds->dst, dev, new_tag_ops, + err = dsa_tree_change_tag_proto(cpu_dp->ds->dst, new_tag_ops, old_tag_ops); if (err) { /* On failure the old tagger is restored, so we don't need the -- cgit v1.2.3 From 5dc760d1208200daaf49a2ff56a0e7dfa2d80bb2 Mon Sep 17 00:00:00 2001 From: Vladimir Oltean Date: Fri, 19 Aug 2022 20:48:18 +0300 Subject: net: dsa: use dsa_tree_for_each_cpu_port in dsa_tree_{setup,teardown}_master More logic will be added to dsa_tree_setup_master() and dsa_tree_teardown_master() in upcoming changes. Reduce the indentation by one level in these functions by introducing and using a dedicated iterator for CPU ports of a tree. Signed-off-by: Vladimir Oltean Reviewed-by: Florian Fainelli Signed-off-by: Paolo Abeni --- include/net/dsa.h | 4 ++++ net/dsa/dsa2.c | 46 +++++++++++++++++++++------------------------- 2 files changed, 25 insertions(+), 25 deletions(-) (limited to 'net/dsa') diff --git a/include/net/dsa.h b/include/net/dsa.h index b902b31bebce..f2ce12860546 100644 --- a/include/net/dsa.h +++ b/include/net/dsa.h @@ -559,6 +559,10 @@ static inline bool dsa_is_user_port(struct dsa_switch *ds, int p) list_for_each_entry((_dp), &(_dst)->ports, list) \ if (dsa_port_is_user((_dp))) +#define dsa_tree_for_each_cpu_port(_dp, _dst) \ + list_for_each_entry((_dp), &(_dst)->ports, list) \ + if (dsa_port_is_cpu((_dp))) + #define dsa_switch_for_each_port(_dp, _ds) \ list_for_each_entry((_dp), &(_ds)->dst->ports, list) \ if ((_dp)->ds == (_ds)) diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c index c7bdf7106d23..ed56c7a554b8 100644 --- a/net/dsa/dsa2.c +++ b/net/dsa/dsa2.c @@ -1060,26 +1060,24 @@ static int dsa_tree_setup_switches(struct dsa_switch_tree *dst) static int dsa_tree_setup_master(struct dsa_switch_tree *dst) { - struct dsa_port *dp; + struct dsa_port *cpu_dp; int err = 0; rtnl_lock(); - list_for_each_entry(dp, &dst->ports, list) { - if (dsa_port_is_cpu(dp)) { - struct net_device *master = dp->master; - bool admin_up = (master->flags & IFF_UP) && - !qdisc_tx_is_noop(master); + dsa_tree_for_each_cpu_port(cpu_dp, dst) { + struct net_device *master = cpu_dp->master; + bool admin_up = (master->flags & IFF_UP) && + !qdisc_tx_is_noop(master); - err = dsa_master_setup(master, dp); - if (err) - break; + err = dsa_master_setup(master, cpu_dp); + if (err) + break; - /* Replay master state event */ - dsa_tree_master_admin_state_change(dst, master, admin_up); - dsa_tree_master_oper_state_change(dst, master, - netif_oper_up(master)); - } + /* Replay master state event */ + dsa_tree_master_admin_state_change(dst, master, admin_up); + dsa_tree_master_oper_state_change(dst, master, + netif_oper_up(master)); } rtnl_unlock(); @@ -1089,22 +1087,20 @@ static int dsa_tree_setup_master(struct dsa_switch_tree *dst) static void dsa_tree_teardown_master(struct dsa_switch_tree *dst) { - struct dsa_port *dp; + struct dsa_port *cpu_dp; rtnl_lock(); - list_for_each_entry(dp, &dst->ports, list) { - if (dsa_port_is_cpu(dp)) { - struct net_device *master = dp->master; + dsa_tree_for_each_cpu_port(cpu_dp, dst) { + struct net_device *master = cpu_dp->master; - /* Synthesizing an "admin down" state is sufficient for - * the switches to get a notification if the master is - * currently up and running. - */ - dsa_tree_master_admin_state_change(dst, master, false); + /* Synthesizing an "admin down" state is sufficient for + * the switches to get a notification if the master is + * currently up and running. + */ + dsa_tree_master_admin_state_change(dst, master, false); - dsa_master_teardown(master); - } + dsa_master_teardown(master); } rtnl_unlock(); -- cgit v1.2.3 From 8f6a19c0316deb48cdfdc5335de9a6d7db5b7b62 Mon Sep 17 00:00:00 2001 From: Vladimir Oltean Date: Sun, 11 Sep 2022 04:06:58 +0300 Subject: net: dsa: introduce dsa_port_get_master() There is a desire to support for DSA masters in a LAG. That configuration is intended to work by simply enslaving the master to a bonding/team device. But the physical DSA master (the LAG slave) still has a dev->dsa_ptr, and that cpu_dp still corresponds to the physical CPU port. However, we would like to be able to retrieve the LAG that's the upper of the physical DSA master. In preparation for that, introduce a helper called dsa_port_get_master() that replaces all occurrences of the dp->cpu_dp->master pattern. The distinction between LAG and non-LAG will be made later within the helper itself. Signed-off-by: Vladimir Oltean Reviewed-by: Florian Fainelli Signed-off-by: Paolo Abeni --- drivers/net/dsa/bcm_sf2.c | 4 ++-- drivers/net/dsa/bcm_sf2_cfp.c | 4 ++-- drivers/net/dsa/lan9303-core.c | 4 ++-- drivers/net/ethernet/mediatek/mtk_ppe_offload.c | 2 +- include/net/dsa.h | 5 +++++ net/dsa/dsa2.c | 8 +++---- net/dsa/dsa_priv.h | 2 +- net/dsa/port.c | 28 ++++++++++++------------- net/dsa/slave.c | 11 +++++----- net/dsa/tag_8021q.c | 4 ++-- 10 files changed, 38 insertions(+), 34 deletions(-) (limited to 'net/dsa') diff --git a/drivers/net/dsa/bcm_sf2.c b/drivers/net/dsa/bcm_sf2.c index 572f7450b527..6507663f35e5 100644 --- a/drivers/net/dsa/bcm_sf2.c +++ b/drivers/net/dsa/bcm_sf2.c @@ -983,7 +983,7 @@ static int bcm_sf2_sw_resume(struct dsa_switch *ds) static void bcm_sf2_sw_get_wol(struct dsa_switch *ds, int port, struct ethtool_wolinfo *wol) { - struct net_device *p = dsa_to_port(ds, port)->cpu_dp->master; + struct net_device *p = dsa_port_to_master(dsa_to_port(ds, port)); struct bcm_sf2_priv *priv = bcm_sf2_to_priv(ds); struct ethtool_wolinfo pwol = { }; @@ -1007,7 +1007,7 @@ static void bcm_sf2_sw_get_wol(struct dsa_switch *ds, int port, static int bcm_sf2_sw_set_wol(struct dsa_switch *ds, int port, struct ethtool_wolinfo *wol) { - struct net_device *p = dsa_to_port(ds, port)->cpu_dp->master; + struct net_device *p = dsa_port_to_master(dsa_to_port(ds, port)); struct bcm_sf2_priv *priv = bcm_sf2_to_priv(ds); s8 cpu_port = dsa_to_port(ds, port)->cpu_dp->index; struct ethtool_wolinfo pwol = { }; diff --git a/drivers/net/dsa/bcm_sf2_cfp.c b/drivers/net/dsa/bcm_sf2_cfp.c index 22bc295bebdb..c4010b7bf089 100644 --- a/drivers/net/dsa/bcm_sf2_cfp.c +++ b/drivers/net/dsa/bcm_sf2_cfp.c @@ -1102,7 +1102,7 @@ static int bcm_sf2_cfp_rule_get_all(struct bcm_sf2_priv *priv, int bcm_sf2_get_rxnfc(struct dsa_switch *ds, int port, struct ethtool_rxnfc *nfc, u32 *rule_locs) { - struct net_device *p = dsa_to_port(ds, port)->cpu_dp->master; + struct net_device *p = dsa_port_to_master(dsa_to_port(ds, port)); struct bcm_sf2_priv *priv = bcm_sf2_to_priv(ds); int ret = 0; @@ -1145,7 +1145,7 @@ int bcm_sf2_get_rxnfc(struct dsa_switch *ds, int port, int bcm_sf2_set_rxnfc(struct dsa_switch *ds, int port, struct ethtool_rxnfc *nfc) { - struct net_device *p = dsa_to_port(ds, port)->cpu_dp->master; + struct net_device *p = dsa_port_to_master(dsa_to_port(ds, port)); struct bcm_sf2_priv *priv = bcm_sf2_to_priv(ds); int ret = 0; diff --git a/drivers/net/dsa/lan9303-core.c b/drivers/net/dsa/lan9303-core.c index 9e04541c3144..438e46af03e9 100644 --- a/drivers/net/dsa/lan9303-core.c +++ b/drivers/net/dsa/lan9303-core.c @@ -1092,7 +1092,7 @@ static int lan9303_port_enable(struct dsa_switch *ds, int port, if (!dsa_port_is_user(dp)) return 0; - vlan_vid_add(dp->cpu_dp->master, htons(ETH_P_8021Q), port); + vlan_vid_add(dsa_port_to_master(dp), htons(ETH_P_8021Q), port); return lan9303_enable_processing_port(chip, port); } @@ -1105,7 +1105,7 @@ static void lan9303_port_disable(struct dsa_switch *ds, int port) if (!dsa_port_is_user(dp)) return; - vlan_vid_del(dp->cpu_dp->master, htons(ETH_P_8021Q), port); + vlan_vid_del(dsa_port_to_master(dp), htons(ETH_P_8021Q), port); lan9303_disable_processing_port(chip, port); lan9303_phy_write(ds, chip->phy_addr_base + port, MII_BMCR, BMCR_PDOWN); diff --git a/drivers/net/ethernet/mediatek/mtk_ppe_offload.c b/drivers/net/ethernet/mediatek/mtk_ppe_offload.c index 25dc3c3aa31d..5a1fc4bcd7a5 100644 --- a/drivers/net/ethernet/mediatek/mtk_ppe_offload.c +++ b/drivers/net/ethernet/mediatek/mtk_ppe_offload.c @@ -173,7 +173,7 @@ mtk_flow_get_dsa_port(struct net_device **dev) if (dp->cpu_dp->tag_ops->proto != DSA_TAG_PROTO_MTK) return -ENODEV; - *dev = dp->cpu_dp->master; + *dev = dsa_port_to_master(dp); return dp->index; #else diff --git a/include/net/dsa.h b/include/net/dsa.h index f2ce12860546..23eac1bda843 100644 --- a/include/net/dsa.h +++ b/include/net/dsa.h @@ -718,6 +718,11 @@ static inline bool dsa_port_offloads_lag(struct dsa_port *dp, return dsa_port_lag_dev_get(dp) == lag->dev; } +static inline struct net_device *dsa_port_to_master(const struct dsa_port *dp) +{ + return dp->cpu_dp->master; +} + static inline struct net_device *dsa_port_to_bridge_port(const struct dsa_port *dp) { diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c index ed56c7a554b8..f1f96e2e56aa 100644 --- a/net/dsa/dsa2.c +++ b/net/dsa/dsa2.c @@ -1263,11 +1263,11 @@ int dsa_tree_change_tag_proto(struct dsa_switch_tree *dst, * attempts to change the tagging protocol. If we ever lift the IFF_UP * restriction, there needs to be another mutex which serializes this. */ - list_for_each_entry(dp, &dst->ports, list) { - if (dsa_port_is_cpu(dp) && (dp->master->flags & IFF_UP)) + dsa_tree_for_each_user_port(dp, dst) { + if (dsa_port_to_master(dp)->flags & IFF_UP) goto out_unlock; - if (dsa_port_is_user(dp) && (dp->slave->flags & IFF_UP)) + if (dp->slave->flags & IFF_UP) goto out_unlock; } @@ -1797,7 +1797,7 @@ void dsa_switch_shutdown(struct dsa_switch *ds) rtnl_lock(); dsa_switch_for_each_user_port(dp, ds) { - master = dp->cpu_dp->master; + master = dsa_port_to_master(dp); slave_dev = dp->slave; netdev_upper_dev_unlink(master, slave_dev); diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h index 614fbba8fe39..c48c5c8ba790 100644 --- a/net/dsa/dsa_priv.h +++ b/net/dsa/dsa_priv.h @@ -322,7 +322,7 @@ dsa_slave_to_master(const struct net_device *dev) { struct dsa_port *dp = dsa_slave_to_port(dev); - return dp->cpu_dp->master; + return dsa_port_to_master(dp); } /* If under a bridge with vlan_filtering=0, make sure to send pvid-tagged diff --git a/net/dsa/port.c b/net/dsa/port.c index 7afc35db0c29..4183e60db4f9 100644 --- a/net/dsa/port.c +++ b/net/dsa/port.c @@ -1026,7 +1026,7 @@ int dsa_port_standalone_host_fdb_add(struct dsa_port *dp, int dsa_port_bridge_host_fdb_add(struct dsa_port *dp, const unsigned char *addr, u16 vid) { - struct dsa_port *cpu_dp = dp->cpu_dp; + struct net_device *master = dsa_port_to_master(dp); struct dsa_db db = { .type = DSA_DB_BRIDGE, .bridge = *dp->bridge, @@ -1037,8 +1037,8 @@ int dsa_port_bridge_host_fdb_add(struct dsa_port *dp, * requires rtnl_lock(), since we can't guarantee that is held here, * and we can't take it either. */ - if (cpu_dp->master->priv_flags & IFF_UNICAST_FLT) { - err = dev_uc_add(cpu_dp->master, addr); + if (master->priv_flags & IFF_UNICAST_FLT) { + err = dev_uc_add(master, addr); if (err) return err; } @@ -1077,15 +1077,15 @@ int dsa_port_standalone_host_fdb_del(struct dsa_port *dp, int dsa_port_bridge_host_fdb_del(struct dsa_port *dp, const unsigned char *addr, u16 vid) { - struct dsa_port *cpu_dp = dp->cpu_dp; + struct net_device *master = dsa_port_to_master(dp); struct dsa_db db = { .type = DSA_DB_BRIDGE, .bridge = *dp->bridge, }; int err; - if (cpu_dp->master->priv_flags & IFF_UNICAST_FLT) { - err = dev_uc_del(cpu_dp->master, addr); + if (master->priv_flags & IFF_UNICAST_FLT) { + err = dev_uc_del(master, addr); if (err) return err; } @@ -1208,14 +1208,14 @@ int dsa_port_standalone_host_mdb_add(const struct dsa_port *dp, int dsa_port_bridge_host_mdb_add(const struct dsa_port *dp, const struct switchdev_obj_port_mdb *mdb) { - struct dsa_port *cpu_dp = dp->cpu_dp; + struct net_device *master = dsa_port_to_master(dp); struct dsa_db db = { .type = DSA_DB_BRIDGE, .bridge = *dp->bridge, }; int err; - err = dev_mc_add(cpu_dp->master, mdb->addr); + err = dev_mc_add(master, mdb->addr); if (err) return err; @@ -1252,14 +1252,14 @@ int dsa_port_standalone_host_mdb_del(const struct dsa_port *dp, int dsa_port_bridge_host_mdb_del(const struct dsa_port *dp, const struct switchdev_obj_port_mdb *mdb) { - struct dsa_port *cpu_dp = dp->cpu_dp; + struct net_device *master = dsa_port_to_master(dp); struct dsa_db db = { .type = DSA_DB_BRIDGE, .bridge = *dp->bridge, }; int err; - err = dev_mc_del(cpu_dp->master, mdb->addr); + err = dev_mc_del(master, mdb->addr); if (err) return err; @@ -1294,19 +1294,19 @@ int dsa_port_host_vlan_add(struct dsa_port *dp, const struct switchdev_obj_port_vlan *vlan, struct netlink_ext_ack *extack) { + struct net_device *master = dsa_port_to_master(dp); struct dsa_notifier_vlan_info info = { .dp = dp, .vlan = vlan, .extack = extack, }; - struct dsa_port *cpu_dp = dp->cpu_dp; int err; err = dsa_port_notify(dp, DSA_NOTIFIER_HOST_VLAN_ADD, &info); if (err && err != -EOPNOTSUPP) return err; - vlan_vid_add(cpu_dp->master, htons(ETH_P_8021Q), vlan->vid); + vlan_vid_add(master, htons(ETH_P_8021Q), vlan->vid); return err; } @@ -1314,18 +1314,18 @@ int dsa_port_host_vlan_add(struct dsa_port *dp, int dsa_port_host_vlan_del(struct dsa_port *dp, const struct switchdev_obj_port_vlan *vlan) { + struct net_device *master = dsa_port_to_master(dp); struct dsa_notifier_vlan_info info = { .dp = dp, .vlan = vlan, }; - struct dsa_port *cpu_dp = dp->cpu_dp; int err; err = dsa_port_notify(dp, DSA_NOTIFIER_HOST_VLAN_DEL, &info); if (err && err != -EOPNOTSUPP) return err; - vlan_vid_del(cpu_dp->master, htons(ETH_P_8021Q), vlan->vid); + vlan_vid_del(master, htons(ETH_P_8021Q), vlan->vid); return err; } diff --git a/net/dsa/slave.c b/net/dsa/slave.c index 345106b1ed78..55094b94a5ae 100644 --- a/net/dsa/slave.c +++ b/net/dsa/slave.c @@ -1503,8 +1503,7 @@ static int dsa_slave_setup_tc_block(struct net_device *dev, static int dsa_slave_setup_ft_block(struct dsa_switch *ds, int port, void *type_data) { - struct dsa_port *cpu_dp = dsa_to_port(ds, port)->cpu_dp; - struct net_device *master = cpu_dp->master; + struct net_device *master = dsa_port_to_master(dsa_to_port(ds, port)); if (!master->netdev_ops->ndo_setup_tc) return -EOPNOTSUPP; @@ -2147,13 +2146,14 @@ static int dsa_slave_fill_forward_path(struct net_device_path_ctx *ctx, struct net_device_path *path) { struct dsa_port *dp = dsa_slave_to_port(ctx->dev); + struct net_device *master = dsa_port_to_master(dp); struct dsa_port *cpu_dp = dp->cpu_dp; path->dev = ctx->dev; path->type = DEV_PATH_DSA; path->dsa.proto = cpu_dp->tag_ops->proto; path->dsa.port = dp->index; - ctx->dev = cpu_dp->master; + ctx->dev = master; return 0; } @@ -2271,9 +2271,9 @@ static int dsa_slave_phy_setup(struct net_device *slave_dev) void dsa_slave_setup_tagger(struct net_device *slave) { struct dsa_port *dp = dsa_slave_to_port(slave); + struct net_device *master = dsa_port_to_master(dp); struct dsa_slave_priv *p = netdev_priv(slave); const struct dsa_port *cpu_dp = dp->cpu_dp; - struct net_device *master = cpu_dp->master; const struct dsa_switch *ds = dp->ds; slave->needed_headroom = cpu_dp->tag_ops->needed_headroom; @@ -2330,8 +2330,7 @@ int dsa_slave_resume(struct net_device *slave_dev) int dsa_slave_create(struct dsa_port *port) { - const struct dsa_port *cpu_dp = port->cpu_dp; - struct net_device *master = cpu_dp->master; + struct net_device *master = dsa_port_to_master(port); struct dsa_switch *ds = port->ds; const char *name = port->name; struct net_device *slave_dev; diff --git a/net/dsa/tag_8021q.c b/net/dsa/tag_8021q.c index b5f80bc45ceb..34e5ec5d3e23 100644 --- a/net/dsa/tag_8021q.c +++ b/net/dsa/tag_8021q.c @@ -330,7 +330,7 @@ static int dsa_tag_8021q_port_setup(struct dsa_switch *ds, int port) if (!dsa_port_is_user(dp)) return 0; - master = dp->cpu_dp->master; + master = dsa_port_to_master(dp); err = dsa_port_tag_8021q_vlan_add(dp, vid, false); if (err) { @@ -359,7 +359,7 @@ static void dsa_tag_8021q_port_teardown(struct dsa_switch *ds, int port) if (!dsa_port_is_user(dp)) return; - master = dp->cpu_dp->master; + master = dsa_port_to_master(dp); dsa_port_tag_8021q_vlan_del(dp, vid, false); -- cgit v1.2.3 From 95f510d0b792f308d3d748242fe960c35bdc2c62 Mon Sep 17 00:00:00 2001 From: Vladimir Oltean Date: Sun, 11 Sep 2022 04:06:59 +0300 Subject: net: dsa: allow the DSA master to be seen and changed through rtnetlink Some DSA switches have multiple CPU ports, which can be used to improve CPU termination throughput, but DSA, through dsa_tree_setup_cpu_ports(), sets up only the first one, leading to suboptimal use of hardware. The desire is to not change the default configuration but to permit the user to create a dynamic mapping between individual user ports and the CPU port that they are served by, configurable through rtnetlink. It is also intended to permit load balancing between CPU ports, and in that case, the foreseen model is for the DSA master to be a bonding interface whose lowers are the physical DSA masters. To that end, we create a struct rtnl_link_ops for DSA user ports with the "dsa" kind. We expose the IFLA_DSA_MASTER link attribute that contains the ifindex of the newly desired DSA master. Signed-off-by: Vladimir Oltean Signed-off-by: Paolo Abeni --- include/net/dsa.h | 8 +++ include/uapi/linux/if_link.h | 10 ++++ net/dsa/Makefile | 10 +++- net/dsa/dsa.c | 9 +++ net/dsa/dsa2.c | 14 +++++ net/dsa/dsa_priv.h | 10 ++++ net/dsa/netlink.c | 62 +++++++++++++++++++++ net/dsa/port.c | 130 +++++++++++++++++++++++++++++++++++++++++++ net/dsa/slave.c | 120 +++++++++++++++++++++++++++++++++++++++ 9 files changed, 372 insertions(+), 1 deletion(-) create mode 100644 net/dsa/netlink.c (limited to 'net/dsa') diff --git a/include/net/dsa.h b/include/net/dsa.h index 23eac1bda843..3f717c3fcba0 100644 --- a/include/net/dsa.h +++ b/include/net/dsa.h @@ -559,6 +559,10 @@ static inline bool dsa_is_user_port(struct dsa_switch *ds, int p) list_for_each_entry((_dp), &(_dst)->ports, list) \ if (dsa_port_is_user((_dp))) +#define dsa_tree_for_each_user_port_continue_reverse(_dp, _dst) \ + list_for_each_entry_continue_reverse((_dp), &(_dst)->ports, list) \ + if (dsa_port_is_user((_dp))) + #define dsa_tree_for_each_cpu_port(_dp, _dst) \ list_for_each_entry((_dp), &(_dst)->ports, list) \ if (dsa_port_is_cpu((_dp))) @@ -830,6 +834,10 @@ struct dsa_switch_ops { int (*connect_tag_protocol)(struct dsa_switch *ds, enum dsa_tag_protocol proto); + int (*port_change_master)(struct dsa_switch *ds, int port, + struct net_device *master, + struct netlink_ext_ack *extack); + /* Optional switch-wide initialization and destruction methods */ int (*setup)(struct dsa_switch *ds); void (*teardown)(struct dsa_switch *ds); diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h index 0bfa9a99ebb6..3d39fb398d65 100644 --- a/include/uapi/linux/if_link.h +++ b/include/uapi/linux/if_link.h @@ -1375,4 +1375,14 @@ enum { #define IFLA_MCTP_MAX (__IFLA_MCTP_MAX - 1) +/* DSA section */ + +enum { + IFLA_DSA_UNSPEC, + IFLA_DSA_MASTER, + __IFLA_DSA_MAX, +}; + +#define IFLA_DSA_MAX (__IFLA_DSA_MAX - 1) + #endif /* _UAPI_LINUX_IF_LINK_H */ diff --git a/net/dsa/Makefile b/net/dsa/Makefile index af28c24ead18..bf57ef3bce2a 100644 --- a/net/dsa/Makefile +++ b/net/dsa/Makefile @@ -1,7 +1,15 @@ # SPDX-License-Identifier: GPL-2.0 # the core obj-$(CONFIG_NET_DSA) += dsa_core.o -dsa_core-y += dsa.o dsa2.o master.o port.o slave.o switch.o tag_8021q.o +dsa_core-y += \ + dsa.o \ + dsa2.o \ + master.o \ + netlink.o \ + port.o \ + slave.o \ + switch.o \ + tag_8021q.o # tagging formats obj-$(CONFIG_NET_DSA_TAG_AR9331) += tag_ar9331.o diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c index be7b320cda76..64b14f655b23 100644 --- a/net/dsa/dsa.c +++ b/net/dsa/dsa.c @@ -536,8 +536,16 @@ static int __init dsa_init_module(void) dsa_tag_driver_register(&DSA_TAG_DRIVER_NAME(none_ops), THIS_MODULE); + rc = rtnl_link_register(&dsa_link_ops); + if (rc) + goto netlink_register_fail; + return 0; +netlink_register_fail: + dsa_tag_driver_unregister(&DSA_TAG_DRIVER_NAME(none_ops)); + dsa_slave_unregister_notifier(); + dev_remove_pack(&dsa_pack_type); register_notifier_fail: destroy_workqueue(dsa_owq); @@ -547,6 +555,7 @@ module_init(dsa_init_module); static void __exit dsa_cleanup_module(void) { + rtnl_link_unregister(&dsa_link_ops); dsa_tag_driver_unregister(&DSA_TAG_DRIVER_NAME(none_ops)); dsa_slave_unregister_notifier(); diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c index f1f96e2e56aa..42422ddea59b 100644 --- a/net/dsa/dsa2.c +++ b/net/dsa/dsa2.c @@ -387,6 +387,20 @@ static struct dsa_port *dsa_tree_find_first_cpu(struct dsa_switch_tree *dst) return NULL; } +struct net_device *dsa_tree_find_first_master(struct dsa_switch_tree *dst) +{ + struct device_node *ethernet; + struct net_device *master; + struct dsa_port *cpu_dp; + + cpu_dp = dsa_tree_find_first_cpu(dst); + ethernet = of_parse_phandle(cpu_dp->dn, "ethernet", 0); + master = of_find_net_device_by_node(ethernet); + of_node_put(ethernet); + + return master; +} + /* Assign the default CPU port (the first one in the tree) to all ports of the * fabric which don't already have one as part of their own switch. */ diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h index c48c5c8ba790..d252a04ed725 100644 --- a/net/dsa/dsa_priv.h +++ b/net/dsa/dsa_priv.h @@ -200,6 +200,9 @@ static inline struct net_device *dsa_master_find_slave(struct net_device *dev, return NULL; } +/* netlink.c */ +extern struct rtnl_link_ops dsa_link_ops __read_mostly; + /* port.c */ void dsa_port_set_tag_protocol(struct dsa_port *cpu_dp, const struct dsa_device_ops *tag_ops); @@ -292,6 +295,8 @@ void dsa_port_hsr_leave(struct dsa_port *dp, struct net_device *hsr); int dsa_port_tag_8021q_vlan_add(struct dsa_port *dp, u16 vid, bool broadcast); void dsa_port_tag_8021q_vlan_del(struct dsa_port *dp, u16 vid, bool broadcast); void dsa_port_set_host_flood(struct dsa_port *dp, bool uc, bool mc); +int dsa_port_change_master(struct dsa_port *dp, struct net_device *master, + struct netlink_ext_ack *extack); /* slave.c */ extern const struct dsa_device_ops notag_netdev_ops; @@ -305,8 +310,12 @@ int dsa_slave_suspend(struct net_device *slave_dev); int dsa_slave_resume(struct net_device *slave_dev); int dsa_slave_register_notifier(void); void dsa_slave_unregister_notifier(void); +void dsa_slave_sync_ha(struct net_device *dev); +void dsa_slave_unsync_ha(struct net_device *dev); void dsa_slave_setup_tagger(struct net_device *slave); int dsa_slave_change_mtu(struct net_device *dev, int new_mtu); +int dsa_slave_change_master(struct net_device *dev, struct net_device *master, + struct netlink_ext_ack *extack); int dsa_slave_manage_vlan_filtering(struct net_device *dev, bool vlan_filtering); @@ -542,6 +551,7 @@ void dsa_lag_map(struct dsa_switch_tree *dst, struct dsa_lag *lag); void dsa_lag_unmap(struct dsa_switch_tree *dst, struct dsa_lag *lag); struct dsa_lag *dsa_tree_lag_find(struct dsa_switch_tree *dst, const struct net_device *lag_dev); +struct net_device *dsa_tree_find_first_master(struct dsa_switch_tree *dst); int dsa_tree_notify(struct dsa_switch_tree *dst, unsigned long e, void *v); int dsa_broadcast(unsigned long e, void *v); int dsa_tree_change_tag_proto(struct dsa_switch_tree *dst, diff --git a/net/dsa/netlink.c b/net/dsa/netlink.c new file mode 100644 index 000000000000..0f43bbb94769 --- /dev/null +++ b/net/dsa/netlink.c @@ -0,0 +1,62 @@ +// SPDX-License-Identifier: GPL-2.0 +/* Copyright 2022 NXP + */ +#include +#include + +#include "dsa_priv.h" + +static const struct nla_policy dsa_policy[IFLA_DSA_MAX + 1] = { + [IFLA_DSA_MASTER] = { .type = NLA_U32 }, +}; + +static int dsa_changelink(struct net_device *dev, struct nlattr *tb[], + struct nlattr *data[], + struct netlink_ext_ack *extack) +{ + int err; + + if (!data) + return 0; + + if (data[IFLA_DSA_MASTER]) { + u32 ifindex = nla_get_u32(data[IFLA_DSA_MASTER]); + struct net_device *master; + + master = __dev_get_by_index(dev_net(dev), ifindex); + if (!master) + return -EINVAL; + + err = dsa_slave_change_master(dev, master, extack); + if (err) + return err; + } + + return 0; +} + +static size_t dsa_get_size(const struct net_device *dev) +{ + return nla_total_size(sizeof(u32)) + /* IFLA_DSA_MASTER */ + 0; +} + +static int dsa_fill_info(struct sk_buff *skb, const struct net_device *dev) +{ + struct net_device *master = dsa_slave_to_master(dev); + + if (nla_put_u32(skb, IFLA_DSA_MASTER, master->ifindex)) + return -EMSGSIZE; + + return 0; +} + +struct rtnl_link_ops dsa_link_ops __read_mostly = { + .kind = "dsa", + .priv_size = sizeof(struct dsa_port), + .maxtype = IFLA_DSA_MAX, + .policy = dsa_policy, + .changelink = dsa_changelink, + .get_size = dsa_get_size, + .fill_info = dsa_fill_info, +}; diff --git a/net/dsa/port.c b/net/dsa/port.c index 4183e60db4f9..e557b42de9f2 100644 --- a/net/dsa/port.c +++ b/net/dsa/port.c @@ -7,6 +7,7 @@ */ #include +#include #include #include #include @@ -1374,6 +1375,135 @@ int dsa_port_mrp_del_ring_role(const struct dsa_port *dp, return ds->ops->port_mrp_del_ring_role(ds, dp->index, mrp); } +static int dsa_port_assign_master(struct dsa_port *dp, + struct net_device *master, + struct netlink_ext_ack *extack, + bool fail_on_err) +{ + struct dsa_switch *ds = dp->ds; + int port = dp->index, err; + + err = ds->ops->port_change_master(ds, port, master, extack); + if (err && !fail_on_err) + dev_err(ds->dev, "port %d failed to assign master %s: %pe\n", + port, master->name, ERR_PTR(err)); + + if (err && fail_on_err) + return err; + + dp->cpu_dp = master->dsa_ptr; + + return 0; +} + +/* Change the dp->cpu_dp affinity for a user port. Note that both cross-chip + * notifiers and drivers have implicit assumptions about user-to-CPU-port + * mappings, so we unfortunately cannot delay the deletion of the objects + * (switchdev, standalone addresses, standalone VLANs) on the old CPU port + * until the new CPU port has been set up. So we need to completely tear down + * the old CPU port before changing it, and restore it on errors during the + * bringup of the new one. + */ +int dsa_port_change_master(struct dsa_port *dp, struct net_device *master, + struct netlink_ext_ack *extack) +{ + struct net_device *bridge_dev = dsa_port_bridge_dev_get(dp); + struct net_device *old_master = dsa_port_to_master(dp); + struct net_device *dev = dp->slave; + struct dsa_switch *ds = dp->ds; + bool vlan_filtering; + int err, tmp; + + /* Bridges may hold host FDB, MDB and VLAN objects. These need to be + * migrated, so dynamically unoffload and later reoffload the bridge + * port. + */ + if (bridge_dev) { + dsa_port_pre_bridge_leave(dp, bridge_dev); + dsa_port_bridge_leave(dp, bridge_dev); + } + + /* The port might still be VLAN filtering even if it's no longer + * under a bridge, either due to ds->vlan_filtering_is_global or + * ds->needs_standalone_vlan_filtering. In turn this means VLANs + * on the CPU port. + */ + vlan_filtering = dsa_port_is_vlan_filtering(dp); + if (vlan_filtering) { + err = dsa_slave_manage_vlan_filtering(dev, false); + if (err) { + NL_SET_ERR_MSG_MOD(extack, + "Failed to remove standalone VLANs"); + goto rewind_old_bridge; + } + } + + /* Standalone addresses, and addresses of upper interfaces like + * VLAN, LAG, HSR need to be migrated. + */ + dsa_slave_unsync_ha(dev); + + err = dsa_port_assign_master(dp, master, extack, true); + if (err) + goto rewind_old_addrs; + + dsa_slave_sync_ha(dev); + + if (vlan_filtering) { + err = dsa_slave_manage_vlan_filtering(dev, true); + if (err) { + NL_SET_ERR_MSG_MOD(extack, + "Failed to restore standalone VLANs"); + goto rewind_new_addrs; + } + } + + if (bridge_dev) { + err = dsa_port_bridge_join(dp, bridge_dev, extack); + if (err && err == -EOPNOTSUPP) { + NL_SET_ERR_MSG_MOD(extack, + "Failed to reoffload bridge"); + goto rewind_new_vlan; + } + } + + return 0; + +rewind_new_vlan: + if (vlan_filtering) + dsa_slave_manage_vlan_filtering(dev, false); + +rewind_new_addrs: + dsa_slave_unsync_ha(dev); + + dsa_port_assign_master(dp, old_master, NULL, false); + +/* Restore the objects on the old CPU port */ +rewind_old_addrs: + dsa_slave_sync_ha(dev); + + if (vlan_filtering) { + tmp = dsa_slave_manage_vlan_filtering(dev, true); + if (tmp) { + dev_err(ds->dev, + "port %d failed to restore standalone VLANs: %pe\n", + dp->index, ERR_PTR(tmp)); + } + } + +rewind_old_bridge: + if (bridge_dev) { + tmp = dsa_port_bridge_join(dp, bridge_dev, extack); + if (tmp) { + dev_err(ds->dev, + "port %d failed to rejoin bridge %s: %pe\n", + dp->index, bridge_dev->name, ERR_PTR(tmp)); + } + } + + return err; +} + void dsa_port_set_tag_protocol(struct dsa_port *cpu_dp, const struct dsa_device_ops *tag_ops) { diff --git a/net/dsa/slave.c b/net/dsa/slave.c index 55094b94a5ae..00df6cf07866 100644 --- a/net/dsa/slave.c +++ b/net/dsa/slave.c @@ -164,6 +164,48 @@ static int dsa_slave_unsync_mc(struct net_device *dev, return dsa_slave_schedule_standalone_work(dev, DSA_MC_DEL, addr, 0); } +void dsa_slave_sync_ha(struct net_device *dev) +{ + struct dsa_port *dp = dsa_slave_to_port(dev); + struct dsa_switch *ds = dp->ds; + struct netdev_hw_addr *ha; + + netif_addr_lock_bh(dev); + + netdev_for_each_synced_mc_addr(ha, dev) + dsa_slave_sync_mc(dev, ha->addr); + + netdev_for_each_synced_uc_addr(ha, dev) + dsa_slave_sync_uc(dev, ha->addr); + + netif_addr_unlock_bh(dev); + + if (dsa_switch_supports_uc_filtering(ds) || + dsa_switch_supports_mc_filtering(ds)) + dsa_flush_workqueue(); +} + +void dsa_slave_unsync_ha(struct net_device *dev) +{ + struct dsa_port *dp = dsa_slave_to_port(dev); + struct dsa_switch *ds = dp->ds; + struct netdev_hw_addr *ha; + + netif_addr_lock_bh(dev); + + netdev_for_each_synced_uc_addr(ha, dev) + dsa_slave_unsync_uc(dev, ha->addr); + + netdev_for_each_synced_mc_addr(ha, dev) + dsa_slave_unsync_mc(dev, ha->addr); + + netif_addr_unlock_bh(dev); + + if (dsa_switch_supports_uc_filtering(ds) || + dsa_switch_supports_mc_filtering(ds)) + dsa_flush_workqueue(); +} + /* slave mii_bus handling ***************************************************/ static int dsa_slave_phy_read(struct mii_bus *bus, int addr, int reg) { @@ -2346,6 +2388,7 @@ int dsa_slave_create(struct dsa_port *port) if (slave_dev == NULL) return -ENOMEM; + slave_dev->rtnl_link_ops = &dsa_link_ops; slave_dev->ethtool_ops = &dsa_slave_ethtool_ops; #if IS_ENABLED(CONFIG_DCB) slave_dev->dcbnl_ops = &dsa_slave_dcbnl_ops; @@ -2462,6 +2505,83 @@ void dsa_slave_destroy(struct net_device *slave_dev) free_netdev(slave_dev); } +int dsa_slave_change_master(struct net_device *dev, struct net_device *master, + struct netlink_ext_ack *extack) +{ + struct net_device *old_master = dsa_slave_to_master(dev); + struct dsa_port *dp = dsa_slave_to_port(dev); + struct dsa_switch *ds = dp->ds; + struct net_device *upper; + struct list_head *iter; + int err; + + if (master == old_master) + return 0; + + if (!ds->ops->port_change_master) { + NL_SET_ERR_MSG_MOD(extack, + "Driver does not support changing DSA master"); + return -EOPNOTSUPP; + } + + if (!netdev_uses_dsa(master)) { + NL_SET_ERR_MSG_MOD(extack, + "Interface not eligible as DSA master"); + return -EOPNOTSUPP; + } + + netdev_for_each_upper_dev_rcu(master, upper, iter) { + if (dsa_slave_dev_check(upper)) + continue; + if (netif_is_bridge_master(upper)) + continue; + NL_SET_ERR_MSG_MOD(extack, "Cannot join master with unknown uppers"); + return -EOPNOTSUPP; + } + + /* Since we allow live-changing the DSA master, plus we auto-open the + * DSA master when the user port opens => we need to ensure that the + * new DSA master is open too. + */ + if (dev->flags & IFF_UP) { + err = dev_open(master, extack); + if (err) + return err; + } + + netdev_upper_dev_unlink(old_master, dev); + + err = netdev_upper_dev_link(master, dev, extack); + if (err) + goto out_revert_old_master_unlink; + + err = dsa_port_change_master(dp, master, extack); + if (err) + goto out_revert_master_link; + + /* Update the MTU of the new CPU port through cross-chip notifiers */ + err = dsa_slave_change_mtu(dev, dev->mtu); + if (err && err != -EOPNOTSUPP) { + netdev_warn(dev, + "nonfatal error updating MTU with new master: %pe\n", + ERR_PTR(err)); + } + + /* If the port doesn't have its own MAC address and relies on the DSA + * master's one, inherit it again from the new DSA master. + */ + if (is_zero_ether_addr(dp->mac)) + eth_hw_addr_inherit(dev, master); + + return 0; + +out_revert_master_link: + netdev_upper_dev_unlink(master, dev); +out_revert_old_master_unlink: + netdev_upper_dev_link(old_master, dev, NULL); + return err; +} + bool dsa_slave_dev_check(const struct net_device *dev) { return dev->netdev_ops == &dsa_slave_netdev_ops; -- cgit v1.2.3 From 6e61b55c6d7f20f5619e831fa171f65bfbcf03c7 Mon Sep 17 00:00:00 2001 From: Vladimir Oltean Date: Sun, 11 Sep 2022 04:07:00 +0300 Subject: net: dsa: don't keep track of admin/oper state on LAG DSA masters We store information about the DSA master's state in cpu_dp->master_admin_up and cpu_dp->master_oper_up, and this assumes a bijective association between a CPU port and a DSA master. However, when we have CPU ports in a LAG (and DSA masters in a LAG too), the way in which we set up things is that the physical DSA masters still have dev->dsa_ptr pointing to our cpu_dp, but the bonding/team device itself also has its dev->dsa_ptr pointing towards one of the CPU port structures (the first one). So logically speaking, that first cpu_dp can't keep track of both the physical master's admin/oper state, and of the bonding master's state. This isn't even needed; the reason why we keep track of the DSA master's state is to know when it is available for Ethernet-based register access. For that use case, we don't even need LAG; we just need to decide upon one of the physical DSA masters (if there is more than 1 available) and use that. This change suppresses dsa_tree_master_{admin,oper}_state_change() calls on LAG DSA masters (which will be supported in a future change), to allow the tracking of just physical DSA masters. Link: https://lore.kernel.org/netdev/628cc94d.1c69fb81.15b0d.422d@mx.google.com/ Suggested-by: Christian Marangi Signed-off-by: Vladimir Oltean Signed-off-by: Paolo Abeni --- net/dsa/dsa2.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) (limited to 'net/dsa') diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c index 42422ddea59b..7024e2120de1 100644 --- a/net/dsa/dsa2.c +++ b/net/dsa/dsa2.c @@ -1326,6 +1326,12 @@ void dsa_tree_master_admin_state_change(struct dsa_switch_tree *dst, struct dsa_port *cpu_dp = master->dsa_ptr; bool notify = false; + /* Don't keep track of admin state on LAG DSA masters, + * but rather just of physical DSA masters + */ + if (netif_is_lag_master(master)) + return; + if ((dsa_port_master_is_operational(cpu_dp)) != (up && cpu_dp->master_oper_up)) notify = true; @@ -1343,6 +1349,12 @@ void dsa_tree_master_oper_state_change(struct dsa_switch_tree *dst, struct dsa_port *cpu_dp = master->dsa_ptr; bool notify = false; + /* Don't keep track of oper state on LAG DSA masters, + * but rather just of physical DSA masters + */ + if (netif_is_lag_master(master)) + return; + if ((dsa_port_master_is_operational(cpu_dp)) != (cpu_dp->master_admin_up && up)) notify = true; -- cgit v1.2.3 From cfeb84a52fcbdb12c7364c2cab4529c5c5558c35 Mon Sep 17 00:00:00 2001 From: Vladimir Oltean Date: Sun, 11 Sep 2022 04:07:01 +0300 Subject: net: dsa: suppress appending ethtool stats to LAG DSA masters Similar to the discussion about tracking the admin/oper state of LAG DSA masters, we have the problem here that struct dsa_port *cpu_dp caches a single pair of orig_ethtool_ops and netdev_ops pointers. So if we call dsa_master_setup(bond0, cpu_dp) where cpu_dp is also the dev->dsa_ptr of one of the physical DSA masters, we'd effectively overwrite what we cached from that physical netdev with what replaced from the bonding interface. We don't need DSA ethtool stats on the bonding interface when used as DSA master, it's good enough to have them just on the physical DSA masters, so suppress this logic. Signed-off-by: Vladimir Oltean Signed-off-by: Paolo Abeni --- net/dsa/master.c | 9 +++++++++ 1 file changed, 9 insertions(+) (limited to 'net/dsa') diff --git a/net/dsa/master.c b/net/dsa/master.c index fb810edc8281..99d773b24223 100644 --- a/net/dsa/master.c +++ b/net/dsa/master.c @@ -226,6 +226,9 @@ static int dsa_master_ethtool_setup(struct net_device *dev) struct dsa_switch *ds = cpu_dp->ds; struct ethtool_ops *ops; + if (netif_is_lag_master(dev)) + return 0; + ops = devm_kzalloc(ds->dev, sizeof(*ops), GFP_KERNEL); if (!ops) return -ENOMEM; @@ -250,6 +253,9 @@ static void dsa_master_ethtool_teardown(struct net_device *dev) { struct dsa_port *cpu_dp = dev->dsa_ptr; + if (netif_is_lag_master(dev)) + return; + dev->ethtool_ops = cpu_dp->orig_ethtool_ops; cpu_dp->orig_ethtool_ops = NULL; } @@ -257,6 +263,9 @@ static void dsa_master_ethtool_teardown(struct net_device *dev) static void dsa_netdev_ops_set(struct net_device *dev, const struct dsa_netdevice_ops *ops) { + if (netif_is_lag_master(dev)) + return; + dev->dsa_ptr->netdev_ops = ops; } -- cgit v1.2.3 From 13eccc1bbb2e98df5d616398cd9215d9edab522f Mon Sep 17 00:00:00 2001 From: Vladimir Oltean Date: Sun, 11 Sep 2022 04:07:02 +0300 Subject: net: dsa: suppress device links to LAG DSA masters These don't work (print a harmless error about the operation failing) and make little sense to have anyway, because when a LAG DSA master goes away, we will introduce logic to move our CPU port back to the first physical DSA master. So suppress these device links in preparation for adding support for LAG DSA masters. Signed-off-by: Vladimir Oltean Signed-off-by: Paolo Abeni --- net/dsa/master.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) (limited to 'net/dsa') diff --git a/net/dsa/master.c b/net/dsa/master.c index 99d773b24223..2176c14b97a8 100644 --- a/net/dsa/master.c +++ b/net/dsa/master.c @@ -364,12 +364,14 @@ int dsa_master_setup(struct net_device *dev, struct dsa_port *cpu_dp) mtu = ETH_DATA_LEN + dsa_tag_protocol_overhead(tag_ops); /* The DSA master must use SET_NETDEV_DEV for this to work. */ - consumer_link = device_link_add(ds->dev, dev->dev.parent, - DL_FLAG_AUTOREMOVE_CONSUMER); - if (!consumer_link) - netdev_err(dev, - "Failed to create a device link to DSA switch %s\n", - dev_name(ds->dev)); + if (!netif_is_lag_master(dev)) { + consumer_link = device_link_add(ds->dev, dev->dev.parent, + DL_FLAG_AUTOREMOVE_CONSUMER); + if (!consumer_link) + netdev_err(dev, + "Failed to create a device link to DSA switch %s\n", + dev_name(ds->dev)); + } /* The switch driver may not implement ->port_change_mtu(), case in * which dsa_slave_change_mtu() will not update the master MTU either, -- cgit v1.2.3 From 2e359b00a11715c7a89d7448e6e4cb4d84543520 Mon Sep 17 00:00:00 2001 From: Vladimir Oltean Date: Sun, 11 Sep 2022 04:07:03 +0300 Subject: net: dsa: propagate extack to port_lag_join Drivers could refuse to offload a LAG configuration for a variety of reasons, mainly having to do with its TX type. Additionally, since DSA masters may now also be LAG interfaces, and this will translate into a call to port_lag_join on the CPU ports, there may be extra restrictions there. Propagate the netlink extack to this DSA method in order for drivers to give a meaningful error message back to the user. Signed-off-by: Vladimir Oltean Signed-off-by: Paolo Abeni --- drivers/net/dsa/mv88e6xxx/chip.c | 27 +++++++++++++++++++-------- drivers/net/dsa/ocelot/felix.c | 5 +++-- drivers/net/dsa/qca/qca8k-common.c | 23 +++++++++++++++++------ drivers/net/dsa/qca/qca8k.h | 3 ++- drivers/net/ethernet/mscc/ocelot.c | 8 ++++++-- drivers/net/ethernet/mscc/ocelot_net.c | 7 +++---- include/net/dsa.h | 6 ++++-- include/soc/mscc/ocelot.h | 3 ++- net/dsa/dsa_priv.h | 1 + net/dsa/port.c | 1 + net/dsa/switch.c | 4 ++-- 11 files changed, 60 insertions(+), 28 deletions(-) (limited to 'net/dsa') diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c index 6f4ea39ab466..5f178faa110f 100644 --- a/drivers/net/dsa/mv88e6xxx/chip.c +++ b/drivers/net/dsa/mv88e6xxx/chip.c @@ -6593,14 +6593,17 @@ out: static bool mv88e6xxx_lag_can_offload(struct dsa_switch *ds, struct dsa_lag lag, - struct netdev_lag_upper_info *info) + struct netdev_lag_upper_info *info, + struct netlink_ext_ack *extack) { struct mv88e6xxx_chip *chip = ds->priv; struct dsa_port *dp; int members = 0; - if (!mv88e6xxx_has_lag(chip)) + if (!mv88e6xxx_has_lag(chip)) { + NL_SET_ERR_MSG_MOD(extack, "Chip does not support LAG offload"); return false; + } if (!lag.id) return false; @@ -6609,14 +6612,20 @@ static bool mv88e6xxx_lag_can_offload(struct dsa_switch *ds, /* Includes the port joining the LAG */ members++; - if (members > 8) + if (members > 8) { + NL_SET_ERR_MSG_MOD(extack, + "Cannot offload more than 8 LAG ports"); return false; + } /* We could potentially relax this to include active * backup in the future. */ - if (info->tx_type != NETDEV_LAG_TX_TYPE_HASH) + if (info->tx_type != NETDEV_LAG_TX_TYPE_HASH) { + NL_SET_ERR_MSG_MOD(extack, + "Can only offload LAG using hash TX type"); return false; + } /* Ideally we would also validate that the hash type matches * the hardware. Alas, this is always set to unknown on team @@ -6769,12 +6778,13 @@ static int mv88e6xxx_port_lag_change(struct dsa_switch *ds, int port) static int mv88e6xxx_port_lag_join(struct dsa_switch *ds, int port, struct dsa_lag lag, - struct netdev_lag_upper_info *info) + struct netdev_lag_upper_info *info, + struct netlink_ext_ack *extack) { struct mv88e6xxx_chip *chip = ds->priv; int err, id; - if (!mv88e6xxx_lag_can_offload(ds, lag, info)) + if (!mv88e6xxx_lag_can_offload(ds, lag, info, extack)) return -EOPNOTSUPP; /* DSA LAG IDs are one-based */ @@ -6827,12 +6837,13 @@ static int mv88e6xxx_crosschip_lag_change(struct dsa_switch *ds, int sw_index, static int mv88e6xxx_crosschip_lag_join(struct dsa_switch *ds, int sw_index, int port, struct dsa_lag lag, - struct netdev_lag_upper_info *info) + struct netdev_lag_upper_info *info, + struct netlink_ext_ack *extack) { struct mv88e6xxx_chip *chip = ds->priv; int err; - if (!mv88e6xxx_lag_can_offload(ds, lag, info)) + if (!mv88e6xxx_lag_can_offload(ds, lag, info, extack)) return -EOPNOTSUPP; mv88e6xxx_reg_lock(chip); diff --git a/drivers/net/dsa/ocelot/felix.c b/drivers/net/dsa/ocelot/felix.c index c73ef5f7aa64..82dcc21a7172 100644 --- a/drivers/net/dsa/ocelot/felix.c +++ b/drivers/net/dsa/ocelot/felix.c @@ -861,11 +861,12 @@ static void felix_bridge_leave(struct dsa_switch *ds, int port, static int felix_lag_join(struct dsa_switch *ds, int port, struct dsa_lag lag, - struct netdev_lag_upper_info *info) + struct netdev_lag_upper_info *info, + struct netlink_ext_ack *extack) { struct ocelot *ocelot = ds->priv; - return ocelot_port_lag_join(ocelot, port, lag.dev, info); + return ocelot_port_lag_join(ocelot, port, lag.dev, info, extack); } static int felix_lag_leave(struct dsa_switch *ds, int port, diff --git a/drivers/net/dsa/qca/qca8k-common.c b/drivers/net/dsa/qca/qca8k-common.c index bba95613e218..fb45b598847b 100644 --- a/drivers/net/dsa/qca/qca8k-common.c +++ b/drivers/net/dsa/qca/qca8k-common.c @@ -1017,7 +1017,8 @@ int qca8k_port_vlan_del(struct dsa_switch *ds, int port, static bool qca8k_lag_can_offload(struct dsa_switch *ds, struct dsa_lag lag, - struct netdev_lag_upper_info *info) + struct netdev_lag_upper_info *info, + struct netlink_ext_ack *extack) { struct dsa_port *dp; int members = 0; @@ -1029,15 +1030,24 @@ static bool qca8k_lag_can_offload(struct dsa_switch *ds, /* Includes the port joining the LAG */ members++; - if (members > QCA8K_NUM_PORTS_FOR_LAG) + if (members > QCA8K_NUM_PORTS_FOR_LAG) { + NL_SET_ERR_MSG_MOD(extack, + "Cannot offload more than 4 LAG ports"); return false; + } - if (info->tx_type != NETDEV_LAG_TX_TYPE_HASH) + if (info->tx_type != NETDEV_LAG_TX_TYPE_HASH) { + NL_SET_ERR_MSG_MOD(extack, + "Can only offload LAG using hash TX type"); return false; + } if (info->hash_type != NETDEV_LAG_HASH_L2 && - info->hash_type != NETDEV_LAG_HASH_L23) + info->hash_type != NETDEV_LAG_HASH_L23) { + NL_SET_ERR_MSG_MOD(extack, + "Can only offload L2 or L2+L3 TX hash"); return false; + } return true; } @@ -1160,11 +1170,12 @@ static int qca8k_lag_refresh_portmap(struct dsa_switch *ds, int port, } int qca8k_port_lag_join(struct dsa_switch *ds, int port, struct dsa_lag lag, - struct netdev_lag_upper_info *info) + struct netdev_lag_upper_info *info, + struct netlink_ext_ack *extack) { int ret; - if (!qca8k_lag_can_offload(ds, lag, info)) + if (!qca8k_lag_can_offload(ds, lag, info, extack)) return -EOPNOTSUPP; ret = qca8k_lag_setup_hash(ds, lag, info); diff --git a/drivers/net/dsa/qca/qca8k.h b/drivers/net/dsa/qca/qca8k.h index e36ecc9777f4..0b7a5cb12321 100644 --- a/drivers/net/dsa/qca/qca8k.h +++ b/drivers/net/dsa/qca/qca8k.h @@ -512,7 +512,8 @@ int qca8k_port_vlan_del(struct dsa_switch *ds, int port, /* Common port LAG function */ int qca8k_port_lag_join(struct dsa_switch *ds, int port, struct dsa_lag lag, - struct netdev_lag_upper_info *info); + struct netdev_lag_upper_info *info, + struct netlink_ext_ack *extack); int qca8k_port_lag_leave(struct dsa_switch *ds, int port, struct dsa_lag lag); diff --git a/drivers/net/ethernet/mscc/ocelot.c b/drivers/net/ethernet/mscc/ocelot.c index 874fb2a5874e..5c18f8986975 100644 --- a/drivers/net/ethernet/mscc/ocelot.c +++ b/drivers/net/ethernet/mscc/ocelot.c @@ -2132,10 +2132,14 @@ static void ocelot_migrate_lag_fdbs(struct ocelot *ocelot, int ocelot_port_lag_join(struct ocelot *ocelot, int port, struct net_device *bond, - struct netdev_lag_upper_info *info) + struct netdev_lag_upper_info *info, + struct netlink_ext_ack *extack) { - if (info->tx_type != NETDEV_LAG_TX_TYPE_HASH) + if (info->tx_type != NETDEV_LAG_TX_TYPE_HASH) { + NL_SET_ERR_MSG_MOD(extack, + "Can only offload LAG using hash TX type"); return -EOPNOTSUPP; + } mutex_lock(&ocelot->fwd_domain_lock); diff --git a/drivers/net/ethernet/mscc/ocelot_net.c b/drivers/net/ethernet/mscc/ocelot_net.c index 2979fb1ba0f7..50858cc10fef 100644 --- a/drivers/net/ethernet/mscc/ocelot_net.c +++ b/drivers/net/ethernet/mscc/ocelot_net.c @@ -1412,11 +1412,10 @@ static int ocelot_netdevice_lag_join(struct net_device *dev, int port = priv->port.index; int err; - err = ocelot_port_lag_join(ocelot, port, bond, info); - if (err == -EOPNOTSUPP) { - NL_SET_ERR_MSG_MOD(extack, "Offloading not supported"); + err = ocelot_port_lag_join(ocelot, port, bond, info, extack); + if (err == -EOPNOTSUPP) + /* Offloading not supported, fall back to software LAG */ return 0; - } bridge_dev = netdev_master_upper_dev_get(bond); if (!bridge_dev || !netif_is_bridge_master(bridge_dev)) diff --git a/include/net/dsa.h b/include/net/dsa.h index 3f717c3fcba0..1c80e75b3cad 100644 --- a/include/net/dsa.h +++ b/include/net/dsa.h @@ -1094,7 +1094,8 @@ struct dsa_switch_ops { int port); int (*crosschip_lag_join)(struct dsa_switch *ds, int sw_index, int port, struct dsa_lag lag, - struct netdev_lag_upper_info *info); + struct netdev_lag_upper_info *info, + struct netlink_ext_ack *extack); int (*crosschip_lag_leave)(struct dsa_switch *ds, int sw_index, int port, struct dsa_lag lag); @@ -1169,7 +1170,8 @@ struct dsa_switch_ops { int (*port_lag_change)(struct dsa_switch *ds, int port); int (*port_lag_join)(struct dsa_switch *ds, int port, struct dsa_lag lag, - struct netdev_lag_upper_info *info); + struct netdev_lag_upper_info *info, + struct netlink_ext_ack *extack); int (*port_lag_leave)(struct dsa_switch *ds, int port, struct dsa_lag lag); diff --git a/include/soc/mscc/ocelot.h b/include/soc/mscc/ocelot.h index 355cfdedc43b..ea19e8ef1f61 100644 --- a/include/soc/mscc/ocelot.h +++ b/include/soc/mscc/ocelot.h @@ -1229,7 +1229,8 @@ int ocelot_port_mdb_del(struct ocelot *ocelot, int port, const struct net_device *bridge); int ocelot_port_lag_join(struct ocelot *ocelot, int port, struct net_device *bond, - struct netdev_lag_upper_info *info); + struct netdev_lag_upper_info *info, + struct netlink_ext_ack *extack); void ocelot_port_lag_leave(struct ocelot *ocelot, int port, struct net_device *bond); void ocelot_port_lag_change(struct ocelot *ocelot, int port, bool lag_tx_active); diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h index d252a04ed725..f0ae54d0435e 100644 --- a/net/dsa/dsa_priv.h +++ b/net/dsa/dsa_priv.h @@ -88,6 +88,7 @@ struct dsa_notifier_lag_info { const struct dsa_port *dp; struct dsa_lag lag; struct netdev_lag_upper_info *info; + struct netlink_ext_ack *extack; }; /* DSA_NOTIFIER_VLAN_* */ diff --git a/net/dsa/port.c b/net/dsa/port.c index e557b42de9f2..98f7fa0cdd5c 100644 --- a/net/dsa/port.c +++ b/net/dsa/port.c @@ -635,6 +635,7 @@ int dsa_port_lag_join(struct dsa_port *dp, struct net_device *lag_dev, struct dsa_notifier_lag_info info = { .dp = dp, .info = uinfo, + .extack = extack, }; struct net_device *bridge_dev; int err; diff --git a/net/dsa/switch.c b/net/dsa/switch.c index 4dfd68cf61c5..c2cb15e21324 100644 --- a/net/dsa/switch.c +++ b/net/dsa/switch.c @@ -507,12 +507,12 @@ static int dsa_switch_lag_join(struct dsa_switch *ds, { if (info->dp->ds == ds && ds->ops->port_lag_join) return ds->ops->port_lag_join(ds, info->dp->index, info->lag, - info->info); + info->info, info->extack); if (info->dp->ds != ds && ds->ops->crosschip_lag_join) return ds->ops->crosschip_lag_join(ds, info->dp->ds->index, info->dp->index, info->lag, - info->info); + info->info, info->extack); return -EOPNOTSUPP; } -- cgit v1.2.3 From acc43b7bf52a015221d989164c2600c1e1f28790 Mon Sep 17 00:00:00 2001 From: Vladimir Oltean Date: Sun, 11 Sep 2022 04:07:04 +0300 Subject: net: dsa: allow masters to join a LAG There are 2 ways in which a DSA user port may become handled by 2 CPU ports in a LAG: (1) its current DSA master joins a LAG ip link del bond0 && ip link add bond0 type bond mode 802.3ad ip link set eno2 master bond0 When this happens, all user ports with "eno2" as DSA master get automatically migrated to "bond0" as DSA master. (2) it is explicitly configured as such by the user # Before, the DSA master was eno3 ip link set swp0 type dsa master bond0 The design of this configuration is that the LAG device dynamically becomes a DSA master through dsa_master_setup() when the first physical DSA master becomes a LAG slave, and stops being so through dsa_master_teardown() when the last physical DSA master leaves. A LAG interface is considered as a valid DSA master only if it contains existing DSA masters, and no other lower interfaces. Therefore, we mainly rely on method (1) to enter this configuration. Each physical DSA master (LAG slave) retains its dev->dsa_ptr for when it becomes a standalone DSA master again. But the LAG master also has a dev->dsa_ptr, and this is actually duplicated from one of the physical LAG slaves, and therefore needs to be balanced when LAG slaves come and go. To the switch driver, putting DSA masters in a LAG is seen as putting their associated CPU ports in a LAG. We need to prepare cross-chip host FDB notifiers for CPU ports in a LAG, by calling the driver's ->lag_fdb_add method rather than ->port_fdb_add. Signed-off-by: Vladimir Oltean Signed-off-by: Paolo Abeni --- include/net/dsa.h | 12 +++ net/dsa/dsa_priv.h | 5 ++ net/dsa/master.c | 49 ++++++++++++ net/dsa/port.c | 1 + net/dsa/slave.c | 231 +++++++++++++++++++++++++++++++++++++++++++++++++++-- net/dsa/switch.c | 22 ++++- 6 files changed, 310 insertions(+), 10 deletions(-) (limited to 'net/dsa') diff --git a/include/net/dsa.h b/include/net/dsa.h index 1c80e75b3cad..d777eac5694f 100644 --- a/include/net/dsa.h +++ b/include/net/dsa.h @@ -300,6 +300,9 @@ struct dsa_port { u8 master_admin_up:1; u8 master_oper_up:1; + /* Valid only on user ports */ + u8 cpu_port_in_lag:1; + u8 setup:1; struct device_node *dn; @@ -724,6 +727,9 @@ static inline bool dsa_port_offloads_lag(struct dsa_port *dp, static inline struct net_device *dsa_port_to_master(const struct dsa_port *dp) { + if (dp->cpu_port_in_lag) + return dsa_port_lag_dev_get(dp->cpu_dp); + return dp->cpu_dp->master; } @@ -811,6 +817,12 @@ dsa_tree_offloads_bridge_dev(struct dsa_switch_tree *dst, return false; } +static inline bool dsa_port_tree_same(const struct dsa_port *a, + const struct dsa_port *b) +{ + return a->ds->dst == b->ds->dst; +} + typedef int dsa_fdb_dump_cb_t(const unsigned char *addr, u16 vid, bool is_static, void *data); struct dsa_switch_ops { diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h index f0ae54d0435e..129e4a649c7e 100644 --- a/net/dsa/dsa_priv.h +++ b/net/dsa/dsa_priv.h @@ -185,6 +185,11 @@ static inline int dsa_tag_protocol_overhead(const struct dsa_device_ops *ops) /* master.c */ int dsa_master_setup(struct net_device *dev, struct dsa_port *cpu_dp); void dsa_master_teardown(struct net_device *dev); +int dsa_master_lag_setup(struct net_device *lag_dev, struct dsa_port *cpu_dp, + struct netdev_lag_upper_info *uinfo, + struct netlink_ext_ack *extack); +void dsa_master_lag_teardown(struct net_device *lag_dev, + struct dsa_port *cpu_dp); static inline struct net_device *dsa_master_find_slave(struct net_device *dev, int device, int port) diff --git a/net/dsa/master.c b/net/dsa/master.c index 2176c14b97a8..40367ab41cf8 100644 --- a/net/dsa/master.c +++ b/net/dsa/master.c @@ -428,3 +428,52 @@ void dsa_master_teardown(struct net_device *dev) */ wmb(); } + +int dsa_master_lag_setup(struct net_device *lag_dev, struct dsa_port *cpu_dp, + struct netdev_lag_upper_info *uinfo, + struct netlink_ext_ack *extack) +{ + bool master_setup = false; + int err; + + if (!netdev_uses_dsa(lag_dev)) { + err = dsa_master_setup(lag_dev, cpu_dp); + if (err) + return err; + + master_setup = true; + } + + err = dsa_port_lag_join(cpu_dp, lag_dev, uinfo, extack); + if (err) { + if (extack && !extack->_msg) + NL_SET_ERR_MSG_MOD(extack, + "CPU port failed to join LAG"); + goto out_master_teardown; + } + + return 0; + +out_master_teardown: + if (master_setup) + dsa_master_teardown(lag_dev); + return err; +} + +/* Tear down a master if there isn't any other user port on it, + * optionally also destroying LAG information. + */ +void dsa_master_lag_teardown(struct net_device *lag_dev, + struct dsa_port *cpu_dp) +{ + struct net_device *upper; + struct list_head *iter; + + dsa_port_lag_leave(cpu_dp, lag_dev); + + netdev_for_each_upper_dev_rcu(lag_dev, upper, iter) + if (dsa_slave_dev_check(upper)) + return; + + dsa_master_teardown(lag_dev); +} diff --git a/net/dsa/port.c b/net/dsa/port.c index 98f7fa0cdd5c..e6289a1db0a0 100644 --- a/net/dsa/port.c +++ b/net/dsa/port.c @@ -1393,6 +1393,7 @@ static int dsa_port_assign_master(struct dsa_port *dp, return err; dp->cpu_dp = master->dsa_ptr; + dp->cpu_port_in_lag = netif_is_lag_master(master); return 0; } diff --git a/net/dsa/slave.c b/net/dsa/slave.c index 00df6cf07866..aa47ddc19fdf 100644 --- a/net/dsa/slave.c +++ b/net/dsa/slave.c @@ -2818,11 +2818,45 @@ dsa_slave_prechangeupper_sanity_check(struct net_device *dev, return NOTIFY_DONE; } +/* To be eligible as a DSA master, a LAG must have all lower interfaces be + * eligible DSA masters. Additionally, all LAG slaves must be DSA masters of + * switches in the same switch tree. + */ +static int dsa_lag_master_validate(struct net_device *lag_dev, + struct netlink_ext_ack *extack) +{ + struct net_device *lower1, *lower2; + struct list_head *iter1, *iter2; + + netdev_for_each_lower_dev(lag_dev, lower1, iter1) { + netdev_for_each_lower_dev(lag_dev, lower2, iter2) { + if (!netdev_uses_dsa(lower1) || + !netdev_uses_dsa(lower2)) { + NL_SET_ERR_MSG_MOD(extack, + "All LAG ports must be eligible as DSA masters"); + return notifier_from_errno(-EINVAL); + } + + if (lower1 == lower2) + continue; + + if (!dsa_port_tree_same(lower1->dsa_ptr, + lower2->dsa_ptr)) { + NL_SET_ERR_MSG_MOD(extack, + "LAG contains DSA masters of disjoint switch trees"); + return notifier_from_errno(-EINVAL); + } + } + } + + return NOTIFY_DONE; +} + static int dsa_master_prechangeupper_sanity_check(struct net_device *master, struct netdev_notifier_changeupper_info *info) { - struct netlink_ext_ack *extack; + struct netlink_ext_ack *extack = netdev_notifier_info_to_extack(&info->info); if (!netdev_uses_dsa(master)) return NOTIFY_DONE; @@ -2840,13 +2874,51 @@ dsa_master_prechangeupper_sanity_check(struct net_device *master, if (netif_is_bridge_master(info->upper_dev)) return NOTIFY_DONE; - extack = netdev_notifier_info_to_extack(&info->info); + /* Allow LAG uppers, subject to further restrictions in + * dsa_lag_master_prechangelower_sanity_check() + */ + if (netif_is_lag_master(info->upper_dev)) + return dsa_lag_master_validate(info->upper_dev, extack); NL_SET_ERR_MSG_MOD(extack, "DSA master cannot join unknown upper interfaces"); return notifier_from_errno(-EBUSY); } +static int +dsa_lag_master_prechangelower_sanity_check(struct net_device *dev, + struct netdev_notifier_changeupper_info *info) +{ + struct netlink_ext_ack *extack = netdev_notifier_info_to_extack(&info->info); + struct net_device *lag_dev = info->upper_dev; + struct net_device *lower; + struct list_head *iter; + + if (!netdev_uses_dsa(lag_dev) || !netif_is_lag_master(lag_dev)) + return NOTIFY_DONE; + + if (!info->linking) + return NOTIFY_DONE; + + if (!netdev_uses_dsa(dev)) { + NL_SET_ERR_MSG(extack, + "Only DSA masters can join a LAG DSA master"); + return notifier_from_errno(-EINVAL); + } + + netdev_for_each_lower_dev(lag_dev, lower, iter) { + if (!dsa_port_tree_same(dev->dsa_ptr, lower->dsa_ptr)) { + NL_SET_ERR_MSG(extack, + "Interface is DSA master for a different switch tree than this LAG"); + return notifier_from_errno(-EINVAL); + } + + break; + } + + return NOTIFY_DONE; +} + /* Don't allow bridging of DSA masters, since the bridge layer rx_handler * prevents the DSA fake ethertype handler to be invoked, so we don't get the * chance to strip off and parse the DSA switch tag protocol header (the bridge @@ -2887,6 +2959,136 @@ dsa_bridge_prechangelower_sanity_check(struct net_device *new_lower, return NOTIFY_DONE; } +static void dsa_tree_migrate_ports_from_lag_master(struct dsa_switch_tree *dst, + struct net_device *lag_dev) +{ + struct net_device *new_master = dsa_tree_find_first_master(dst); + struct dsa_port *dp; + int err; + + dsa_tree_for_each_user_port(dp, dst) { + if (dsa_port_to_master(dp) != lag_dev) + continue; + + err = dsa_slave_change_master(dp->slave, new_master, NULL); + if (err) { + netdev_err(dp->slave, + "failed to restore master to %s: %pe\n", + new_master->name, ERR_PTR(err)); + } + } +} + +static int dsa_master_lag_join(struct net_device *master, + struct net_device *lag_dev, + struct netdev_lag_upper_info *uinfo, + struct netlink_ext_ack *extack) +{ + struct dsa_port *cpu_dp = master->dsa_ptr; + struct dsa_switch_tree *dst = cpu_dp->dst; + struct dsa_port *dp; + int err; + + err = dsa_master_lag_setup(lag_dev, cpu_dp, uinfo, extack); + if (err) + return err; + + dsa_tree_for_each_user_port(dp, dst) { + if (dsa_port_to_master(dp) != master) + continue; + + err = dsa_slave_change_master(dp->slave, lag_dev, extack); + if (err) + goto restore; + } + + return 0; + +restore: + dsa_tree_for_each_user_port_continue_reverse(dp, dst) { + if (dsa_port_to_master(dp) != lag_dev) + continue; + + err = dsa_slave_change_master(dp->slave, master, NULL); + if (err) { + netdev_err(dp->slave, + "failed to restore master to %s: %pe\n", + master->name, ERR_PTR(err)); + } + } + + dsa_master_lag_teardown(lag_dev, master->dsa_ptr); + + return err; +} + +static void dsa_master_lag_leave(struct net_device *master, + struct net_device *lag_dev) +{ + struct dsa_port *dp, *cpu_dp = lag_dev->dsa_ptr; + struct dsa_switch_tree *dst = cpu_dp->dst; + struct dsa_port *new_cpu_dp = NULL; + struct net_device *lower; + struct list_head *iter; + + netdev_for_each_lower_dev(lag_dev, lower, iter) { + if (netdev_uses_dsa(lower)) { + new_cpu_dp = lower->dsa_ptr; + break; + } + } + + if (new_cpu_dp) { + /* Update the CPU port of the user ports still under the LAG + * so that dsa_port_to_master() continues to work properly + */ + dsa_tree_for_each_user_port(dp, dst) + if (dsa_port_to_master(dp) == lag_dev) + dp->cpu_dp = new_cpu_dp; + + /* Update the index of the virtual CPU port to match the lowest + * physical CPU port + */ + lag_dev->dsa_ptr = new_cpu_dp; + wmb(); + } else { + /* If the LAG DSA master has no ports left, migrate back all + * user ports to the first physical CPU port + */ + dsa_tree_migrate_ports_from_lag_master(dst, lag_dev); + } + + /* This DSA master has left its LAG in any case, so let + * the CPU port leave the hardware LAG as well + */ + dsa_master_lag_teardown(lag_dev, master->dsa_ptr); +} + +static int dsa_master_changeupper(struct net_device *dev, + struct netdev_notifier_changeupper_info *info) +{ + struct netlink_ext_ack *extack; + int err = NOTIFY_DONE; + + if (!netdev_uses_dsa(dev)) + return err; + + extack = netdev_notifier_info_to_extack(&info->info); + + if (netif_is_lag_master(info->upper_dev)) { + if (info->linking) { + err = dsa_master_lag_join(dev, info->upper_dev, + info->upper_info, extack); + err = notifier_from_errno(err); + } else { + dsa_master_lag_leave(dev, info->upper_dev); + err = NOTIFY_OK; + } + } + + return err; +} + static int dsa_slave_netdevice_event(struct notifier_block *nb, unsigned long event, void *ptr) { @@ -2905,6 +3107,10 @@ static int dsa_slave_netdevice_event(struct notifier_block *nb, if (notifier_to_errno(err)) return err; + err = dsa_lag_master_prechangelower_sanity_check(dev, info); + if (notifier_to_errno(err)) + return err; + err = dsa_bridge_prechangelower_sanity_check(dev, info); if (notifier_to_errno(err)) return err; @@ -2930,6 +3136,10 @@ static int dsa_slave_netdevice_event(struct notifier_block *nb, if (notifier_to_errno(err)) return err; + err = dsa_master_changeupper(dev, ptr); + if (notifier_to_errno(err)) + return err; + break; } case NETDEV_CHANGELOWERSTATE: { @@ -2937,12 +3147,21 @@ static int dsa_slave_netdevice_event(struct notifier_block *nb, struct dsa_port *dp; int err; - if (!dsa_slave_dev_check(dev)) - break; + if (dsa_slave_dev_check(dev)) { + dp = dsa_slave_to_port(dev); + + err = dsa_port_lag_change(dp, info->lower_state_info); + } - dp = dsa_slave_to_port(dev); + /* Mirror LAG port events on DSA masters that are in + * a LAG towards their respective switch CPU ports + */ + if (netdev_uses_dsa(dev)) { + dp = dev->dsa_ptr; + + err = dsa_port_lag_change(dp, info->lower_state_info); + } - err = dsa_port_lag_change(dp, info->lower_state_info); return notifier_from_errno(err); } case NETDEV_CHANGE: diff --git a/net/dsa/switch.c b/net/dsa/switch.c index c2cb15e21324..ce56acdba203 100644 --- a/net/dsa/switch.c +++ b/net/dsa/switch.c @@ -398,8 +398,15 @@ static int dsa_switch_host_fdb_add(struct dsa_switch *ds, dsa_switch_for_each_port(dp, ds) { if (dsa_port_host_address_match(dp, info->dp)) { - err = dsa_port_do_fdb_add(dp, info->addr, info->vid, - info->db); + if (dsa_port_is_cpu(dp) && info->dp->cpu_port_in_lag) { + err = dsa_switch_do_lag_fdb_add(ds, dp->lag, + info->addr, + info->vid, + info->db); + } else { + err = dsa_port_do_fdb_add(dp, info->addr, + info->vid, info->db); + } if (err) break; } @@ -419,8 +426,15 @@ static int dsa_switch_host_fdb_del(struct dsa_switch *ds, dsa_switch_for_each_port(dp, ds) { if (dsa_port_host_address_match(dp, info->dp)) { - err = dsa_port_do_fdb_del(dp, info->addr, info->vid, - info->db); + if (dsa_port_is_cpu(dp) && info->dp->cpu_port_in_lag) { + err = dsa_switch_do_lag_fdb_del(ds, dp->lag, + info->addr, + info->vid, + info->db); + } else { + err = dsa_port_do_fdb_del(dp, info->addr, + info->vid, info->db); + } if (err) break; } -- cgit v1.2.3 From 56378f3ccb83328b9ea6e4c1bccd4ea6055763ab Mon Sep 17 00:00:00 2001 From: Vladimir Oltean Date: Wed, 21 Sep 2022 21:54:28 +0300 Subject: net: dsa: make user ports return to init_net on netns deletion As pointed out during review, currently the following set of commands crashes the kernel: $ ip netns add ns0 $ ip link set swp0 netns ns0 $ ip netns del ns0 WARNING: CPU: 1 PID: 27 at net/core/dev.c:10884 unregister_netdevice_many+0xaa4/0xaec Workqueue: netns cleanup_net pstate: 20000005 (nzCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--) pc : unregister_netdevice_many+0xaa4/0xaec lr : unregister_netdevice_many+0x700/0xaec Call trace: unregister_netdevice_many+0xaa4/0xaec default_device_exit_batch+0x294/0x340 ops_exit_list+0xac/0xc4 cleanup_net+0x2e4/0x544 process_one_work+0x4ec/0xb40 ---[ end trace 0000000000000000 ]--- unregister_netdevice: waiting for swp0 to become free. Usage count = 2 This is because since DSA user ports, since they started populating dev->rtnl_link_ops in the blamed commit, gained a different treatment from default_device_exit_net(), which thinks these interfaces can now be unregistered. They can't; so set netns_refund = true to restore the behavior prior to populating dev->rtnl_link_ops. Fixes: 95f510d0b792 ("net: dsa: allow the DSA master to be seen and changed through rtnetlink") Suggested-by: Jakub Kicinski Signed-off-by: Vladimir Oltean Link: https://lore.kernel.org/r/20220921185428.1767001-1-vladimir.oltean@nxp.com Signed-off-by: Jakub Kicinski --- net/dsa/netlink.c | 1 + 1 file changed, 1 insertion(+) (limited to 'net/dsa') diff --git a/net/dsa/netlink.c b/net/dsa/netlink.c index 0f43bbb94769..ecf9ed1de185 100644 --- a/net/dsa/netlink.c +++ b/net/dsa/netlink.c @@ -59,4 +59,5 @@ struct rtnl_link_ops dsa_link_ops __read_mostly = { .changelink = dsa_changelink, .get_size = dsa_get_size, .fill_info = dsa_fill_info, + .netns_refund = true, }; -- cgit v1.2.3 From d82acd85cc41a8e5d5e0e4c2a3f4b645def29723 Mon Sep 17 00:00:00 2001 From: Jiri Pirko Date: Thu, 29 Sep 2022 09:28:59 +0200 Subject: net: dsa: move port_setup/teardown to be called outside devlink port registered area Move port_setup() op to be called before devlink_port_register() and port_teardown() after devlink_port_unregister(). Note it makes sense to move this alongside the rest of the devlink port code, the reinit() function also gets much nicer, as clearly the fact that port_setup()->devlink_port_region_create() was called in dsa_port_setup did not fit the flow. Signed-off-by: Jiri Pirko Signed-off-by: Jakub Kicinski --- net/dsa/dsa2.c | 68 ++++++++++++++++++++++------------------------------------ 1 file changed, 26 insertions(+), 42 deletions(-) (limited to 'net/dsa') diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c index 7024e2120de1..6f555b1bb483 100644 --- a/net/dsa/dsa2.c +++ b/net/dsa/dsa2.c @@ -472,12 +472,6 @@ static int dsa_port_setup(struct dsa_port *dp) if (dp->setup) return 0; - if (ds->ops->port_setup) { - err = ds->ops->port_setup(ds, dp->index); - if (err) - return err; - } - switch (dp->type) { case DSA_PORT_TYPE_UNUSED: dsa_port_disable(dp); @@ -532,11 +526,8 @@ static int dsa_port_setup(struct dsa_port *dp) dsa_port_disable(dp); if (err && dsa_port_link_registered) dsa_shared_port_link_unregister_of(dp); - if (err) { - if (ds->ops->port_teardown) - ds->ops->port_teardown(ds, dp->index); + if (err) return err; - } dp->setup = true; @@ -549,17 +540,26 @@ static int dsa_port_devlink_setup(struct dsa_port *dp) struct dsa_switch_tree *dst = dp->ds->dst; struct devlink_port_attrs attrs = {}; struct devlink *dl = dp->ds->devlink; + struct dsa_switch *ds = dp->ds; const unsigned char *id; unsigned char len; int err; + memset(dlp, 0, sizeof(*dlp)); + devlink_port_init(dl, dlp); + + if (ds->ops->port_setup) { + err = ds->ops->port_setup(ds, dp->index); + if (err) + return err; + } + id = (const unsigned char *)&dst->index; len = sizeof(dst->index); attrs.phys.port_number = dp->index; memcpy(attrs.switch_id.id, id, len); attrs.switch_id.id_len = len; - memset(dlp, 0, sizeof(*dlp)); switch (dp->type) { case DSA_PORT_TYPE_UNUSED: @@ -578,24 +578,23 @@ static int dsa_port_devlink_setup(struct dsa_port *dp) devlink_port_attrs_set(dlp, &attrs); err = devlink_port_register(dl, dlp, dp->index); + if (err) { + if (ds->ops->port_teardown) + ds->ops->port_teardown(ds, dp->index); + return err; + } + dp->devlink_port_setup = true; - if (!err) - dp->devlink_port_setup = true; - - return err; + return 0; } static void dsa_port_teardown(struct dsa_port *dp) { struct devlink_port *dlp = &dp->devlink_port; - struct dsa_switch *ds = dp->ds; if (!dp->setup) return; - if (ds->ops->port_teardown) - ds->ops->port_teardown(ds, dp->index); - devlink_port_type_clear(dlp); switch (dp->type) { @@ -625,40 +624,25 @@ static void dsa_port_teardown(struct dsa_port *dp) static void dsa_port_devlink_teardown(struct dsa_port *dp) { struct devlink_port *dlp = &dp->devlink_port; + struct dsa_switch *ds = dp->ds; - if (dp->devlink_port_setup) + if (dp->devlink_port_setup) { devlink_port_unregister(dlp); + if (ds->ops->port_teardown) + ds->ops->port_teardown(ds, dp->index); + devlink_port_fini(dlp); + } dp->devlink_port_setup = false; } /* Destroy the current devlink port, and create a new one which has the UNUSED - * flavour. At this point, any call to ds->ops->port_setup has been already - * balanced out by a call to ds->ops->port_teardown, so we know that any - * devlink port regions the driver had are now unregistered. We then call its - * ds->ops->port_setup again, in order for the driver to re-create them on the - * new devlink port. + * flavour. */ static int dsa_port_reinit_as_unused(struct dsa_port *dp) { - struct dsa_switch *ds = dp->ds; - int err; - dsa_port_devlink_teardown(dp); dp->type = DSA_PORT_TYPE_UNUSED; - err = dsa_port_devlink_setup(dp); - if (err) - return err; - - if (ds->ops->port_setup) { - /* On error, leave the devlink port registered, - * dsa_switch_teardown will clean it up later. - */ - err = ds->ops->port_setup(ds, dp->index); - if (err) - return err; - } - - return 0; + return dsa_port_devlink_setup(dp); } static int dsa_devlink_info_get(struct devlink *dl, -- cgit v1.2.3 From cf5ca4ddc37a693b17fdb653cb84b920b1185d71 Mon Sep 17 00:00:00 2001 From: Vladimir Oltean Date: Thu, 29 Sep 2022 09:29:00 +0200 Subject: 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 Signed-off-by: Jiri Pirko Signed-off-by: Jakub Kicinski --- net/dsa/dsa_priv.h | 1 + net/dsa/port.c | 22 +++++++++++++++------- net/dsa/slave.c | 6 +++--- 3 files changed, 19 insertions(+), 10 deletions(-) (limited to 'net/dsa') diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h index 129e4a649c7e..6e65c7ffd6f3 100644 --- a/net/dsa/dsa_priv.h +++ b/net/dsa/dsa_priv.h @@ -294,6 +294,7 @@ int dsa_port_mrp_add_ring_role(const struct dsa_port *dp, int dsa_port_mrp_del_ring_role(const struct dsa_port *dp, const struct switchdev_obj_ring_role_mrp *mrp); int dsa_port_phylink_create(struct dsa_port *dp); +void dsa_port_phylink_destroy(struct dsa_port *dp); int dsa_shared_port_link_register_of(struct dsa_port *dp); void dsa_shared_port_link_unregister_of(struct dsa_port *dp); int dsa_port_hsr_join(struct dsa_port *dp, struct net_device *hsr); diff --git a/net/dsa/port.c b/net/dsa/port.c index e6289a1db0a0..e4a0513816bb 100644 --- a/net/dsa/port.c +++ b/net/dsa/port.c @@ -1661,6 +1661,7 @@ int dsa_port_phylink_create(struct dsa_port *dp) { struct dsa_switch *ds = dp->ds; phy_interface_t mode; + struct phylink *pl; int err; err = of_get_phy_mode(dp->dn, &mode); @@ -1677,16 +1678,24 @@ int dsa_port_phylink_create(struct dsa_port *dp) if (ds->ops->phylink_get_caps) ds->ops->phylink_get_caps(ds, dp->index, &dp->pl_config); - dp->pl = phylink_create(&dp->pl_config, of_fwnode_handle(dp->dn), - mode, &dsa_port_phylink_mac_ops); - if (IS_ERR(dp->pl)) { + pl = phylink_create(&dp->pl_config, of_fwnode_handle(dp->dn), + mode, &dsa_port_phylink_mac_ops); + if (IS_ERR(pl)) { pr_err("error creating PHYLINK: %ld\n", PTR_ERR(dp->pl)); - return PTR_ERR(dp->pl); + return PTR_ERR(pl); } + dp->pl = pl; + return 0; } +void dsa_port_phylink_destroy(struct dsa_port *dp) +{ + phylink_destroy(dp->pl); + dp->pl = NULL; +} + static int dsa_shared_port_setup_phy_of(struct dsa_port *dp, bool enable) { struct dsa_switch *ds = dp->ds; @@ -1781,7 +1790,7 @@ static int dsa_shared_port_phylink_register(struct dsa_port *dp) return 0; err_phy_connect: - phylink_destroy(dp->pl); + dsa_port_phylink_destroy(dp); return err; } @@ -1983,8 +1992,7 @@ void dsa_shared_port_link_unregister_of(struct dsa_port *dp) rtnl_lock(); phylink_disconnect_phy(dp->pl); rtnl_unlock(); - phylink_destroy(dp->pl); - dp->pl = NULL; + dsa_port_phylink_destroy(dp); return; } 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); -- cgit v1.2.3 From c698a5fbf7fd9f5bb909d09626319b59d55db36b Mon Sep 17 00:00:00 2001 From: Jiri Pirko Date: Thu, 29 Sep 2022 09:29:01 +0200 Subject: net: dsa: don't do devlink port setup early Commit 3122433eb533 ("net: dsa: Register devlink ports before calling DSA driver setup()") moved devlink port setup to be done early before driver setup() was called. That is no longer needed, so move the devlink port initialization back to dsa_port_setup(), as the first thing done there. Note there is no longer needed to reinit port as unused if dsa_port_setup() fails, as it unregisters the devlink port instance on the error path. Signed-off-by: Jiri Pirko Signed-off-by: Jakub Kicinski --- net/dsa/dsa2.c | 176 +++++++++++++++++++++++++++------------------------------ 1 file changed, 82 insertions(+), 94 deletions(-) (limited to 'net/dsa') diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c index 6f555b1bb483..747c0364fb0f 100644 --- a/net/dsa/dsa2.c +++ b/net/dsa/dsa2.c @@ -461,6 +461,74 @@ static void dsa_tree_teardown_cpu_ports(struct dsa_switch_tree *dst) dp->cpu_dp = NULL; } +static int dsa_port_devlink_setup(struct dsa_port *dp) +{ + struct devlink_port *dlp = &dp->devlink_port; + struct dsa_switch_tree *dst = dp->ds->dst; + struct devlink_port_attrs attrs = {}; + struct devlink *dl = dp->ds->devlink; + struct dsa_switch *ds = dp->ds; + const unsigned char *id; + unsigned char len; + int err; + + memset(dlp, 0, sizeof(*dlp)); + devlink_port_init(dl, dlp); + + if (ds->ops->port_setup) { + err = ds->ops->port_setup(ds, dp->index); + if (err) + return err; + } + + id = (const unsigned char *)&dst->index; + len = sizeof(dst->index); + + attrs.phys.port_number = dp->index; + memcpy(attrs.switch_id.id, id, len); + attrs.switch_id.id_len = len; + + switch (dp->type) { + case DSA_PORT_TYPE_UNUSED: + attrs.flavour = DEVLINK_PORT_FLAVOUR_UNUSED; + break; + case DSA_PORT_TYPE_CPU: + attrs.flavour = DEVLINK_PORT_FLAVOUR_CPU; + break; + case DSA_PORT_TYPE_DSA: + attrs.flavour = DEVLINK_PORT_FLAVOUR_DSA; + break; + case DSA_PORT_TYPE_USER: + attrs.flavour = DEVLINK_PORT_FLAVOUR_PHYSICAL; + break; + } + + devlink_port_attrs_set(dlp, &attrs); + err = devlink_port_register(dl, dlp, dp->index); + if (err) { + if (ds->ops->port_teardown) + ds->ops->port_teardown(ds, dp->index); + return err; + } + dp->devlink_port_setup = true; + + return 0; +} + +static void dsa_port_devlink_teardown(struct dsa_port *dp) +{ + struct devlink_port *dlp = &dp->devlink_port; + struct dsa_switch *ds = dp->ds; + + if (dp->devlink_port_setup) { + devlink_port_unregister(dlp); + if (ds->ops->port_teardown) + ds->ops->port_teardown(ds, dp->index); + devlink_port_fini(dlp); + } + dp->devlink_port_setup = false; +} + static int dsa_port_setup(struct dsa_port *dp) { struct devlink_port *dlp = &dp->devlink_port; @@ -472,6 +540,10 @@ static int dsa_port_setup(struct dsa_port *dp) if (dp->setup) return 0; + err = dsa_port_devlink_setup(dp); + if (err) + return err; + switch (dp->type) { case DSA_PORT_TYPE_UNUSED: dsa_port_disable(dp); @@ -526,64 +598,12 @@ static int dsa_port_setup(struct dsa_port *dp) dsa_port_disable(dp); if (err && dsa_port_link_registered) dsa_shared_port_link_unregister_of(dp); - if (err) - return err; - - dp->setup = true; - - return 0; -} - -static int dsa_port_devlink_setup(struct dsa_port *dp) -{ - struct devlink_port *dlp = &dp->devlink_port; - struct dsa_switch_tree *dst = dp->ds->dst; - struct devlink_port_attrs attrs = {}; - struct devlink *dl = dp->ds->devlink; - struct dsa_switch *ds = dp->ds; - const unsigned char *id; - unsigned char len; - int err; - - memset(dlp, 0, sizeof(*dlp)); - devlink_port_init(dl, dlp); - - if (ds->ops->port_setup) { - err = ds->ops->port_setup(ds, dp->index); - if (err) - return err; - } - - id = (const unsigned char *)&dst->index; - len = sizeof(dst->index); - - attrs.phys.port_number = dp->index; - memcpy(attrs.switch_id.id, id, len); - attrs.switch_id.id_len = len; - - switch (dp->type) { - case DSA_PORT_TYPE_UNUSED: - attrs.flavour = DEVLINK_PORT_FLAVOUR_UNUSED; - break; - case DSA_PORT_TYPE_CPU: - attrs.flavour = DEVLINK_PORT_FLAVOUR_CPU; - break; - case DSA_PORT_TYPE_DSA: - attrs.flavour = DEVLINK_PORT_FLAVOUR_DSA; - break; - case DSA_PORT_TYPE_USER: - attrs.flavour = DEVLINK_PORT_FLAVOUR_PHYSICAL; - break; - } - - devlink_port_attrs_set(dlp, &attrs); - err = devlink_port_register(dl, dlp, dp->index); if (err) { - if (ds->ops->port_teardown) - ds->ops->port_teardown(ds, dp->index); + dsa_port_devlink_teardown(dp); return err; } - dp->devlink_port_setup = true; + + dp->setup = true; return 0; } @@ -618,31 +638,15 @@ static void dsa_port_teardown(struct dsa_port *dp) break; } - dp->setup = false; -} - -static void dsa_port_devlink_teardown(struct dsa_port *dp) -{ - struct devlink_port *dlp = &dp->devlink_port; - struct dsa_switch *ds = dp->ds; + dsa_port_devlink_teardown(dp); - if (dp->devlink_port_setup) { - devlink_port_unregister(dlp); - if (ds->ops->port_teardown) - ds->ops->port_teardown(ds, dp->index); - devlink_port_fini(dlp); - } - dp->devlink_port_setup = false; + dp->setup = false; } -/* 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); } static int dsa_devlink_info_get(struct devlink *dl, @@ -866,7 +870,6 @@ static int dsa_switch_setup(struct dsa_switch *ds) { struct dsa_devlink_priv *dl_priv; struct device_node *dn; - struct dsa_port *dp; int err; if (ds->setup) @@ -889,18 +892,9 @@ static int dsa_switch_setup(struct dsa_switch *ds) dl_priv = devlink_priv(ds->devlink); dl_priv->ds = ds; - /* Setup devlink port instances now, so that the switch - * setup() can register regions etc, against the ports - */ - dsa_switch_for_each_port(dp, ds) { - err = dsa_port_devlink_setup(dp); - if (err) - goto unregister_devlink_ports; - } - err = dsa_switch_register_notifier(ds); if (err) - goto unregister_devlink_ports; + goto devlink_free; ds->configure_vlan_while_not_filtering = true; @@ -941,9 +935,7 @@ teardown: ds->ops->teardown(ds); unregister_notifier: dsa_switch_unregister_notifier(ds); -unregister_devlink_ports: - dsa_switch_for_each_port(dp, ds) - dsa_port_devlink_teardown(dp); +devlink_free: devlink_free(ds->devlink); ds->devlink = NULL; return err; @@ -951,8 +943,6 @@ unregister_devlink_ports: static void dsa_switch_teardown(struct dsa_switch *ds) { - struct dsa_port *dp; - if (!ds->setup) return; @@ -971,8 +961,6 @@ static void dsa_switch_teardown(struct dsa_switch *ds) dsa_switch_unregister_notifier(ds); if (ds->devlink) { - dsa_switch_for_each_port(dp, ds) - dsa_port_devlink_teardown(dp); devlink_free(ds->devlink); ds->devlink = NULL; } @@ -1025,7 +1013,7 @@ static int dsa_tree_setup_ports(struct dsa_switch_tree *dst) if (dsa_port_is_user(dp) || dsa_port_is_unused(dp)) { err = dsa_port_setup(dp); if (err) { - err = dsa_port_reinit_as_unused(dp); + err = dsa_port_setup_as_unused(dp); if (err) goto teardown; } -- cgit v1.2.3 From 61e4a51621587c939672d6a9354f6d0aa3d4e131 Mon Sep 17 00:00:00 2001 From: Vladimir Oltean Date: Thu, 29 Sep 2022 09:29:02 +0200 Subject: net: dsa: remove bool devlink_port_setup Since dsa_port_devlink_setup() and dsa_port_devlink_teardown() are already called from code paths which only execute once per port (due to the existing bool dp->setup), keeping another dp->devlink_port_setup is redundant, because we can already manage to balance the calls properly (and not call teardown when setup was never called, or call setup twice, or things like that). Signed-off-by: Vladimir Oltean Signed-off-by: Jiri Pirko Signed-off-by: Jakub Kicinski --- include/net/dsa.h | 2 -- net/dsa/dsa2.c | 14 ++++++-------- 2 files changed, 6 insertions(+), 10 deletions(-) (limited to 'net/dsa') diff --git a/include/net/dsa.h b/include/net/dsa.h index d777eac5694f..ee369670e20e 100644 --- a/include/net/dsa.h +++ b/include/net/dsa.h @@ -294,8 +294,6 @@ struct dsa_port { u8 lag_tx_enabled:1; - u8 devlink_port_setup:1; - /* Master state bits, valid only on CPU ports */ u8 master_admin_up:1; u8 master_oper_up:1; diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c index 747c0364fb0f..af0e2c0394ac 100644 --- a/net/dsa/dsa2.c +++ b/net/dsa/dsa2.c @@ -510,7 +510,6 @@ static int dsa_port_devlink_setup(struct dsa_port *dp) ds->ops->port_teardown(ds, dp->index); return err; } - dp->devlink_port_setup = true; return 0; } @@ -520,13 +519,12 @@ static void dsa_port_devlink_teardown(struct dsa_port *dp) struct devlink_port *dlp = &dp->devlink_port; struct dsa_switch *ds = dp->ds; - if (dp->devlink_port_setup) { - devlink_port_unregister(dlp); - if (ds->ops->port_teardown) - ds->ops->port_teardown(ds, dp->index); - devlink_port_fini(dlp); - } - dp->devlink_port_setup = false; + devlink_port_unregister(dlp); + + if (ds->ops->port_teardown) + ds->ops->port_teardown(ds, dp->index); + + devlink_port_fini(dlp); } static int dsa_port_setup(struct dsa_port *dp) -- cgit v1.2.3