[PATCH 1/3] libibumad: Support arbitrary number of IB devices

[PATCH 1/3] libibumad: Support arbitrary number of IB devices

[Haim Boozaglo]

From: Vladimir Koushnir <vladimirk@mellanox.com>

Added new function returning a list of available InfiniBand device names.
The returned list is not limited to 32 devices.

Signed-off-by: Vladimir Koushnir <vladimirk@mellanox.com>
Signed-off-by: Haim Boozaglo <haimbo@mellanox.com>
---
 libibumad/CMakeLists.txt              |  2 +-
 libibumad/libibumad.map               |  6 +++++
 libibumad/man/umad_free_ca_namelist.3 | 28 ++++++++++++++++++++++++
 libibumad/man/umad_get_ca_namelist.3  | 34 +++++++++++++++++++++++++++++
 libibumad/umad.c                      | 41 +++++++++++++++++++++++++++++++++++
 libibumad/umad.h                      |  2 ++
 6 files changed, 112 insertions(+), 1 deletion(-)
 create mode 100644 libibumad/man/umad_free_ca_namelist.3
 create mode 100644 libibumad/man/umad_get_ca_namelist.3

diff --git a/libibumad/CMakeLists.txt b/libibumad/CMakeLists.txt
index 1f600a0..9d0a425 100644
--- a/libibumad/CMakeLists.txt
+++ b/libibumad/CMakeLists.txt
@@ -10,7 +10,7 @@ publish_headers(infiniband
 
 rdma_library(ibumad libibumad.map
   # See Documentation/versioning.md
-  3 3.0.${PACKAGE_VERSION}
+  3 3.1.${PACKAGE_VERSION}
   sysfs.c
   umad.c
   umad_str.c
diff --git a/libibumad/libibumad.map b/libibumad/libibumad.map
index 8bf474e..1b8e7e1 100644
--- a/libibumad/libibumad.map
+++ b/libibumad/libibumad.map
@@ -39,3 +39,9 @@ IBUMAD_1.0 {
 		umad_attribute_str;
 	local: *;
 };
+
+IBUMAD_1.1 {
+	global:
+		umad_get_ca_namelist;
+		umad_free_ca_namelist;
+} IBUMAD_1.0;
diff --git a/libibumad/man/umad_free_ca_namelist.3 b/libibumad/man/umad_free_ca_namelist.3
new file mode 100644
index 0000000..f15958b
--- /dev/null
+++ b/libibumad/man/umad_free_ca_namelist.3
@@ -0,0 +1,28 @@
+.\" -*- nroff -*-
+.\"
+.TH UMAD_FREE_CA_NAMELIST 3  "May 1, 2018" "OpenIB" "OpenIB Programmer\'s Manual"
+.SH "NAME"
+umad_free_ca_namelist \- free InfiniBand devices name list
+.SH "SYNOPSIS"
+.nf
+.B #include <infiniband/umad.h>
+.sp
+.BI "void umad_free_ca_namelist(char " "*cas" );
+.fi
+.SH "DESCRIPTION"
+.B umad_get_free_namelist()
+frees the InfiniBand devices name list previously allocated with
+.B umad_get_ca_namelist()\fR.
+The argument
+.I cas
+is a character array of InfiniBand devices names
+.SH "RETURN VALUE"
+.B umad_free_ca_namelist()
+returns no value.
+.SH "SEE ALSO"
+.BR umad_get_ca_namelist(3)
+.SH "AUTHORS"
+.TP
+Vladimir Koushnir <vladimirk@mellanox.com>
+.TP
+Hal Rosenstock <hal@mellanox.com>
diff --git a/libibumad/man/umad_get_ca_namelist.3 b/libibumad/man/umad_get_ca_namelist.3
new file mode 100644
index 0000000..5b30209
--- /dev/null
+++ b/libibumad/man/umad_get_ca_namelist.3
@@ -0,0 +1,34 @@
+.\" -*- nroff -*-
+.\"
+.TH UMAD_GET_CA_NAMELIST 3  "May 1, 2018" "OpenIB" "OpenIB Programmer\'s Manual"
+.SH "NAME"
+umad_get_ca_namelist \- get list of available InfiniBand device names
+.SH "SYNOPSIS"
+.nf
+.B #include <infiniband/umad.h>
+.sp
+.BI "int umad_get_ca_namelist(char " "**cas" );
+.fi
+.SH "DESCRIPTION"
+.B umad_get_ca_namelist()
+fills the
+.I cas
+array with arbitrary number of local IB devices (CAs) names.
+The argument
+.I cas
+is a character array that will be allocated by the function to include number of entries, each with
+.B UMAD_CA_NAME_LEN
+characters.
+.SH "RETURN VALUE"
+.B umad_get_ca_namelist()
+returns a non-negative value equal to the number of entries filled,
+or \-1 on errors.
+.SH "SEE ALSO"
+.BR umad_get_ca_portguids (3),
+.BR umad_open_port (3)
+.BR umad_free_ca_namelist(3)
+.SH "AUTHORS"
+.TP
+Vladimir Koushnir <vladimirk@mellanox.com>
+.TP
+Hal Rosenstock <hal@mellanox.com>
diff --git a/libibumad/umad.c b/libibumad/umad.c
index 5f8656e..9d0303b 100644
--- a/libibumad/umad.c
+++ b/libibumad/umad.c
@@ -1123,3 +1123,44 @@ void umad_dump(void *umad)
 	       mad->agent_id, mad->status, mad->timeout_ms);
 	umad_addr_dump(&mad->addr);
 }
+
+int umad_get_ca_namelist(char **cas)
+{
+	struct dirent **namelist;
+	int n, i, j = 0;
+
+	n = scandir(SYS_INFINIBAND, &namelist, NULL, alphasort);
+
+	if (n > 0) {
+		*cas = (char *) calloc(1, n * sizeof(char) * UMAD_CA_NAME_LEN);
+		for (i = 0; i < n; i++) {
+			if (*cas && strcmp(namelist[i]->d_name, ".") &&
+			    strcmp(namelist[i]->d_name, "..")) {
+				if (is_ib_type(namelist[i]->d_name)) {
+					strncpy(*cas + j * UMAD_CA_NAME_LEN,
+						namelist[i]->d_name,
+						UMAD_CA_NAME_LEN);
+					j++;
+				}
+			}
+			free(namelist[i]);
+		}
+		DEBUG("return %d cas", j);
+	} else {
+		/* Is this still needed ? */
+		if ((*cas = calloc(1, UMAD_CA_NAME_LEN * sizeof(char)))) {
+			strncpy(*cas, def_ca_name, UMAD_CA_NAME_LEN);
+			DEBUG("return 1 ca");
+			j = 1;
+		}
+	}
+	if (n >= 0)
+		free(namelist);
+
+	return j;
+}
+
+void umad_free_ca_namelist(char *cas)
+{
+	free(cas);
+}
diff --git a/libibumad/umad.h b/libibumad/umad.h
index 3cc551f..70bc213 100644
--- a/libibumad/umad.h
+++ b/libibumad/umad.h
@@ -208,6 +208,8 @@ int umad_register(int portid, int mgmt_class, int mgmt_version,
 int umad_register_oui(int portid, int mgmt_class, uint8_t rmpp_version,
 		      uint8_t oui[3], long method_mask[16 / sizeof(long)]);
 int umad_unregister(int portid, int agentid);
+int umad_get_ca_namelist(char **cas);
+void umad_free_ca_namelist(char *cas);
 
 enum {
 	UMAD_USER_RMPP = (1 << 0)
-- 
1.8.3.1

[PATCH 2/3] libibumad: Redesign resolve_ca_name to support arbitrary number of IB devices

[Haim Boozaglo]

From: Vladimir Koushnir <vladimirk@mellanox.com>

resolve_ca_name is now based on the new umad_get_ca_namelist API
rather than the old umad_get_cas_names API.

Signed-off-by: Vladimir Koushnir <vladimirk@mellanox.com>
Signed-off-by: Haim Boozaglo <haimbo@mellanox.com>
---
 libibumad/umad.c | 100 +++++++++++++++++++++++++++++++++++--------------------
 1 file changed, 64 insertions(+), 36 deletions(-)

diff --git a/libibumad/umad.c b/libibumad/umad.c
index 9d0303b..0fdf85e 100644
--- a/libibumad/umad.c
+++ b/libibumad/umad.c
@@ -331,42 +331,52 @@ Exit:
 	return ret;
 }
 
-static const char *resolve_ca_name(const char *ca_name, int *best_port)
+static int resolve_ca_name(const char *ca_in, int *best_port,
+			   char ca_name[UMAD_CA_NAME_LEN])
 {
-	static char names[UMAD_MAX_DEVICES][UMAD_CA_NAME_LEN];
+	char *names;
+	char *name_found;
 	int phys_found = -1, port_found = 0, port, port_type;
 	int caidx, n;
 
-	if (ca_name && (!best_port || *best_port))
-		return ca_name;
+	if (ca_in && (!best_port || *best_port)) {
+		strncpy(ca_name, ca_in, UMAD_CA_NAME_LEN);
+		return 0;
+	}
 
-	if (ca_name) {
-		if (resolve_ca_port(ca_name, best_port) < 0)
-			return NULL;
-		return ca_name;
+	if (ca_in) {
+		if (resolve_ca_port(ca_in, best_port) < 0)
+			return -1;
+		strncpy(ca_name, ca_in, UMAD_CA_NAME_LEN);
+		return 0;
 	}
 
 	/* Get the list of CA names */
-	if ((n = umad_get_cas_names((void *)names, UMAD_MAX_DEVICES)) < 0)
-		return NULL;
+	if ((n = umad_get_ca_namelist(&names)) < 0)
+		return -1;
 
 	/* Find the first existing CA with an active port */
 	for (caidx = 0; caidx < n; caidx++) {
-		TRACE("checking ca '%s'", names[caidx]);
+		name_found = &names[caidx * UMAD_CA_NAME_LEN];
+
+		TRACE("checking ca '%s'", name_found);
 
 		port = best_port ? *best_port : 0;
-		if ((port_type = resolve_ca_port(names[caidx], &port)) < 0)
+		if ((port_type = resolve_ca_port(name_found,
+						 &port)) < 0)
 			continue;
 
 		DEBUG("found ca %s with port %d type %d",
-		      names[caidx], port, port_type);
+		      name_found, port, port_type);
 
 		if (port_type > 0) {
 			if (best_port)
 				*best_port = port;
 			DEBUG("found ca %s with active port %d",
-			      names[caidx], port);
-			return (char *)(names + caidx);
+			      name_found, port);
+			strncpy(ca_name, name_found, UMAD_CA_NAME_LEN);
+			umad_free_ca_namelist(names);
+			return 0;
 		}
 
 		if (phys_found == -1) {
@@ -376,17 +386,30 @@ static const char *resolve_ca_name(const char *ca_name, int *best_port)
 	}
 
 	DEBUG("phys found %d on %s port %d",
-	      phys_found, phys_found >= 0 ? names[phys_found] : NULL,
+	      phys_found,
+	      phys_found >= 0 ? &names[phys_found *  UMAD_CA_NAME_LEN] : NULL,
 	      port_found);
+
 	if (phys_found >= 0) {
+		name_found = &names[phys_found * UMAD_CA_NAME_LEN];
+		DEBUG("phys found %d on %s port %d",
+			phys_found,
+			phys_found >= 0 ? name_found : NULL,
+			port_found);
 		if (best_port)
 			*best_port = port_found;
-		return names[phys_found];
+		strncpy(ca_name, name_found, UMAD_CA_NAME_LEN);
+		umad_free_ca_namelist(names);
+		return 0;
 	}
 
+	umad_free_ca_namelist(names);
+
 	if (best_port)
 		*best_port = def_ca_port;
-	return def_ca_name;
+
+	strncpy(ca_name, def_ca_name, UMAD_CA_NAME_LEN);
+	return 0;
 }
 
 static int get_ca(const char *ca_name, umad_ca_t * ca)
@@ -577,16 +600,17 @@ int umad_get_cas_names(char cas[][UMAD_CA_NAME_LEN], int max)
 	return j;
 }
 
-int umad_get_ca_portguids(const char *ca_name, __be64 *portguids, int max)
+int umad_get_ca_portguids(const char *ca_name, __be64 * portguids, int max)
 {
 	umad_ca_t ca;
 	int ports = 0, i;
+	char found_ca_name[UMAD_CA_NAME_LEN];
 
 	TRACE("ca name %s max port guids %d", ca_name, max);
-	if (!(ca_name = resolve_ca_name(ca_name, NULL)))
+	if (resolve_ca_name(ca_name, NULL, found_ca_name) < 0)
 		return -ENODEV;
 
-	if (umad_get_ca(ca_name, &ca) < 0)
+	if (umad_get_ca(found_ca_name, &ca) < 0)
 		return -1;
 
 	if (portguids) {
@@ -596,12 +620,12 @@ int umad_get_ca_portguids(const char *ca_name, __be64 *portguids, int max)
 		}
 
 		for (i = 0; i <= ca.numports; i++)
-			portguids[ports++] = ca.ports[i] ?
-				ca.ports[i]->port_guid : htobe64(0);
+			portguids[ports++] =
+			    ca.ports[i] ? ca.ports[i]->port_guid : 0;
 	}
 
 	release_ca(&ca);
-	DEBUG("%s: %d ports", ca_name, ports);
+	DEBUG("%s: %d ports", found_ca_name, ports);
 
 	return ports;
 }
@@ -609,13 +633,14 @@ int umad_get_ca_portguids(const char *ca_name, __be64 *portguids, int max)
 int umad_get_issm_path(const char *ca_name, int portnum, char path[], int max)
 {
 	int umad_id;
+	char found_ca_name[UMAD_CA_NAME_LEN];
 
 	TRACE("ca %s port %d", ca_name, portnum);
 
-	if (!(ca_name = resolve_ca_name(ca_name, &portnum)))
+	if (resolve_ca_name(ca_name, &portnum, found_ca_name) < 0)
 		return -ENODEV;
 
-	if ((umad_id = dev_to_umad_id(ca_name, portnum)) < 0)
+	if ((umad_id = dev_to_umad_id(found_ca_name, portnum)) < 0)
 		return -EINVAL;
 
 	snprintf(path, max, "%s/issm%u", RDMA_CDEV_DIR, umad_id);
@@ -628,18 +653,19 @@ int umad_open_port(const char *ca_name, int portnum)
 	char dev_file[UMAD_DEV_FILE_SZ];
 	int umad_id, fd;
 	unsigned int abi_version = get_abi_version();
+	char found_ca_name[UMAD_CA_NAME_LEN];
 
 	TRACE("ca %s port %d", ca_name, portnum);
 
 	if (!abi_version)
 		return -EOPNOTSUPP;
 
-	if (!(ca_name = resolve_ca_name(ca_name, &portnum)))
+	if (resolve_ca_name(ca_name, &portnum, found_ca_name) < 0 )
 		return -ENODEV;
 
-	DEBUG("opening %s port %d", ca_name, portnum);
+	DEBUG("opening %s port %d", found_ca_name, portnum);
 
-	if ((umad_id = dev_to_umad_id(ca_name, portnum)) < 0)
+	if ((umad_id = dev_to_umad_id(found_ca_name, portnum)) < 0)
 		return -EINVAL;
 
 	snprintf(dev_file, sizeof(dev_file), "%s/umad%d",
@@ -662,18 +688,19 @@ int umad_open_port(const char *ca_name, int portnum)
 int umad_get_ca(const char *ca_name, umad_ca_t * ca)
 {
 	int r;
+	char found_ca_name[UMAD_CA_NAME_LEN];
 
 	TRACE("ca_name %s", ca_name);
-	if (!(ca_name = resolve_ca_name(ca_name, NULL)))
+	if (resolve_ca_name(ca_name, NULL, found_ca_name) < 0)
 		return -ENODEV;
 
-	if (find_cached_ca(ca_name, ca) > 0)
+	if (find_cached_ca(found_ca_name, ca) > 0)
 		return 0;
 
-	if ((r = get_ca(ca_name, ca)) < 0)
+	if ((r = get_ca(found_ca_name, ca)) < 0)
 		return r;
 
-	DEBUG("opened %s", ca_name);
+	DEBUG("opened %s", found_ca_name);
 	return 0;
 }
 
@@ -695,16 +722,17 @@ int umad_release_ca(umad_ca_t * ca)
 int umad_get_port(const char *ca_name, int portnum, umad_port_t * port)
 {
 	char dir_name[256];
+	char found_ca_name[UMAD_CA_NAME_LEN];
 
 	TRACE("ca_name %s portnum %d", ca_name, portnum);
 
-	if (!(ca_name = resolve_ca_name(ca_name, &portnum)))
+	if (resolve_ca_name(ca_name, &portnum, found_ca_name) < 0)
 		return -ENODEV;
 
 	snprintf(dir_name, sizeof(dir_name), "%s/%s/%s",
-		 SYS_INFINIBAND, ca_name, SYS_CA_PORTS_DIR);
+		 SYS_INFINIBAND, found_ca_name, SYS_CA_PORTS_DIR);
 
-	return get_port(ca_name, dir_name, portnum, port);
+	return get_port(found_ca_name, dir_name, portnum, port);
 }
 
 int umad_release_port(umad_port_t * port)
-- 
1.8.3.1

[PATCH 3/3] ibdiags: Support arbitrary number of IB devices in ibstat

[Haim Boozaglo]

From: Vladimir Koushnir <vladimirk@mellanox.com>

Allow showing more than 32 IB devices on the server.
umad_get_ca_namelist API is used for this purpose.

Signed-off-by: Vladimir Koushnir <vladimirk@mellanox.com>
Signed-off-by: Haim Boozaglo <haimbo@mellanox.com>
---
 infiniband-diags/ibstat.c | 47 ++++++++++++++++++++++++++---------------------
 1 file changed, 26 insertions(+), 21 deletions(-)

diff --git a/infiniband-diags/ibstat.c b/infiniband-diags/ibstat.c
index 4601f2a..9c87ff8 100644
--- a/infiniband-diags/ibstat.c
+++ b/infiniband-diags/ibstat.c
@@ -232,23 +232,22 @@ static int ca_stat(char *ca_name, int portnum, int no_ports)
 	return 0;
 }
 
-static int ports_list(char names[][UMAD_CA_NAME_LEN], int n)
+static int ports_list(char *names, int n)
 {
 	__be64 guids[64];
-	int found, ports, i;
+	int ports, i, j;
 
-	for (i = 0, found = 0; i < n && found < 64; i++) {
+	for (i = 0; i < n ; i++) {
 		if ((ports =
-		     umad_get_ca_portguids(names[i], guids + found,
-					   64 - found)) < 0)
+		     umad_get_ca_portguids(&names[i * UMAD_CA_NAME_LEN],
+					   &guids[0], 64)) < 0)
 			return -1;
-		found += ports;
-	}
 
-	for (i = 0; i < found; i++)
-		if (guids[i])
-			printf("0x%016" PRIx64 "\n", be64toh(guids[i]));
-	return found;
+		for (j = 0; j < ports; j++)
+			if (guids[j])
+				printf("0x%016" PRIx64 "\n", be64toh(guids[j]));
+	}
+	return 0;
 }
 
 static int list_only, short_format, list_ports;
@@ -273,9 +272,10 @@ static int process_opt(void *context, int ch)
 
 int main(int argc, char *argv[])
 {
-	char names[UMAD_MAX_DEVICES][UMAD_CA_NAME_LEN];
+	char *names;
 	int dev_port = -1;
 	int n, i;
+	char *nptr;
 
 	const struct ibdiag_opt opts[] = {
 		{"list_of_cas", 'l', 0, NULL, "list all IB devices"},
@@ -302,33 +302,38 @@ int main(int argc, char *argv[])
 	if (umad_init() < 0)
 		IBPANIC("can't init UMAD library");
 
-	if ((n = umad_get_cas_names(names, UMAD_MAX_DEVICES)) < 0)
+	if ((n = umad_get_ca_namelist(&names)) < 0)
 		IBPANIC("can't list IB device names");
 
 	if (argc) {
 		for (i = 0; i < n; i++)
-			if (!strncmp(names[i], argv[0], sizeof names[i]))
+			if (!strncmp(&names[i * UMAD_CA_NAME_LEN],
+				     argv[0], UMAD_CA_NAME_LEN))
 				break;
 		if (i >= n)
 			IBPANIC("'%s' IB device can't be found", argv[0]);
-
-		strncpy(names[0], argv[0], sizeof(names[0])-1);
-		names[0][sizeof(names[0])-1] = '\0';
+		if (i != 0) {
+			strncpy(&names[0],
+				&names[i * UMAD_CA_NAME_LEN],
+				UMAD_CA_NAME_LEN);
+		}
 		n = 1;
 	}
 
 	if (list_ports) {
 		if (ports_list(names, n) < 0)
 			IBPANIC("can't list ports");
+		umad_free_ca_namelist(names);
 		return 0;
 	}
 
 	for (i = 0; i < n; i++) {
+		nptr = &names[i * UMAD_CA_NAME_LEN];
 		if (list_only)
-			printf("%s\n", names[i]);
-		else if (ca_stat(names[i], dev_port, short_format) < 0)
-			IBPANIC("stat of IB device '%s' failed", names[i]);
+			printf("%s\n", nptr);
+		else if (ca_stat(nptr, dev_port, short_format) < 0)
+			IBPANIC("stat of IB device '%s' failed", nptr);
 	}
-
+	umad_free_ca_namelist(names);
 	return 0;
 }
-- 
1.8.3.1

Re: [PATCH 1/3] libibumad: Support arbitrary number of IB devices

[Leon Romanovsky]

On Tue, Aug 06, 2019 at 02:38:52PM +0000, Haim Boozaglo wrote:
> From: Vladimir Koushnir <vladimirk@mellanox.com>
>
> Added new function returning a list of available InfiniBand device names.
> The returned list is not limited to 32 devices.
>
> Signed-off-by: Vladimir Koushnir <vladimirk@mellanox.com>
> Signed-off-by: Haim Boozaglo <haimbo@mellanox.com>
> ---
>  libibumad/CMakeLists.txt              |  2 +-
>  libibumad/libibumad.map               |  6 +++++
>  libibumad/man/umad_free_ca_namelist.3 | 28 ++++++++++++++++++++++++
>  libibumad/man/umad_get_ca_namelist.3  | 34 +++++++++++++++++++++++++++++
>  libibumad/umad.c                      | 41 +++++++++++++++++++++++++++++++++++
>  libibumad/umad.h                      |  2 ++
>  6 files changed, 112 insertions(+), 1 deletion(-)
>  create mode 100644 libibumad/man/umad_free_ca_namelist.3
>  create mode 100644 libibumad/man/umad_get_ca_namelist.3

1. Please use cover letter for patch series.
2. There is a need to update debian package too.
3. Need to use the same discovery mechanism as used in libibverbs - netlink based.
4. Does it work with hot-plug where device name can change?

Thanks

RE: [PATCH 1/3] libibumad: Support arbitrary number of IB devices

[Vladimir Koushnir]

Leon,

For comment #3:
We are not planning to change implementation of libibumad. 
The patches were developed with existing libibumad code and extending existing capability to provide list of devices on the node beyond 32 devocs

For comment #4:
Hot-plug is out-of-scope of the libibumad as no persistent data is maintained by libibumad.

-----Original Message-----
From: Leon Romanovsky <leon@kernel.org> 
Sent: Tuesday, August 6, 2019 6:52 PM
To: Haim Boozaglo <haimbo@mellanox.com>
Cc: linux-rdma@vger.kernel.org; Vladimir Koushnir <vladimirk@mellanox.com>
Subject: Re: [PATCH 1/3] libibumad: Support arbitrary number of IB devices

On Tue, Aug 06, 2019 at 02:38:52PM +0000, Haim Boozaglo wrote:
> From: Vladimir Koushnir <vladimirk@mellanox.com>
>
> Added new function returning a list of available InfiniBand device names.
> The returned list is not limited to 32 devices.
>
> Signed-off-by: Vladimir Koushnir <vladimirk@mellanox.com>
> Signed-off-by: Haim Boozaglo <haimbo@mellanox.com>
> ---
>  libibumad/CMakeLists.txt              |  2 +-
>  libibumad/libibumad.map               |  6 +++++
>  libibumad/man/umad_free_ca_namelist.3 | 28 ++++++++++++++++++++++++
>  libibumad/man/umad_get_ca_namelist.3  | 34 +++++++++++++++++++++++++++++
>  libibumad/umad.c                      | 41 +++++++++++++++++++++++++++++++++++
>  libibumad/umad.h                      |  2 ++
>  6 files changed, 112 insertions(+), 1 deletion(-)  create mode 100644 
> libibumad/man/umad_free_ca_namelist.3
>  create mode 100644 libibumad/man/umad_get_ca_namelist.3

1. Please use cover letter for patch series.
2. There is a need to update debian package too.
3. Need to use the same discovery mechanism as used in libibverbs - netlink based.
4. Does it work with hot-plug where device name can change?

Thanks

Re: [PATCH 1/3] libibumad: Support arbitrary number of IB devices

[Leon Romanovsky]

On Wed, Aug 07, 2019 at 07:26:35AM +0000, Vladimir Koushnir wrote:
> Leon,
>
> For comment #3:
> We are not planning to change implementation of libibumad.
> The patches were developed with existing libibumad code and extending existing capability to provide list of devices on the node beyond 32 devocs

You are proposing patches for inclusion in upstream rdma-core and
our request is to use the same functionality as already available
in rdma-core. We don't ask you to rewrite existing libibumad code,
but don't submit wrong code.

>
> For comment #4:
> Hot-plug is out-of-scope of the libibumad as no persistent data is maintained by libibumad.

Fair enough.

Thanks

>
> -----Original Message-----
> From: Leon Romanovsky <leon@kernel.org>
> Sent: Tuesday, August 6, 2019 6:52 PM
> To: Haim Boozaglo <haimbo@mellanox.com>
> Cc: linux-rdma@vger.kernel.org; Vladimir Koushnir <vladimirk@mellanox.com>
> Subject: Re: [PATCH 1/3] libibumad: Support arbitrary number of IB devices
>
> On Tue, Aug 06, 2019 at 02:38:52PM +0000, Haim Boozaglo wrote:
> > From: Vladimir Koushnir <vladimirk@mellanox.com>
> >
> > Added new function returning a list of available InfiniBand device names.
> > The returned list is not limited to 32 devices.
> >
> > Signed-off-by: Vladimir Koushnir <vladimirk@mellanox.com>
> > Signed-off-by: Haim Boozaglo <haimbo@mellanox.com>
> > ---
> >  libibumad/CMakeLists.txt              |  2 +-
> >  libibumad/libibumad.map               |  6 +++++
> >  libibumad/man/umad_free_ca_namelist.3 | 28 ++++++++++++++++++++++++
> >  libibumad/man/umad_get_ca_namelist.3  | 34 +++++++++++++++++++++++++++++
> >  libibumad/umad.c                      | 41 +++++++++++++++++++++++++++++++++++
> >  libibumad/umad.h                      |  2 ++
> >  6 files changed, 112 insertions(+), 1 deletion(-)  create mode 100644
> > libibumad/man/umad_free_ca_namelist.3
> >  create mode 100644 libibumad/man/umad_get_ca_namelist.3
>
> 1. Please use cover letter for patch series.
> 2. There is a need to update debian package too.
> 3. Need to use the same discovery mechanism as used in libibverbs - netlink based.
> 4. Does it work with hot-plug where device name can change?
>
> Thanks

RE: [PATCH 1/3] libibumad: Support arbitrary number of IB devices

[Vladimir Koushnir]

After discussing with Leon, libibumad and probably infiniband-diags should be redesigned to follow new rdma-core policy of using netlink API.

This redesign will be added to the POR and will be provided on top of provided patches.

So in short-term, the patches will be accepted after fixing comments #1-#2.
In long-term, libibumad support for netlink API will be provided.

-----Original Message-----
From: Leon Romanovsky <leon@kernel.org> 
Sent: Wednesday, August 7, 2019 10:45 AM
To: Vladimir Koushnir <vladimirk@mellanox.com>
Cc: Haim Boozaglo <haimbo@mellanox.com>; linux-rdma@vger.kernel.org
Subject: Re: [PATCH 1/3] libibumad: Support arbitrary number of IB devices

On Wed, Aug 07, 2019 at 07:26:35AM +0000, Vladimir Koushnir wrote:
> Leon,
>
> For comment #3:
> We are not planning to change implementation of libibumad.
> The patches were developed with existing libibumad code and extending 
> existing capability to provide list of devices on the node beyond 32 
> devocs

You are proposing patches for inclusion in upstream rdma-core and our request is to use the same functionality as already available in rdma-core. We don't ask you to rewrite existing libibumad code, but don't submit wrong code.

>
> For comment #4:
> Hot-plug is out-of-scope of the libibumad as no persistent data is maintained by libibumad.

Fair enough.

Thanks

>
> -----Original Message-----
> From: Leon Romanovsky <leon@kernel.org>
> Sent: Tuesday, August 6, 2019 6:52 PM
> To: Haim Boozaglo <haimbo@mellanox.com>
> Cc: linux-rdma@vger.kernel.org; Vladimir Koushnir 
> <vladimirk@mellanox.com>
> Subject: Re: [PATCH 1/3] libibumad: Support arbitrary number of IB 
> devices
>
> On Tue, Aug 06, 2019 at 02:38:52PM +0000, Haim Boozaglo wrote:
> > From: Vladimir Koushnir <vladimirk@mellanox.com>
> >
> > Added new function returning a list of available InfiniBand device names.
> > The returned list is not limited to 32 devices.
> >
> > Signed-off-by: Vladimir Koushnir <vladimirk@mellanox.com>
> > Signed-off-by: Haim Boozaglo <haimbo@mellanox.com>
> > ---
> >  libibumad/CMakeLists.txt              |  2 +-
> >  libibumad/libibumad.map               |  6 +++++
> >  libibumad/man/umad_free_ca_namelist.3 | 28 ++++++++++++++++++++++++
> >  libibumad/man/umad_get_ca_namelist.3  | 34 +++++++++++++++++++++++++++++
> >  libibumad/umad.c                      | 41 +++++++++++++++++++++++++++++++++++
> >  libibumad/umad.h                      |  2 ++
> >  6 files changed, 112 insertions(+), 1 deletion(-)  create mode 
> > 100644
> > libibumad/man/umad_free_ca_namelist.3
> >  create mode 100644 libibumad/man/umad_get_ca_namelist.3
>
> 1. Please use cover letter for patch series.
> 2. There is a need to update debian package too.
> 3. Need to use the same discovery mechanism as used in libibverbs - netlink based.
> 4. Does it work with hot-plug where device name can change?
>
> Thanks

[PATCH 3/3] ibdiags: Support arbitrary number of IB devices in ibstat

[Haim Boozaglo]

From: Vladimir Koushnir <vladimirk@mellanox.com>

Allow showing more than 32 IB devices on the server.
umad_get_ca_namelist API is used for this purpose.

Signed-off-by: Vladimir Koushnir <vladimirk@mellanox.com>
Signed-off-by: Haim Boozaglo <haimbo@mellanox.com>
---
 infiniband-diags/ibstat.c | 47 ++++++++++++++++++++++++++---------------------
 1 file changed, 26 insertions(+), 21 deletions(-)

diff --git a/infiniband-diags/ibstat.c b/infiniband-diags/ibstat.c
index 4601f2a..9c87ff8 100644
--- a/infiniband-diags/ibstat.c
+++ b/infiniband-diags/ibstat.c
@@ -232,23 +232,22 @@ static int ca_stat(char *ca_name, int portnum, int no_ports)
 	return 0;
 }
 
-static int ports_list(char names[][UMAD_CA_NAME_LEN], int n)
+static int ports_list(char *names, int n)
 {
 	__be64 guids[64];
-	int found, ports, i;
+	int ports, i, j;
 
-	for (i = 0, found = 0; i < n && found < 64; i++) {
+	for (i = 0; i < n ; i++) {
 		if ((ports =
-		     umad_get_ca_portguids(names[i], guids + found,
-					   64 - found)) < 0)
+		     umad_get_ca_portguids(&names[i * UMAD_CA_NAME_LEN],
+					   &guids[0], 64)) < 0)
 			return -1;
-		found += ports;
-	}
 
-	for (i = 0; i < found; i++)
-		if (guids[i])
-			printf("0x%016" PRIx64 "\n", be64toh(guids[i]));
-	return found;
+		for (j = 0; j < ports; j++)
+			if (guids[j])
+				printf("0x%016" PRIx64 "\n", be64toh(guids[j]));
+	}
+	return 0;
 }
 
 static int list_only, short_format, list_ports;
@@ -273,9 +272,10 @@ static int process_opt(void *context, int ch)
 
 int main(int argc, char *argv[])
 {
-	char names[UMAD_MAX_DEVICES][UMAD_CA_NAME_LEN];
+	char *names;
 	int dev_port = -1;
 	int n, i;
+	char *nptr;
 
 	const struct ibdiag_opt opts[] = {
 		{"list_of_cas", 'l', 0, NULL, "list all IB devices"},
@@ -302,33 +302,38 @@ int main(int argc, char *argv[])
 	if (umad_init() < 0)
 		IBPANIC("can't init UMAD library");
 
-	if ((n = umad_get_cas_names(names, UMAD_MAX_DEVICES)) < 0)
+	if ((n = umad_get_ca_namelist(&names)) < 0)
 		IBPANIC("can't list IB device names");
 
 	if (argc) {
 		for (i = 0; i < n; i++)
-			if (!strncmp(names[i], argv[0], sizeof names[i]))
+			if (!strncmp(&names[i * UMAD_CA_NAME_LEN],
+				     argv[0], UMAD_CA_NAME_LEN))
 				break;
 		if (i >= n)
 			IBPANIC("'%s' IB device can't be found", argv[0]);
-
-		strncpy(names[0], argv[0], sizeof(names[0])-1);
-		names[0][sizeof(names[0])-1] = '\0';
+		if (i != 0) {
+			strncpy(&names[0],
+				&names[i * UMAD_CA_NAME_LEN],
+				UMAD_CA_NAME_LEN);
+		}
 		n = 1;
 	}
 
 	if (list_ports) {
 		if (ports_list(names, n) < 0)
 			IBPANIC("can't list ports");
+		umad_free_ca_namelist(names);
 		return 0;
 	}
 
 	for (i = 0; i < n; i++) {
+		nptr = &names[i * UMAD_CA_NAME_LEN];
 		if (list_only)
-			printf("%s\n", names[i]);
-		else if (ca_stat(names[i], dev_port, short_format) < 0)
-			IBPANIC("stat of IB device '%s' failed", names[i]);
+			printf("%s\n", nptr);
+		else if (ca_stat(nptr, dev_port, short_format) < 0)
+			IBPANIC("stat of IB device '%s' failed", nptr);
 	}
-
+	umad_free_ca_namelist(names);
 	return 0;
 }
-- 
1.8.3.1