| Message ID | d20778039a791b9721bb449d493836edb742d1dc.1597570323.git.lucien.xin@gmail.com |
|---|---|
| State | Accepted |
| Delegated to: | David Miller |
| Headers | show |
| Series | [net] tipc: not enable tipc when ipv6 works as a module | expand |
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?
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.
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.
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.
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.
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.
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.
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.
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".
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.
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.
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.
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.
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.
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.
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.
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?
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
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(+)