diff mbox series

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

Message ID 5f5132fd657daa503c709b86c87ae147e28a78ad.1603469145.git.gnault@redhat.com
State New
Headers show
Series mpls: fix dependencies on mpls_gso.ko | expand

Commit Message

Guillaume Nault Oct. 23, 2020, 4:19 p.m. UTC
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(+)

Comments

Jakub Kicinski Oct. 23, 2020, 6:23 p.m. UTC | #1
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.
>
Guillaume Nault Oct. 23, 2020, 6:48 p.m. UTC | #2
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.
> >  
>
Jakub Kicinski Oct. 25, 2020, 9:43 p.m. UTC | #3
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.
David Ahern Oct. 26, 2020, 2:48 a.m. UTC | #4
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.
Guillaume Nault Oct. 26, 2020, 9:04 a.m. UTC | #5
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 mbox series

Patch

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.