| Message ID | 20221027210830.3577793-1-vladimir.oltean@nxp.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Autoload DSA tagging driver when dynamically changing protocol | expand |
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
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!
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
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?
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.
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.
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(-)