opensm: Add a name to IB subnet and include it in syslog messages

Added a text based name to an IB subnet to help user in identifying an IB subnet or understanding its function in a multi-fabric IB cluster. For example, in a dual-fabric (or dual-rail) IB cluster, one subnet could be named "mpi" and the other subnet could be named "storage".

Summary of changes:
o Added the option 'subnet_name' to OpenSM command line and config file.
o Enhanced OpenSM logging facility to include the subnet name in syslog messages.

Signed-off-by Arputham Benjamin <abenjamin-sJ/[email protected]>

---
diff -rup a/include/opensm/osm_log.h b/include/opensm/osm_log.h

include/opensm/osm_log.h +1 -0

@@ -120,6 +120,7 @@ typedef struct osm_log {
 	boolean_t accum_log_file;
 	boolean_t daemon;
 	char *log_file_name;
+	char *subnet_name;
 } osm_log_t;
 /*********/
 

diff -rup a/include/opensm/osm_subnet.h b/include/opensm/osm_subnet.h

include/opensm/osm_subnet.h +1 -0

@@ -224,6 +224,7 @@ typedef struct osm_subn_opt {
 	char *event_plugin_name;
 	char *node_name_map_name;
 	char *prefix_routes_file;
+	char *subnet_name;
 	boolean_t consolidate_ipv6_snm_req;
 	struct osm_subn_opt *file_opts; /* used for update */
 	uint8_t lash_start_vl;			/* starting vl to use in lash */

diff -rup a/man/opensm.8.in b/man/opensm.8.in

man/opensm.8.in +4 -0

@@ -49,6 +49,7 @@ opensm \- InfiniBand subnet manager and 
 [\-\-perfmgr_sweep_time_s <seconds>]
 [\-\-prefix_routes_file <path>]
 [\-\-consolidate_ipv6_snm_req]
+[\-\-subnet_name <subnet name>]
 [\-v(erbose)] [\-V] [\-D <flags>] [\-d(ebug) <number>]
 [\-h(elp)] [\-?]
 

@@ -345,6 +346,9 @@ effect if --enable-perfmgr was specified
 Use shared MLID for IPv6 Solicited Node Multicast groups per MGID scope
 and P_Key.
 .TP
+\fB\-\-subnet_name\fR <subnet name>
+This option specifies the text based name of the subnet.
+.TP
 \fB\-v\fR, \fB\-\-verbose\fR
 This option increases the log verbosity level.
 The -v option may be specified multiple times

diff -rup a/opensm/main.c b/opensm/main.c

opensm/main.c +8 -0

@@ -324,6 +324,8 @@ static void show_usage(void)
 	printf("--consolidate_ipv6_snm_req\n"
 	       "          Use shared MLID for IPv6 Solicited Node Multicast groups\n"
 	       "          per MGID scope and P_Key.\n\n");
+	printf("--subnet_name <subnet name>\n"
+	       "          Text based name of the IB subnet.\n\n");
 	printf("--verbose, -v\n"
 	       "          This option increases the log verbosity level.\n"
 	       "          The -v option may be specified multiple times\n"

@@ -607,6 +609,7 @@ int main(int argc, char *argv[])
 		{"lash_start_vl", 1, NULL, 6},
 		{"sm_sl", 1, NULL, 7},
 		{"retries", 1, NULL, 8},
+		{"subnet_name", 1, NULL, 9},
 		{NULL, 0, NULL, 0}	/* Required at the end of the array */
 	};
 

@@ -985,6 +988,11 @@ int main(int argc, char *argv[])
 			printf(" Transaction retries = %u\n",
 			       opt.transaction_retries);
 			break;
+		case 9:
+			SET_STR_OPT(opt.subnet_name, optarg);
+			printf("IB subnet name = %s\n",
+			       opt.subnet_name);
+			break;
 		case 'h':
 		case '?':
 		case ':':

diff -rup a/opensm/osm_log.c b/opensm/osm_log.c

opensm/osm_log.c +1 -0

@@ -107,6 +107,7 @@ void osm_log(IN osm_log_t * p_log, IN os
 	char buffer[LOG_ENTRY_SIZE_MAX];
 	va_list args;
 	int ret;
+	uint8_t ind = 0;
 #ifdef __WIN__
 	SYSTEMTIME st;
 	uint32_t pid = GetCurrentThreadId();

opensm/osm_log.c +5 -0

+		vsprintf(&buffer[ind], p_str, args);
+	}
+	else {
+		vsprintf(buffer, p_str, args);
+	}
 	va_end(args);
 
 	/* this is a call to the syslog */

diff -rup a/opensm/osm_opensm.c b/opensm/osm_opensm.c

opensm/osm_opensm.c +1 -0

@@ -337,6 +337,7 @@ ib_api_status_t osm_opensm_init(IN osm_o
 				 p_opt->log_max_size, p_opt->accum_log_file);
 	if (status != IB_SUCCESS)
 		return status;
+	p_osm->log.subnet_name = p_opt->subnet_name;
 
 	/* If there is a log level defined - add the OSM_VERSION to it */
 	osm_log(&p_osm->log,

diff -rup a/opensm/osm_subnet.c b/opensm/osm_subnet.c

opensm/osm_subnet.c +1 -0

@@ -395,6 +395,7 @@ static const opt_rec_t opt_tbl[] = {
 	{ "consolidate_ipv6_snm_req", OPT_OFFSET(consolidate_ipv6_snm_req), opts_parse_boolean, NULL, 1 },
 	{ "lash_start_vl", OPT_OFFSET(lash_start_vl), opts_parse_uint8, NULL, 1 },
 	{ "sm_sl", OPT_OFFSET(sm_sl), opts_parse_uint8, NULL, 1 },
+	{ "subnet_name", OPT_OFFSET(subnet_name), opts_parse_charp, NULL, 1 },
 	{0}
 };
 

@@ -1629,6 +1630,11 @@ int osm_subn_output_conf(FILE *out, IN o
 		"consolidate_ipv6_snm_req %s\n\n",

Hi Benjamin,

Quoted 9 lines · Arputham Benjamin

Added a text based name to an IB subnet to help user in identifying an IB subnet or understanding its function in a multi-fabric IB cluster. For example, in a dual-fabric (or dual-rail) IB cluster, one subnet could be named "mpi" and the other subnet could be named "storage".

Summary of changes:
o Added the option 'subnet_name' to OpenSM command line and config file.
o Enhanced OpenSM logging facility to include the subnet name in syslog messages.

Looking at the usage below I can see that his is done as adding "free text" prefix to syslog prints. This is fine and seems could be useful for any purpose (not only different subnets) when syslog message mark is desired.

Assuming so, wouldn't it be better to change "subnet_name" to something more generic, let say "log_prefix"?

Quoted 89 lines

Signed-off-by Arputham Benjamin <abenjamin-sJ/[email protected]>

---
diff -rup a/include/opensm/osm_log.h b/include/opensm/osm_log.h

include/opensm/osm_log.h +1 -0

@@ -120,6 +120,7 @@ typedef struct osm_log {
 	boolean_t accum_log_file;
 	boolean_t daemon;
 	char *log_file_name;
+	char *subnet_name;
 } osm_log_t;
 /*********/
 

diff -rup a/include/opensm/osm_subnet.h b/include/opensm/osm_subnet.h

include/opensm/osm_subnet.h +1 -0

@@ -224,6 +224,7 @@ typedef struct osm_subn_opt {
 	char *event_plugin_name;
 	char *node_name_map_name;
 	char *prefix_routes_file;
+	char *subnet_name;
 	boolean_t consolidate_ipv6_snm_req;
 	struct osm_subn_opt *file_opts; /* used for update */
 	uint8_t lash_start_vl;			/* starting vl to use in lash */

diff -rup a/man/opensm.8.in b/man/opensm.8.in

man/opensm.8.in +4 -0

@@ -49,6 +49,7 @@ opensm \- InfiniBand subnet manager and 
 [\-\-perfmgr_sweep_time_s <seconds>]
 [\-\-prefix_routes_file <path>]
 [\-\-consolidate_ipv6_snm_req]
+[\-\-subnet_name <subnet name>]
 [\-v(erbose)] [\-V] [\-D <flags>] [\-d(ebug) <number>]
 [\-h(elp)] [\-?]
 

@@ -345,6 +346,9 @@ effect if --enable-perfmgr was specified
 Use shared MLID for IPv6 Solicited Node Multicast groups per MGID scope
 and P_Key.
 .TP
+\fB\-\-subnet_name\fR <subnet name>
+This option specifies the text based name of the subnet.
+.TP
 \fB\-v\fR, \fB\-\-verbose\fR
 This option increases the log verbosity level.
 The -v option may be specified multiple times

diff -rup a/opensm/main.c b/opensm/main.c

opensm/main.c +8 -0

@@ -324,6 +324,8 @@ static void show_usage(void)
 	printf("--consolidate_ipv6_snm_req\n"
 	       "          Use shared MLID for IPv6 Solicited Node Multicast groups\n"
 	       "          per MGID scope and P_Key.\n\n");
+	printf("--subnet_name <subnet name>\n"
+	       "          Text based name of the IB subnet.\n\n");
 	printf("--verbose, -v\n"
 	       "          This option increases the log verbosity level.\n"
 	       "          The -v option may be specified multiple times\n"

@@ -607,6 +609,7 @@ int main(int argc, char *argv[])
 		{"lash_start_vl", 1, NULL, 6},
 		{"sm_sl", 1, NULL, 7},
 		{"retries", 1, NULL, 8},
+		{"subnet_name", 1, NULL, 9},
 		{NULL, 0, NULL, 0}	/* Required at the end of the array */
 	};
 

@@ -985,6 +988,11 @@ int main(int argc, char *argv[])
 			printf(" Transaction retries = %u\n",
 			       opt.transaction_retries);
 			break;
+		case 9:
+			SET_STR_OPT(opt.subnet_name, optarg);
+			printf("IB subnet name = %s\n",
+			       opt.subnet_name);
+			break;
 		case 'h':
 		case '?':
 		case ':':

diff -rup a/opensm/osm_log.c b/opensm/osm_log.c

opensm/osm_log.c +1 -0

@@ -107,6 +107,7 @@ void osm_log(IN osm_log_t * p_log, IN os
 	char buffer[LOG_ENTRY_SIZE_MAX];
 	va_list args;
 	int ret;
+	uint8_t ind = 0;
 #ifdef __WIN__
 	SYSTEMTIME st;
 	uint32_t pid = GetCurrentThreadId();

opensm/osm_log.c void osm_log(IN osm_log_t * p_log, IN os · line 129

Quoted 7 lines

opensm/osm_log.c +3 -1

@@ -123,7 +124,14 @@ void osm_log(IN osm_log_t * p_log, IN os
 		return;
 
 	va_start(args, p_str);
-	vsprintf(buffer, p_str, args);
+	if(p_log->subnet_name) {
+		snprintf(buffer, LOG_ENTRY_SIZE_MAX, "%s: ", p_log->subnet_name);
+		ind = strlen(buffer);

strlen() returns size_t (not uint8_t). And actually you don't need this because snprintf() already returns number of printed characters. All this part can be done as:

	int n = snprintf(buffer, sizeof(buffer), "%s: ", p_log->subnet_name);
	vsprintf(buffer + n, p_str, args);

Sasha

Quoted 41 lines

opensm/osm_log.c +5 -0

+		vsprintf(&buffer[ind], p_str, args);
+	}
+	else {
+		vsprintf(buffer, p_str, args);
+	}
 	va_end(args);
 
 	/* this is a call to the syslog */

diff -rup a/opensm/osm_opensm.c b/opensm/osm_opensm.c

opensm/osm_opensm.c +1 -0

@@ -337,6 +337,7 @@ ib_api_status_t osm_opensm_init(IN osm_o
 				 p_opt->log_max_size, p_opt->accum_log_file);
 	if (status != IB_SUCCESS)
 		return status;
+	p_osm->log.subnet_name = p_opt->subnet_name;
 
 	/* If there is a log level defined - add the OSM_VERSION to it */
 	osm_log(&p_osm->log,

diff -rup a/opensm/osm_subnet.c b/opensm/osm_subnet.c

opensm/osm_subnet.c +6 -0

@@ -395,6 +395,7 @@ static const opt_rec_t opt_tbl[] = {
 	{ "consolidate_ipv6_snm_req", OPT_OFFSET(consolidate_ipv6_snm_req), opts_parse_boolean, NULL, 1 },
 	{ "lash_start_vl", OPT_OFFSET(lash_start_vl), opts_parse_uint8, NULL, 1 },
 	{ "sm_sl", OPT_OFFSET(sm_sl), opts_parse_uint8, NULL, 1 },
+	{ "subnet_name", OPT_OFFSET(subnet_name), opts_parse_charp, NULL, 1 },
 	{0}
 };
 

@@ -1629,6 +1630,11 @@ int osm_subn_output_conf(FILE *out, IN o
 		"consolidate_ipv6_snm_req %s\n\n",
 		p_opts->consolidate_ipv6_snm_req ? "TRUE" : "FALSE");
 
+	fprintf(out,
+		"# Subnet name\n"
+		"subnet_name %s\n\n",
+		p_opts->subnet_name);
+
 	/* optional string attributes ... */
 

opensm/osm_subnet.c int osm_subn_output_conf(FILE *out, IN o · line 1640

Quoted 2 lines

opensm/osm_subnet.c +0 -0

 	return 0;