[PATCH rdma-core 0/3] get more than 32 available InfiniBand device names

[PATCH rdma-core 0/3] get more than 32 available InfiniBand device names

[Haim Boozaglo]

Hi,

This series from Vladimir Koushnir adds the ability to get
list of InfiniBand device names greater than 32.

Thanks

Vladimir Koushnir (3):
  libibumad: Support arbitrary number of IB devices
  libibumad: Redesign resolve_ca_name to support arbitrary number of IB
    devices
  ibdiags: Support arbitrary number of IB devices in ibstat

 debian/libibumad3.symbols             |   2 +
 infiniband-diags/ibstat.c             |  47 +++++++-----
 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                      | 141 +++++++++++++++++++++++++---------
 libibumad/umad.h                      |   2 +
 8 files changed, 204 insertions(+), 58 deletions(-)
 create mode 100644 libibumad/man/umad_free_ca_namelist.3
 create mode 100644 libibumad/man/umad_get_ca_namelist.3

-- 
1.8.3.1

[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>
---
 debian/libibumad3.symbols             |  2 ++
 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 ++
 7 files changed, 114 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/debian/libibumad3.symbols b/debian/libibumad3.symbols
index 31a749f..d4b9bde 100644
--- a/debian/libibumad3.symbols
+++ b/debian/libibumad3.symbols
@@ -9,7 +9,9 @@ libibumad.so.3 libibumad3 #MINVER#
  umad_debug@IBUMAD_1.0 1.3.9
  umad_done@IBUMAD_1.0 1.3.9
  umad_dump@IBUMAD_1.0 1.3.9
+ umad_free_ca_namelist@IBUMAD_1.1 3.1.26
  umad_get_ca@IBUMAD_1.0 1.3.9
+ umad_get_ca_namelist@IBUMAD_1.1 3.1.26
  umad_get_ca_portguids@IBUMAD_1.0 1.3.9
  umad_get_cas_names@IBUMAD_1.0 1.3.9
  umad_get_fd@IBUMAD_1.0 1.3.9
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..1f66cc3 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_free_ca_namelist;
+		umad_get_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 rdma-core 0/3] get more than 32 available InfiniBand device names

[Doug Ledford]

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

On Sun, 2019-08-11 at 16:29 +0000, Haim Boozaglo wrote:
> Hi,
> 
> This series from Vladimir Koushnir adds the ability to get
> list of InfiniBand device names greater than 32.
> 
> Thanks
> 
> Vladimir Koushnir (3):
>   libibumad: Support arbitrary number of IB devices
>   libibumad: Redesign resolve_ca_name to support arbitrary number of
> IB
>     devices
>   ibdiags: Support arbitrary number of IB devices in ibstat
> 
>  debian/libibumad3.symbols             |   2 +
>  infiniband-diags/ibstat.c             |  47 +++++++-----
>  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                      | 141
> +++++++++++++++++++++++++---------
>  libibumad/umad.h                      |   2 +
>  8 files changed, 204 insertions(+), 58 deletions(-)
>  create mode 100644 libibumad/man/umad_free_ca_namelist.3
>  create mode 100644 libibumad/man/umad_get_ca_namelist.3
> 

Changes have been requested from this series before it can be merged:

https://github.com/linux-rdma/rdma-core/pull/561

-- 
Doug Ledford <dledford@redhat.com>
    GPG KeyID: B826A3330E572FDD
    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: 833 bytes --]

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

[Ira Weiny]

On Sun, Aug 11, 2019 at 04:29:20PM +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>
> ---

[snip]

> 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++;
> +				}

This all seems overly complicated to avoid allocating the strings separate from
the pointer array.  Why not just allocate the pointer array and strdup() the
names into the array?  And then make unamd_free_ca_namelist() do some work?

Ira

> +			}
> +			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
> 

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

[Haim Boozaglo]

On 8/13/2019 7:39 PM, Ira Weiny wrote:
> On Sun, Aug 11, 2019 at 04:29:20PM +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>
>> ---
> 
> [snip]
> 
>> 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++;
>> +				}
> 
> This all seems overly complicated to avoid allocating the strings separate from
> the pointer array.  Why not just allocate the pointer array and strdup() the
> names into the array?  And then make unamd_free_ca_namelist() do some work?
> 
> Ira

Hi Ira,
Thats what I am going to do and fix (to use strdup),

Thanks,
Haim.

> 
>> +			}
>> +			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
>>