[ofa-general] [PATCH] opensm: multi lid routing balancing for updn/minhop

[Sasha Khapyorsky]

On 11:01 Tue 04 Mar     , Al Chu wrote:
> From 391e2e3cdd08b205bdb94203660c0aacb8212d5a Mon Sep 17 00:00:00 2001
> From: Albert L. Chu <chu11 at llnl.gov>
> Date: Mon, 3 Mar 2008 10:39:43 -0800
> Subject: [PATCH] support balanced multi-lid routing
> 
> 
> Signed-off-by: Albert L. Chu <chu11 at llnl.gov>

Applied. Thanks.

I just have a few stylistic comments below. I applied the patch as is
with hope that we can fix it later in subsequent patches (fixed some
already).

[snip...]

> diff --git a/opensm/include/opensm/osm_switch.h b/opensm/include/opensm/osm_switch.h
> index e2fe86d..dbb2552 100644
> --- a/opensm/include/opensm/osm_switch.h
> +++ b/opensm/include/opensm/osm_switch.h
> @@ -158,6 +158,33 @@ typedef struct _osm_switch {
>  *	Switch object
>  *********/
>  
> +/****s* OpenSM: Switch/osm_switch_guid_count_t
> +* NAME
> +*       osm_switch_guid_count_t
> +*
> +* DESCRIPTION
> +*       Stores system and node guids and the number of

Tab character instead of spaces.

[snip...]

> --- a/opensm/opensm/osm_switch.c
> +++ b/opensm/opensm/osm_switch.c
> @@ -219,16 +219,99 @@ osm_switch_get_fwd_tbl_block(IN const osm_switch_t * const p_sw,
>  
>  /**********************************************************************
>   **********************************************************************/
> +static osm_switch_guid_count_t *
> +osm_switch_find_guid_common(IN const osm_switch_t * const p_sw,
> +			    IN osm_switch_guid_count_t * remote_guids,
> +			    IN uint16_t * p_num_remote_guids,
> +			    IN uint8_t port_num,
> +			    IN int find_sys_guid,
> +			    IN int find_node_guid)

We are using osm_ prefixes only for public functions, for statics names
can be shorter.

[snip...]

> diff --git a/opensm/opensm/osm_ucast_mgr.c b/opensm/opensm/osm_ucast_mgr.c
> index 1aa5ea9..d7fc4d3 100644
> --- a/opensm/opensm/osm_ucast_mgr.c
> +++ b/opensm/opensm/osm_ucast_mgr.c
> @@ -209,31 +209,21 @@ __osm_ucast_mgr_process_port(IN osm_ucast_mgr_t * const p_mgr,
>  	   in providing better routing in LMC > 0 situations
>  	 */
>  	uint16_t lids_per_port = 1 << p_mgr->p_subn->opt.lmc;
> -	uint64_t *remote_sys_guids = NULL;
> -	uint64_t *remote_node_guids = NULL;
> -	uint16_t num_used_sys = 0;
> -	uint16_t num_used_nodes = 0;
> +	osm_switch_guid_count_t *remote_guids = NULL;
> +	uint16_t num_used_guids = 0;
> +	osm_switch_guid_count_t *p_remote_guid_used = NULL;
>  
>  	OSM_LOG_ENTER(p_mgr->p_log);
>  
>  	if (lids_per_port > 1) {
> -		remote_sys_guids = malloc(sizeof(uint64_t) * lids_per_port);
> -		if (remote_sys_guids == NULL) {
> -			OSM_LOG(p_mgr->p_log, OSM_LOG_ERROR, "ERR 3A09: "
> +		remote_guids = malloc(sizeof(osm_switch_guid_count_t) * lids_per_port);
> +		if (remote_guids == NULL) {
> +			osm_log(p_mgr->p_log, OSM_LOG_ERROR,
> +				"__osm_ucast_mgr_process_port: ERR 3A0B: "

An error code (ie 'ERR XXXX') should be unique, so it is better to not
change this.

Sasha