[ofa-general] Re: [PATCH 2/3 - no ibcommon] Convert iblinkinfo.pl to C and use new ibnetdisc library.

Sasha Khapyorsky sashak at voltaire.com
Wed Jan 21 08:19:46 PST 2009


On 15:47 Fri 09 Jan     , Ira Weiny wrote:
> From 139d0ad5ecffecd5c325d865e96cdf038e03a3e5 Mon Sep 17 00:00:00 2001
> From: Ira Weiny <weiny2 at llnl.gov>
> Date: Mon, 1 Dec 2008 14:55:10 -0800
> Subject: [PATCH] Convert iblinkinfo.pl to C and use new ibnetdisc library.
> 
> 
> Signed-off-by: weiny2 at llnl.gov <weiny2 at llnl.gov>
> ---
>  infiniband-diags/Makefile.am           |   12 +-
>  infiniband-diags/scripts/iblinkinfo.pl |  327 ------------------------
>  infiniband-diags/src/iblinkinfo.c      |  423 ++++++++++++++++++++++++++++++++
>  3 files changed, 432 insertions(+), 330 deletions(-)
>  delete mode 100755 infiniband-diags/scripts/iblinkinfo.pl
>  create mode 100644 infiniband-diags/src/iblinkinfo.c
> 
> diff --git a/infiniband-diags/Makefile.am b/infiniband-diags/Makefile.am
> index 8e8c3c1..d127a4d 100644
> --- a/infiniband-diags/Makefile.am
> +++ b/infiniband-diags/Makefile.am
> @@ -1,6 +1,7 @@
>  SUBDIRS = libibnetdisc
>  
> -INCLUDES = -I$(top_builddir)/include/ -I$(srcdir)/include -I$(includedir) -I$(includedir)/infiniband
> +INCLUDES = -I$(top_builddir)/include/ -I$(srcdir)/include -I$(includedir) -I$(includedir)/infiniband \
> +	-I$(top_builddir)/libibnetdisc/include
>  
>  if DEBUG
>  DBGFLAGS = -ggdb -D_DEBUG_
> @@ -11,7 +12,7 @@ endif
>  sbin_PROGRAMS = src/ibaddr src/ibnetdiscover src/ibping src/ibportstate \
>  	        src/ibroute src/ibstat src/ibsysstat src/ibtracert \
>  	        src/perfquery src/sminfo src/smpdump src/smpquery \
> -	        src/saquery src/vendstat
> +	        src/saquery src/vendstat src/iblinkinfo.pl

Is it a good idea to make binary file with .pl extension? Probably you
like to preserve backward compatibility, but in this case I think it
would be better to have real tiny perl script which executes iblinkinfo
program.

>  
>  if ENABLE_TEST_UTILS
>  sbin_PROGRAMS += src/ibsendtrap src/mcm_rereg_test
> @@ -28,7 +29,7 @@ sbin_SCRIPTS = scripts/ibcheckerrs scripts/ibchecknet scripts/ibchecknode \
>  	       scripts/dump_lfts.sh scripts/dump_mfts.sh \
>  	       scripts/set_nodedesc.sh \
>  	       scripts/ibqueryerrors.pl scripts/ibswportwatch.pl \
> -	       scripts/iblinkinfo.pl scripts/ibprintswitch.pl \
> +	       scripts/ibprintswitch.pl \
>  	       scripts/ibprintca.pl scripts/ibprintrt.pl \
>  	       scripts/ibfindnodesusing.pl scripts/ibidsverify.pl \
>  	       scripts/check_lft_balance.pl
> @@ -40,6 +41,11 @@ src_ibnetdiscover_SOURCES = src/ibnetdiscover.c src/grouping.c src/ibdiag_common
>  src_ibnetdiscover_CFLAGS = -Wall $(DBGFLAGS)
>  src_ibnetdiscover_LDFLAGS = -Wl,--rpath -Wl,$(libdir)
>  
> +src_iblinkinfo_pl_SOURCES = src/iblinkinfo.c
> +src_iblinkinfo_pl_CFLAGS = -Wall $(DBGFLAGS)
> +src_iblinkinfo_pl_LDFLAGS = -Wl,--rpath -Wl,$(libdir) \
> +			-L$(srcdir)/libibnetdisc -libnetdisc
> +
>  src_ibping_SOURCES = src/ibping.c src/ibdiag_common.c
>  src_ibping_CFLAGS = -Wall $(DBGFLAGS)
> 

[snip..]

> diff --git a/infiniband-diags/src/iblinkinfo.c b/infiniband-diags/src/iblinkinfo.c
> new file mode 100644
> index 0000000..c8f224e
> --- /dev/null
> +++ b/infiniband-diags/src/iblinkinfo.c
> @@ -0,0 +1,423 @@
> +/*
> + * Copyright (c) 2004-2007 Voltaire Inc.  All rights reserved.
> + * Copyright (c) 2007 Xsigo Systems Inc.  All rights reserved.
> + * Copyright (c) 2008 Lawrence Livermore National Lab.  All rights reserved.
> + *
> + * This software is available to you under a choice of one of two
> + * licenses.  You may choose to be licensed under the terms of the GNU
> + * General Public License (GPL) Version 2, available from the file
> + * COPYING in the main directory of this source tree, or the
> + * OpenIB.org BSD license below:
> + *
> + *     Redistribution and use in source and binary forms, with or
> + *     without modification, are permitted provided that the following
> + *     conditions are met:
> + *
> + *      - Redistributions of source code must retain the above
> + *        copyright notice, this list of conditions and the following
> + *        disclaimer.
> + *
> + *      - Redistributions in binary form must reproduce the above
> + *        copyright notice, this list of conditions and the following
> + *        disclaimer in the documentation and/or other materials
> + *        provided with the distribution.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> + * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
> + * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
> + * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
> + * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
> + * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
> + * SOFTWARE.
> + *
> + */
> +
> +#if HAVE_CONFIG_H
> +#  include <config.h>
> +#endif /* HAVE_CONFIG_H */
> +
> +#define _GNU_SOURCE
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <unistd.h>
> +#include <stdarg.h>
> +#include <time.h>
> +#include <string.h>
> +#include <getopt.h>
> +#include <errno.h>
> +#include <inttypes.h>
> +
> +#include <infiniband/complib/cl_nodenamemap.h>
> +#include <infiniband/ibnetdisc.h>
> +
> +char *argv0 = "iblinkinfotest";
> +static FILE *f;
> +
> +static char *node_name_map_file = NULL;
> +static nn_map_t *node_name_map = NULL;
> +
> +static int timeout_ms = 500;
> +
> +static int down_links_only = 0;
> +static int line_mode = 0;
> +static int add_sw_settings = 0;
> +static int print_port_guids = 0;
> +static int old_output = 0;
> +
> +static unsigned int
> +get_max(unsigned int num)
> +{
> +	unsigned int v = num; // 32-bit word to find the log base 2 of
> +	unsigned r = 0; // r will be lg(v)
> +
> +	while (v >>= 1) // unroll for more speed...
> +	{
> +		r++;
> +	}
> +
> +	return (1 << r);
> +}
> +
> +static char *
> +get_linkspeed_str(int link_speed)
> +{
> +	return (ibnd_linkspeed_str(link_speed, old_output));
> +}
> +
> +void
> +get_msg(char *width_msg, char *speed_msg, int msg_size, ibnd_port_t *port)
> +{
> +	int max_speed = 0;
> +
> +	int max_width = get_max(port->info.link_width_supported
> +				& port->remoteport->info.link_width_supported);
> +	if ((max_width & port->info.link_width_active) == 0) {
> +		// we are not at the max supported width
> +		// print what we could be at.
> +		snprintf(width_msg, msg_size, "Could be %s",
> +			ibnd_linkwidth_str(max_width));
> +	}
> +
> +	max_speed = get_max(port->info.link_speed_supported
> +				& port->remoteport->info.link_speed_supported);
> +	if ((max_speed & port->info.link_speed_active) == 0) {
> +		// we are not at the max supported speed
> +		// print what we could be at.
> +		snprintf(speed_msg, msg_size, "Could be %s",
> +			get_linkspeed_str(max_speed));
> +	}
> +}
> +
> +void
> +print_port(ibnd_node_t *node, ibnd_port_t *port)
> +{
> +	static char remote_guid_str[256];
> +	static char remote_str[256];
> +	static char link_str[256];
> +	static char width_msg[256];
> +	static char speed_msg[256];
> +	static char ext_port_str[256];
> +	static char loc_sma_lid[16];
> +
> +	if (!port)
> +		return;
> +
> +	remote_guid_str[0] = '\0';
> +	remote_str[0] = '\0';
> +	link_str[0] = '\0';
> +	width_msg[0] = '\0';
> +	speed_msg[0] = '\0';
> +
> +	snprintf(loc_sma_lid, 16, "%d", node->smalid);
> +	if (port->remoteport) {
> +		static char  remote_name_buf[256];

Should it be static?

> +		strncpy(remote_name_buf, port->remoteport->node->nodedesc, 256);
> +
> +		if (port->remoteport->ext_portnum)
> +			snprintf(ext_port_str, 256, "%d", port->remoteport->ext_portnum);
> +		else
> +			ext_port_str[0] = '\0';
> +
> +		get_msg(width_msg, speed_msg, 256, port);
> +		if (line_mode) {
> +			if (print_port_guids) {
> +				snprintf(remote_guid_str, 256,
> +					"0x%016"PRIx64" ",
> +					port->remoteport->guid);
> +			} else {
> +				snprintf(remote_guid_str, 256,
> +					"0x%016"PRIx64" ",
> +					port->remoteport->node->info.nodeguid);
> +			}

Really nit (pretty optional - just saves few lines)...

			snprintf(remote_guid_str, sizeof(remote_guid_str),
				 "0x%016"PRIx64" ", print_port_guids ?
				 port->remoteport->guid :
				 port->remoteport->node->info.nodeguid);

> +		}
> +
> +		snprintf(remote_str, 256,
> +			"%s%6d %4d[%2s] \"%s\" ( %s %s)\n",
> +			remote_guid_str,
> +			port->remoteport->info.base_lid ?
> +				port->remoteport->info.base_lid :
> +				port->remoteport->node->smalid,
> +			port->remoteport->portnum,
> +			ext_port_str,
> +			remap_node_name(node_name_map,
> +				port->remoteport->node->info.nodeguid,
> +				remote_name_buf),
> +			width_msg,
> +			speed_msg
> +			);
> +	} else {
> +		snprintf(remote_str, 256,
> +			"%19s%6s %4s[%2s] \"\" (  )\n", "", "", "", "");
> +		if (old_output) {
> +			loc_sma_lid[0] = '\0';
> +		}
> +	}
> +
> +
> +	if (add_sw_settings) {
> +		snprintf(link_str, 256,
> +			"(%3s %s %6s / %8s) (HOQ:%d VL_Stall:%d)",
> +			ibnd_linkwidth_str(port->info.link_width_active),
> +			get_linkspeed_str(port->info.link_speed_active),
> +			ibnd_linkstate_str(port->info.port_state),
> +			ibnd_physstate_str(port->info.phys_state),
> +			port->info.hoq_lifetime,
> +			port->info.vl_stall_count
> +			);
> +	} else {
> +		snprintf(link_str, 256,
> +			"(%3s %s %6s / %8s)",
> +			ibnd_linkwidth_str(port->info.link_width_active),
> +			get_linkspeed_str(port->info.link_speed_active),
> +			ibnd_linkstate_str(port->info.port_state),
> +			ibnd_physstate_str(port->info.phys_state)
> +			);
> +	}

similar...

	n = snprintf(link_str, sizeof(link_str), "(%3s %s %6s / %8s)",
		ibnd_linkwidth_str(port->info.link_width_active),
		get_linkspeed_str(port->info.link_speed_active),
		ibnd_linkstate_str(port->info.port_state),
		ibnd_physstate_str(port->info.phys_state));
	if (add_sw_settings && n < sizeof(link_str))
		snprintf(link_str + n, sizeof(link_str) - n,
			" (HOQ:%d VL_Stall:%d)",
			port->info.hoq_lifetime, port->info.vl_stall_count);

> +
> +	if (port->ext_portnum)
> +		snprintf(ext_port_str, 256, "%d", port->ext_portnum);
> +	else
> +		ext_port_str[0] = '\0';
> +
> +	if (line_mode) {
> +		static char  name_buf[256];
> +		char *node_name = "";
> +
> +		if (old_output && (!port->remoteport)) {
> +			node_name = "";
> +		} else {
> +			strncpy(name_buf, node->nodedesc, 256);
> +			node_name = remap_node_name(node_name_map,
> +				node->info.nodeguid,
> +				name_buf);
> +		}
> +
> +		printf("0x%016"PRIx64" \"%30s\" %6s %4d[%2s]  ==%s==>  %s",
> +			node->info.nodeguid,
> +			node_name,
> +			loc_sma_lid, port->portnum,
> +			ext_port_str,
> +			link_str,
> +			remote_str
> +			);
> +	} else {
> +		printf(" %6s %4d[%2s]  ==%s==>  %s",
> +			loc_sma_lid, port->portnum,
> +			ext_port_str,
> +			link_str,
> +			remote_str
> +			);
> +	}
> +}
> +
> +void
> +print_switch(ibnd_node_t *node, void *user_data)
> +{
> +	int i = 0;
> +
> +	if (!line_mode) {
> +		char  name_buf[256];
> +		strncpy(name_buf, node->nodedesc, 256);
> +		printf("Switch 0x%016"PRIx64" %s:\n",
> +			node->info.nodeguid,
> +			remap_node_name(node_name_map,
> +				node->info.nodeguid,
> +				name_buf));
> +	}
> +
> +	for (i = 1; i <= node->info.numports; i++) {
> +		ibnd_port_t *port = node->ports[i];
> +		if (!port)
> +			continue;
> +		if (!down_links_only || port->info.port_state == IBND_LINK_DOWN) {
> +			print_port(node, port);
> +		}
> +	}
> +}
> +
> +void
> +usage(void)
> +{
> +	fprintf(stderr,
> +		"Usage: %s [-hclp -S <guid> -D <direct route> -C <ca_name> -P <ca_port>]\n"
> +		"   Report link speed and connection for each port of each switch which is active\n"
> +		"   -h This help message\n"
> +		"   -S <guid> output only the node specified by guid\n"
> +		"   -D <direct route> print only node specified by <direct route>\n"
> +		"   -f <dr_path> specify node to start \"from\"\n"
> +		"   -n <hops> Number of hops to include away from specified node\n"
> +		"   -d print only down links\n"
> +		"   -l (line mode) print all information for each link on each line\n"
> +		"   -p print additional switch settings (PktLifeTime,HoqLife,VLStallCount)\n"
> +
> +
> +		"   -t <timeout_ms> timeout for any single fabric query\n"
> +		"   -s show progress during scan\n"
> +		"   --node-name-map <map_file> use specified node name map\n"
> +
> +		"   -C <ca_name> use selected Channel Adaptor name for queries\n"
> +		"   -P <ca_port> use selected channel adaptor port for queries\n"
> +		"   -g print port guids instead of node guids\n"
> +		"   --debug print debug messages\n"
> +		"   -R (this option is obsolete and does nothing)\n"
> +		,
> +			argv0);
> +	exit(-1);
> +}
> +
> +int
> +main(int argc, char **argv)
> +{
> +	char *ca = 0;
> +	int ca_port = 0;
> +	ibnd_fabric_t *fabric = NULL;
> +	uint64_t guid = 0;
> +	char *dr_path = NULL;
> +	char *from = NULL;
> +	int hops = 0;
> +	ib_portid_t port_id;
> +
> +	static char const str_opts[] = "S:D:n:C:P:t:sldgphuf:R";
> +	static const struct option long_opts[] = {
> +		{ "S", 1, 0, 'S'},
> +		{ "D", 1, 0, 'D'},
> +		{ "num-hops", 1, 0, 'n'},
> +		{ "down-links-only", 0, 0, 'd'},
> +		{ "line-mode", 0, 0, 'l'},
> +		{ "ca-name", 1, 0, 'C'},
> +		{ "ca-port", 1, 0, 'P'},
> +		{ "timeout", 1, 0, 't'},
> +		{ "show", 0, 0, 's'},
> +		{ "print-port-guids", 0, 0, 'g'},
> +		{ "print-additional", 0, 0, 'p'},
> +		{ "help", 0, 0, 'h'},
> +		{ "usage", 0, 0, 'u'},
> +		{ "node-name-map", 1, 0, 1},
> +		{ "debug", 0, 0, 2},
> +		{ "compat", 0, 0, 3},
> +		{ "from", 1, 0, 'f'},
> +		{ "R", 0, 0, 'R'},
> +		{ }
> +	};
> +
> +	f = stdout;

Where is 'f' used?

> +
> +	argv0 = argv[0];
> +
> +	while (1) {
> +		int ch = getopt_long(argc, argv, str_opts, long_opts, NULL);
> +		if ( ch == -1 )
> +			break;
> +		switch(ch) {
> +		case 1:
> +			node_name_map_file = strdup(optarg);

Should we use strdup() here and below? If yes, I don't see any free().

Sasha

> +			break;
> +		case 2:
> +			ibnd_debug(1);
> +			break;
> +		case 3:
> +			old_output = 1;
> +			break;
> +		case 'f':
> +			from = strdup(optarg);
> +			break;
> +		case 'C':
> +			ca = strdup(optarg);
> +			break;
> +		case 'P':
> +			ca_port = strtoul(optarg, 0, 0);
> +			break;
> +		case 'D':
> +			dr_path = strdup(optarg);
> +			break;
> +		case 'n':
> +			hops = (int)strtol(optarg, NULL, 0);
> +			break;
> +		case 'd':
> +			down_links_only = 1;
> +			break;
> +		case 'l':
> +			line_mode = 1;
> +			break;
> +		case 't':
> +			timeout_ms = strtoul(optarg, 0, 0);
> +			break;
> +		case 's':
> +			ibnd_show_progress(1);
> +			break;
> +		case 'g':
> +			print_port_guids = 1;
> +			break;
> +		case 'S':
> +			guid = (uint64_t)strtoull(optarg, 0, 0);
> +			break;
> +		case 'p':
> +			add_sw_settings = 1;
> +			break;
> +		case 'R':
> +			/* GNDN */
> +			break;
> +		default:
> +			usage();
> +			break;
> +		}
> +	}
> +	argc -= optind;
> +	argv += optind;
> +
> +	if (argc && !(f = fopen(argv[0], "w")))
> +		fprintf(stderr, "can't open file %s for writing", argv[0]);
> +
> +	node_name_map = open_node_name_map(node_name_map_file);
> +
> +	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, &port_id, hops)) == NULL) {
> +			fprintf(stderr, "discover failed\n");
> +			exit(1);
> +		}
> +		guid = 0;
> +	} else {
> +		if ((fabric = ibnd_discover_fabric(ca, ca_port, timeout_ms, NULL, -1)) == NULL) {
> +			fprintf(stderr, "discover failed\n");
> +			exit(1);
> +		}
> +	}
> +
> +	if (guid) {
> +		ibnd_node_t *sw = ibnd_find_node_guid(fabric, guid);
> +		print_switch(sw, NULL);
> +	} else if (dr_path) {
> +		ibnd_node_t *sw = ibnd_find_node_dr(fabric, dr_path);
> +		print_switch(sw, NULL);
> +	} else {
> +		ibnd_iter_nodes_type(fabric, print_switch, IBND_SWITCH_NODE, NULL);
> +	}
> +
> +	ibnd_destroy_fabric(fabric);
> +
> +	close_node_name_map(node_name_map);
> +	exit(0);
> +}
> -- 
> 1.5.4.5
> 



More information about the general mailing list