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

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

[Leon Romanovsky]

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.

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) {
+		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

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Doug Ledford]

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

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

> +		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: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

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

[Leon Romanovsky]

[-- 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 --]

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

[Doug Ledford]

[-- Attachment #1.1: Type: text/plain, Size: 2489 bytes --]

On 1/25/2017 1:51 AM, Leon Romanovsky wrote:
> 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.

OK, that makes sense, I just hadn't seen it that way.  I'll add this to
the -rc queue.


-- 
Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
    GPG Key ID: B826A3330E572FDD
    Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 884 bytes --]