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

[Sasha Khapyorsky]

On 15:42 Fri 03 Apr     , Ira Weiny wrote:
> From a677ae35fe7a5966f05b5859df8f00e9b18df864 Mon Sep 17 00:00:00 2001

You know :)

> From: Ira Weiny <weiny2 at llnl.gov>
> Date: Fri, 3 Apr 2009 15:28:18 -0700
> Subject: [PATCH] Convert iblinkinfo.pl to C and use new ibnetdisc library.
> 
> Signed-off-by: Ira Weiny <weiny2 at llnl.gov>

Applied. Thanks. Couple of notes are below.

> ---
>  infiniband-diags/Makefile.am              |    7 +-
>  infiniband-diags/configure.in             |    1 +
>  infiniband-diags/scripts/iblinkinfo.pl    |  327 ------------------------
>  infiniband-diags/scripts/iblinkinfo.pl.in |   40 +++
>  infiniband-diags/src/iblinkinfo.c         |  386 +++++++++++++++++++++++++++++
>  5 files changed, 432 insertions(+), 329 deletions(-)
>  delete mode 100755 infiniband-diags/scripts/iblinkinfo.pl
>  create mode 100755 infiniband-diags/scripts/iblinkinfo.pl.in
>  create mode 100644 infiniband-diags/src/iblinkinfo.c
> 
> diff --git a/infiniband-diags/Makefile.am b/infiniband-diags/Makefile.am
> index 7b8523a..b480a4a 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

Here $(top_srcdir) should be used instead of $(top_builddir).
'top_builddir' points project build directory, not source directory
(where not generated header files are located). And build like:

  mkdir /tmp/ib-diags && cd /tmp/ib-diags
  /path/to/management/infiniband-diags/configure && make

will fail.

I'm fixing this.

>  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
>  
>  if ENABLE_TEST_UTILS
>  sbin_PROGRAMS += src/ibsendtrap src/mcm_rereg_test
> @@ -55,6 +56,8 @@ src_saquery_SOURCES = src/saquery.c
>  src_ibsendtrap_SOURCES = src/ibsendtrap.c
>  src_vendstat_SOURCES = src/vendstat.c
>  src_mcm_rereg_test_SOURCES = src/mcm_rereg_test.c
> +src_iblinkinfo_SOURCES = src/iblinkinfo.c
> +src_iblinkinfo_LDADD = -libnetdisc

I think here should be '-L$(top_builddir)/libibnetdisc -libnetdisc'.
Otherwise we are assuming pre-installed library (we could run
'make install', but it fails due to 'make' error :)).

Adding this too.

>  
>  man_MANS = man/ibaddr.8 man/ibcheckerrors.8 man/ibcheckerrs.8 \
>  	man/ibchecknet.8 man/ibchecknode.8 man/ibcheckport.8 \

[snip...]

> +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;
> +
> +	argv0 = argv[0];
> +
> +	while (1) {
> +		int ch = getopt_long(argc, argv, str_opts, long_opts, NULL);

Any reason to not use new ibdiag_process_opts() here? It should simplify
an options processing and unify the usage message.

Of course this can be done as subsequent patch.

Sasha