[ofa-general] Re: [PATCH] opensm/osm_state_mgr.c: bug fix in unicast cache

[Yevgeny Kliteynik]

Sasha Khapyorsky wrote:
> Hi Yevgeny,
> 
> On 11:51 Wed 19 Nov     , Yevgeny Kliteynik wrote:
>> Hi Sasha,
>>
>> When there are errors during initialization and new
>> heavy sweep is forced, unicast cache might hold a
>> snapshot of the previous routing, and since there
>> might be no *topology* changes, ucast cache will
>> apply that cached routing, which might be wrong.
>>
>> This patch invalidates cache explicitly if there
>> were initialization errors in addition to few other
>> cases.
>>
>> This fix addresses bug #1398.
>>
>> Signed-off-by: Yevgeny Kliteynik <kliteyn at dev.mellanox.co.il>
>> ---
>>  opensm/opensm/osm_state_mgr.c |   16 ++++++++++++----
>>  1 files changed, 12 insertions(+), 4 deletions(-)
>>
>> diff --git a/opensm/opensm/osm_state_mgr.c b/opensm/opensm/osm_state_mgr.c
>> index 841438c..d00e8ff 100644
>> --- a/opensm/opensm/osm_state_mgr.c
>> +++ b/opensm/opensm/osm_state_mgr.c
>> @@ -1064,6 +1064,18 @@ static void do_sweep(osm_sm_t * sm)
>>  	}
>>
>>  	/*
>> +	 * Unicast cache should be invalidated if:
>> +	 *  - every sweep is a heavy sweep
>> +	 *  - there were errors during initialization
>> +	 *  - subnet re-route is requested
>> +	 */
>> +	if (sm->p_subn->opt.use_ucast_cache &&
>> +	    (sm->p_subn->opt.force_heavy_sweep ||
> 
> Why 'opt.force_heavy_sweep' should be there? It is possible to enforce
> heavy sweep without routing cache just by using:
> 
> opt.force_heavy_sweep TRUE
> opt.use_ucast_cache FALSE

Well, it doesn't have to be there.
The opt.force_heavy_sweep is kind of debug mode of opensm,
so I just wanted to disable cache in that case.
Want me to remove it and repost the patch?

-- Yevgeny

> Sasha
> 
>> +	     sm->p_subn->subnet_initialization_error ||
>> +	     sm->p_subn->force_reroute))
>> +		osm_ucast_cache_invalidate(&sm->ucast_mgr);
>> +
>> +	/*
>>  	 * If we don't need to do a heavy sweep and we want to do a reroute,
>>  	 * just reroute only.
>>  	 */
>> @@ -1079,10 +1091,6 @@ static void do_sweep(osm_sm_t * sm)
>>  		/* Re-program the switches fully */
>>  		sm->p_subn->ignore_existing_lfts = TRUE;
>>
>> -		/* we want to re-route, so cache should be invalidated */
>> -		if (sm->p_subn->opt.use_ucast_cache)
>> -			osm_ucast_cache_invalidate(&sm->ucast_mgr);
>> -
>>  		osm_ucast_mgr_process(&sm->ucast_mgr);
>>
>>  		/* Reset flag */
>> -- 
>> 1.5.1.4
>>
>