[PATCH] opensm/opensm/osm_subnet.c: add checks for HOQ and Leaf HOQ input values (Was: Re: [ofa-general] XmtDiscards)

[Hal Rosenstock]

On Mon, 2008-04-07 at 09:49 -0700, Ira Weiny wrote:
> On Sat, 05 Apr 2008 06:17:59 -0700
> Hal Rosenstock <hrosenstock at xsigo.com> wrote:
> 
> > On Fri, 2008-04-04 at 17:48 -0700, Boris Shpolyansky wrote:
> > > Bernd,
> > > 
> > > 0x14 is the maximal value for HOQ lifetime, which effectively disables
> > > the mechanism. I think you shouldn't exceed this value. 
> > 
> > True about the maximal value but any 5 bit value > 19 (up through 31)
> > should effectively be the same thing according to the spec.
> > 
> > I also think that OpenSM could do a better job validating and setting
> > this and other similar optional parameters.
> > 
> 
> As a start here is a patch which checks the HOQ life values.
> 
> Ira
> 
> From 9e05f091a3c9173045f523aee245e98af1bf74f3 Mon Sep 17 00:00:00 2001
> From: Ira K. Weiny <weiny2 at llnl.gov>
> Date: Mon, 7 Apr 2008 08:31:46 -0700
> Subject: [PATCH] opensm/opensm/osm_subnet.c: add checks for HOQ and Leaf HOQ input values
> 
> 
> Signed-off-by: Ira K. Weiny <weiny2 at llnl.gov>
> ---
>  opensm/opensm/osm_subnet.c |   22 ++++++++++++++++++++++
>  1 files changed, 22 insertions(+), 0 deletions(-)
> 
> diff --git a/opensm/opensm/osm_subnet.c b/opensm/opensm/osm_subnet.c
> index 47d735f..29d7cdc 100644
> --- a/opensm/opensm/osm_subnet.c
> +++ b/opensm/opensm/osm_subnet.c
> @@ -1045,6 +1045,28 @@ static void subn_verify_conf_file(IN osm_subn_opt_t * const p_opts)
>  		p_opts->force_link_speed = IB_PORT_LINK_SPEED_ENABLED_MASK;
>  	}
>  
> +	if (0x14 < p_opts->head_of_queue_lifetime) {
> +		sprintf(buff,
> +			" Invalid Cached Option Value:head_of_queue_lifetime = %u:"
> +			"Using Default:%u\n", p_opts->head_of_queue_lifetime,
> +			OSM_DEFAULT_HEAD_OF_QUEUE_LIFE);
> +		printf(buff);
> +		cl_log_event("OpenSM", CL_LOG_INFO, buff, NULL, 0);
> +		p_opts->head_of_queue_lifetime =
> +		OSM_DEFAULT_HEAD_OF_QUEUE_LIFE;
> +	}
> +
> +	if (0x14 < p_opts->leaf_head_of_queue_lifetime) {
> +		sprintf(buff,
> +			" Invalid Cached Option Value:leaf_head_of_queue_lifetime = %u:"
> +			"Using Default:%u\n", p_opts->leaf_head_of_queue_lifetime,
> +			OSM_DEFAULT_LEAF_HEAD_OF_QUEUE_LIFE);
> +		printf(buff);
> +		cl_log_event("OpenSM", CL_LOG_INFO, buff, NULL, 0);
> +		p_opts->leaf_head_of_queue_lifetime =
> +		OSM_DEFAULT_LEAF_HEAD_OF_QUEUE_LIFE;
> +	}
> +

Should these be set to max rather than default as it seems that that's
what they're more likely trying to do ?

-- Hal

>  	if (strcmp(p_opts->console, OSM_DISABLE_CONSOLE)
>  	    && strcmp(p_opts->console, OSM_LOCAL_CONSOLE)
>  #ifdef ENABLE_OSM_CONSOLE_SOCKET