| Message ID | 1425976885-18258-1-git-send-email-ying.xue@windriver.com |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
On Tue, Mar 10, 2015 at 4:41 AM, Ying Xue <ying.xue@windriver.com> wrote: > When CONFIG_IPV6 option is disabled, below error will appear while > building TIPC module: > > ERROR: "__ipv6_sock_mc_join" [net/tipc/tipc.ko] undefined! > make[1]: *** [__modpost] Error 1 > make: *** [net/tipc/tipc.ko] Error 1 > > This is because we don't check whether or not the CONFIG_IPV6 is > enabled when calling __ipv6_sock_mc_join(). > > In addition, especially when TIPC=y, TIPC_MEDIA_UDP=y, and IPV6=m, TIPC > module is also unable to be successfully built. Therefore, we add a > dependency condition like (IPV6 || IPV6=n) to avoid the error. > > Fixes: d0f91938bede ("tipc: add ip/udp media type") > Reported-by: Wu Fengguang <fengguang.wu@intel.com> > Cc: Kbuild test robot <kbuild-all@01.org> > Signed-off-by: Ying Xue <ying.xue@windriver.com> > --- > v2: > Fix another compile error when TIPC=y, TIPC_MEDIA_UDP=y, and IPV6=m > > net/tipc/Kconfig | 1 + > net/tipc/udp_media.c | 2 ++ > 2 files changed, 3 insertions(+) > > diff --git a/net/tipc/Kconfig b/net/tipc/Kconfig > index c25a3a1..6f217ba 100644 > --- a/net/tipc/Kconfig > +++ b/net/tipc/Kconfig > @@ -5,6 +5,7 @@ > menuconfig TIPC > tristate "The TIPC Protocol" > depends on INET > + depends on (IPV6 || IPV6=n) Should this be limited to TIPC_MEDIA_UDP? > ---help--- > The Transparent Inter Process Communication (TIPC) protocol is > specially designed for intra cluster communication. This protocol > diff --git a/net/tipc/udp_media.c b/net/tipc/udp_media.c > index fc2fb11..6763002 100644 > --- a/net/tipc/udp_media.c > +++ b/net/tipc/udp_media.c > @@ -247,10 +247,12 @@ static int enable_mcast(struct udp_bearer *ub, struct udp_media_addr *remote) > mreqn.imr_multiaddr = remote->ipv4; > mreqn.imr_ifindex = ub->ifindex; > err = __ip_mc_join_group(sk, &mreqn); > +#if IS_ENABLED(CONFIG_IPV6) > } else { > if (!ipv6_addr_is_multicast(&remote->ipv6)) > return 0; > err = __ipv6_sock_mc_join(sk, ub->ifindex, &remote->ipv6); > +#endif > } > return err; It may also be prudent to initialize err to -EAFNOSUPPORT instead of 0. This function is currently only called when the protocol family has been checked, but relying on that is a bit fragile. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Willem de Bruijn <willemb@google.com> Date: Tue, 10 Mar 2015 11:41:03 -0400 > On Tue, Mar 10, 2015 at 4:41 AM, Ying Xue <ying.xue@windriver.com> wrote: >> @@ -5,6 +5,7 @@ >> menuconfig TIPC >> tristate "The TIPC Protocol" >> depends on INET >> + depends on (IPV6 || IPV6=n) > > Should this be limited to TIPC_MEDIA_UDP? I think it should. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello. On 03/10/2015 11:41 AM, Ying Xue wrote: > When CONFIG_IPV6 option is disabled, below error will appear while > building TIPC module: > ERROR: "__ipv6_sock_mc_join" [net/tipc/tipc.ko] undefined! > make[1]: *** [__modpost] Error 1 > make: *** [net/tipc/tipc.ko] Error 1 > This is because we don't check whether or not the CONFIG_IPV6 is > enabled when calling __ipv6_sock_mc_join(). > In addition, especially when TIPC=y, TIPC_MEDIA_UDP=y, and IPV6=m, TIPC > module is also unable to be successfully built. Therefore, we add a > dependency condition like (IPV6 || IPV6=n) to avoid the error. > Fixes: d0f91938bede ("tipc: add ip/udp media type") > Reported-by: Wu Fengguang <fengguang.wu@intel.com> > Cc: Kbuild test robot <kbuild-all@01.org> > Signed-off-by: Ying Xue <ying.xue@windriver.com> > --- > v2: > Fix another compile error when TIPC=y, TIPC_MEDIA_UDP=y, and IPV6=m > net/tipc/Kconfig | 1 + > net/tipc/udp_media.c | 2 ++ > 2 files changed, 3 insertions(+) > diff --git a/net/tipc/Kconfig b/net/tipc/Kconfig > index c25a3a1..6f217ba 100644 > --- a/net/tipc/Kconfig > +++ b/net/tipc/Kconfig > @@ -5,6 +5,7 @@ > menuconfig TIPC > tristate "The TIPC Protocol" > depends on INET > + depends on (IPV6 || IPV6=n) Parens not needed. [...] > diff --git a/net/tipc/udp_media.c b/net/tipc/udp_media.c > index fc2fb11..6763002 100644 > --- a/net/tipc/udp_media.c > +++ b/net/tipc/udp_media.c > @@ -247,10 +247,12 @@ static int enable_mcast(struct udp_bearer *ub, struct udp_media_addr *remote) > mreqn.imr_multiaddr = remote->ipv4; > mreqn.imr_ifindex = ub->ifindex; > err = __ip_mc_join_group(sk, &mreqn); > +#if IS_ENABLED(CONFIG_IPV6) > } else { How about: } else if (IS_ENABLED(CONFIG_IPV6)) { > if (!ipv6_addr_is_multicast(&remote->ipv6)) > return 0; > err = __ipv6_sock_mc_join(sk, ub->ifindex, &remote->ipv6); > +#endif > } > return err; > } WBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 03/11/2015 12:47 AM, David Miller wrote: > From: Willem de Bruijn <willemb@google.com> > Date: Tue, 10 Mar 2015 11:41:03 -0400 > >> On Tue, Mar 10, 2015 at 4:41 AM, Ying Xue <ying.xue@windriver.com> wrote: >>> @@ -5,6 +5,7 @@ >>> menuconfig TIPC >>> tristate "The TIPC Protocol" >>> depends on INET >>> + depends on (IPV6 || IPV6=n) >> >> Should this be limited to TIPC_MEDIA_UDP? > > I think it should. > > My first thought is also same as you. But when the dependency like (IPV6 || IPV6=n) is enforced to TIPC_MEDIA_UDP, it doesn't work in the below case: IPV6=m and TIPC=Y. In above condition, TIPC_MEDIA_UDP is still set to "Y". However, if I change the dependency to IPV6=y and IPV6=n, and allow it to limit TIPC_MEDIA_UDP, TIPC modules can be built successfully in kinds of cases. Thank for your review! Please wait a moment, and I will fix it in next version. Regards, Ying -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 03/11/2015 02:50 AM, Sergei Shtylyov wrote: > Hello. > > On 03/10/2015 11:41 AM, Ying Xue wrote: > >> When CONFIG_IPV6 option is disabled, below error will appear while >> building TIPC module: > >> ERROR: "__ipv6_sock_mc_join" [net/tipc/tipc.ko] undefined! >> make[1]: *** [__modpost] Error 1 >> make: *** [net/tipc/tipc.ko] Error 1 > >> This is because we don't check whether or not the CONFIG_IPV6 is >> enabled when calling __ipv6_sock_mc_join(). > >> In addition, especially when TIPC=y, TIPC_MEDIA_UDP=y, and IPV6=m, TIPC >> module is also unable to be successfully built. Therefore, we add a >> dependency condition like (IPV6 || IPV6=n) to avoid the error. > >> Fixes: d0f91938bede ("tipc: add ip/udp media type") >> Reported-by: Wu Fengguang <fengguang.wu@intel.com> >> Cc: Kbuild test robot <kbuild-all@01.org> >> Signed-off-by: Ying Xue <ying.xue@windriver.com> >> --- >> v2: >> Fix another compile error when TIPC=y, TIPC_MEDIA_UDP=y, and IPV6=m > >> net/tipc/Kconfig | 1 + >> net/tipc/udp_media.c | 2 ++ >> 2 files changed, 3 insertions(+) > >> diff --git a/net/tipc/Kconfig b/net/tipc/Kconfig >> index c25a3a1..6f217ba 100644 >> --- a/net/tipc/Kconfig >> +++ b/net/tipc/Kconfig >> @@ -5,6 +5,7 @@ >> menuconfig TIPC >> tristate "The TIPC Protocol" >> depends on INET >> + depends on (IPV6 || IPV6=n) > > Parens not needed. > > [...] >> diff --git a/net/tipc/udp_media.c b/net/tipc/udp_media.c >> index fc2fb11..6763002 100644 >> --- a/net/tipc/udp_media.c >> +++ b/net/tipc/udp_media.c >> @@ -247,10 +247,12 @@ static int enable_mcast(struct udp_bearer *ub, struct >> udp_media_addr *remote) >> mreqn.imr_multiaddr = remote->ipv4; >> mreqn.imr_ifindex = ub->ifindex; >> err = __ip_mc_join_group(sk, &mreqn); >> +#if IS_ENABLED(CONFIG_IPV6) >> } else { > > How about: > > } else if (IS_ENABLED(CONFIG_IPV6)) { > Sorry, I try to the suggestion, but it doesn't work. Thanks, Ying >> if (!ipv6_addr_is_multicast(&remote->ipv6)) >> return 0; >> err = __ipv6_sock_mc_join(sk, ub->ifindex, &remote->ipv6); >> +#endif >> } >> return err; >> } > > WBR, Sergei > > > -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 03/10/2015 11:41 PM, Willem de Bruijn wrote: >> l >> > diff --git a/net/tipc/udp_media.c b/net/tipc/udp_media.c >> > index fc2fb11..6763002 100644 >> > --- a/net/tipc/udp_media.c >> > +++ b/net/tipc/udp_media.c >> > @@ -247,10 +247,12 @@ static int enable_mcast(struct udp_bearer *ub, struct udp_media_addr *remote) > >> > mreqn.imr_multiaddr = remote->ipv4; >> > mreqn.imr_ifindex = ub->ifindex; >> > err = __ip_mc_join_group(sk, &mreqn); >> > +#if IS_ENABLED(CONFIG_IPV6) >> > } else { >> > if (!ipv6_addr_is_multicast(&remote->ipv6)) >> > return 0; >> > err = __ipv6_sock_mc_join(sk, ub->ifindex, &remote->ipv6); >> > +#endif >> > } >> > return err; > It may also be prudent to initialize err to -EAFNOSUPPORT instead of > 0. This function is currently only called when the protocol family has > been checked, but relying on that is a bit fragile. > > Thanks for your suggestion, and I will fix it in next version. Regards, Ying -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Ying Xue <ying.xue@windriver.com> Date: Wed, 11 Mar 2015 11:11:37 +0800 > On 03/11/2015 12:47 AM, David Miller wrote: >> From: Willem de Bruijn <willemb@google.com> >> Date: Tue, 10 Mar 2015 11:41:03 -0400 >> >>> On Tue, Mar 10, 2015 at 4:41 AM, Ying Xue <ying.xue@windriver.com> wrote: >>>> @@ -5,6 +5,7 @@ >>>> menuconfig TIPC >>>> tristate "The TIPC Protocol" >>>> depends on INET >>>> + depends on (IPV6 || IPV6=n) >>> >>> Should this be limited to TIPC_MEDIA_UDP? >> >> I think it should. >> >> > > My first thought is also same as you. But when the dependency like (IPV6 || > IPV6=n) is enforced to TIPC_MEDIA_UDP, it doesn't work in the below case: > > IPV6=m and TIPC=Y. > > In above condition, TIPC_MEDIA_UDP is still set to "Y". It's because TIPC_MEDIA_UDP is bool, it needs to be a tristate. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 03/11/2015 12:05 PM, David Miller wrote: > From: Ying Xue <ying.xue@windriver.com> > Date: Wed, 11 Mar 2015 11:11:37 +0800 > >> On 03/11/2015 12:47 AM, David Miller wrote: >>> From: Willem de Bruijn <willemb@google.com> >>> Date: Tue, 10 Mar 2015 11:41:03 -0400 >>> >>>> On Tue, Mar 10, 2015 at 4:41 AM, Ying Xue <ying.xue@windriver.com> wrote: >>>>> @@ -5,6 +5,7 @@ >>>>> menuconfig TIPC >>>>> tristate "The TIPC Protocol" >>>>> depends on INET >>>>> + depends on (IPV6 || IPV6=n) >>>> >>>> Should this be limited to TIPC_MEDIA_UDP? >>> >>> I think it should. >>> >>> >> >> My first thought is also same as you. But when the dependency like (IPV6 || >> IPV6=n) is enforced to TIPC_MEDIA_UDP, it doesn't work in the below case: >> >> IPV6=m and TIPC=Y. >> >> In above condition, TIPC_MEDIA_UDP is still set to "Y". > > It's because TIPC_MEDIA_UDP is bool, it needs to be a tristate. > Yes, you are right. But as we don't implement UDP bearer as a module, I guess we have to set TIPC_MEDIA_UDP as boolean type. Regards, Ying > -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello. On 3/11/2015 6:34 AM, Ying Xue wrote: >>> When CONFIG_IPV6 option is disabled, below error will appear while >>> building TIPC module: >>> ERROR: "__ipv6_sock_mc_join" [net/tipc/tipc.ko] undefined! >>> make[1]: *** [__modpost] Error 1 >>> make: *** [net/tipc/tipc.ko] Error 1 >>> This is because we don't check whether or not the CONFIG_IPV6 is >>> enabled when calling __ipv6_sock_mc_join(). >>> In addition, especially when TIPC=y, TIPC_MEDIA_UDP=y, and IPV6=m, TIPC >>> module is also unable to be successfully built. Therefore, we add a >>> dependency condition like (IPV6 || IPV6=n) to avoid the error. >>> Fixes: d0f91938bede ("tipc: add ip/udp media type") >>> Reported-by: Wu Fengguang <fengguang.wu@intel.com> >>> Cc: Kbuild test robot <kbuild-all@01.org> >>> Signed-off-by: Ying Xue <ying.xue@windriver.com> >>> --- >>> v2: >>> Fix another compile error when TIPC=y, TIPC_MEDIA_UDP=y, and IPV6=m [...] >>> diff --git a/net/tipc/udp_media.c b/net/tipc/udp_media.c >>> index fc2fb11..6763002 100644 >>> --- a/net/tipc/udp_media.c >>> +++ b/net/tipc/udp_media.c >>> @@ -247,10 +247,12 @@ static int enable_mcast(struct udp_bearer *ub, struct >>> udp_media_addr *remote) >>> mreqn.imr_multiaddr = remote->ipv4; >>> mreqn.imr_ifindex = ub->ifindex; >>> err = __ip_mc_join_group(sk, &mreqn); >>> +#if IS_ENABLED(CONFIG_IPV6) >>> } else { >> How about: >> } else if (IS_ENABLED(CONFIG_IPV6)) { > Sorry, I try to the suggestion, but it doesn't work. You mean that the linking error you reported doesn't go away? > Thanks, > Ying >>> if (!ipv6_addr_is_multicast(&remote->ipv6)) >>> return 0; >>> err = __ipv6_sock_mc_join(sk, ub->ifindex, &remote->ipv6); >>> +#endif >>> } >>> return err; >>> } WBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/net/tipc/Kconfig b/net/tipc/Kconfig index c25a3a1..6f217ba 100644 --- a/net/tipc/Kconfig +++ b/net/tipc/Kconfig @@ -5,6 +5,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 diff --git a/net/tipc/udp_media.c b/net/tipc/udp_media.c index fc2fb11..6763002 100644 --- a/net/tipc/udp_media.c +++ b/net/tipc/udp_media.c @@ -247,10 +247,12 @@ static int enable_mcast(struct udp_bearer *ub, struct udp_media_addr *remote) mreqn.imr_multiaddr = remote->ipv4; mreqn.imr_ifindex = ub->ifindex; err = __ip_mc_join_group(sk, &mreqn); +#if IS_ENABLED(CONFIG_IPV6) } else { if (!ipv6_addr_is_multicast(&remote->ipv6)) return 0; err = __ipv6_sock_mc_join(sk, ub->ifindex, &remote->ipv6); +#endif } return err; }
When CONFIG_IPV6 option is disabled, below error will appear while building TIPC module: ERROR: "__ipv6_sock_mc_join" [net/tipc/tipc.ko] undefined! make[1]: *** [__modpost] Error 1 make: *** [net/tipc/tipc.ko] Error 1 This is because we don't check whether or not the CONFIG_IPV6 is enabled when calling __ipv6_sock_mc_join(). In addition, especially when TIPC=y, TIPC_MEDIA_UDP=y, and IPV6=m, TIPC module is also unable to be successfully built. Therefore, we add a dependency condition like (IPV6 || IPV6=n) to avoid the error. Fixes: d0f91938bede ("tipc: add ip/udp media type") Reported-by: Wu Fengguang <fengguang.wu@intel.com> Cc: Kbuild test robot <kbuild-all@01.org> Signed-off-by: Ying Xue <ying.xue@windriver.com> --- v2: Fix another compile error when TIPC=y, TIPC_MEDIA_UDP=y, and IPV6=m net/tipc/Kconfig | 1 + net/tipc/udp_media.c | 2 ++ 2 files changed, 3 insertions(+)