Re: [PATCH] RDMA/cma: Fix unknown symbol when CONFIG_IPV6 is not enabled

[Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>]

[-- Attachment #1: Type: text/plain, Size: 3218 bytes --]

On Tue, Jan 24, 2017 at 04:28:50PM -0500, Doug Ledford wrote:
> On Sun, 2017-01-15 at 20:15 +0200, Leon Romanovsky wrote:
> > From: Jack Morgenstein <jackm-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org>
> >
> > If IPV6 has not been enabled in the underlying kernel, we must avoid
> > calling IPV6 procedures in rdma_cm.ko.
> >
> > This requires using "IS_ENABLED(CONFIG_IPV6)" in "if" statements
> > surrounding any code which calls external IPV6 procedures.
>
> This seems strange....
>
> > In the instance fixed here, procedure cma_bind_addr() called
> > ipv6_addr_type() -- which resulted in calling external procedure
> > __ipv6_addr_type().
> >
> > Fixes: 6c26a77124ff ("RDMA/cma: fix IPv6 address resolution")
> > Cc: <stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org> # v4.2+
> > Cc: Spencer Baugh <sbaugh-n2IX9UM++eDQT0dZR+AlfA@public.gmane.org>
> > Signed-off-by: Jack Morgenstein <jackm-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org>
> > Reviewed-by: Moni Shoua <monis-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> > Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> > ---
> >  drivers/infiniband/core/cma.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/infiniband/core/cma.c
> > b/drivers/infiniband/core/cma.c
> > index bd8d051..e19f19c 100644
> > --- a/drivers/infiniband/core/cma.c
> > +++ b/drivers/infiniband/core/cma.c
> > @@ -2822,7 +2822,8 @@ static int cma_bind_addr(struct rdma_cm_id *id,
> > struct sockaddr *src_addr,
> >  	if (!src_addr || !src_addr->sa_family) {
> >  		src_addr = (struct sockaddr *) &id-
> > >route.addr.src_addr;
> >  		src_addr->sa_family = dst_addr->sa_family;
> > -		if (dst_addr->sa_family == AF_INET6) {
>
> Why this construct?  Isn't the norm to simply surround the entire if
> statement with
> #if IS_ENABLED(CONFIG_IPV6)
> ...
> #endif

It is common way to add dependency on specific config option directly to
the flow if other "if" already exists. It gives clear view on the flow,
eliminates the need to find corresponding #endif and put all constraints
in one place.

For example latest patches, bet there is a lot of code like this:
commit 7ede8665f27cde7da69e8b2fbeaa1ed0664879c5
Author: Alexander Popov <alex.popov-vYTEC60ixJUAvxtiuMwx3w@public.gmane.org>
Date:   Mon Dec 19 16:23:06 2016 -0800

    arm64: setup: introduce kaslr_offset()

commit b6f8a92c9ca835b4a079ecee8433d0d110398448
Author: Nicolas Pitre <nicolas.pitre-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Date:   Wed Dec 14 15:06:13 2016 -0800

    posix-timers: give lazy compilers some help optimizing code away


.....

>
> > +		if (IS_ENABLED(CONFIG_IPV6) &&
> > +		    dst_addr->sa_family == AF_INET6) {
> >  			struct sockaddr_in6 *src_addr6 = (struct
> > sockaddr_in6 *) src_addr;
> >  			struct sockaddr_in6 *dst_addr6 = (struct
> > sockaddr_in6 *) dst_addr;
> >  			src_addr6->sin6_scope_id = dst_addr6-
> > >sin6_scope_id;
> > --
> > 2.10.2
> >
> --
> Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>     GPG KeyID: B826A3330E572FDD
>    
> Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD



[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]