[PATCH net] tipc: not enable tipc when ipv6 works as a module

[PATCH net] tipc: not enable tipc when ipv6 works as a module

[Xin Long]

When using ipv6_dev_find() in one module, it requires ipv6 not to
work as a module. Otherwise, this error occurs in build:

  undefined reference to `ipv6_dev_find'.

So fix it by adding "depends on IPV6 || IPV6=n" to tipc/Kconfig,
as it does in sctp/Kconfig.

Fixes: 5a6f6f579178 ("tipc: set ub->ifindex for local ipv6 address")
Reported-by: kernel test robot <lkp@intel.com>
Acked-by: Randy Dunlap <rdunlap@infradead.org>
Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
 net/tipc/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/tipc/Kconfig b/net/tipc/Kconfig
index 9dd7802..be1c400 100644
--- a/net/tipc/Kconfig
+++ b/net/tipc/Kconfig
@@ -6,6 +6,7 @@
 menuconfig TIPC
 	tristate "The TIPC Protocol"
 	depends on INET
+	depends on IPV6 || IPV6=n
 	help
 	  The Transparent Inter Process Communication (TIPC) protocol is
 	  specially designed for intra cluster communication. This protocol
-- 
2.1.0

Re: [PATCH net] tipc: not enable tipc when ipv6 works as a module

[Cong Wang]

On Sun, Aug 16, 2020 at 4:54 AM Xin Long <lucien.xin@gmail.com> wrote:
>
> When using ipv6_dev_find() in one module, it requires ipv6 not to
> work as a module. Otherwise, this error occurs in build:
>
>   undefined reference to `ipv6_dev_find'.
>
> So fix it by adding "depends on IPV6 || IPV6=n" to tipc/Kconfig,
> as it does in sctp/Kconfig.

Or put it into struct ipv6_stub?

Re: [PATCH net] tipc: not enable tipc when ipv6 works as a module

[Xin Long]

On Mon, Aug 17, 2020 at 2:29 AM Cong Wang <xiyou.wangcong@gmail.com> wrote:
>
> On Sun, Aug 16, 2020 at 4:54 AM Xin Long <lucien.xin@gmail.com> wrote:
> >
> > When using ipv6_dev_find() in one module, it requires ipv6 not to
> > work as a module. Otherwise, this error occurs in build:
> >
> >   undefined reference to `ipv6_dev_find'.
> >
> > So fix it by adding "depends on IPV6 || IPV6=n" to tipc/Kconfig,
> > as it does in sctp/Kconfig.
>
> Or put it into struct ipv6_stub?
Hi Cong,

That could be one way. We may do it when this new function becomes more common.
By now, I think it's okay to make TIPC depend on IPV6 || IPV6=n.

Thanks.

Re: [PATCH net] tipc: not enable tipc when ipv6 works as a module

[Cong Wang]

On Sun, Aug 16, 2020 at 11:37 PM Xin Long <lucien.xin@gmail.com> wrote:
>
> On Mon, Aug 17, 2020 at 2:29 AM Cong Wang <xiyou.wangcong@gmail.com> wrote:
> >
> > Or put it into struct ipv6_stub?
> Hi Cong,
>
> That could be one way. We may do it when this new function becomes more common.
> By now, I think it's okay to make TIPC depend on IPV6 || IPV6=n.

I am not a fan of IPV6=m, but disallowing it for one symbol seems
too harsh.

Re: [PATCH net] tipc: not enable tipc when ipv6 works as a module

[Randy Dunlap]

On 8/17/20 11:31 AM, Cong Wang wrote:
> On Sun, Aug 16, 2020 at 11:37 PM Xin Long <lucien.xin@gmail.com> wrote:
>>
>> On Mon, Aug 17, 2020 at 2:29 AM Cong Wang <xiyou.wangcong@gmail.com> wrote:
>>>
>>> Or put it into struct ipv6_stub?
>> Hi Cong,
>>
>> That could be one way. We may do it when this new function becomes more common.
>> By now, I think it's okay to make TIPC depend on IPV6 || IPV6=n.
> 
> I am not a fan of IPV6=m, but disallowing it for one symbol seems
> too harsh.

Hi,

Maybe I'm not following you, but this doesn't disallow IPV6=m.

It just restricts how TIPC can be built, so that
TIPC=y and IPV6=m cannot happen together, which causes
a build error.

-- 
~Randy

Re: [PATCH net] tipc: not enable tipc when ipv6 works as a module

[Cong Wang]

On Mon, Aug 17, 2020 at 11:49 AM Randy Dunlap <rdunlap@infradead.org> wrote:
>
> On 8/17/20 11:31 AM, Cong Wang wrote:
> > On Sun, Aug 16, 2020 at 11:37 PM Xin Long <lucien.xin@gmail.com> wrote:
> >>
> >> On Mon, Aug 17, 2020 at 2:29 AM Cong Wang <xiyou.wangcong@gmail.com> wrote:
> >>>
> >>> Or put it into struct ipv6_stub?
> >> Hi Cong,
> >>
> >> That could be one way. We may do it when this new function becomes more common.
> >> By now, I think it's okay to make TIPC depend on IPV6 || IPV6=n.
> >
> > I am not a fan of IPV6=m, but disallowing it for one symbol seems
> > too harsh.
>
> Hi,
>
> Maybe I'm not following you, but this doesn't disallow IPV6=m.

Well, by "disallowing IPV6=m" I meant "disallowing IPV6=m when
enabling TIPC" for sure... Sorry that it misleads you to believe
completely disallowing IPV6=m globally.

>
> It just restricts how TIPC can be built, so that
> TIPC=y and IPV6=m cannot happen together, which causes
> a build error.

It also disallows TIPC=m and IPV6=m, right? In short, it disalows
IPV6=m when TIPC is enabled. And this is exactly what I complain,
as it looks too harsh.

Thanks.

Re: [PATCH net] tipc: not enable tipc when ipv6 works as a module

[Randy Dunlap]

On 8/17/20 11:55 AM, Cong Wang wrote:
> On Mon, Aug 17, 2020 at 11:49 AM Randy Dunlap <rdunlap@infradead.org> wrote:
>>
>> On 8/17/20 11:31 AM, Cong Wang wrote:
>>> On Sun, Aug 16, 2020 at 11:37 PM Xin Long <lucien.xin@gmail.com> wrote:
>>>>
>>>> On Mon, Aug 17, 2020 at 2:29 AM Cong Wang <xiyou.wangcong@gmail.com> wrote:
>>>>>
>>>>> Or put it into struct ipv6_stub?
>>>> Hi Cong,
>>>>
>>>> That could be one way. We may do it when this new function becomes more common.
>>>> By now, I think it's okay to make TIPC depend on IPV6 || IPV6=n.
>>>
>>> I am not a fan of IPV6=m, but disallowing it for one symbol seems
>>> too harsh.
>>
>> Hi,
>>
>> Maybe I'm not following you, but this doesn't disallow IPV6=m.
> 
> Well, by "disallowing IPV6=m" I meant "disallowing IPV6=m when
> enabling TIPC" for sure... Sorry that it misleads you to believe
> completely disallowing IPV6=m globally.
> 
>>
>> It just restricts how TIPC can be built, so that
>> TIPC=y and IPV6=m cannot happen together, which causes
>> a build error.
> 
> It also disallows TIPC=m and IPV6=m, right? In short, it disalows
> IPV6=m when TIPC is enabled. And this is exactly what I complain,
> as it looks too harsh.

I haven't tested that specifically, but that should work.
This patch won't prevent that from working.

We have loadable modules calling other loadable modules
all over the kernel.

-- 
~Randy

Re: [PATCH net] tipc: not enable tipc when ipv6 works as a module

[Cong Wang]

On Mon, Aug 17, 2020 at 12:00 PM Randy Dunlap <rdunlap@infradead.org> wrote:
>
> On 8/17/20 11:55 AM, Cong Wang wrote:
> > On Mon, Aug 17, 2020 at 11:49 AM Randy Dunlap <rdunlap@infradead.org> wrote:
> >>
> >> On 8/17/20 11:31 AM, Cong Wang wrote:
> >>> On Sun, Aug 16, 2020 at 11:37 PM Xin Long <lucien.xin@gmail.com> wrote:
> >>>>
> >>>> On Mon, Aug 17, 2020 at 2:29 AM Cong Wang <xiyou.wangcong@gmail.com> wrote:
> >>>>>
> >>>>> Or put it into struct ipv6_stub?
> >>>> Hi Cong,
> >>>>
> >>>> That could be one way. We may do it when this new function becomes more common.
> >>>> By now, I think it's okay to make TIPC depend on IPV6 || IPV6=n.
> >>>
> >>> I am not a fan of IPV6=m, but disallowing it for one symbol seems
> >>> too harsh.
> >>
> >> Hi,
> >>
> >> Maybe I'm not following you, but this doesn't disallow IPV6=m.
> >
> > Well, by "disallowing IPV6=m" I meant "disallowing IPV6=m when
> > enabling TIPC" for sure... Sorry that it misleads you to believe
> > completely disallowing IPV6=m globally.
> >
> >>
> >> It just restricts how TIPC can be built, so that
> >> TIPC=y and IPV6=m cannot happen together, which causes
> >> a build error.
> >
> > It also disallows TIPC=m and IPV6=m, right? In short, it disalows
> > IPV6=m when TIPC is enabled. And this is exactly what I complain,
> > as it looks too harsh.
>
> I haven't tested that specifically, but that should work.
> This patch won't prevent that from working.

Please give it a try. I do not see how it allows IPV6=m and TIPC=m
but disallows IPV6=m and TIPC=y.

>
> We have loadable modules calling other loadable modules
> all over the kernel.

True, we rely on request_module(). But I do not see TIPC calls
request_module() to request IPV6 module to load "ipv6_dev_find".

Thanks.

Re: [PATCH net] tipc: not enable tipc when ipv6 works as a module

[Randy Dunlap]

On 8/17/20 12:26 PM, Cong Wang wrote:
> On Mon, Aug 17, 2020 at 12:00 PM Randy Dunlap <rdunlap@infradead.org> wrote:
>>
>> On 8/17/20 11:55 AM, Cong Wang wrote:
>>> On Mon, Aug 17, 2020 at 11:49 AM Randy Dunlap <rdunlap@infradead.org> wrote:
>>>>
>>>> On 8/17/20 11:31 AM, Cong Wang wrote:
>>>>> On Sun, Aug 16, 2020 at 11:37 PM Xin Long <lucien.xin@gmail.com> wrote:
>>>>>>
>>>>>> On Mon, Aug 17, 2020 at 2:29 AM Cong Wang <xiyou.wangcong@gmail.com> wrote:
>>>>>>>
>>>>>>> Or put it into struct ipv6_stub?
>>>>>> Hi Cong,
>>>>>>
>>>>>> That could be one way. We may do it when this new function becomes more common.
>>>>>> By now, I think it's okay to make TIPC depend on IPV6 || IPV6=n.
>>>>>
>>>>> I am not a fan of IPV6=m, but disallowing it for one symbol seems
>>>>> too harsh.
>>>>
>>>> Hi,
>>>>
>>>> Maybe I'm not following you, but this doesn't disallow IPV6=m.
>>>
>>> Well, by "disallowing IPV6=m" I meant "disallowing IPV6=m when
>>> enabling TIPC" for sure... Sorry that it misleads you to believe
>>> completely disallowing IPV6=m globally.
>>>
>>>>
>>>> It just restricts how TIPC can be built, so that
>>>> TIPC=y and IPV6=m cannot happen together, which causes
>>>> a build error.
>>>
>>> It also disallows TIPC=m and IPV6=m, right? In short, it disalows
>>> IPV6=m when TIPC is enabled. And this is exactly what I complain,
>>> as it looks too harsh.
>>
>> I haven't tested that specifically, but that should work.
>> This patch won't prevent that from working.
> 
> Please give it a try. I do not see how it allows IPV6=m and TIPC=m
> but disallows IPV6=m and TIPC=y.

TIPC=m and IPV6=m builds just fine.

Having tipc autoload ipv6 is a different problem. (IMO)


This Kconfig entry:
 menuconfig TIPC
 	tristate "The TIPC Protocol"
 	depends on INET
+	depends on IPV6 || IPV6=n

says:
If IPV6=n, TIPC can be y/m/n.
If IPV6=y/m, TIPC is limited to whatever IPV6 is set to.
TIPC cannot be =y unless IPV6=y.


>> We have loadable modules calling other loadable modules
>> all over the kernel.
> 
> True, we rely on request_module(). But I do not see TIPC calls
> request_module() to request IPV6 module to load "ipv6_dev_find".


-- 
~Randy

Re: [PATCH net] tipc: not enable tipc when ipv6 works as a module

[Cong Wang]

On Mon, Aug 17, 2020 at 12:55 PM Randy Dunlap <rdunlap@infradead.org> wrote:
>
> TIPC=m and IPV6=m builds just fine.
>
> Having tipc autoload ipv6 is a different problem. (IMO)
>
>
> This Kconfig entry:
>  menuconfig TIPC
>         tristate "The TIPC Protocol"
>         depends on INET
> +       depends on IPV6 || IPV6=n
>
> says:
> If IPV6=n, TIPC can be y/m/n.
> If IPV6=y/m, TIPC is limited to whatever IPV6 is set to.

Hmm, nowadays we _do_ have IPV6=y on popular distros.
So this means TIPC would have to be builtin after this patch??
Still sounds harsh, right?

At least on my OpenSUSE I have CONFIG_IPV6=y and
CONFIG_TIPC=m.

> TIPC cannot be =y unless IPV6=y.

Interesting, I never correctly understand that "depends on"
behavior.

But even if it builds, how could TIPC module find and load
IPV6 module? Does IPV6 module automatically become its
dependency now I think?

Thanks.

Re: [PATCH net] tipc: not enable tipc when ipv6 works as a module

[Randy Dunlap]

On 8/17/20 1:29 PM, Cong Wang wrote:
> On Mon, Aug 17, 2020 at 12:55 PM Randy Dunlap <rdunlap@infradead.org> wrote:
>>
>> TIPC=m and IPV6=m builds just fine.
>>
>> Having tipc autoload ipv6 is a different problem. (IMO)
>>
>>
>> This Kconfig entry:
>>  menuconfig TIPC
>>         tristate "The TIPC Protocol"
>>         depends on INET
>> +       depends on IPV6 || IPV6=n
>>
>> says:
>> If IPV6=n, TIPC can be y/m/n.
>> If IPV6=y/m, TIPC is limited to whatever IPV6 is set to.
> 
> Hmm, nowadays we _do_ have IPV6=y on popular distros.
> So this means TIPC would have to be builtin after this patch??

No, it does not mean that. We can still have IPV6=y and TIPC=m.

Hm, maybe I should have said this instead:

  If IPV6=y/m, TIPC is limited _by_ whatever IPV6 is set to.
                (instead of    _to_ )

Does that help any?

The "limited" in Kconfig rules is a "less than or equal to"
limit, where 'm' < 'y'.



> Still sounds harsh, right?
> 
> At least on my OpenSUSE I have CONFIG_IPV6=y and
> CONFIG_TIPC=m.
> 
>> TIPC cannot be =y unless IPV6=y.
> 
> Interesting, I never correctly understand that "depends on"
> behavior.
> 
> But even if it builds, how could TIPC module find and load
> IPV6 module? Does IPV6 module automatically become its
> dependency now I think?

Sorry, I don't know about this.


-- 
~Randy

Re: [PATCH net] tipc: not enable tipc when ipv6 works as a module

[Cong Wang]

On Mon, Aug 17, 2020 at 1:43 PM Randy Dunlap <rdunlap@infradead.org> wrote:
>
> On 8/17/20 1:29 PM, Cong Wang wrote:
> > On Mon, Aug 17, 2020 at 12:55 PM Randy Dunlap <rdunlap@infradead.org> wrote:
> >>
> >> TIPC=m and IPV6=m builds just fine.
> >>
> >> Having tipc autoload ipv6 is a different problem. (IMO)
> >>
> >>
> >> This Kconfig entry:
> >>  menuconfig TIPC
> >>         tristate "The TIPC Protocol"
> >>         depends on INET
> >> +       depends on IPV6 || IPV6=n
> >>
> >> says:
> >> If IPV6=n, TIPC can be y/m/n.
> >> If IPV6=y/m, TIPC is limited to whatever IPV6 is set to.
> >
> > Hmm, nowadays we _do_ have IPV6=y on popular distros.
> > So this means TIPC would have to be builtin after this patch??
>
> No, it does not mean that. We can still have IPV6=y and TIPC=m.
>
> Hm, maybe I should have said this instead:
>
>   If IPV6=y/m, TIPC is limited _by_ whatever IPV6 is set to.
>                 (instead of    _to_ )
>
> Does that help any?
>
> The "limited" in Kconfig rules is a "less than or equal to"
> limit, where 'm' < 'y'.

Yeah, this is more clear now. If so, that means all the symbols
we have in ipv6_stub can be gone now, assuming module
dependency is automatically solved when both are modules.

Is this a new Kconfig feature? ipv6_stub was introduced for
VXLAN, at that time I don't remember we have such kind of
Kconfig rules, otherwise it would not be needed.

I also glanced at Documentation/kbuild/kconfig-language.txt,
I do not see such a rule, I guess the doc is not updated.


> > Still sounds harsh, right?
> >
> > At least on my OpenSUSE I have CONFIG_IPV6=y and
> > CONFIG_TIPC=m.
> >
> >> TIPC cannot be =y unless IPV6=y.
> >
> > Interesting, I never correctly understand that "depends on"
> > behavior.
> >
> > But even if it builds, how could TIPC module find and load
> > IPV6 module? Does IPV6 module automatically become its
> > dependency now I think?
>
> Sorry, I don't know about this.

You can check `modinfo tipc` output after compiling both as
modules. (Sorry that I only have CONFIG_MODULES=n here.)

Thanks.

Re: [PATCH net] tipc: not enable tipc when ipv6 works as a module

[David Miller]

From: Cong Wang <xiyou.wangcong@gmail.com>
Date: Mon, 17 Aug 2020 13:59:46 -0700

> Is this a new Kconfig feature? ipv6_stub was introduced for
> VXLAN, at that time I don't remember we have such kind of
> Kconfig rules, otherwise it would not be needed.

The ipv6_stub exists in order to allow the troublesome
"ipv6=m && feature_using_ipv6=y" combination.

Re: [PATCH net] tipc: not enable tipc when ipv6 works as a module

[Cong Wang]

On Mon, Aug 17, 2020 at 2:39 PM David Miller <davem@davemloft.net> wrote:
>
> From: Cong Wang <xiyou.wangcong@gmail.com>
> Date: Mon, 17 Aug 2020 13:59:46 -0700
>
> > Is this a new Kconfig feature? ipv6_stub was introduced for
> > VXLAN, at that time I don't remember we have such kind of
> > Kconfig rules, otherwise it would not be needed.
>
> The ipv6_stub exists in order to allow the troublesome
> "ipv6=m && feature_using_ipv6=y" combination.

Hmm, so "IPV6=m && TIPC=y" is not a concern here as you pick
this patch over adding a ipv6_stub?

Thanks.

Re: [PATCH net] tipc: not enable tipc when ipv6 works as a module

[Xin Long]

On Tue, Aug 18, 2020 at 6:20 AM Cong Wang <xiyou.wangcong@gmail.com> wrote:
>
> On Mon, Aug 17, 2020 at 2:39 PM David Miller <davem@davemloft.net> wrote:
> >
> > From: Cong Wang <xiyou.wangcong@gmail.com>
> > Date: Mon, 17 Aug 2020 13:59:46 -0700
> >
> > > Is this a new Kconfig feature? ipv6_stub was introduced for
> > > VXLAN, at that time I don't remember we have such kind of
> > > Kconfig rules, otherwise it would not be needed.
> >
> > The ipv6_stub exists in order to allow the troublesome
> > "ipv6=m && feature_using_ipv6=y" combination.
For certain code, instead of IS_ENABLE(), use IS_REACHABLE().

>
> Hmm, so "IPV6=m && TIPC=y" is not a concern here as you pick
> this patch over adding a ipv6_stub?
>
This is more a question for TIPC users.

Hi, Jon and Ying,

Have you met any users having "IPV6=m && TIPC=y" in their kernels?

Re: [PATCH net] tipc: not enable tipc when ipv6 works as a module

[David Miller]

From: Cong Wang <xiyou.wangcong@gmail.com>
Date: Mon, 17 Aug 2020 13:29:40 -0700

> On Mon, Aug 17, 2020 at 12:55 PM Randy Dunlap <rdunlap@infradead.org> wrote:
>>
>> TIPC=m and IPV6=m builds just fine.
>>
>> Having tipc autoload ipv6 is a different problem. (IMO)
>>
>>
>> This Kconfig entry:
>>  menuconfig TIPC
>>         tristate "The TIPC Protocol"
>>         depends on INET
>> +       depends on IPV6 || IPV6=n
>>
>> says:
>> If IPV6=n, TIPC can be y/m/n.
>> If IPV6=y/m, TIPC is limited to whatever IPV6 is set to.
> 
> Hmm, nowadays we _do_ have IPV6=y on popular distros.
> So this means TIPC would have to be builtin after this patch??

Note the word "limited", ipv6=y allows y and m, ipv6=m (more limited)
allows only m.

Re: [PATCH net] tipc: not enable tipc when ipv6 works as a module

[David Miller]

From: Cong Wang <xiyou.wangcong@gmail.com>
Date: Mon, 17 Aug 2020 11:55:55 -0700

> On Mon, Aug 17, 2020 at 11:49 AM Randy Dunlap <rdunlap@infradead.org> wrote:
>>
>> It just restricts how TIPC can be built, so that
>> TIPC=y and IPV6=m cannot happen together, which causes
>> a build error.
> 
> It also disallows TIPC=m and IPV6=m, right?

That combination is allowed.

The whole "X || X=n" construct means X must be off or equal to the
value of the Kconfig entry this dependency is for.

Thus you'll see "depends IPV6 || IPV6=n" everywhere.

Re: [PATCH net] tipc: not enable tipc when ipv6 works as a module

[David Miller]

From: Xin Long <lucien.xin@gmail.com>
Date: Sun, 16 Aug 2020 17:32:03 +0800

> When using ipv6_dev_find() in one module, it requires ipv6 not to
> work as a module. Otherwise, this error occurs in build:
> 
>   undefined reference to `ipv6_dev_find'.
> 
> So fix it by adding "depends on IPV6 || IPV6=n" to tipc/Kconfig,
> as it does in sctp/Kconfig.
> 
> Fixes: 5a6f6f579178 ("tipc: set ub->ifindex for local ipv6 address")
> Reported-by: kernel test robot <lkp@intel.com>
> Acked-by: Randy Dunlap <rdunlap@infradead.org>
> Signed-off-by: Xin Long <lucien.xin@gmail.com>

Applied.