[RFC PATCH net-next 0/3] Autoload DSA tagging driver when dynamically changing protocol

[RFC PATCH net-next 0/3] Autoload DSA tagging driver when dynamically changing protocol

[Vladimir Oltean]

This patch set solves the issue reported by Michael and Heiko here:
https://lore.kernel.org/lkml/20221027113248.420216-1-michael@walle.cc/
making full use of Michael's suggestion of having two modaliases: one
gets used for loading the tagging protocol when it's the default one
reported by the switch driver, the other gets loaded at user's request,
by name.

  # modinfo tag_ocelot_8021q
  filename:       /lib/modules/6.1.0-rc2+/kernel/net/dsa/tag_ocelot_8021q.ko
  alias:          dsa_tag-ocelot-8021q
  alias:          dsa_tag-20
  license:        GPL v2
  depends:        dsa_core
  intree:         Y
  name:           tag_ocelot_8021q
  vermagic:       6.1.0-rc2+ SMP preempt mod_unload modversions aarch64

Tested on NXP LS1028A-RDB with the following device tree addition:

&mscc_felix_port4 {
	dsa-tag-protocol = "ocelot-8021q";
};

&mscc_felix_port5 {
	dsa-tag-protocol = "ocelot-8021q";
};

CONFIG_NET_DSA and everything that depends on it is built as module.
Everything auto-loads, and "cat /sys/class/net/eno2/dsa/tagging" shows
"ocelot-8021q". Traffic works as well.

Note: I included patch 1/3 because I secretly want to see if the
patchwork build tests pass :) But I also submitted it separately to
"net" already, and without it, patch 3/3 doesn't apply to current net-next.
So if you want to leave comments on 1/3, make sure to leave them here:
https://patchwork.kernel.org/project/netdevbpf/patch/20221027145439.3086017-1-vladimir.oltean@nxp.com/

Vladimir Oltean (3):
  net: dsa: fall back to default tagger if we can't load the one from DT
  net: dsa: provide a second modalias to tag proto drivers based on
    their name
  net: dsa: autoload tag driver module on tagging protocol change

 include/net/dsa.h          |  5 +++--
 net/dsa/dsa.c              |  8 +++++---
 net/dsa/dsa2.c             | 15 +++++++++++----
 net/dsa/dsa_priv.h         |  4 ++--
 net/dsa/master.c           |  4 ++--
 net/dsa/tag_ar9331.c       |  6 ++++--
 net/dsa/tag_brcm.c         | 16 ++++++++++------
 net/dsa/tag_dsa.c          | 11 +++++++----
 net/dsa/tag_gswip.c        |  6 ++++--
 net/dsa/tag_hellcreek.c    |  6 ++++--
 net/dsa/tag_ksz.c          | 21 +++++++++++++--------
 net/dsa/tag_lan9303.c      |  6 ++++--
 net/dsa/tag_mtk.c          |  6 ++++--
 net/dsa/tag_ocelot.c       | 11 +++++++----
 net/dsa/tag_ocelot_8021q.c |  6 ++++--
 net/dsa/tag_qca.c          |  6 ++++--
 net/dsa/tag_rtl4_a.c       |  6 ++++--
 net/dsa/tag_rtl8_4.c       |  7 +++++--
 net/dsa/tag_rzn1_a5psw.c   |  6 ++++--
 net/dsa/tag_sja1105.c      | 11 +++++++----
 net/dsa/tag_trailer.c      |  6 ++++--
 net/dsa/tag_xrs700x.c      |  6 ++++--
 22 files changed, 116 insertions(+), 63 deletions(-)

-- 
2.34.1

[RFC PATCH net-next 1/3] net: dsa: fall back to default tagger if we can't load the one from DT

[Vladimir Oltean]

DSA tagging protocol drivers can be changed at runtime through sysfs and
at probe time through the device tree (support for the latter was added
later).

When changing through sysfs, it is assumed that the module for the new
tagging protocol was already loaded into the kernel (in fact this is
only a concern for Ocelot/Felix switches, where we have tag_ocelot.ko
and tag_ocelot_8021q.ko; for every other switch, the default and
alternative protocols are compiled within the same .ko, so there is
nothing for the user to load).

The kernel cannot currently call request_module(), because it has no way
of constructing the modalias name of the tagging protocol driver
("dsa_tag-%d", where the number is one of DSA_TAG_PROTO_*_VALUE).
The device tree only contains the string name of the tagging protocol
("ocelot-8021q"), and the only mapping between the string and the
DSA_TAG_PROTO_OCELOT_8021Q_VALUE is present in tag_ocelot_8021q.ko.
So this is a chicken-and-egg situation and dsa_core.ko has nothing based
on which it can automatically request the insertion of the module.

As a consequence, if CONFIG_NET_DSA_TAG_OCELOT_8021Q is built as module,
the switch will forever defer probing.

The long-term solution is to make DSA call request_module() somehow,
but that probably needs some refactoring.

What we can do to keep operating with existing device tree blobs is to
cancel the attempt to change the tagging protocol with the one specified
there, and to remain operating with the default one. Depending on the
situation, the default protocol might still allow some functionality
(in the case of ocelot, it does), and it's better to have that than to
fail to probe.

Fixes: deff710703d8 ("net: dsa: Allow default tag protocol to be overridden from DT")
Link: https://lore.kernel.org/lkml/20221027113248.420216-1-michael@walle.cc/
Reported-by: Heiko Thiery <heiko.thiery@gmail.com>
Reported-by: Michael Walle <michael@walle.cc>
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 net/dsa/dsa2.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index af0e2c0394ac..e504a18fc125 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -1409,9 +1409,9 @@ static enum dsa_tag_protocol dsa_get_tag_protocol(struct dsa_port *dp,
 static int dsa_port_parse_cpu(struct dsa_port *dp, struct net_device *master,
 			      const char *user_protocol)
 {
+	const struct dsa_device_ops *tag_ops = NULL;
 	struct dsa_switch *ds = dp->ds;
 	struct dsa_switch_tree *dst = ds->dst;
-	const struct dsa_device_ops *tag_ops;
 	enum dsa_tag_protocol default_proto;
 
 	/* Find out which protocol the switch would prefer. */
@@ -1434,10 +1434,17 @@ static int dsa_port_parse_cpu(struct dsa_port *dp, struct net_device *master,
 		}
 
 		tag_ops = dsa_find_tagger_by_name(user_protocol);
-	} else {
-		tag_ops = dsa_tag_driver_get(default_proto);
+		if (IS_ERR(tag_ops)) {
+			dev_warn(ds->dev,
+				 "Failed to find a tagging driver for protocol %s, using default\n",
+				 user_protocol);
+			tag_ops = NULL;
+		}
 	}
 
+	if (!tag_ops)
+		tag_ops = dsa_tag_driver_get(default_proto);
+
 	if (IS_ERR(tag_ops)) {
 		if (PTR_ERR(tag_ops) == -ENOPROTOOPT)
 			return -EPROBE_DEFER;
-- 
2.34.1

[RFC PATCH net-next 2/3] net: dsa: provide a second modalias to tag proto drivers based on their name

[Vladimir Oltean]

Currently, tagging protocol drivers have a modalias of "dsa_tag-<number>",
where the number is one of DSA_TAG_PROTO_*_VALUE.

This modalias makes it possible for the request_module() call in
dsa_tag_driver_get() to work, given the input it has - an integer
returned by ds->ops->get_tag_protocol().

It is also possible to change tagging protocols at (pseudo-)runtime, via
sysfs or via device tree, and this works via the name string of the
tagging protocol rather than via its id (DSA_TAG_PROTO_*_VALUE).

In the latter case, there is no request_module() call, because there is
no association that the DSA core has between the string name and the ID,
to construct the modalias. The module is simply assumed to have been
inserted. This is actually slightly problematic when the tagging
protocol change should take place at probe time, since it's expected
that the dependency module should get autoloaded.

For this purpose, let's introduce a second modalias, so that the DSA
core can call request_module() by name. There is no reason to make the
modalias by name optional, so just modify the MODULE_ALIAS_DSA_TAG_DRIVER()
macro to take both the ID and the name as arguments, and generate two
modaliases behind the scenes.

Suggested-by: Michael Walle <michael@walle.cc>
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 include/net/dsa.h          |  5 +++--
 net/dsa/tag_ar9331.c       |  6 ++++--
 net/dsa/tag_brcm.c         | 16 ++++++++++------
 net/dsa/tag_dsa.c          | 11 +++++++----
 net/dsa/tag_gswip.c        |  6 ++++--
 net/dsa/tag_hellcreek.c    |  6 ++++--
 net/dsa/tag_ksz.c          | 21 +++++++++++++--------
 net/dsa/tag_lan9303.c      |  6 ++++--
 net/dsa/tag_mtk.c          |  6 ++++--
 net/dsa/tag_ocelot.c       | 11 +++++++----
 net/dsa/tag_ocelot_8021q.c |  6 ++++--
 net/dsa/tag_qca.c          |  6 ++++--
 net/dsa/tag_rtl4_a.c       |  6 ++++--
 net/dsa/tag_rtl8_4.c       |  7 +++++--
 net/dsa/tag_rzn1_a5psw.c   |  6 ++++--
 net/dsa/tag_sja1105.c      | 11 +++++++----
 net/dsa/tag_trailer.c      |  6 ++++--
 net/dsa/tag_xrs700x.c      |  6 ++++--
 18 files changed, 96 insertions(+), 52 deletions(-)

diff --git a/include/net/dsa.h b/include/net/dsa.h
index ee369670e20e..001972bb38fe 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -119,8 +119,9 @@ struct dsa_netdevice_ops {
 };
 
 #define DSA_TAG_DRIVER_ALIAS "dsa_tag-"
-#define MODULE_ALIAS_DSA_TAG_DRIVER(__proto)				\
-	MODULE_ALIAS(DSA_TAG_DRIVER_ALIAS __stringify(__proto##_VALUE))
+#define MODULE_ALIAS_DSA_TAG_DRIVER(__proto, __name)			\
+	MODULE_ALIAS(DSA_TAG_DRIVER_ALIAS __stringify(__proto##_VALUE)); \
+	MODULE_ALIAS(DSA_TAG_DRIVER_ALIAS __name)
 
 struct dsa_lag {
 	struct net_device *dev;
diff --git a/net/dsa/tag_ar9331.c b/net/dsa/tag_ar9331.c
index 8a02ac44282f..bfa161a4f502 100644
--- a/net/dsa/tag_ar9331.c
+++ b/net/dsa/tag_ar9331.c
@@ -9,6 +9,8 @@
 
 #include "dsa_priv.h"
 
+#define AR9331_NAME			"ar9331"
+
 #define AR9331_HDR_LEN			2
 #define AR9331_HDR_VERSION		1
 
@@ -80,7 +82,7 @@ static struct sk_buff *ar9331_tag_rcv(struct sk_buff *skb,
 }
 
 static const struct dsa_device_ops ar9331_netdev_ops = {
-	.name	= "ar9331",
+	.name	= AR9331_NAME,
 	.proto	= DSA_TAG_PROTO_AR9331,
 	.xmit	= ar9331_tag_xmit,
 	.rcv	= ar9331_tag_rcv,
@@ -88,5 +90,5 @@ static const struct dsa_device_ops ar9331_netdev_ops = {
 };
 
 MODULE_LICENSE("GPL v2");
-MODULE_ALIAS_DSA_TAG_DRIVER(DSA_TAG_PROTO_AR9331);
+MODULE_ALIAS_DSA_TAG_DRIVER(DSA_TAG_PROTO_AR9331, AR9331_NAME);
 module_dsa_tag_driver(ar9331_netdev_ops);
diff --git a/net/dsa/tag_brcm.c b/net/dsa/tag_brcm.c
index 16889ea3e0a7..9e7477ed70f1 100644
--- a/net/dsa/tag_brcm.c
+++ b/net/dsa/tag_brcm.c
@@ -12,6 +12,10 @@
 
 #include "dsa_priv.h"
 
+#define BRCM_NAME		"brcm"
+#define BRCM_LEGACY_NAME	"brcm-legacy"
+#define BRCM_PREPEND_NAME	"brcm-prepend"
+
 /* Legacy Broadcom tag (6 bytes) */
 #define BRCM_LEG_TAG_LEN	6
 
@@ -196,7 +200,7 @@ static struct sk_buff *brcm_tag_rcv(struct sk_buff *skb, struct net_device *dev)
 }
 
 static const struct dsa_device_ops brcm_netdev_ops = {
-	.name	= "brcm",
+	.name	= BRCM_NAME,
 	.proto	= DSA_TAG_PROTO_BRCM,
 	.xmit	= brcm_tag_xmit,
 	.rcv	= brcm_tag_rcv,
@@ -204,7 +208,7 @@ static const struct dsa_device_ops brcm_netdev_ops = {
 };
 
 DSA_TAG_DRIVER(brcm_netdev_ops);
-MODULE_ALIAS_DSA_TAG_DRIVER(DSA_TAG_PROTO_BRCM);
+MODULE_ALIAS_DSA_TAG_DRIVER(DSA_TAG_PROTO_BRCM, BRCM_NAME);
 #endif
 
 #if IS_ENABLED(CONFIG_NET_DSA_TAG_BRCM_LEGACY)
@@ -273,7 +277,7 @@ static struct sk_buff *brcm_leg_tag_rcv(struct sk_buff *skb,
 }
 
 static const struct dsa_device_ops brcm_legacy_netdev_ops = {
-	.name = "brcm-legacy",
+	.name = BRCM_LEGACY_NAME,
 	.proto = DSA_TAG_PROTO_BRCM_LEGACY,
 	.xmit = brcm_leg_tag_xmit,
 	.rcv = brcm_leg_tag_rcv,
@@ -281,7 +285,7 @@ static const struct dsa_device_ops brcm_legacy_netdev_ops = {
 };
 
 DSA_TAG_DRIVER(brcm_legacy_netdev_ops);
-MODULE_ALIAS_DSA_TAG_DRIVER(DSA_TAG_PROTO_BRCM_LEGACY);
+MODULE_ALIAS_DSA_TAG_DRIVER(DSA_TAG_PROTO_BRCM_LEGACY, BRCM_LEGACY_NAME);
 #endif /* CONFIG_NET_DSA_TAG_BRCM_LEGACY */
 
 #if IS_ENABLED(CONFIG_NET_DSA_TAG_BRCM_PREPEND)
@@ -300,7 +304,7 @@ static struct sk_buff *brcm_tag_rcv_prepend(struct sk_buff *skb,
 }
 
 static const struct dsa_device_ops brcm_prepend_netdev_ops = {
-	.name	= "brcm-prepend",
+	.name	= BRCM_PREPEND_NAME,
 	.proto	= DSA_TAG_PROTO_BRCM_PREPEND,
 	.xmit	= brcm_tag_xmit_prepend,
 	.rcv	= brcm_tag_rcv_prepend,
@@ -308,7 +312,7 @@ static const struct dsa_device_ops brcm_prepend_netdev_ops = {
 };
 
 DSA_TAG_DRIVER(brcm_prepend_netdev_ops);
-MODULE_ALIAS_DSA_TAG_DRIVER(DSA_TAG_PROTO_BRCM_PREPEND);
+MODULE_ALIAS_DSA_TAG_DRIVER(DSA_TAG_PROTO_BRCM_PREPEND, BRCM_PREPEND_NAME);
 #endif
 
 static struct dsa_tag_driver *dsa_tag_driver_array[] =	{
diff --git a/net/dsa/tag_dsa.c b/net/dsa/tag_dsa.c
index e4b6e3f2a3db..9fe77f5cc759 100644
--- a/net/dsa/tag_dsa.c
+++ b/net/dsa/tag_dsa.c
@@ -52,6 +52,9 @@
 
 #include "dsa_priv.h"
 
+#define DSA_NAME	"dsa"
+#define EDSA_NAME	"edsa"
+
 #define DSA_HLEN	4
 
 /**
@@ -339,7 +342,7 @@ static struct sk_buff *dsa_rcv(struct sk_buff *skb, struct net_device *dev)
 }
 
 static const struct dsa_device_ops dsa_netdev_ops = {
-	.name	  = "dsa",
+	.name	  = DSA_NAME,
 	.proto	  = DSA_TAG_PROTO_DSA,
 	.xmit	  = dsa_xmit,
 	.rcv	  = dsa_rcv,
@@ -347,7 +350,7 @@ static const struct dsa_device_ops dsa_netdev_ops = {
 };
 
 DSA_TAG_DRIVER(dsa_netdev_ops);
-MODULE_ALIAS_DSA_TAG_DRIVER(DSA_TAG_PROTO_DSA);
+MODULE_ALIAS_DSA_TAG_DRIVER(DSA_TAG_PROTO_DSA, DSA_NAME);
 #endif	/* CONFIG_NET_DSA_TAG_DSA */
 
 #if IS_ENABLED(CONFIG_NET_DSA_TAG_EDSA)
@@ -381,7 +384,7 @@ static struct sk_buff *edsa_rcv(struct sk_buff *skb, struct net_device *dev)
 }
 
 static const struct dsa_device_ops edsa_netdev_ops = {
-	.name	  = "edsa",
+	.name	  = EDSA_NAME,
 	.proto	  = DSA_TAG_PROTO_EDSA,
 	.xmit	  = edsa_xmit,
 	.rcv	  = edsa_rcv,
@@ -389,7 +392,7 @@ static const struct dsa_device_ops edsa_netdev_ops = {
 };
 
 DSA_TAG_DRIVER(edsa_netdev_ops);
-MODULE_ALIAS_DSA_TAG_DRIVER(DSA_TAG_PROTO_EDSA);
+MODULE_ALIAS_DSA_TAG_DRIVER(DSA_TAG_PROTO_EDSA, EDSA_NAME);
 #endif	/* CONFIG_NET_DSA_TAG_EDSA */
 
 static struct dsa_tag_driver *dsa_tag_drivers[] = {
diff --git a/net/dsa/tag_gswip.c b/net/dsa/tag_gswip.c
index df7140984da3..020050dff3e4 100644
--- a/net/dsa/tag_gswip.c
+++ b/net/dsa/tag_gswip.c
@@ -12,6 +12,8 @@
 
 #include "dsa_priv.h"
 
+#define GSWIP_NAME			"gswip"
+
 #define GSWIP_TX_HEADER_LEN		4
 
 /* special tag in TX path header */
@@ -98,7 +100,7 @@ static struct sk_buff *gswip_tag_rcv(struct sk_buff *skb,
 }
 
 static const struct dsa_device_ops gswip_netdev_ops = {
-	.name = "gswip",
+	.name = GSWIP_NAME,
 	.proto	= DSA_TAG_PROTO_GSWIP,
 	.xmit = gswip_tag_xmit,
 	.rcv = gswip_tag_rcv,
@@ -106,6 +108,6 @@ static const struct dsa_device_ops gswip_netdev_ops = {
 };
 
 MODULE_LICENSE("GPL");
-MODULE_ALIAS_DSA_TAG_DRIVER(DSA_TAG_PROTO_GSWIP);
+MODULE_ALIAS_DSA_TAG_DRIVER(DSA_TAG_PROTO_GSWIP, GSWIP_NAME);
 
 module_dsa_tag_driver(gswip_netdev_ops);
diff --git a/net/dsa/tag_hellcreek.c b/net/dsa/tag_hellcreek.c
index 846588c0070a..03fd5f2877c8 100644
--- a/net/dsa/tag_hellcreek.c
+++ b/net/dsa/tag_hellcreek.c
@@ -13,6 +13,8 @@
 
 #include "dsa_priv.h"
 
+#define HELLCREEK_NAME		"hellcreek"
+
 #define HELLCREEK_TAG_LEN	1
 
 static struct sk_buff *hellcreek_xmit(struct sk_buff *skb,
@@ -57,7 +59,7 @@ static struct sk_buff *hellcreek_rcv(struct sk_buff *skb,
 }
 
 static const struct dsa_device_ops hellcreek_netdev_ops = {
-	.name	  = "hellcreek",
+	.name	  = HELLCREEK_NAME,
 	.proto	  = DSA_TAG_PROTO_HELLCREEK,
 	.xmit	  = hellcreek_xmit,
 	.rcv	  = hellcreek_rcv,
@@ -65,6 +67,6 @@ static const struct dsa_device_ops hellcreek_netdev_ops = {
 };
 
 MODULE_LICENSE("Dual MIT/GPL");
-MODULE_ALIAS_DSA_TAG_DRIVER(DSA_TAG_PROTO_HELLCREEK);
+MODULE_ALIAS_DSA_TAG_DRIVER(DSA_TAG_PROTO_HELLCREEK, HELLCREEK_NAME);
 
 module_dsa_tag_driver(hellcreek_netdev_ops);
diff --git a/net/dsa/tag_ksz.c b/net/dsa/tag_ksz.c
index 38fa19c1e2d5..37db5156f9a3 100644
--- a/net/dsa/tag_ksz.c
+++ b/net/dsa/tag_ksz.c
@@ -9,6 +9,11 @@
 #include <net/dsa.h>
 #include "dsa_priv.h"
 
+#define KSZ8795_NAME "ksz8795"
+#define KSZ9477_NAME "ksz9477"
+#define KSZ9893_NAME "ksz9893"
+#define LAN937X_NAME "lan937x"
+
 /* Typically only one byte is used for tail tag. */
 #define KSZ_EGRESS_TAG_LEN		1
 #define KSZ_INGRESS_TAG_LEN		1
@@ -74,7 +79,7 @@ static struct sk_buff *ksz8795_rcv(struct sk_buff *skb, struct net_device *dev)
 }
 
 static const struct dsa_device_ops ksz8795_netdev_ops = {
-	.name	= "ksz8795",
+	.name	= KSZ8795_NAME,
 	.proto	= DSA_TAG_PROTO_KSZ8795,
 	.xmit	= ksz8795_xmit,
 	.rcv	= ksz8795_rcv,
@@ -82,7 +87,7 @@ static const struct dsa_device_ops ksz8795_netdev_ops = {
 };
 
 DSA_TAG_DRIVER(ksz8795_netdev_ops);
-MODULE_ALIAS_DSA_TAG_DRIVER(DSA_TAG_PROTO_KSZ8795);
+MODULE_ALIAS_DSA_TAG_DRIVER(DSA_TAG_PROTO_KSZ8795, KSZ8795_NAME);
 
 /*
  * For Ingress (Host -> KSZ9477), 2 bytes are added before FCS.
@@ -147,7 +152,7 @@ static struct sk_buff *ksz9477_rcv(struct sk_buff *skb, struct net_device *dev)
 }
 
 static const struct dsa_device_ops ksz9477_netdev_ops = {
-	.name	= "ksz9477",
+	.name	= KSZ9477_NAME,
 	.proto	= DSA_TAG_PROTO_KSZ9477,
 	.xmit	= ksz9477_xmit,
 	.rcv	= ksz9477_rcv,
@@ -155,7 +160,7 @@ static const struct dsa_device_ops ksz9477_netdev_ops = {
 };
 
 DSA_TAG_DRIVER(ksz9477_netdev_ops);
-MODULE_ALIAS_DSA_TAG_DRIVER(DSA_TAG_PROTO_KSZ9477);
+MODULE_ALIAS_DSA_TAG_DRIVER(DSA_TAG_PROTO_KSZ9477, KSZ9477_NAME);
 
 #define KSZ9893_TAIL_TAG_OVERRIDE	BIT(5)
 #define KSZ9893_TAIL_TAG_LOOKUP		BIT(6)
@@ -183,7 +188,7 @@ static struct sk_buff *ksz9893_xmit(struct sk_buff *skb,
 }
 
 static const struct dsa_device_ops ksz9893_netdev_ops = {
-	.name	= "ksz9893",
+	.name	= KSZ9893_NAME,
 	.proto	= DSA_TAG_PROTO_KSZ9893,
 	.xmit	= ksz9893_xmit,
 	.rcv	= ksz9477_rcv,
@@ -191,7 +196,7 @@ static const struct dsa_device_ops ksz9893_netdev_ops = {
 };
 
 DSA_TAG_DRIVER(ksz9893_netdev_ops);
-MODULE_ALIAS_DSA_TAG_DRIVER(DSA_TAG_PROTO_KSZ9893);
+MODULE_ALIAS_DSA_TAG_DRIVER(DSA_TAG_PROTO_KSZ9893, KSZ9893_NAME);
 
 /* For xmit, 2 bytes are added before FCS.
  * ---------------------------------------------------------------------------
@@ -241,7 +246,7 @@ static struct sk_buff *lan937x_xmit(struct sk_buff *skb,
 }
 
 static const struct dsa_device_ops lan937x_netdev_ops = {
-	.name	= "lan937x",
+	.name	= LAN937X_NAME,
 	.proto	= DSA_TAG_PROTO_LAN937X,
 	.xmit	= lan937x_xmit,
 	.rcv	= ksz9477_rcv,
@@ -249,7 +254,7 @@ static const struct dsa_device_ops lan937x_netdev_ops = {
 };
 
 DSA_TAG_DRIVER(lan937x_netdev_ops);
-MODULE_ALIAS_DSA_TAG_DRIVER(DSA_TAG_PROTO_LAN937X);
+MODULE_ALIAS_DSA_TAG_DRIVER(DSA_TAG_PROTO_LAN937X, LAN937X_NAME);
 
 static struct dsa_tag_driver *dsa_tag_driver_array[] = {
 	&DSA_TAG_DRIVER_NAME(ksz8795_netdev_ops),
diff --git a/net/dsa/tag_lan9303.c b/net/dsa/tag_lan9303.c
index 98d7d7120bab..4118292ed218 100644
--- a/net/dsa/tag_lan9303.c
+++ b/net/dsa/tag_lan9303.c
@@ -30,6 +30,8 @@
  * Required when no forwarding between the external ports should happen.
  */
 
+#define LAN9303_NAME "lan9303"
+
 #define LAN9303_TAG_LEN 4
 # define LAN9303_TAG_TX_USE_ALR BIT(3)
 # define LAN9303_TAG_TX_STP_OVERRIDE BIT(4)
@@ -110,7 +112,7 @@ static struct sk_buff *lan9303_rcv(struct sk_buff *skb, struct net_device *dev)
 }
 
 static const struct dsa_device_ops lan9303_netdev_ops = {
-	.name = "lan9303",
+	.name = LAN9303_NAME,
 	.proto	= DSA_TAG_PROTO_LAN9303,
 	.xmit = lan9303_xmit,
 	.rcv = lan9303_rcv,
@@ -118,6 +120,6 @@ static const struct dsa_device_ops lan9303_netdev_ops = {
 };
 
 MODULE_LICENSE("GPL");
-MODULE_ALIAS_DSA_TAG_DRIVER(DSA_TAG_PROTO_LAN9303);
+MODULE_ALIAS_DSA_TAG_DRIVER(DSA_TAG_PROTO_LAN9303, LAN9303_NAME);
 
 module_dsa_tag_driver(lan9303_netdev_ops);
diff --git a/net/dsa/tag_mtk.c b/net/dsa/tag_mtk.c
index 415d8ece242a..ba37495ab5f4 100644
--- a/net/dsa/tag_mtk.c
+++ b/net/dsa/tag_mtk.c
@@ -10,6 +10,8 @@
 
 #include "dsa_priv.h"
 
+#define MTK_NAME		"mtk"
+
 #define MTK_HDR_LEN		4
 #define MTK_HDR_XMIT_UNTAGGED		0
 #define MTK_HDR_XMIT_TAGGED_TPID_8100	1
@@ -91,7 +93,7 @@ static struct sk_buff *mtk_tag_rcv(struct sk_buff *skb, struct net_device *dev)
 }
 
 static const struct dsa_device_ops mtk_netdev_ops = {
-	.name		= "mtk",
+	.name		= MTK_NAME,
 	.proto		= DSA_TAG_PROTO_MTK,
 	.xmit		= mtk_tag_xmit,
 	.rcv		= mtk_tag_rcv,
@@ -99,6 +101,6 @@ static const struct dsa_device_ops mtk_netdev_ops = {
 };
 
 MODULE_LICENSE("GPL");
-MODULE_ALIAS_DSA_TAG_DRIVER(DSA_TAG_PROTO_MTK);
+MODULE_ALIAS_DSA_TAG_DRIVER(DSA_TAG_PROTO_MTK, MTK_NAME);
 
 module_dsa_tag_driver(mtk_netdev_ops);
diff --git a/net/dsa/tag_ocelot.c b/net/dsa/tag_ocelot.c
index 0d81f172b7a6..8cc31ab47e28 100644
--- a/net/dsa/tag_ocelot.c
+++ b/net/dsa/tag_ocelot.c
@@ -4,6 +4,9 @@
 #include <linux/dsa/ocelot.h>
 #include "dsa_priv.h"
 
+#define OCELOT_NAME	"ocelot"
+#define SEVILLE_NAME	"seville"
+
 /* If the port is under a VLAN-aware bridge, remove the VLAN header from the
  * payload and move it into the DSA tag, which will make the switch classify
  * the packet to the bridge VLAN. Otherwise, leave the classified VLAN at zero,
@@ -183,7 +186,7 @@ static struct sk_buff *ocelot_rcv(struct sk_buff *skb,
 }
 
 static const struct dsa_device_ops ocelot_netdev_ops = {
-	.name			= "ocelot",
+	.name			= OCELOT_NAME,
 	.proto			= DSA_TAG_PROTO_OCELOT,
 	.xmit			= ocelot_xmit,
 	.rcv			= ocelot_rcv,
@@ -192,10 +195,10 @@ static const struct dsa_device_ops ocelot_netdev_ops = {
 };
 
 DSA_TAG_DRIVER(ocelot_netdev_ops);
-MODULE_ALIAS_DSA_TAG_DRIVER(DSA_TAG_PROTO_OCELOT);
+MODULE_ALIAS_DSA_TAG_DRIVER(DSA_TAG_PROTO_OCELOT, OCELOT_NAME);
 
 static const struct dsa_device_ops seville_netdev_ops = {
-	.name			= "seville",
+	.name			= SEVILLE_NAME,
 	.proto			= DSA_TAG_PROTO_SEVILLE,
 	.xmit			= seville_xmit,
 	.rcv			= ocelot_rcv,
@@ -204,7 +207,7 @@ static const struct dsa_device_ops seville_netdev_ops = {
 };
 
 DSA_TAG_DRIVER(seville_netdev_ops);
-MODULE_ALIAS_DSA_TAG_DRIVER(DSA_TAG_PROTO_SEVILLE);
+MODULE_ALIAS_DSA_TAG_DRIVER(DSA_TAG_PROTO_SEVILLE, SEVILLE_NAME);
 
 static struct dsa_tag_driver *ocelot_tag_driver_array[] = {
 	&DSA_TAG_DRIVER_NAME(ocelot_netdev_ops),
diff --git a/net/dsa/tag_ocelot_8021q.c b/net/dsa/tag_ocelot_8021q.c
index 37ccf00404ea..d1ec68001487 100644
--- a/net/dsa/tag_ocelot_8021q.c
+++ b/net/dsa/tag_ocelot_8021q.c
@@ -12,6 +12,8 @@
 #include <linux/dsa/ocelot.h>
 #include "dsa_priv.h"
 
+#define OCELOT_8021Q_NAME "ocelot-8021q"
+
 struct ocelot_8021q_tagger_private {
 	struct ocelot_8021q_tagger_data data; /* Must be first */
 	struct kthread_worker *xmit_worker;
@@ -119,7 +121,7 @@ static int ocelot_connect(struct dsa_switch *ds)
 }
 
 static const struct dsa_device_ops ocelot_8021q_netdev_ops = {
-	.name			= "ocelot-8021q",
+	.name			= OCELOT_8021Q_NAME,
 	.proto			= DSA_TAG_PROTO_OCELOT_8021Q,
 	.xmit			= ocelot_xmit,
 	.rcv			= ocelot_rcv,
@@ -130,6 +132,6 @@ static const struct dsa_device_ops ocelot_8021q_netdev_ops = {
 };
 
 MODULE_LICENSE("GPL v2");
-MODULE_ALIAS_DSA_TAG_DRIVER(DSA_TAG_PROTO_OCELOT_8021Q);
+MODULE_ALIAS_DSA_TAG_DRIVER(DSA_TAG_PROTO_OCELOT_8021Q, OCELOT_8021Q_NAME);
 
 module_dsa_tag_driver(ocelot_8021q_netdev_ops);
diff --git a/net/dsa/tag_qca.c b/net/dsa/tag_qca.c
index 57d2e00f1e5d..73d6e111228d 100644
--- a/net/dsa/tag_qca.c
+++ b/net/dsa/tag_qca.c
@@ -10,6 +10,8 @@
 
 #include "dsa_priv.h"
 
+#define QCA_NAME "qca"
+
 static struct sk_buff *qca_tag_xmit(struct sk_buff *skb, struct net_device *dev)
 {
 	struct dsa_port *dp = dsa_slave_to_port(dev);
@@ -107,7 +109,7 @@ static void qca_tag_disconnect(struct dsa_switch *ds)
 }
 
 static const struct dsa_device_ops qca_netdev_ops = {
-	.name	= "qca",
+	.name	= QCA_NAME,
 	.proto	= DSA_TAG_PROTO_QCA,
 	.connect = qca_tag_connect,
 	.disconnect = qca_tag_disconnect,
@@ -118,6 +120,6 @@ static const struct dsa_device_ops qca_netdev_ops = {
 };
 
 MODULE_LICENSE("GPL");
-MODULE_ALIAS_DSA_TAG_DRIVER(DSA_TAG_PROTO_QCA);
+MODULE_ALIAS_DSA_TAG_DRIVER(DSA_TAG_PROTO_QCA, QCA_NAME);
 
 module_dsa_tag_driver(qca_netdev_ops);
diff --git a/net/dsa/tag_rtl4_a.c b/net/dsa/tag_rtl4_a.c
index 6d928ee3ef7a..18b52d77d200 100644
--- a/net/dsa/tag_rtl4_a.c
+++ b/net/dsa/tag_rtl4_a.c
@@ -20,6 +20,8 @@
 
 #include "dsa_priv.h"
 
+#define RTL4_A_NAME		"rtl4a"
+
 #define RTL4_A_HDR_LEN		4
 #define RTL4_A_ETHERTYPE	0x8899
 #define RTL4_A_PROTOCOL_SHIFT	12
@@ -112,7 +114,7 @@ static struct sk_buff *rtl4a_tag_rcv(struct sk_buff *skb,
 }
 
 static const struct dsa_device_ops rtl4a_netdev_ops = {
-	.name	= "rtl4a",
+	.name	= RTL4_A_NAME,
 	.proto	= DSA_TAG_PROTO_RTL4_A,
 	.xmit	= rtl4a_tag_xmit,
 	.rcv	= rtl4a_tag_rcv,
@@ -121,4 +123,4 @@ static const struct dsa_device_ops rtl4a_netdev_ops = {
 module_dsa_tag_driver(rtl4a_netdev_ops);
 
 MODULE_LICENSE("GPL");
-MODULE_ALIAS_DSA_TAG_DRIVER(DSA_TAG_PROTO_RTL4_A);
+MODULE_ALIAS_DSA_TAG_DRIVER(DSA_TAG_PROTO_RTL4_A, RTL4_A_NAME);
diff --git a/net/dsa/tag_rtl8_4.c b/net/dsa/tag_rtl8_4.c
index a593ead7ff26..030a8cf0ad48 100644
--- a/net/dsa/tag_rtl8_4.c
+++ b/net/dsa/tag_rtl8_4.c
@@ -84,6 +84,9 @@
  * 0x04 = RTL8365MB DSA protocol
  */
 
+#define RTL8_4_NAME			"rtl8_4"
+#define RTL8_4T_NAME			"rtl8_4t"
+
 #define RTL8_4_TAG_LEN			8
 
 #define RTL8_4_PROTOCOL			GENMASK(15, 8)
@@ -234,7 +237,7 @@ static const struct dsa_device_ops rtl8_4_netdev_ops = {
 
 DSA_TAG_DRIVER(rtl8_4_netdev_ops);
 
-MODULE_ALIAS_DSA_TAG_DRIVER(DSA_TAG_PROTO_RTL8_4);
+MODULE_ALIAS_DSA_TAG_DRIVER(DSA_TAG_PROTO_RTL8_4, RTL8_4_NAME);
 
 /* Tail version */
 static const struct dsa_device_ops rtl8_4t_netdev_ops = {
@@ -247,7 +250,7 @@ static const struct dsa_device_ops rtl8_4t_netdev_ops = {
 
 DSA_TAG_DRIVER(rtl8_4t_netdev_ops);
 
-MODULE_ALIAS_DSA_TAG_DRIVER(DSA_TAG_PROTO_RTL8_4T);
+MODULE_ALIAS_DSA_TAG_DRIVER(DSA_TAG_PROTO_RTL8_4T, RTL8_4T_NAME);
 
 static struct dsa_tag_driver *dsa_tag_drivers[] = {
 	&DSA_TAG_DRIVER_NAME(rtl8_4_netdev_ops),
diff --git a/net/dsa/tag_rzn1_a5psw.c b/net/dsa/tag_rzn1_a5psw.c
index e2a5ee6ae688..b9135069f9fc 100644
--- a/net/dsa/tag_rzn1_a5psw.c
+++ b/net/dsa/tag_rzn1_a5psw.c
@@ -22,6 +22,8 @@
  * See struct a5psw_tag for layout
  */
 
+#define A5PSW_NAME			"a5psw"
+
 #define ETH_P_DSA_A5PSW			0xE001
 #define A5PSW_TAG_LEN			8
 #define A5PSW_CTRL_DATA_FORCE_FORWARD	BIT(0)
@@ -101,7 +103,7 @@ static struct sk_buff *a5psw_tag_rcv(struct sk_buff *skb,
 }
 
 static const struct dsa_device_ops a5psw_netdev_ops = {
-	.name	= "a5psw",
+	.name	= A5PSW_NAME,
 	.proto	= DSA_TAG_PROTO_RZN1_A5PSW,
 	.xmit	= a5psw_tag_xmit,
 	.rcv	= a5psw_tag_rcv,
@@ -109,5 +111,5 @@ static const struct dsa_device_ops a5psw_netdev_ops = {
 };
 
 MODULE_LICENSE("GPL v2");
-MODULE_ALIAS_DSA_TAG_DRIVER(DSA_TAG_PROTO_A5PSW);
+MODULE_ALIAS_DSA_TAG_DRIVER(DSA_TAG_PROTO_A5PSW, A5PSW_NAME);
 module_dsa_tag_driver(a5psw_netdev_ops);
diff --git a/net/dsa/tag_sja1105.c b/net/dsa/tag_sja1105.c
index 83e4136516b0..3b6e642a90e9 100644
--- a/net/dsa/tag_sja1105.c
+++ b/net/dsa/tag_sja1105.c
@@ -7,6 +7,9 @@
 #include <linux/packing.h>
 #include "dsa_priv.h"
 
+#define SJA1105_NAME				"sja1105"
+#define SJA1110_NAME				"sja1110"
+
 /* Is this a TX or an RX header? */
 #define SJA1110_HEADER_HOST_TO_SWITCH		BIT(15)
 
@@ -786,7 +789,7 @@ static int sja1105_connect(struct dsa_switch *ds)
 }
 
 static const struct dsa_device_ops sja1105_netdev_ops = {
-	.name = "sja1105",
+	.name = SJA1105_NAME,
 	.proto = DSA_TAG_PROTO_SJA1105,
 	.xmit = sja1105_xmit,
 	.rcv = sja1105_rcv,
@@ -798,10 +801,10 @@ static const struct dsa_device_ops sja1105_netdev_ops = {
 };
 
 DSA_TAG_DRIVER(sja1105_netdev_ops);
-MODULE_ALIAS_DSA_TAG_DRIVER(DSA_TAG_PROTO_SJA1105);
+MODULE_ALIAS_DSA_TAG_DRIVER(DSA_TAG_PROTO_SJA1105, SJA1105_NAME);
 
 static const struct dsa_device_ops sja1110_netdev_ops = {
-	.name = "sja1110",
+	.name = SJA1110_NAME,
 	.proto = DSA_TAG_PROTO_SJA1110,
 	.xmit = sja1110_xmit,
 	.rcv = sja1110_rcv,
@@ -813,7 +816,7 @@ static const struct dsa_device_ops sja1110_netdev_ops = {
 };
 
 DSA_TAG_DRIVER(sja1110_netdev_ops);
-MODULE_ALIAS_DSA_TAG_DRIVER(DSA_TAG_PROTO_SJA1110);
+MODULE_ALIAS_DSA_TAG_DRIVER(DSA_TAG_PROTO_SJA1110, SJA1110_NAME);
 
 static struct dsa_tag_driver *sja1105_tag_driver_array[] = {
 	&DSA_TAG_DRIVER_NAME(sja1105_netdev_ops),
diff --git a/net/dsa/tag_trailer.c b/net/dsa/tag_trailer.c
index 5749ba85c2b8..8754dfe680f6 100644
--- a/net/dsa/tag_trailer.c
+++ b/net/dsa/tag_trailer.c
@@ -10,6 +10,8 @@
 
 #include "dsa_priv.h"
 
+#define TRAILER_NAME "trailer"
+
 static struct sk_buff *trailer_xmit(struct sk_buff *skb, struct net_device *dev)
 {
 	struct dsa_port *dp = dsa_slave_to_port(dev);
@@ -50,7 +52,7 @@ static struct sk_buff *trailer_rcv(struct sk_buff *skb, struct net_device *dev)
 }
 
 static const struct dsa_device_ops trailer_netdev_ops = {
-	.name	= "trailer",
+	.name	= TRAILER_NAME,
 	.proto	= DSA_TAG_PROTO_TRAILER,
 	.xmit	= trailer_xmit,
 	.rcv	= trailer_rcv,
@@ -58,6 +60,6 @@ static const struct dsa_device_ops trailer_netdev_ops = {
 };
 
 MODULE_LICENSE("GPL");
-MODULE_ALIAS_DSA_TAG_DRIVER(DSA_TAG_PROTO_TRAILER);
+MODULE_ALIAS_DSA_TAG_DRIVER(DSA_TAG_PROTO_TRAILER, TRAILER_NAME);
 
 module_dsa_tag_driver(trailer_netdev_ops);
diff --git a/net/dsa/tag_xrs700x.c b/net/dsa/tag_xrs700x.c
index ff442b8af636..dc935dd90f98 100644
--- a/net/dsa/tag_xrs700x.c
+++ b/net/dsa/tag_xrs700x.c
@@ -9,6 +9,8 @@
 
 #include "dsa_priv.h"
 
+#define XRS700X_NAME "xrs700x"
+
 static struct sk_buff *xrs700x_xmit(struct sk_buff *skb, struct net_device *dev)
 {
 	struct dsa_port *partner, *dp = dsa_slave_to_port(dev);
@@ -51,7 +53,7 @@ static struct sk_buff *xrs700x_rcv(struct sk_buff *skb, struct net_device *dev)
 }
 
 static const struct dsa_device_ops xrs700x_netdev_ops = {
-	.name	= "xrs700x",
+	.name	= XRS700X_NAME,
 	.proto	= DSA_TAG_PROTO_XRS700X,
 	.xmit	= xrs700x_xmit,
 	.rcv	= xrs700x_rcv,
@@ -59,6 +61,6 @@ static const struct dsa_device_ops xrs700x_netdev_ops = {
 };
 
 MODULE_LICENSE("GPL");
-MODULE_ALIAS_DSA_TAG_DRIVER(DSA_TAG_PROTO_XRS700X);
+MODULE_ALIAS_DSA_TAG_DRIVER(DSA_TAG_PROTO_XRS700X, XRS700X_NAME);
 
 module_dsa_tag_driver(xrs700x_netdev_ops);
-- 
2.34.1

[RFC PATCH net-next 3/3] net: dsa: autoload tag driver module on tagging protocol change

[Vladimir Oltean]

Issue a request_module() call when an attempt to change the tagging
protocol is made, either by sysfs or by device tree. In the case of
ocelot (the only driver for which the default and the alternative
tagging protocol are compiled as different modules), the user is now no
longer required to insert tag_ocelot_8021q.ko manually.

In the particular case of ocelot, this solves a problem where
tag_ocelot_8021q.ko is built as module, and this is present in the
device tree:

&mscc_felix_port4 {
	dsa-tag-protocol = "ocelot-8021q";
};

&mscc_felix_port5 {
	dsa-tag-protocol = "ocelot-8021q";
};

Because no one attempts to load the module into the kernel at boot time,
the switch driver will fail to probe (actually forever defer) until
someone manually inserts tag_ocelot_8021q.ko. This is now no longer
necessary and happens automatically.

Link: https://lore.kernel.org/lkml/20221027113248.420216-1-michael@walle.cc/
Suggested-by: Michael Walle <michael@walle.cc>
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 net/dsa/dsa.c      | 8 +++++---
 net/dsa/dsa2.c     | 4 ++--
 net/dsa/dsa_priv.h | 4 ++--
 net/dsa/master.c   | 4 ++--
 4 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
index 64b14f655b23..20af0759052b 100644
--- a/net/dsa/dsa.c
+++ b/net/dsa/dsa.c
@@ -78,16 +78,18 @@ const char *dsa_tag_protocol_to_str(const struct dsa_device_ops *ops)
 /* Function takes a reference on the module owning the tagger,
  * so dsa_tag_driver_put must be called afterwards.
  */
-const struct dsa_device_ops *dsa_find_tagger_by_name(const char *buf)
+const struct dsa_device_ops *dsa_tag_driver_get_by_name(const char *name)
 {
 	const struct dsa_device_ops *ops = ERR_PTR(-ENOPROTOOPT);
 	struct dsa_tag_driver *dsa_tag_driver;
 
+	request_module("%s%s", DSA_TAG_DRIVER_ALIAS, name);
+
 	mutex_lock(&dsa_tag_drivers_lock);
 	list_for_each_entry(dsa_tag_driver, &dsa_tag_drivers_list, list) {
 		const struct dsa_device_ops *tmp = dsa_tag_driver->ops;
 
-		if (!sysfs_streq(buf, tmp->name))
+		if (!sysfs_streq(name, tmp->name))
 			continue;
 
 		if (!try_module_get(dsa_tag_driver->owner))
@@ -101,7 +103,7 @@ const struct dsa_device_ops *dsa_find_tagger_by_name(const char *buf)
 	return ops;
 }
 
-const struct dsa_device_ops *dsa_tag_driver_get(int tag_protocol)
+const struct dsa_device_ops *dsa_tag_driver_get_by_id(int tag_protocol)
 {
 	struct dsa_tag_driver *dsa_tag_driver;
 	const struct dsa_device_ops *ops;
diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index e504a18fc125..cba07ddfbcca 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -1433,7 +1433,7 @@ static int dsa_port_parse_cpu(struct dsa_port *dp, struct net_device *master,
 			return -EINVAL;
 		}
 
-		tag_ops = dsa_find_tagger_by_name(user_protocol);
+		tag_ops = dsa_tag_driver_get_by_name(user_protocol);
 		if (IS_ERR(tag_ops)) {
 			dev_warn(ds->dev,
 				 "Failed to find a tagging driver for protocol %s, using default\n",
@@ -1443,7 +1443,7 @@ static int dsa_port_parse_cpu(struct dsa_port *dp, struct net_device *master,
 	}
 
 	if (!tag_ops)
-		tag_ops = dsa_tag_driver_get(default_proto);
+		tag_ops = dsa_tag_driver_get_by_id(default_proto);
 
 	if (IS_ERR(tag_ops)) {
 		if (PTR_ERR(tag_ops) == -ENOPROTOOPT)
diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
index 6e65c7ffd6f3..4d5b631bd39a 100644
--- a/net/dsa/dsa_priv.h
+++ b/net/dsa/dsa_priv.h
@@ -168,9 +168,9 @@ struct dsa_slave_priv {
 };
 
 /* dsa.c */
-const struct dsa_device_ops *dsa_tag_driver_get(int tag_protocol);
+const struct dsa_device_ops *dsa_tag_driver_get_by_id(int tag_protocol);
+const struct dsa_device_ops *dsa_tag_driver_get_by_name(const char *buf);
 void dsa_tag_driver_put(const struct dsa_device_ops *ops);
-const struct dsa_device_ops *dsa_find_tagger_by_name(const char *buf);
 
 bool dsa_db_equal(const struct dsa_db *a, const struct dsa_db *b);
 
diff --git a/net/dsa/master.c b/net/dsa/master.c
index 40367ab41cf8..3fbbf51c8e97 100644
--- a/net/dsa/master.c
+++ b/net/dsa/master.c
@@ -305,8 +305,8 @@ static ssize_t tagging_store(struct device *d, struct device_attribute *attr,
 	int err;
 
 	old_tag_ops = cpu_dp->tag_ops;
-	new_tag_ops = dsa_find_tagger_by_name(buf);
-	/* Bad tagger name, or module is not loaded? */
+	new_tag_ops = dsa_tag_driver_get_by_name(buf);
+	/* Bad tagger name, or module does not exist? */
 	if (IS_ERR(new_tag_ops))
 		return PTR_ERR(new_tag_ops);
 
-- 
2.34.1

Re: [RFC PATCH net-next 0/3] Autoload DSA tagging driver when dynamically changing protocol

[Michael Walle]

Am 2022-10-27 23:08, schrieb Vladimir Oltean:
> This patch set solves the issue reported by Michael and Heiko here:
> https://lore.kernel.org/lkml/20221027113248.420216-1-michael@walle.cc/
> making full use of Michael's suggestion of having two modaliases: one
> gets used for loading the tagging protocol when it's the default one
> reported by the switch driver, the other gets loaded at user's request,
> by name.
> 
>   # modinfo tag_ocelot_8021q
>   filename:       
> /lib/modules/6.1.0-rc2+/kernel/net/dsa/tag_ocelot_8021q.ko
>   alias:          dsa_tag-ocelot-8021q
>   alias:          dsa_tag-20

I know that name clashes are not to be expected because one is a 
numerical id
and the other is a string, but it might still make sense to have a 
different
prefix so the user of modinfo can figure that out more easily.

Presuming that backwards compatibility is not an issue, maybe:
dsa_tag-ocelot-8021q
dsa_tag-id-20

>   license:        GPL v2
>   depends:        dsa_core
>   intree:         Y
>   name:           tag_ocelot_8021q
>   vermagic:       6.1.0-rc2+ SMP preempt mod_unload modversions aarch64
> 
> Tested on NXP LS1028A-RDB with the following device tree addition:
> 
> &mscc_felix_port4 {
> 	dsa-tag-protocol = "ocelot-8021q";
> };
> 
> &mscc_felix_port5 {
> 	dsa-tag-protocol = "ocelot-8021q";
> };
> 
> CONFIG_NET_DSA and everything that depends on it is built as module.
> Everything auto-loads, and "cat /sys/class/net/eno2/dsa/tagging" shows
> "ocelot-8021q". Traffic works as well.
> 
> Note: I included patch 1/3 because I secretly want to see if the
> patchwork build tests pass :) But I also submitted it separately to
> "net" already, and without it, patch 3/3 doesn't apply to current 
> net-next.
> So if you want to leave comments on 1/3, make sure to leave them here:
> https://patchwork.kernel.org/project/netdevbpf/patch/20221027145439.3086017-1-vladimir.oltean@nxp.com/
> 
> Vladimir Oltean (3):
>   net: dsa: fall back to default tagger if we can't load the one from 
> DT
>   net: dsa: provide a second modalias to tag proto drivers based on
>     their name
>   net: dsa: autoload tag driver module on tagging protocol change
> 
>  include/net/dsa.h          |  5 +++--
>  net/dsa/dsa.c              |  8 +++++---
>  net/dsa/dsa2.c             | 15 +++++++++++----
>  net/dsa/dsa_priv.h         |  4 ++--
>  net/dsa/master.c           |  4 ++--
>  net/dsa/tag_ar9331.c       |  6 ++++--
>  net/dsa/tag_brcm.c         | 16 ++++++++++------
>  net/dsa/tag_dsa.c          | 11 +++++++----
>  net/dsa/tag_gswip.c        |  6 ++++--
>  net/dsa/tag_hellcreek.c    |  6 ++++--
>  net/dsa/tag_ksz.c          | 21 +++++++++++++--------
>  net/dsa/tag_lan9303.c      |  6 ++++--
>  net/dsa/tag_mtk.c          |  6 ++++--
>  net/dsa/tag_ocelot.c       | 11 +++++++----
>  net/dsa/tag_ocelot_8021q.c |  6 ++++--
>  net/dsa/tag_qca.c          |  6 ++++--
>  net/dsa/tag_rtl4_a.c       |  6 ++++--
>  net/dsa/tag_rtl8_4.c       |  7 +++++--
>  net/dsa/tag_rzn1_a5psw.c   |  6 ++++--
>  net/dsa/tag_sja1105.c      | 11 +++++++----
>  net/dsa/tag_trailer.c      |  6 ++++--
>  net/dsa/tag_xrs700x.c      |  6 ++++--
>  22 files changed, 116 insertions(+), 63 deletions(-)

FWIW
Tested-by: Michael Walle <michael@walle.cc> # on kontron-sl28 w/ 
ocelot_8021q

Thanks,
-michael

Re: [RFC PATCH net-next 0/3] Autoload DSA tagging driver when dynamically changing protocol

[Vladimir Oltean]

On Fri, Oct 28, 2022 at 11:17:40AM +0200, Michael Walle wrote:
> >   alias:          dsa_tag-ocelot-8021q
> >   alias:          dsa_tag-20
> 
> I know that name clashes are not to be expected because one is a numerical id
> and the other is a string, but it might still make sense to have a different
> prefix so the user of modinfo can figure that out more easily.
> 
> Presuming that backwards compatibility is not an issue, maybe:
> dsa_tag-ocelot-8021q
> dsa_tag-id-20

Hm, it probably isn't an issue, but I'd like to hear from
Andrew/Florian/Vivien as well?

In the same note, "enum dsa_tag_protocol" isn't considered ABI either,
or in other words, we can reorder them, and DSA_TAG_PROTO_OCELOT_8021Q_VALUE
could become, say, 19 instead of 20 in a future kernel version, and that
would still be fine, because we would never load a module built for
kernel 6.1 in a 6.2 kernel?

> FWIW
> Tested-by: Michael Walle <michael@walle.cc> # on kontron-sl28 w/
> ocelot_8021q

Thanks for reporting the problem and for testing!

Re: [RFC PATCH net-next 0/3] Autoload DSA tagging driver when dynamically changing protocol

[Andrew Lunn]

On Fri, Oct 28, 2022 at 09:28:41AM +0000, Vladimir Oltean wrote:
> On Fri, Oct 28, 2022 at 11:17:40AM +0200, Michael Walle wrote:
> > >   alias:          dsa_tag-ocelot-8021q
> > >   alias:          dsa_tag-20
> > 
> > I know that name clashes are not to be expected because one is a numerical id
> > and the other is a string, but it might still make sense to have a different
> > prefix so the user of modinfo can figure that out more easily.
> > 
> > Presuming that backwards compatibility is not an issue, maybe:
> > dsa_tag-ocelot-8021q
> > dsa_tag-id-20
> 
> Hm, it probably isn't an issue, but I'd like to hear from
> Andrew/Florian/Vivien as well?

I don't see it being a big issue either way. This is not ABI, as
Vladimir points out. These module strings are also somewhat black
magic:

pci:v00001269d000000BBsv*sd*bc*sc*i*
usb:v17E9p*d*dc*dsc*dp*icFFisc00ip00in*
virtio:d00000005v*
pcmcia:m*c*f02fn*pfn*pa*pb*pc*pd*
acpi*:TPF0001:*

I don't think they are meant to be human readable.

I do however wounder if they should be dsa_tag:ocelot-8021q,
dsa_tag:20 ?

	   Andrew

Re: [RFC PATCH net-next 0/3] Autoload DSA tagging driver when dynamically changing protocol

[Vladimir Oltean]

On Sun, Oct 30, 2022 at 10:22:25PM +0100, Andrew Lunn wrote:
> On Fri, Oct 28, 2022 at 09:28:41AM +0000, Vladimir Oltean wrote:
> > On Fri, Oct 28, 2022 at 11:17:40AM +0200, Michael Walle wrote:
> > > Presuming that backwards compatibility is not an issue, maybe:
> > > dsa_tag-id-20
> 
> I don't think they are meant to be human readable.
> 
> I do however wounder if they should be dsa_tag:ocelot-8021q,
> dsa_tag:20 ?

dsa_tag:20 or dsa_tag:id-20?

Re: [RFC PATCH net-next 0/3] Autoload DSA tagging driver when dynamically changing protocol

[Florian Fainelli]

On 11/11/22 14:53, Vladimir Oltean wrote:
> On Sun, Oct 30, 2022 at 10:22:25PM +0100, Andrew Lunn wrote:
>> On Fri, Oct 28, 2022 at 09:28:41AM +0000, Vladimir Oltean wrote:
>>> On Fri, Oct 28, 2022 at 11:17:40AM +0200, Michael Walle wrote:
>>>> Presuming that backwards compatibility is not an issue, maybe:
>>>> dsa_tag-id-20
>>
>> I don't think they are meant to be human readable.
>>
>> I do however wounder if they should be dsa_tag:ocelot-8021q,
>> dsa_tag:20 ?
> 
> dsa_tag:20 or dsa_tag:id-20?

The latter IMHO would be more explicit.
-- 
Florian

Re: [RFC PATCH net-next 0/3] Autoload DSA tagging driver when dynamically changing protocol

[Vladimir Oltean]

On Fri, Nov 11, 2022 at 02:54:18PM -0800, Florian Fainelli wrote:
> On 11/11/22 14:53, Vladimir Oltean wrote:
> > On Sun, Oct 30, 2022 at 10:22:25PM +0100, Andrew Lunn wrote:
> > > On Fri, Oct 28, 2022 at 09:28:41AM +0000, Vladimir Oltean wrote:
> > > > On Fri, Oct 28, 2022 at 11:17:40AM +0200, Michael Walle wrote:
> > > > > Presuming that backwards compatibility is not an issue, maybe:
> > > > > dsa_tag-id-20
> > > 
> > > I don't think they are meant to be human readable.
> > > 
> > > I do however wounder if they should be dsa_tag:ocelot-8021q,
> > > dsa_tag:20 ?
> > 
> > dsa_tag:20 or dsa_tag:id-20?
> 
> The latter IMHO would be more explicit.

Okay, thanks.