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

PATCH In review Leon Romanovsky 2 participants 4 messages Jan 15, 2017 – Jan 26, 2017 3 messages not in the archive InfiniBand

lore

Share & tools

lore

Leon Romanovsky

author

2017-01-15 18:15 UTC #0 lore raw

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

From: Jack Morgenstein <[email protected]>

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 <[email protected]> # v4.2+

Cc Spencer Baugh <[email protected]>

Signed-off-by Jack Morgenstein <[email protected]>

Reviewed-by Moni Shoua <monis-VPRAkNaXOzVWk0Htik3J/[email protected]>

Signed-off-by Leon Romanovsky <[email protected]>

---
 drivers/infiniband/core/cma.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

drivers/infiniband/core/cma.c +2 -1

index bd8d051..e19f19c 100644
@@ -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 [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

A message that is not in the archive (sent off-list or not yet received): 20170115181500.4465-1-leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org

Doug Ledford

2017-01-24 21:28 UTC #2 lore raw

Quoted 7 lines · Leon Romanovsky

From: Jack Morgenstein <[email protected]>
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....

Quoted 25 lines

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 <[email protected]> # v4.2+
Cc Spencer Baugh <[email protected]>
Signed-off-by Jack Morgenstein <[email protected]>
Reviewed-by Moni Shoua <monis-VPRAkNaXOzVWk0Htik3J/[email protected]>
Signed-off-by Leon Romanovsky <[email protected]>
---
drivers/infiniband/core/cma.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
drivers/infiniband/core/cma.c +0 -0
index bd8d051..e19f19c 100644
@@ -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-
Quoted 1 line
route.addr.src_addr;
src_addr->sa_family = dst_addr->sa_family;

drivers/infiniband/core/cma.c static int cma_bind_addr(struct rdma_cm_id *id, struct sockaddr *src_addr, · line 2825

Quoted 1 line

drivers/infiniband/core/cma.c +0 -1
-		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

Quoted 11 lines

drivers/infiniband/core/cma.c +2 -0
+		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-
Quoted 1 line
sin6_scope_id;
--
2.10.2

Signature · 4 lines

Doug Ledford <[email protected]>
    GPG KeyID: B826A3330E572FDD
   
Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD

A message that is not in the archive (sent off-list or not yet received): 1485293330.43764.80.camel-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org

Leon Romanovsky

author

2017-01-25 06:51 UTC #4 lore raw

Quoted 35 lines · Doug Ledford

Quoted 7 lines · Leon Romanovsky
From: Jack Morgenstein <[email protected]>
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....
Quoted 24 lines
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 <[email protected]> # v4.2+
Cc Spencer Baugh <[email protected]>
Signed-off-by Jack Morgenstein <[email protected]>
Reviewed-by Moni Shoua <monis-VPRAkNaXOzVWk0Htik3J/[email protected]>
Signed-off-by Leon Romanovsky <[email protected]>
---
drivers/infiniband/core/cma.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
drivers/infiniband/core/cma.c +0 -0
index bd8d051..e19f19c 100644
@@ -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-
Quoted 1 line
route.addr.src_addr;

Quoted 8 lines

drivers/infiniband/core/cma.c static int cma_bind_addr(struct rdma_cm_id *id, struct sockaddr *src_addr, · line 2825
Quoted 2 lines
src_addr->sa_family = dst_addr->sa_family;
drivers/infiniband/core/cma.c +0 -1
-		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 <[email protected]>
Date:   Mon Dec 19 16:23:06 2016 -0800

arm64: setup: introduce kaslr_offset()

commit b6f8a92c9ca835b4a079ecee8433d0d110398448

Author: Nicolas Pitre <[email protected]>
Date:   Wed Dec 14 15:06:13 2016 -0800

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

.....

Quoted 17 lines

Quoted 8 lines
drivers/infiniband/core/cma.c +2 -0
+		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-
Quoted 4 lines
Quoted 1 line
sin6_scope_id;
--
2.10.2
--
Doug Ledford <[email protected]> GPG KeyID: B826A3330E572FDD

Key fingerprint = AE6B 1BDA 122B 23B4 265B 1274 B826 A333 0E57 2FDD

A message that is not in the archive (sent off-list or not yet received): 20170125065115.GM6005-U/[email protected]

Doug Ledford

2017-01-26 16:14 UTC #6 lore raw

Quoted 45 lines · Leon Romanovsky

Quoted 35 lines · Doug Ledford
Quoted 7 lines · Leon Romanovsky
From: Jack Morgenstein <[email protected]>
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....
Quoted 24 lines
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 <[email protected]> # v4.2+
Cc Spencer Baugh <[email protected]>
Signed-off-by Jack Morgenstein <[email protected]>
Reviewed-by Moni Shoua <monis-VPRAkNaXOzVWk0Htik3J/[email protected]>
Signed-off-by Leon Romanovsky <[email protected]>
---
 drivers/infiniband/core/cma.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
drivers/infiniband/core/cma.c +0 -0
index bd8d051..e19f19c 100644
@@ -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;
Quoted 8 lines
drivers/infiniband/core/cma.c static int cma_bind_addr(struct rdma_cm_id *id, struct sockaddr *src_addr, · line 2825
Quoted 2 lines
drivers/infiniband/core/cma.c +0 -1
 		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

Quoted 4 lines

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.

Signature · 3 lines

Doug Ledford <[email protected]>
    GPG Key ID: B826A3330E572FDD
    Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD