[ofa-general] Re: [PATCH 2/2] opensm: setup routing engine when in use and delete when fail

[Sasha Khapyorsky]

On 17:36 Thu 26 Feb     , Eli Dorfman (Voltaire) wrote:
>  setup routing engine when in use and delete when fail
>  setup routing engine before use.
>  delete resources when routing algorithm fails
>  this will save allocation for routing algorithms that are not used.
> 
> Signed-off-by: Eli Dorfman <elid at voltaire.com>
> ---
>  opensm/opensm/osm_opensm.c    |   20 ++++++--------------
>  opensm/opensm/osm_ucast_mgr.c |   34 +++++++++++++++++++++++++++++++++-
>  2 files changed, 39 insertions(+), 15 deletions(-)
> 
> diff --git a/opensm/opensm/osm_opensm.c b/opensm/opensm/osm_opensm.c
> index 7de2e5b..a2620d5 100644
> --- a/opensm/opensm/osm_opensm.c
> +++ b/opensm/opensm/osm_opensm.c
> @@ -169,21 +169,14 @@ static void setup_routing_engine(osm_opensm_t *osm, const char *name)
>  			memset(re, 0, sizeof(struct osm_routing_engine));
>  
>  			re->name = m->name;
> -			if (m->setup(re, osm)) {
> -				OSM_LOG(&osm->log, OSM_LOG_VERBOSE,
> -					"setup of routing"
> -					" engine \'%s\' failed\n", name);
> -				return;
> -			}
> -			OSM_LOG(&osm->log, OSM_LOG_DEBUG,
> -				"\'%s\' routing engine set up\n", re->name);
> +			re->setup = m->setup;
>  			append_routing_engine(osm, re);
>  			return;
>  		}
>  	}
>  
>  	OSM_LOG(&osm->log, OSM_LOG_ERROR,
> -		"cannot find or setup routing engine \'%s\'", name);
> +		"cannot find or setup routing engine \'%s\'\n", name);

This is needed fix, but unrelated to the patch.

>  }
>  
>  static void setup_routing_engines(osm_opensm_t *osm, const char *engine_names)
> @@ -224,18 +217,17 @@ void osm_opensm_construct(IN osm_opensm_t * const p_osm)
>  
>  /**********************************************************************
>   **********************************************************************/
> -static void destroy_routing_engines(osm_opensm_t *osm)
> +static void destroy_routing_engines(struct osm_routing_engine **re)

Any reason for function prototype change?

>  {
>  	struct osm_routing_engine *r, *next;
>  
> -	next = osm->routing_engine_list;
> +	next = *re;
>  	while (next) {
>  		r = next;
>  		next = r->next;
> -		if (r->delete)
> -			r->delete(r->context);

What will happen with currently used routing engine? It will be never
deleted, leak? right?

>  		free(r);
>  	}
> +	*re = NULL;
>  }
>  
>  /**********************************************************************
> @@ -289,7 +281,7 @@ void osm_opensm_destroy(IN osm_opensm_t * const p_osm)
>  
>  	/* do the destruction in reverse order as init */
>  	destroy_plugins(p_osm);
> -	destroy_routing_engines(p_osm);
> +	destroy_routing_engines(&p_osm->routing_engine_list);
>  	osm_sa_destroy(&p_osm->sa);
>  	osm_sm_destroy(&p_osm->sm);
>  #ifdef ENABLE_OSM_PERF_MGR
> diff --git a/opensm/opensm/osm_ucast_mgr.c b/opensm/opensm/osm_ucast_mgr.c
> index e404c91..7175926 100644
> --- a/opensm/opensm/osm_ucast_mgr.c
> +++ b/opensm/opensm/osm_ucast_mgr.c
> @@ -886,7 +886,6 @@ int osm_ucast_mgr_process(IN osm_ucast_mgr_t * const p_mgr)
>  
>  	p_sw_guid_tbl = &p_mgr->p_subn->sw_guid_tbl;
>  	p_osm = p_mgr->p_subn->p_osm;
> -	p_routing_eng = p_osm->routing_engine_list;
>  
>  	CL_PLOCK_EXCL_ACQUIRE(p_mgr->p_lock);
>  
> @@ -897,10 +896,30 @@ int osm_ucast_mgr_process(IN osm_ucast_mgr_t * const p_mgr)
>  	    ucast_mgr_setup_all_switches(p_mgr->p_subn) < 0)
>  		goto Exit;
>  
> +	/* update the entry in active list */
> +
>  	p_osm->routing_engine_used = OSM_ROUTING_ENGINE_TYPE_NONE;
> +	p_routing_eng = p_osm->routing_engine_list;
>  	while (p_routing_eng) {
> +		if (!p_routing_eng->initialized && 
> +			p_routing_eng->setup(p_routing_eng, p_osm)) {
                   ^^^^^^^^
> +			OSM_LOG(p_mgr->p_log, OSM_LOG_VERBOSE,
> +				"setup of routing engine \'%s\' failed\n", 
> +					p_routing_eng->name);
                                ^^^^^^^^

Keep indentation please.

> +			p_routing_eng = p_routing_eng->next;
> +			continue;
> +		}
> +		OSM_LOG(p_mgr->p_log, OSM_LOG_DEBUG,
> +			"\'%s\' routing engine set up\n", p_routing_eng->name);
> +		p_routing_eng->initialized = 1;
> +
>  		if (!ucast_mgr_route(p_routing_eng, p_osm))
>  			break;
> +
> +		/* delete unused routing engine */
> +		if (p_routing_eng->delete)
> +			p_routing_eng->delete(p_routing_eng->context);
> +
>  		p_routing_eng = p_routing_eng->next;
>  	}
>  
> @@ -911,6 +930,19 @@ int osm_ucast_mgr_process(IN osm_ucast_mgr_t * const p_mgr)
>  		p_osm->routing_engine_used = OSM_ROUTING_ENGINE_TYPE_MINHOP;
>  	}
>  
> +	/* if for some reason different routing engine is used */
> +	/* cleanup unused routing engine */
> +	p_routing_eng = p_osm->routing_engine_list;
> +	while (p_routing_eng) {
> +		if (p_routing_eng->initialized &&
> +			p_osm->routing_engine_used != 
> +				osm_routing_engine_type(p_routing_eng->name) &&
> +			p_routing_eng->delete) 
> +			p_routing_eng->delete(p_routing_eng->context);
> +
> +		p_routing_eng = p_routing_eng->next;
> +	}
> +

How this section of code is useful?

Sasha