[ofa-general] Re: [PATCH 5/6] opensm/Unicast Routing Cache: integrate cache into opensm

[Yevgeny Kliteynik]

Hi Sasha,

Sasha Khapyorsky wrote:
> Hi Yevgeny,
> 
> On 03:29 Mon 06 Oct     , Yevgeny Kliteynik wrote:
> 
> [snip...]
> 
>> @@ -818,27 +826,37 @@ int osm_ucast_mgr_process(IN osm_ucast_mgr_t * const p_mgr)
>>  	/*
>>  	   If there are no switches in the subnet, we are done.
>>  	 */
>> -	if (cl_qmap_count(p_sw_guid_tbl) == 0 ||
>> -	    ucast_mgr_setup_all_switches(p_mgr->p_subn) < 0)
>> +	if (cl_qmap_count(p_sw_guid_tbl) == 0)
>>  		goto Exit;
>>
>>  	p_osm->routing_engine_used = OSM_ROUTING_ENGINE_TYPE_NONE;
>> -	while (p_routing_eng) {
>> -		if (!ucast_mgr_route(p_routing_eng, p_osm))
>> -			break;
>> -		p_routing_eng = p_routing_eng->next;
>> -	}
>> +	if (p_mgr->p_subn->opt.use_ucast_cache &&
>> +	    osm_ucast_cache_is_valid(p_mgr->p_cache)) {
>> +		OSM_LOG(p_mgr->p_log, OSM_LOG_INFO,
>> +			"Configuring switch tables using cached routing\n");
>> +		osm_ucast_cache_apply(p_mgr->p_cache);
>>
>> -	if (p_osm->routing_engine_used == OSM_ROUTING_ENGINE_TYPE_NONE) {
>> -		/* If configured routing algorithm failed, use default MinHop */
>> -		osm_ucast_mgr_build_lid_matrices(p_mgr);
>> -		ucast_mgr_build_lfts(p_mgr);
>> -		p_osm->routing_engine_used = OSM_ROUTING_ENGINE_TYPE_MINHOP;
>> -	}
>> +	} else {
> 
> I think this will break some routing engines (such LASH) and logging
> because p_osm->routing_engine_used is leaved as
> OSM_ROUTING_ENGINE_TYPE_NONE.
> 
> And since we have cache validation calls in do_sweep() anyway:
> 
> +	if (sm->p_subn->opt.use_ucast_cache)
> +		osm_ucast_cache_validate(sm->ucast_mgr.p_cache);
> 
> Isn't it would be better to not touch osm_ucast_mgr_process() at all and
> instead to replace lines above by:
> 
> 	if (!sm->p_subn->opt.use_ucast_cache ||
> 	    osm_ucast_cache_process(sm->ucast_mgr.p_cache))
> 
> This also saves couple of public calls in ucast cache. I think then the
> patch could look like below. Agreed?

Yes, this is a good idea. Thanks for the patch.

-- Yevgeny