[ofa-general] Re: [PATCH] opensm: fixing handling of opt.max_wire_smps

[Yevgeny Kliteynik]

Hi Sasha,

Sasha Khapyorsky wrote:
> On 17:32 Tue 04 Aug     , Yevgeny Kliteynik wrote:
>> opt.max_wire_smps is uint32, but then when it's propagated
>> into the VL15 poller it's casted to int32. Fixing the
>> parameter handling to protect it from wrong values.
>>
>> Signed-off-by: Yevgeny Kliteynik <kliteyn at dev.mellanox.co.il>
>> ---
>>  opensm/opensm/main.c       |    2 +-
>>  opensm/opensm/osm_subnet.c |    7 +++++++
>>  2 files changed, 8 insertions(+), 1 deletions(-)
>>
>> diff --git a/opensm/opensm/main.c b/opensm/opensm/main.c
>> index 296d5d5..9cb9990 100644
>> --- a/opensm/opensm/main.c
>> +++ b/opensm/opensm/main.c
>> @@ -722,7 +722,7 @@ int main(int argc, char *argv[])
>>
>>  		case 'n':
>>  			opt.max_wire_smps = strtol(optarg, NULL, 0);
> 
> Then you likely want to use strtoul().

Right
 
>> -			if (opt.max_wire_smps <= 0)
>> +			if (opt.max_wire_smps > 0x7FFFFFFF)
>>  				opt.max_wire_smps = 0x7FFFFFFF;
> 
> What about opt.max_wire_smps == 0?

Good point.

> Sasha
> 
>>  			printf(" Max wire smp's = %d\n", opt.max_wire_smps);
>>  			break;
>> diff --git a/opensm/opensm/osm_subnet.c b/opensm/opensm/osm_subnet.c
>> index ec15f8a..c07d823 100644
>> --- a/opensm/opensm/osm_subnet.c
>> +++ b/opensm/opensm/osm_subnet.c
>> @@ -1066,6 +1066,13 @@ int osm_subn_verify_config(IN osm_subn_opt_t * const p_opts)
>>  		p_opts->force_link_speed = IB_PORT_LINK_SPEED_ENABLED_MASK;
>>  	}
>>
>> +	if (p_opts->max_wire_smps > 0x7FFFFFFF) {
>> +		log_report(" Invalid Cached Option Value: max_wire_smps = %u,"
>> +			   " Using Default: %u\n",
>> +			   p_opts->max_wire_smps, OSM_DEFAULT_SMP_MAX_ON_WIRE);
>> +		p_opts->max_wire_smps = OSM_DEFAULT_SMP_MAX_ON_WIRE;
>> +	}
> 
> Ditto.

Right again.
And since we're on this, perhaps the right thing here would
be not using OSM_DEFAULT_SMP_MAX_ON_WIRE, but the maximal
valid value (0x7FFFFFFF)?

-- Yevgeny

> Sasha
>