| Message ID | 5f5132fd657daa503c709b86c87ae147e28a78ad.1603469145.git.gnault@redhat.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | mpls: fix dependencies on mpls_gso.ko | expand |
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. > diff --git a/net/mpls/Kconfig b/net/mpls/Kconfig > index d672ab72ab12..b83093bcb48f 100644 > --- a/net/mpls/Kconfig > +++ b/net/mpls/Kconfig > @@ -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. >
On Fri, Oct 23, 2020 at 11:23:04AM -0700, Jakub Kicinski wrote: > 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. > > diff --git a/net/mpls/Kconfig b/net/mpls/Kconfig > > index d672ab72ab12..b83093bcb48f 100644 > > --- a/net/mpls/Kconfig > > +++ b/net/mpls/Kconfig > > @@ -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. > > >
On Fri, 23 Oct 2020 20:48:16 +0200 Guillaume Nault wrote: > On Fri, Oct 23, 2020 at 11:23:04AM -0700, Jakub Kicinski wrote: > > 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. 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.
On 10/25/20 3:43 PM, Jakub Kicinski wrote: > On Fri, 23 Oct 2020 20:48:16 +0200 Guillaume Nault wrote: >> On Fri, Oct 23, 2020 at 11:23:04AM -0700, Jakub Kicinski wrote: >>> 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. > > 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.
On Sun, Oct 25, 2020 at 08:48:38PM -0600, David Ahern wrote: > On 10/25/20 3:43 PM, Jakub Kicinski wrote: > > On Fri, 23 Oct 2020 20:48:16 +0200 Guillaume Nault wrote: > >> On Fri, Oct 23, 2020 at 11:23:04AM -0700, Jakub Kicinski wrote: > >>> 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. > > > > 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.
diff --git a/net/mpls/Kconfig b/net/mpls/Kconfig index d672ab72ab12..b83093bcb48f 100644 --- a/net/mpls/Kconfig +++ b/net/mpls/Kconfig @@ -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.
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 <gnault@redhat.com> --- net/mpls/Kconfig | 1 + 1 file changed, 1 insertion(+)