Skip to contentSkip to conversation

mpls: fix dependencies on mpls_gso.ko

PATCH In review Guillaume Nault 3 participants 8 messages Oct 23, 2020 – Oct 26, 2020

Subsystems (3)
Networking Network drivers Traffic control (tc)
lore
Share & tools
lore

Guillaume Nault

author
#0 lore raw

cover mpls: fix dependencies on mpls_gso.ko

Since commit b7c24497baea ("mpls: load mpls_gso after mpls_iptunnel"), mpls_iptunnel tries to load mpls_gso. Therefore, we need to build mpls_gso when mpls_iptunnel is selected (patch 1).

There's also the act_mpls module that can push MPLS headers on GSO packets. This module also depends on mpls_gso (patch 2).

Guillaume Nault (2):
mpls: Make MPLS_IPTUNNEL select NET_MPLS_GSO
net/sched: act_mpls: Add softdep on mpls_gso.ko

 net/mpls/Kconfig     | 1 +
 net/sched/Kconfig    | 2 ++
 net/sched/act_mpls.c | 1 +
 3 files changed, 4 insertions(+)
Signature · 1 line
2.21.3
author
#1 lore raw

PATCH 1/2 mpls: Make MPLS_IPTUNNEL select NET_MPLS_GSO

Since commit b7c24497baea ("mpls: load mpls_gso after mpls_iptunnel"), mpls_iptunnel.ko has a softdep on mpls_gso.ko. For this to work, we need to ensure that mpls_gso.ko is built whenever MPLS_IPTUNNEL is set.

Fixes b7c24497baea ("mpls: load mpls_gso after mpls_iptunnel")
Signed-off-by Guillaume Nault <[email protected]>
---
 net/mpls/Kconfig | 1 +
 1 file changed, 1 insertion(+)
net/mpls/Kconfig +1 -0
index d672ab72ab12..b83093bcb48f 100644
@@ -33,6 +33,7 @@ config MPLS_ROUTING
 config MPLS_IPTUNNEL
 	tristate "MPLS: IP over MPLS tunnel support"
 	depends on LWTUNNEL && MPLS_ROUTING
+	select NET_MPLS_GSO
 	help
 	 mpls ip tunnel support.
Signature · 1 line
2.21.3
maintainer
#2 lore raw

Re: [PATCH net 1/2] mpls: Make MPLS_IPTUNNEL select NET_MPLS_GSO

Quoted 3 lines · On Fri, 23 Oct 2020 18:19:43 +0200 Guillaume Nault wrote:

Since commit b7c24497baea ("mpls: load mpls_gso after mpls_iptunnel"), mpls_iptunnel.ko has a softdep on mpls_gso.ko. For this to work, we need to ensure that mpls_gso.ko is built whenever MPLS_IPTUNNEL is set.

Does it generate an error or a warning? I don't know much about soft dependencies, but I'd think it's optional.

Quoted 12 lines
net/mpls/Kconfig +1 -0
index d672ab72ab12..b83093bcb48f 100644
@@ -33,6 +33,7 @@ config MPLS_ROUTING
 config MPLS_IPTUNNEL
 	tristate "MPLS: IP over MPLS tunnel support"
 	depends on LWTUNNEL && MPLS_ROUTING
+	select NET_MPLS_GSO
 	help
 	 mpls ip tunnel support.
author
#3 lore raw

Re: [PATCH net 1/2] mpls: Make MPLS_IPTUNNEL select NET_MPLS_GSO

Quoted 7 lines · Jakub Kicinski
Quoted 3 lines · On Fri, 23 Oct 2020 18:19:43 +0200 Guillaume Nault wrote:

Since commit b7c24497baea ("mpls: load mpls_gso after mpls_iptunnel"), mpls_iptunnel.ko has a softdep on mpls_gso.ko. For this to work, we need to ensure that mpls_gso.ko is built whenever MPLS_IPTUNNEL is set.

Does it generate an error or a warning? I don't know much about soft dependencies, but I'd think it's optional.

Yes, it's optional from a softdep point of view. My point was that having a softdep isn't a complete solution, as a bad .config can still result in inability to properly transmit GSO packets.

Quoted 13 lines
Quoted 12 lines
net/mpls/Kconfig +1 -0
index d672ab72ab12..b83093bcb48f 100644
@@ -33,6 +33,7 @@ config MPLS_ROUTING
 config MPLS_IPTUNNEL
 	tristate "MPLS: IP over MPLS tunnel support"
 	depends on LWTUNNEL && MPLS_ROUTING
+	select NET_MPLS_GSO
 	help
 	 mpls ip tunnel support.
maintainer
#4 lore raw

Re: [PATCH net 1/2] mpls: Make MPLS_IPTUNNEL select NET_MPLS_GSO

Quoted 6 lines · On Fri, 23 Oct 2020 20:48:16 +0200 Guillaume Nault wrote:
Quoted 5 lines · Jakub Kicinski
Quoted 3 lines · On Fri, 23 Oct 2020 18:19:43 +0200 Guillaume Nault wrote:

Since commit b7c24497baea ("mpls: load mpls_gso after mpls_iptunnel"), mpls_iptunnel.ko has a softdep on mpls_gso.ko. For this to work, we need to ensure that mpls_gso.ko is built whenever MPLS_IPTUNNEL is set.

Quoted 6 lines
Quoted 2 lines

Does it generate an error or a warning? I don't know much about soft dependencies, but I'd think it's optional.

Yes, it's optional from a softdep point of view. My point was that having a softdep isn't a complete solution, as a bad .config can still result in inability to properly transmit GSO packets.

IMO the combination of select and softdep feels pretty strange.

It's either a softdep and therefore optional, or we really don't want to be missing it in a correctly functioning system, and the dependency should be made stronger.

#5 lore raw

Re: [PATCH net 1/2] mpls: Make MPLS_IPTUNNEL select NET_MPLS_GSO

Quoted 14 lines · Jakub Kicinski
Quoted 6 lines · On Fri, 23 Oct 2020 20:48:16 +0200 Guillaume Nault wrote:
Quoted 5 lines · Jakub Kicinski
Quoted 3 lines · On Fri, 23 Oct 2020 18:19:43 +0200 Guillaume Nault wrote:

Since commit b7c24497baea ("mpls: load mpls_gso after mpls_iptunnel"), mpls_iptunnel.ko has a softdep on mpls_gso.ko. For this to work, we need to ensure that mpls_gso.ko is built whenever MPLS_IPTUNNEL is set.

Quoted 6 lines
Quoted 2 lines

Does it generate an error or a warning? I don't know much about soft dependencies, but I'd think it's optional.

Yes, it's optional from a softdep point of view. My point was that having a softdep isn't a complete solution, as a bad .config can still result in inability to properly transmit GSO packets.

Quoted 6 lines

IMO the combination of select and softdep feels pretty strange.

It's either a softdep and therefore optional, or we really don't want to be missing it in a correctly functioning system, and the dependency should be made stronger.

That's why I liked the softdep solution - if the module is there, load it.

author
#6 lore raw

Re: [PATCH net 1/2] mpls: Make MPLS_IPTUNNEL select NET_MPLS_GSO

Quoted 15 lines · David Ahern
Quoted 14 lines · Jakub Kicinski
Quoted 12 lines · On Fri, 23 Oct 2020 20:48:16 +0200 Guillaume Nault wrote:
Quoted 7 lines · Jakub Kicinski
Quoted 3 lines · On Fri, 23 Oct 2020 18:19:43 +0200 Guillaume Nault wrote:

Since commit b7c24497baea ("mpls: load mpls_gso after mpls_iptunnel"), mpls_iptunnel.ko has a softdep on mpls_gso.ko. For this to work, we need to ensure that mpls_gso.ko is built whenever MPLS_IPTUNNEL is set.

Does it generate an error or a warning? I don't know much about soft dependencies, but I'd think it's optional.

Yes, it's optional from a softdep point of view. My point was that having a softdep isn't a complete solution, as a bad .config can still result in inability to properly transmit GSO packets.

Quoted 8 lines
Quoted 6 lines

IMO the combination of select and softdep feels pretty strange.

It's either a softdep and therefore optional, or we really don't want to be missing it in a correctly functioning system, and the dependency should be made stronger.

That's why I liked the softdep solution - if the module is there, load it.

Okay, then I'll resubmit without the Kconfig bits.

author
#7 lore raw

PATCH 2/2 net/sched: act_mpls: Add softdep on mpls_gso.ko

In reply to Guillaume Nault

TCA_MPLS_ACT_PUSH and TCA_MPLS_ACT_MAC_PUSH might be used on gso packets. Such packets will thus require mpls_gso.ko for segmentation.

Fixes 2a2ea50870ba ("net: sched: add mpls manipulation actions to TC")
Signed-off-by Guillaume Nault <[email protected]>
---
 net/sched/Kconfig    | 2 ++
 net/sched/act_mpls.c | 1 +
 2 files changed, 3 insertions(+)
net/sched/Kconfig +2 -0
index a3b37d88800e..b08b410c8084 100644
@@ -879,6 +879,8 @@ config NET_ACT_CSUM
 config NET_ACT_MPLS
 	tristate "MPLS manipulation"
 	depends on NET_CLS_ACT
+	select MPLS
+	select NET_MPLS_GSO
 	help
 	  Say Y here to push or pop MPLS headers.
net/sched/act_mpls.c +1 -0
index f40bf9771cb9..5c7456e5b5cf 100644
@@ -426,6 +426,7 @@ static void __exit mpls_cleanup_module(void)
 module_init(mpls_init_module);
 module_exit(mpls_cleanup_module);
 
+MODULE_SOFTDEP("post: mpls_gso");
 MODULE_AUTHOR("Netronome Systems <[email protected]>");
 MODULE_LICENSE("GPL");
 MODULE_DESCRIPTION("MPLS manipulation actions");
Signature · 1 line
2.21.3