[ofa-general] Re: [PATCH 5/5 v2] infiniband-diags/libibnetdisc: remove members of the fabric struct which are used in the scan only.

Ira Weiny weiny2 at llnl.gov
Mon Aug 17 14:03:44 PDT 2009


From: Ira Weiny <weiny2 at llnl.gov>
Date: Mon, 17 Aug 2009 13:16:51 -0700
Subject: [PATCH] infiniband-diags/libibnetdisc: remove members of the fabric struct which are used in the scan only.

	There is no need to have these be in the public interface.  They can
	cause confusion on which variable present the information to the user.

	Adjusted to apply to v2 of "libibnetdisc: make all fields of ibnd_fabric_t
	public"

Signed-off-by: Ira Weiny <weiny2 at llnl.gov>
---
 .../libibnetdisc/include/infiniband/ibnetdisc.h    |    6 --
 infiniband-diags/libibnetdisc/src/chassis.c        |   52 +++++++-------
 infiniband-diags/libibnetdisc/src/chassis.h        |    2 +-
 infiniband-diags/libibnetdisc/src/ibnetdisc.c      |   80 ++++++++++++--------
 infiniband-diags/libibnetdisc/src/internal.h       |   13 +++
 5 files changed, 88 insertions(+), 65 deletions(-)

diff --git a/infiniband-diags/libibnetdisc/include/infiniband/ibnetdisc.h b/infiniband-diags/libibnetdisc/include/infiniband/ibnetdisc.h
index ce1c74f..51a35a3 100644
--- a/infiniband-diags/libibnetdisc/include/infiniband/ibnetdisc.h
+++ b/infiniband-diags/libibnetdisc/include/infiniband/ibnetdisc.h
@@ -127,7 +127,6 @@ typedef struct ibnd_chassis {
 } ibnd_chassis_t;
 
 #define HTSZ 137
-#define MAXHOPS		63
 
 /** =========================================================================
  * Fabric
@@ -148,14 +147,9 @@ typedef struct ibnd_fabric {
 	/* internal use only */
 	ibnd_node_t *nodestbl[HTSZ];
 	ibnd_port_t *portstbl[HTSZ];
-	ibnd_node_t *nodesdist[MAXHOPS + 1];
-	ibnd_chassis_t *first_chassis;
-	ibnd_chassis_t *current_chassis;
-	ibnd_chassis_t *last_chassis;
 	ibnd_node_t *switches;
 	ibnd_node_t *ch_adapters;
 	ibnd_node_t *routers;
-	ib_portid_t selfportid;
 } ibnd_fabric_t;
 
 /** =========================================================================
diff --git a/infiniband-diags/libibnetdisc/src/chassis.c b/infiniband-diags/libibnetdisc/src/chassis.c
index 4886cfc..d11d7df 100644
--- a/infiniband-diags/libibnetdisc/src/chassis.c
+++ b/infiniband-diags/libibnetdisc/src/chassis.c
@@ -96,7 +96,7 @@ static ibnd_chassis_t *find_chassisnum(ibnd_fabric_t * fabric,
 {
 	ibnd_chassis_t *current;
 
-	for (current = fabric->first_chassis; current; current = current->next) {
+	for (current = fabric->chassis; current; current = current->next) {
 		if (current->chassisnum == chassisnum)
 			return current;
 	}
@@ -207,14 +207,14 @@ static uint64_t get_chassisguid(ibnd_node_t * node)
 		return sysimgguid;
 }
 
-static ibnd_chassis_t *find_chassisguid(ibnd_fabric_t * fabric,
+static ibnd_chassis_t *find_chassisguid(struct ibnd_chassis_ctx *ch_ctx,
 					ibnd_node_t * node)
 {
 	ibnd_chassis_t *current;
 	uint64_t chguid;
 
 	chguid = get_chassisguid(node);
-	for (current = fabric->first_chassis; current; current = current->next) {
+	for (current = ch_ctx->first_chassis; current; current = current->next) {
 		if (current->chassisguid == chguid)
 			return current;
 	}
@@ -782,19 +782,19 @@ static void voltaire_portmap(ibnd_port_t * port)
 		port->ext_portnum = int2ext_map_slb8[chipnum][portnum];
 }
 
-static int add_chassis(ibnd_fabric_t * fabric)
+static int add_chassis(struct ibnd_chassis_ctx *ch_ctx)
 {
-	if (!(fabric->current_chassis = calloc(1, sizeof(ibnd_chassis_t)))) {
+	if (!(ch_ctx->current_chassis = calloc(1, sizeof(ibnd_chassis_t)))) {
 		IBND_ERROR("OOM: failed to allocate chassis object\n");
 		return (-1);
 	}
 
-	if (fabric->first_chassis == NULL) {
-		fabric->first_chassis = fabric->current_chassis;
-		fabric->last_chassis = fabric->current_chassis;
+	if (ch_ctx->first_chassis == NULL) {
+		ch_ctx->first_chassis = ch_ctx->current_chassis;
+		ch_ctx->last_chassis = ch_ctx->current_chassis;
 	} else {
-		fabric->last_chassis->next = fabric->current_chassis;
-		fabric->last_chassis = fabric->current_chassis;
+		ch_ctx->last_chassis->next = ch_ctx->current_chassis;
+		ch_ctx->last_chassis = ch_ctx->current_chassis;
 	}
 	return (0);
 }
@@ -818,22 +818,22 @@ static void add_node_to_chassis(ibnd_chassis_t * chassis, ibnd_node_t * node)
 	Returns:
 	0 on success, -1 on failure
 */
-int group_nodes(ibnd_fabric_t * fabric)
+int group_nodes(struct ibnd_scan_ctx *scan_ctx, ibnd_fabric_t * fabric)
 {
 	ibnd_node_t *node;
 	int dist;
 	int chassisnum = 0;
 	ibnd_chassis_t *chassis;
+	struct ibnd_chassis_ctx ch_ctx;
 
-	fabric->first_chassis = NULL;
-	fabric->current_chassis = NULL;
+	memset(&ch_ctx, 0, sizeof ch_ctx);
 
 	/* first pass on switches and build for every Voltaire node */
 	/* an appropriate chassis record (slotnum and position) */
 	/* according to internal connectivity */
 	/* not very efficient but clear code so... */
 	for (dist = 0; dist <= fabric->maxhops_discovered; dist++) {
-		for (node = fabric->nodesdist[dist]; node; node = node->dnext) {
+		for (node = scan_ctx->nodesdist[dist]; node; node = node->dnext) {
 			if (mad_get_field(node->info, 0,
 					  IB_NODE_VENDORID_F) == VTR_VENDOR_ID)
 				if (fill_voltaire_chassis_record(node))
@@ -844,7 +844,7 @@ int group_nodes(ibnd_fabric_t * fabric)
 	/* separate every Voltaire chassis from each other and build linked list of them */
 	/* algorithm: catch spine and find all surrounding nodes */
 	for (dist = 0; dist <= fabric->maxhops_discovered; dist++) {
-		for (node = fabric->nodesdist[dist]; node; node = node->dnext) {
+		for (node = scan_ctx->nodesdist[dist]; node; node = node->dnext) {
 			if (mad_get_field(node->info, 0,
 					  IB_NODE_VENDORID_F) != VTR_VENDOR_ID)
 				continue;
@@ -852,10 +852,10 @@ int group_nodes(ibnd_fabric_t * fabric)
 			    || (node->chassis && node->chassis->chassisnum)
 			    || !is_spine(node))
 				continue;
-			if (add_chassis(fabric))
+			if (add_chassis(&ch_ctx))
 				return (-1);
-			fabric->current_chassis->chassisnum = ++chassisnum;
-			if (build_chassis(node, fabric->current_chassis))
+			ch_ctx.current_chassis->chassisnum = ++chassisnum;
+			if (build_chassis(node, ch_ctx.current_chassis))
 				return (-1);
 		}
 	}
@@ -863,25 +863,25 @@ int group_nodes(ibnd_fabric_t * fabric)
 	/* now make pass on nodes for chassis which are not Voltaire */
 	/* grouped by common SystemImageGUID */
 	for (dist = 0; dist <= fabric->maxhops_discovered; dist++) {
-		for (node = fabric->nodesdist[dist]; node; node = node->dnext) {
+		for (node = scan_ctx->nodesdist[dist]; node; node = node->dnext) {
 			if (mad_get_field(node->info, 0,
 					  IB_NODE_VENDORID_F) == VTR_VENDOR_ID)
 				continue;
 			if (mad_get_field64(node->info, 0,
 					    IB_NODE_SYSTEM_GUID_F)) {
 				chassis =
-				    find_chassisguid(fabric,
+				    find_chassisguid(&ch_ctx,
 						     (ibnd_node_t *) node);
 				if (chassis)
 					chassis->nodecount++;
 				else {
 					/* Possible new chassis */
-					if (add_chassis(fabric))
+					if (add_chassis(&ch_ctx))
 						return (-1);
-					fabric->current_chassis->chassisguid =
+					ch_ctx.current_chassis->chassisguid =
 					    get_chassisguid((ibnd_node_t *)
 							    node);
-					fabric->current_chassis->nodecount = 1;
+					ch_ctx.current_chassis->nodecount = 1;
 				}
 			}
 		}
@@ -890,14 +890,14 @@ int group_nodes(ibnd_fabric_t * fabric)
 	/* now, make another pass to see which nodes are part of chassis */
 	/* (defined as chassis->nodecount > 1) */
 	for (dist = 0; dist <= MAXHOPS;) {
-		for (node = fabric->nodesdist[dist]; node; node = node->dnext) {
+		for (node = scan_ctx->nodesdist[dist]; node; node = node->dnext) {
 			if (mad_get_field(node->info, 0,
 					  IB_NODE_VENDORID_F) == VTR_VENDOR_ID)
 				continue;
 			if (mad_get_field64(node->info, 0,
 					    IB_NODE_SYSTEM_GUID_F)) {
 				chassis =
-				    find_chassisguid(fabric,
+				    find_chassisguid(&ch_ctx,
 						     (ibnd_node_t *) node);
 				if (chassis && chassis->nodecount > 1) {
 					if (!chassis->chassisnum)
@@ -918,6 +918,6 @@ int group_nodes(ibnd_fabric_t * fabric)
 			dist++;
 	}
 
-	fabric->chassis = fabric->first_chassis;
+	fabric->chassis = ch_ctx.first_chassis;
 	return (0);
 }
diff --git a/infiniband-diags/libibnetdisc/src/chassis.h b/infiniband-diags/libibnetdisc/src/chassis.h
index 2191046..707140c 100644
--- a/infiniband-diags/libibnetdisc/src/chassis.h
+++ b/infiniband-diags/libibnetdisc/src/chassis.h
@@ -82,6 +82,6 @@ enum ibnd_chassis_type {
 };
 enum ibnd_chassis_slot_type { UNRESOLVED_CS, LINE_CS, SPINE_CS, SRBD_CS };
 
-int group_nodes(struct ibnd_fabric *fabric);
+int group_nodes(struct ibnd_scan_ctx *scan_ctx, struct ibnd_fabric *fabric);
 
 #endif				/* _CHASSIS_H_ */
diff --git a/infiniband-diags/libibnetdisc/src/ibnetdisc.c b/infiniband-diags/libibnetdisc/src/ibnetdisc.c
index 7295189..84aac0a 100644
--- a/infiniband-diags/libibnetdisc/src/ibnetdisc.c
+++ b/infiniband-diags/libibnetdisc/src/ibnetdisc.c
@@ -189,21 +189,27 @@ static int add_port_to_dpath(ib_dr_path_t * path, int nextport)
 	return path->cnt;
 }
 
-static int extend_dpath(struct ibmad_port *ibmad_port, ibnd_fabric_t * fabric,
+static int extend_dpath(struct ibnd_scan_ctx *scan_ctx,
+			struct ibmad_port *ibmad_port, ibnd_fabric_t * fabric,
 			ib_portid_t * portid, int nextport)
 {
 	int rc = 0;
 
 	if (portid->lid) {
+		if (!scan_ctx) {
+			IBND_ERROR("Invalid internal scan state");
+			return (-1);
+		}
 		/* If we were LID routed we need to set up the drslid */
-		if (!fabric->selfportid.lid)
-			if (ib_resolve_self_via(&fabric->selfportid, NULL, NULL,
-						ibmad_port) < 0) {
+		if (!scan_ctx->selfportid.lid)
+			if (ib_resolve_self_via
+			    (&scan_ctx->selfportid, NULL, NULL,
+			     ibmad_port) < 0) {
 				IBND_ERROR("Failed to resolve self\n");
 				return -1;
 			}
 
-		portid->drpath.drslid = (uint16_t) fabric->selfportid.lid;
+		portid->drpath.drslid = (uint16_t) scan_ctx->selfportid.lid;
 		portid->drpath.drdlid = 0xFFFF;
 	}
 
@@ -408,19 +414,25 @@ static void add_to_type_list(ibnd_node_t * node, ibnd_fabric_t * fabric)
 	}
 }
 
-static void add_to_nodedist(ibnd_node_t * node, ibnd_fabric_t * fabric)
+static void add_to_nodedist(ibnd_node_t * node, struct ibnd_scan_ctx *scan_ctx)
 {
 	int dist = node->dist;
+
+	if (!scan_ctx) {
+		IBND_ERROR("Invalid internal scan state");
+		return;
+	}
+
 	if (node->type != IB_NODE_SWITCH)
 		dist = MAXHOPS;	/* special Ca list */
 
-	node->dnext = fabric->nodesdist[dist];
-	fabric->nodesdist[dist] = node;
+	node->dnext = scan_ctx->nodesdist[dist];
+	scan_ctx->nodesdist[dist] = node;
 }
 
-static ibnd_node_t *create_node(ibnd_fabric_t * fabric,
-				ibnd_node_t * temp, ib_portid_t * path,
-				int dist)
+static ibnd_node_t *create_node(struct ibnd_scan_ctx *scan_ctx,
+				ibnd_fabric_t * fabric, ibnd_node_t * temp,
+				ib_portid_t * path, int dist)
 {
 	ibnd_node_t *node;
 
@@ -441,7 +453,7 @@ static ibnd_node_t *create_node(ibnd_fabric_t * fabric,
 	fabric->nodes = (ibnd_node_t *) node;
 
 	add_to_type_list(node, fabric);
-	add_to_nodedist(node, fabric);
+	add_to_nodedist(node, scan_ctx);
 
 	return node;
 }
@@ -500,9 +512,10 @@ static void link_ports(ibnd_node_t * node, ibnd_port_t * port,
 	remoteport->remoteport = (ibnd_port_t *) port;
 }
 
-static int get_remote_node(ibnd_ctx_t * ctx, ibnd_fabric_t * fabric,
-			   ibnd_node_t * node, ibnd_port_t * port,
-			   ib_portid_t * path, int portnum, int dist)
+static int get_remote_node(ibnd_ctx_t * ctx, struct ibnd_scan_ctx *scan_ctx,
+			   ibnd_fabric_t * fabric, ibnd_node_t * node,
+			   ibnd_port_t * port, ib_portid_t * path,
+			   int portnum, int dist)
 {
 	int rc = 0;
 	struct ibmad_port *ibmad_port = ctx->ibmad_port;
@@ -521,7 +534,7 @@ static int get_remote_node(ibnd_ctx_t * ctx, ibnd_fabric_t * fabric,
 	    != IB_PORT_PHYS_STATE_LINKUP)
 		return 1;	/* positive == non-fatal error */
 
-	if (extend_dpath(ibmad_port, fabric, path, portnum) < 0)
+	if (extend_dpath(scan_ctx, ibmad_port, fabric, path, portnum) < 0)
 		return -1;
 
 	if (query_node(ibmad_port, fabric, &node_buf, &port_buf, path)) {
@@ -534,7 +547,9 @@ static int get_remote_node(ibnd_ctx_t * ctx, ibnd_fabric_t * fabric,
 	oldnode = find_existing_node(fabric, &node_buf);
 	if (oldnode)
 		remotenode = oldnode;
-	else if (!(remotenode = create_node(fabric, &node_buf, path, dist + 1))) {
+	else if (!
+		 (remotenode =
+		  create_node(scan_ctx, fabric, &node_buf, path, dist + 1))) {
 		rc = -1;
 		goto error;
 	}
@@ -574,10 +589,13 @@ ibnd_fabric_t *ibnd_discover_fabric(ibnd_ctx_t * ctx,
 	int dist = 0;
 	ib_portid_t *path;
 	int max_hops = MAXHOPS - 1;	/* default find everything */
+	struct ibnd_scan_ctx scan_ctx;
 
 	if (check_ctx(ctx) < 0)
 		return (NULL);
 
+	memset(&scan_ctx, 0, sizeof scan_ctx);
+
 	/* if not everything how much? */
 	if (hops >= 0) {
 		max_hops = hops;
@@ -606,7 +624,7 @@ ibnd_fabric_t *ibnd_discover_fabric(ibnd_ctx_t * ctx,
 		goto error;
 	}
 
-	node = create_node(fabric, &node_buf, from, 0);
+	node = create_node(&scan_ctx, fabric, &node_buf, from, 0);
 	if (!node)
 		goto error;
 
@@ -616,7 +634,7 @@ ibnd_fabric_t *ibnd_discover_fabric(ibnd_ctx_t * ctx,
 	if (!port)
 		goto error;
 
-	rc = get_remote_node(ctx, fabric, node, port, from,
+	rc = get_remote_node(ctx, &scan_ctx, fabric, node, port, from,
 			     mad_get_field(node->info, 0,
 					   IB_NODE_LOCAL_PORT_F), 0);
 	if (rc < 0)
@@ -626,7 +644,7 @@ ibnd_fabric_t *ibnd_discover_fabric(ibnd_ctx_t * ctx,
 
 	for (dist = 0; dist <= max_hops; dist++) {
 
-		for (node = fabric->nodesdist[dist]; node; node = node->dnext) {
+		for (node = scan_ctx.nodesdist[dist]; node; node = node->dnext) {
 
 			path = &node->path_portid;
 
@@ -663,14 +681,15 @@ ibnd_fabric_t *ibnd_discover_fabric(ibnd_ctx_t * ctx,
 							    IB_NODE_PORT_GUID_F);
 				}
 
-				if (get_remote_node(ctx, fabric, node,
-						    port, path, i, dist) < 0)
+				if (get_remote_node
+				    (ctx, &scan_ctx, fabric, node, port, path,
+				     i, dist) < 0)
 					goto error;
 			}
 		}
 	}
 
-	if (group_nodes(fabric))
+	if (group_nodes(&scan_ctx, fabric))
 		goto error;
 
 	return ((ibnd_fabric_t *) fabric);
@@ -692,7 +711,6 @@ static void destroy_node(ibnd_node_t * node)
 
 void ibnd_destroy_fabric(ibnd_fabric_t * fabric)
 {
-	int dist = 0;
 	ibnd_node_t *node = NULL;
 	ibnd_node_t *next = NULL;
 	ibnd_chassis_t *ch, *ch_next;
@@ -700,19 +718,17 @@ void ibnd_destroy_fabric(ibnd_fabric_t * fabric)
 	if (!fabric)
 		return;
 
-	ch = fabric->first_chassis;
+	ch = fabric->chassis;
 	while (ch) {
 		ch_next = ch->next;
 		free(ch);
 		ch = ch_next;
 	}
-	for (dist = 0; dist <= MAXHOPS; dist++) {
-		node = fabric->nodesdist[dist];
-		while (node) {
-			next = node->dnext;
-			destroy_node(node);
-			node = next;
-		}
+	node = fabric->nodes;
+	while (node) {
+		next = node->next;
+		destroy_node(node);
+		node = next;
 	}
 	free(fabric);
 }
diff --git a/infiniband-diags/libibnetdisc/src/internal.h b/infiniband-diags/libibnetdisc/src/internal.h
index b989b68..c866b12 100644
--- a/infiniband-diags/libibnetdisc/src/internal.h
+++ b/infiniband-diags/libibnetdisc/src/internal.h
@@ -50,6 +50,19 @@
 /* HASH table defines */
 #define HASHGUID(guid) ((uint32_t)(((uint32_t)(guid) * 101) ^ ((uint32_t)((guid) >> 32) * 103)))
 
+#define MAXHOPS		63
+
+struct ibnd_chassis_ctx {
+	ibnd_chassis_t *first_chassis;
+	ibnd_chassis_t *current_chassis;
+	ibnd_chassis_t *last_chassis;
+};
+
+struct ibnd_scan_ctx {
+	ibnd_node_t *nodesdist[MAXHOPS + 1];
+	ib_portid_t selfportid;
+};
+
 struct ibnd_ctx {
 	struct ibmad_port *ibmad_port;
 	int show_progress;
-- 
1.5.4.5




More information about the general mailing list