[ofa-general] [PATCH v2] opensm/osm_ucat_ftree.c Enhance min hops counters usage

[Yevgeny Kliteynik]

Hi Nicolas,

Nicolas Morey Chaisemartin wrote:
> This patch enhances the use of the min hop table done in the Fat-Tree algorithm.
> Before this patch, the algorithm was using the osm_sw hops table to store the minhop values toward any lid (Switch or not).
> As this table is allocated as we need it, it required a lot of malloc calls and quite some time to set the hops values on remote ports.
> 
> This patch corrects this behaviour:
> -The osm_sw hops table is only used for switch lid
> -ftree_sw_t struct now has its own hop table (only 1 dimensionnal as we don't need to know which port is used) to store its minhop value
> 
> 
> Signed-off-by: Nicolas Morey-Chaisemartin <nicolas.morey-chaisemartin at ext.bull.net>
> ---
> Fixed to work after 
> commit a10b57a2de9ace61455176ad5e43b7ca3d148cfb opensm/osm_ucast_ftree.c: lids are always handled in host order.
> Memory allocation fixed (using right byte order + checking if succesfull)
> 
> 
>  opensm/opensm/osm_ucast_ftree.c |   66 +++++++++++++++++++++++++++++++--------
>  1 files changed, 53 insertions(+), 13 deletions(-)
> 
> diff --git a/opensm/opensm/osm_ucast_ftree.c b/opensm/opensm/osm_ucast_ftree.c
> index dfe7009..83c901e 100644
> --- a/opensm/opensm/osm_ucast_ftree.c
> +++ b/opensm/opensm/osm_ucast_ftree.c
> @@ -172,6 +172,7 @@ typedef struct ftree_sw_t_ {
>  	uint8_t up_port_groups_num;
>  	boolean_t is_leaf;
>  	unsigned down_port_groups_idx;
> +	uint8_t *hops;
>  } ftree_sw_t;
>  
>  /***************************************************
> @@ -554,6 +555,11 @@ static ftree_sw_t *sw_create(IN ftree_fabric_t * p_ftree,
>  
>  	/* initialize lft buffer */
>  	memset(p_osm_sw->new_lft, OSM_NO_PATH, IB_LID_UCAST_END_HO + 1);
> +	p_sw->hops =
> +	    malloc(p_osm_sw->max_lid_ho * sizeof(*(p_sw->hops)));
> +	if(p_sw->hops == NULL)
> +		return NULL;
> +	memset(p_sw->hops, OSM_NO_PATH, p_osm_sw->max_lid_ho);
>  
>  	return p_sw;
>  }				/* sw_create() */
> @@ -566,6 +572,7 @@ static void sw_destroy(IN ftree_fabric_t * p_ftree, IN ftree_sw_t * p_sw)
>  
>  	if (!p_sw)
>  		return;
> +	free(p_sw->hops);
>  
>  	for (i = 0; i < p_sw->down_port_groups_num; i++)
>  		port_group_destroy(p_sw->down_port_groups[i]);
> @@ -693,32 +700,54 @@ static void sw_add_port(IN ftree_sw_t * p_sw, IN uint8_t port_num,
>  /***************************************************/
>  
>  static inline cl_status_t sw_set_hops(IN ftree_sw_t * p_sw, IN uint16_t lid,
> -				      IN uint8_t port_num, IN uint8_t hops)
> +				      IN uint8_t port_num, IN uint8_t hops,
> +				      IN boolean_t is_target_sw)
>  {
>  	/* set local min hop table(LID) */
> -	return osm_switch_set_hops(p_sw->p_osm_sw, lid, port_num, hops);
> +	p_sw->hops[lid] = hops;
> +	if (is_target_sw)
> +		return osm_switch_set_hops(p_sw->p_osm_sw, lid, port_num, hops);
> +	return 0;
>  }
>  
>  /***************************************************/
>  
>  static int set_hops_on_remote_sw(IN ftree_port_group_t * p_group,
> -				 IN uint16_t target_lid, IN uint8_t hops)
> +				 IN uint16_t target_lid, IN uint8_t hops,
> +				 IN boolean_t is_target_sw)
>  {
>  	ftree_port_t *p_port;
>  	uint8_t i, ports_num;
>  	ftree_sw_t *p_remote_sw = p_group->remote_hca_or_sw.p_sw;
>  
> +	/* if lid is a switch, we set the min hop table in the osm_switch struct */
>  	CL_ASSERT(p_group->remote_node_type == IB_NODE_TYPE_SWITCH);
> +	p_remote_sw->hops[target_lid] = hops;
> +
> +	/* If taget lid is a switch we set the min hop table values
> +	 * for each port on the associated osm_sw struct */

I could be missing something here, but is the following code correct?

> +	if (!is_target_sw)
> +		return 0;
> +
>  	ports_num = (uint8_t) cl_ptr_vector_get_size(&p_group->ports);
>  	for (i = 0; i < ports_num; i++) {
>  		cl_ptr_vector_at(&p_group->ports, i, (void *)&p_port);
>  		if (sw_set_hops(p_remote_sw, target_lid,
> -				p_port->remote_port_num, hops))
> +				p_port->remote_port_num, hops, is_target_sw))

sw_set_hops() takes care of the hops table - sets local hop count for
all types of targets and sets hops on osm_switch_t for switches only,
so the "return 0;" above will cause the hops not to be set at all for
HCA targets.

-- Yevgeny