[ofa-general] Re: [PATCH 1/5 v2] change ibnd_discover_fabric to receive ibmad_port

Ira Weiny weiny2 at llnl.gov
Wed Apr 22 19:22:52 PDT 2009


I already found a bug in this patch...  You should check for ibmad_port being NULL before you use it.  ;-)

v2 is below.


From: Ira Weiny <weiny2 at llnl.gov>
Date: Wed, 22 Apr 2009 19:20:03 -0700
Subject: [PATCH] change ibnd_discover_fabric to receive ibmad_port

	In order to allow ibmad_port to be opened with additional classes
	libibnetdisc should accept an ibmad_port as a parameter.  The library
	will error out if the classes it needs are not opened.

Signed-off-by: Ira Weiny <weiny2 at llnl.gov>
---
 .../libibnetdisc/include/infiniband/ibnetdisc.h    |    6 +-
 .../libibnetdisc/man/ibnd_discover_fabric.3        |   21 ++++++--
 infiniband-diags/libibnetdisc/src/ibnetdisc.c      |   54 +++++++++-----------
 infiniband-diags/libibnetdisc/src/internal.h       |    1 -
 infiniband-diags/libibnetdisc/test/testleaks.c     |   20 ++++++--
 5 files changed, 60 insertions(+), 42 deletions(-)

diff --git a/infiniband-diags/libibnetdisc/include/infiniband/ibnetdisc.h b/infiniband-diags/libibnetdisc/include/infiniband/ibnetdisc.h
index a882994..7eaca24 100644
--- a/infiniband-diags/libibnetdisc/include/infiniband/ibnetdisc.h
+++ b/infiniband-diags/libibnetdisc/include/infiniband/ibnetdisc.h
@@ -124,6 +124,7 @@ typedef struct chassis {
  * Main fabric object which is returned and represents the data discovered
  */
 typedef struct ib_fabric {
+	struct ibmad_port *ibmad_port;
 	/* the node the discover was initiated from
 	 * "from" parameter in ibnd_discover_fabric
 	 * or by default the node you ar running on
@@ -143,11 +144,10 @@ typedef struct ib_fabric {
 void           ibnd_debug(int i);
 void           ibnd_show_progress(int i);
 
-ibnd_fabric_t *ibnd_discover_fabric(char *dev_name, int dev_port,
+ibnd_fabric_t *ibnd_discover_fabric(struct ibmad_port *ibmad_port,
 			int timeout_ms, ib_portid_t *from, int hops);
 	/**
-	 * dev_name: (required) local device name to use to access the fabric
-	 * dev_port: (required) local device port to use to access the fabric
+	 * open: (required) ibmad_port object from libibmad
 	 * timeout_ms: (required) gives the timeout for a _SINGLE_ query on
 	 *             the fabric.  So if there are multiple nodes not
 	 *             responding this may result in a lengthy delay.
diff --git a/infiniband-diags/libibnetdisc/man/ibnd_discover_fabric.3 b/infiniband-diags/libibnetdisc/man/ibnd_discover_fabric.3
index 44d8c65..c832c11 100644
--- a/infiniband-diags/libibnetdisc/man/ibnd_discover_fabric.3
+++ b/infiniband-diags/libibnetdisc/man/ibnd_discover_fabric.3
@@ -5,7 +5,7 @@ ibnd_discover_fabric, ibnd_destroy_fabric, ibnd_debug ibnd_show_progress \- init
 .nf
 .B #include <infiniband/ibnetdisc.h>
 .sp
-.BI "ibnd_fabric_t *ibnd_discover_fabric(char *dev_name, int dev_port, int timeout_ms, ib_portid_t *from, int hops)"
+.bi "ibnd_fabric_t *ibnd_discover_fabric(struct ibmad_port *ibmad_port, int timeout_ms, ib_portid_t *from, int hops)"
 .BI "void ibnd_destroy_fabric(ibnd_fabric_t *fabric)"
 .BI "void ibnd_debug(int i)"
 .BI "void ibnd_show_progress(int i)"
@@ -13,7 +13,10 @@ ibnd_discover_fabric, ibnd_destroy_fabric, ibnd_debug ibnd_show_progress \- init
 
 .SH "DESCRIPTION"
 .B ibnd_discover_fabric()
-Discover the fabric connected to the port specified by dev_name and dev_port, using a timeout specified.  The "from" and "hops" parameters are optional and allow one to scan part of a fabric by specifying a node "from" and a number of hops away from that node to scan, "hops".  This gives the user a "sub-fabric" which is "centered" anywhere they chose.
+Discover the fabric connected to the port specified by ibmad_port, using a timeout specified.  The "from" and "hops" parameters are optional and allow one to scan part of a fabric by specifying a node "from" and a number of hops away from that node to scan, "hops".  This gives the user a "sub-fabric" which is "centered" anywhere they chose.
+
+ibmad_port must be opened with at least IB_SMI_CLASS and IB_SMI_DIRECT_CLASS
+classes for ibnd_discover_fabric to work.
 
 .B ibnd_destroy_fabric()
 free all memory and resources associated with the fabric.
@@ -36,13 +39,23 @@ NONE
 
 .B Discover the entire fabric connected to device "mthca0", port 1.
 
-	ibnd_discover_fabric("mthca0", 1, 100, NULL, 0);
+	int mgmt_classes[2] = {IB_SMI_CLASS, IB_SMI_DIRECT_CLASS};
+	struct ibmad_port *ibmad_port = mad_rpc_open_port(ca, ca_port, mgmt_classes, 2);
+	ibnd_fabric_t *fabric = ibnd_discover_fabric(ibmad_port, 100, NULL, 0);
+	...
+	ibnd_destroy_fabric(fabric);
+	mad_rpc_close_port(ibmad_port);
 
 .B Discover only a single node and those nodes connected to it.
 
+	...
 	str2drpath(&(port_id.drpath), from, 0, 0);
+	...
+	ibnd_discover_fabric(ibmad_port, 100, &port_id, 1);
+	...
 
-	ibnd_discover_fabric("mthca0", 1, 100, &port_id, 1);
+.SH "SEE ALSO"
+	libibmad, mad_rpc_open_port
 
 .SH "AUTHORS"
 .TP
diff --git a/infiniband-diags/libibnetdisc/src/ibnetdisc.c b/infiniband-diags/libibnetdisc/src/ibnetdisc.c
index 479bae7..410e2dd 100644
--- a/infiniband-diags/libibnetdisc/src/ibnetdisc.c
+++ b/infiniband-diags/libibnetdisc/src/ibnetdisc.c
@@ -79,7 +79,7 @@ get_port_info(struct ibnd_fabric *fabric, struct ibnd_port *port,
 				IB_PORT_LINK_SPEED_ACTIVE_F);
 
 	if (!smp_query_via(port->port.info, portid, IB_ATTR_PORT_INFO, portnum, timeout_ms,
-			fabric->ibmad_port))
+			fabric->fabric.ibmad_port))
 		return -1;
 
 	decode_port_info(&(port->port));
@@ -100,7 +100,7 @@ static int
 query_node_info(struct ibnd_fabric *fabric, struct ibnd_node *node, ib_portid_t *portid)
 {
 	if (!smp_query_via(&(node->node.info), portid, IB_ATTR_NODE_INFO, 0, timeout_ms,
-			fabric->ibmad_port))
+			fabric->fabric.ibmad_port))
 		return -1;
 
 	/* decode just a couple of fields for quicker reference. */
@@ -130,11 +130,11 @@ query_node(struct ibnd_fabric *fabric, struct ibnd_node *inode,
 	port->guid = mad_get_field64(node->info, 0, IB_NODE_PORT_GUID_F);
 
 	if (!smp_query_via(nd, portid, IB_ATTR_NODE_DESC, 0, timeout_ms,
-			fabric->ibmad_port))
+			fabric->fabric.ibmad_port))
 		return -1;
 
 	if (!smp_query_via(port->info, portid, IB_ATTR_PORT_INFO, 0, timeout_ms,
-			fabric->ibmad_port))
+			fabric->fabric.ibmad_port))
 		return -1;
 	decode_port_info(port);
 
@@ -146,7 +146,7 @@ query_node(struct ibnd_fabric *fabric, struct ibnd_node *inode,
 
 	/* after we have the sma information find out the real PortInfo for this port */
 	if (!smp_query_via(port->info, portid, IB_ATTR_PORT_INFO, port->portnum, timeout_ms,
-			fabric->ibmad_port))
+			fabric->fabric.ibmad_port))
 		return -1;
 	decode_port_info(port);
 
@@ -154,7 +154,7 @@ query_node(struct ibnd_fabric *fabric, struct ibnd_node *inode,
 	port->lmc = node->smalmc;
 
         if (!smp_query_via(node->switchinfo, portid, IB_ATTR_SWITCH_INFO, 0, timeout_ms,
-			fabric->ibmad_port))
+			fabric->fabric.ibmad_port))
 		node->smaenhsp0 = 0;	/* assume base SP0 */
 	else
 		mad_decode_field(node->switchinfo, IB_SW_ENHANCED_PORT0_F, &node->smaenhsp0);
@@ -241,7 +241,7 @@ ibnd_update_node(ibnd_node_t *node)
 		return (NULL);
 
 	if (!smp_query_via(nd, &(n->node.path_portid), IB_ATTR_NODE_DESC, 0, timeout_ms,
-			f->ibmad_port))
+			f->fabric.ibmad_port))
 		return (NULL);
 
 	/* update all the port info's */
@@ -253,14 +253,14 @@ ibnd_update_node(ibnd_node_t *node)
 		goto done;
 
 	if (!smp_query_via(portinfo_port0, &(n->node.path_portid), IB_ATTR_PORT_INFO, 0, timeout_ms,
-			f->ibmad_port))
+			f->fabric.ibmad_port))
 		return (NULL);
 
 	n->node.smalid = mad_get_field(portinfo_port0, 0, IB_PORT_LID_F);
 	n->node.smalmc = mad_get_field(portinfo_port0, 0, IB_PORT_LMC_F);
 
         if (!smp_query_via(node->switchinfo, &(n->node.path_portid), IB_ATTR_SWITCH_INFO, 0, timeout_ms,
-			f->ibmad_port))
+			f->fabric.ibmad_port))
                 node->smaenhsp0 = 0;	/* assume base SP0 */
 	else
 		mad_decode_field(node->switchinfo, IB_SW_ENHANCED_PORT0_F, &n->node.smaenhsp0);
@@ -476,17 +476,8 @@ get_remote_node(struct ibnd_fabric *fabric, struct ibnd_node *node, struct ibnd_
 	return 0;
 }
 
-static void *
-ibnd_init_port(char *dev_name, int dev_port)
-{
-	int mgmt_classes[2] = {IB_SMI_CLASS, IB_SMI_DIRECT_CLASS};
-
-	/* Crank up the mad lib */
-	return (mad_rpc_open_port(dev_name, dev_port, mgmt_classes, 2));
-}
-
 ibnd_fabric_t *
-ibnd_discover_fabric(char *dev_name, int dev_port, int timeout_ms,
+ibnd_discover_fabric(struct ibmad_port *ibmad_port, int timeout_ms,
 			ib_portid_t *from, int hops)
 {
 	struct ibnd_fabric *fabric = NULL;
@@ -500,15 +491,27 @@ ibnd_discover_fabric(char *dev_name, int dev_port, int timeout_ms,
 	ib_portid_t *path;
 	int max_hops = MAXHOPS-1; /* default find everything */
 
+	if (!ibmad_port) {
+		IBPANIC("ibmad_port must be specified to "
+			"ibnd_discover_fabric\n");
+		return (NULL);
+	}
+	if (mad_rpc_class_agent(ibmad_port, IB_SMI_CLASS) == -1
+			||
+		mad_rpc_class_agent(ibmad_port, IB_SMI_DIRECT_CLASS) == -1) {
+		IBPANIC("ibmad_port must be opened with "
+			"IB_SMI_CLASS && IB_SMI_DIRECT_CLASS\n");
+		return (NULL);
+	}
+
 	/* if not everything how much? */
 	if (hops >= 0) {
 		max_hops = hops;
 	}
 
 	/* If not specified start from "my" port */
-	if (!from) {
+	if (!from)
 		from = &my_portid;
-	}
 
 	fabric = malloc(sizeof(*fabric));
 
@@ -519,12 +522,7 @@ ibnd_discover_fabric(char *dev_name, int dev_port, int timeout_ms,
 
 	memset(fabric, 0, sizeof(*fabric));
 
-	fabric->ibmad_port = ibnd_init_port(dev_name, dev_port);
-	if (!fabric->ibmad_port) {
-		IBPANIC("OOM: failed to open \"%s\" port %d\n",
-			dev_name, dev_port);
-		goto error;
-	}
+	fabric->fabric.ibmad_port = ibmad_port;
 
 	IBND_DEBUG("from %s\n", portid2str(from));
 
@@ -633,8 +631,6 @@ ibnd_destroy_fabric(ibnd_fabric_t *fabric)
 			node = next;
 		}
 	}
-	if (f->ibmad_port)
-		mad_rpc_close_port(f->ibmad_port);
 	free(f);
 }
 
diff --git a/infiniband-diags/libibnetdisc/src/internal.h b/infiniband-diags/libibnetdisc/src/internal.h
index afed25e..4e6bb18 100644
--- a/infiniband-diags/libibnetdisc/src/internal.h
+++ b/infiniband-diags/libibnetdisc/src/internal.h
@@ -79,7 +79,6 @@ struct ibnd_fabric {
 	ibnd_fabric_t fabric;
 
 	/* internal use only */
-	void *ibmad_port;
 	struct ibnd_node *nodestbl[HTSZ];
 	struct ibnd_port *portstbl[HTSZ];
 	struct ibnd_node *nodesdist[MAXHOPS+1];
diff --git a/infiniband-diags/libibnetdisc/test/testleaks.c b/infiniband-diags/libibnetdisc/test/testleaks.c
index 1fabaac..0d009c3 100644
--- a/infiniband-diags/libibnetdisc/test/testleaks.c
+++ b/infiniband-diags/libibnetdisc/test/testleaks.c
@@ -84,6 +84,7 @@ usage(void)
 int
 main(int argc, char **argv)
 {
+	int rc = 0;
 	char *ca = 0;
 	int ca_port = 0;
 	ibnd_fabric_t *fabric = NULL;
@@ -94,6 +95,9 @@ main(int argc, char **argv)
 	ib_portid_t port_id;
 	int iters = -1;
 
+	struct ibmad_port *ibmad_port;
+	int mgmt_classes[2] = {IB_SMI_CLASS, IB_SMI_DIRECT_CLASS};
+
 	static char const str_opts[] = "S:D:n:C:P:t:shuf:i:";
 	static const struct option long_opts[] = {
 		{ "S", 1, 0, 'S'},
@@ -155,25 +159,31 @@ main(int argc, char **argv)
 	argc -= optind;
 	argv += optind;
 
+	ibmad_port = mad_rpc_open_port(ca, ca_port, mgmt_classes, 2);
+
 	while (iters == -1 || iters-- > 0) {
 		if (from) {
 			/* only scan part of the fabric */
 			str2drpath(&(port_id.drpath), from, 0, 0);
-			if ((fabric = ibnd_discover_fabric(ca, ca_port, timeout_ms,
+			if ((fabric = ibnd_discover_fabric(ibmad_port, timeout_ms,
 					&port_id, hops)) == NULL) {
 				fprintf(stderr, "discover failed\n");
-				exit(1);
+				rc = 1;
+				goto close_port;
 			}
 			guid = 0;
 		} else {
-			if ((fabric = ibnd_discover_fabric(ca, ca_port, timeout_ms, NULL, -1)) == NULL) {
+			if ((fabric = ibnd_discover_fabric(ibmad_port, timeout_ms, NULL, -1)) == NULL) {
 				fprintf(stderr, "discover failed\n");
-				exit(1);
+				rc = 1;
+				goto close_port;
 			}
 		}
 
 		ibnd_destroy_fabric(fabric);
 	}
 
-	exit(0);
+close_port:
+	mad_rpc_close_port(ibmad_port);
+	exit(rc);
 }
-- 
1.5.4.5




More information about the general mailing list