[openib-general] [PATCHv2] OpenSM: Support configurable SL per partition

[Sasha Khapyorsky]

Hi Hal,

The minor comment is below.

On 07:11 Fri 14 Jul     , Hal Rosenstock wrote:
> OpenSM: Support configurable SL per partition
> 
> Signed-off-by: Sasha Khapyorsky <sashak at voltaire.com>
> Signed-off-by: Hal Rosenstock <halr at voltaire.com>
> 
> Index: include/opensm/osm_partition.h
> ===================================================================
> --- include/opensm/osm_partition.h	(revision 8520)
> +++ include/opensm/osm_partition.h	(working copy)
> @@ -102,6 +102,7 @@
>  {
>  	cl_map_item_t	map_item;
>  	uint16_t	pkey;
> +	uint8_t		sl; 
>  	cl_map_t	full_guid_tbl;
>  	cl_map_t	part_guid_tbl;
>  	char		name[32];
> @@ -114,6 +115,9 @@
>  *	pkey
>  *		The IBA defined P_KEY of this Partition.
>  *
> +*	sl
> +*		The Service Level (SL) associated with this Partiton.
> +*
>  *	port_guid_tbl
>  *		Container of pointers to all Port objects in the Partition,
>  *		indexed by port GUID.
> Index: opensm/osm_prtn.c
> ===================================================================
> --- opensm/osm_prtn.c	(revision 8520)
> +++ opensm/osm_prtn.c	(working copy)
> @@ -79,6 +79,7 @@
>  
>  	memset(p, 0, sizeof(*p));
>  	p->pkey = pkey;
> +	p->sl = OSM_DEFAULT_SL;
>  	cl_map_construct(&p->full_guid_tbl);
>  	cl_map_init(&p->full_guid_tbl, 32);
>  	cl_map_construct(&p->part_guid_tbl);
> @@ -220,7 +221,7 @@
>  	mc_rec.pkey = pkey;
>  	mc_rec.rate = rate ? rate : 0x3; /* 10Gb/sec */
>  	mc_rec.pkt_life = OSM_DEFAULT_SUBNET_TIMEOUT;
> -	mc_rec.sl_flow_hop = OSM_DEFAULT_SL << 28;
> +	mc_rec.sl_flow_hop = ib_member_set_sl_flow_hop(p->sl, 0, 0);
>  	/* Note: scope needs to be consistent with MGID */
>  	mc_rec.scope_state = 0x21;
>  
> Index: opensm/osm_prtn_config.c
> ===================================================================
> --- opensm/osm_prtn_config.c	(revision 8520)
> +++ opensm/osm_prtn_config.c	(working copy)
> @@ -52,8 +52,10 @@
>  #include <string.h>
>  #include <errno.h>
>  #include <ctype.h>
> +#include <limits.h>
            ^^^^^^^^^
I guess you don't need to include limits.h now, since MAX_ constants are
not used.

Sasha

>  
>  #include <iba/ib_types.h>
> +#include <opensm/osm_base.h>
>  #include <opensm/osm_partition.h>
>  #include <opensm/osm_subnet.h>
>  #include <opensm/osm_log.h>
> @@ -82,7 +84,7 @@
>  	osm_log_t  *p_log;
>  	osm_subn_t *p_subn;
>  	osm_prtn_t *p_prtn;
> -	unsigned    is_ipoib, mtu, rate;
> +	unsigned    is_ipoib, mtu, rate, sl;
>  };
>  
>  
> @@ -122,6 +124,16 @@
>  	if (!conf->p_prtn)
>  		return -1;
>  
> +	if (conf->p_subn->opt.no_qos) {
> +		if (conf->sl != OSM_DEFAULT_SL) {
> +			osm_log(conf->p_log, OSM_LOG_ERROR,
> +				"partition_create: Overriding SL %d to default SL %d on partition %s as QoS not enabled\n",
> +				conf->sl, OSM_DEFAULT_SL, name);
> +		  conf->sl = OSM_DEFAULT_SL;
> +		}
> +	}
> +	conf->p_prtn->sl = conf->sl;
> +
>  	if (conf->is_ipoib)
>  		osm_prtn_add_mcgroup(conf->p_log, conf->p_subn, conf->p_prtn,
>  			     conf->is_ipoib, conf->rate, conf->mtu);
> @@ -145,6 +157,17 @@
>  			PARSEWARN(conf->p_log, lineno,
>  				"flag \'rate\' requires valid value"
>  				" - skipped.\n");
> +	} else if (!strncmp(flag, "sl", len)) {
> +		unsigned sl;
> +		char *end;
> +
> +		if (!val || !*val || (sl = strtoul(val, &end, 0)) > 15 ||
> +		    (*end && !isspace(*end)))
> +			PARSEWARN(conf->p_log, lineno,
> +				"flag \'sl\' requires valid value"
> +				" - skipped.\n");
> +		else
> +			conf->sl = sl;
>  	} else {
>  		PARSEWARN(conf->p_log, lineno,
>  			"unrecognized partition flag \'%s\'"
> @@ -254,6 +277,8 @@
>  	conf->p_log = p_log;
>  	conf->p_subn = p_subn;
>  	conf->p_prtn = NULL;
> +	conf->is_ipoib = 0;
> +	conf->sl = OSM_DEFAULT_SL;
>  	return conf;
>  }
>  
> Index: doc/partition-config.txt
> ===================================================================
> --- doc/partition-config.txt	(revision 8520)
> +++ doc/partition-config.txt	(working copy)
> @@ -46,6 +46,7 @@
>               result IPoIB capable MC group will be created.
>  rate=<val> - specifies rate for this IPoIB MC group (default is 3 (10GBps))
>  mtu=<val>  - specifies MTU for this IPoIB MC group (default is 4 (2048))
> +sl=<val>   - specifies SL for this IPoIB MC group (default is 0)
>  
>  Note that values for 'rate' and 'mtu' should be specified as defined in
>  IBTA specification (for example mtu=4 for 2048).
> 
> 
>