[ofa-general] [PATCH] opensm/osm_ucast_ftree.c: some refactoring

[Nicolas Morey Chaisemartin]

Hi,

Are you sure of this?
Seems to me as if there is a problem
Yevgeny Kliteynik wrote:

> @@ -710,13 +713,23 @@ sw_add_port(IN ftree_sw_t * p_sw,
> 
>  /***************************************************/
> 
> -static inline cl_status_t
> +static int
>  sw_set_hops(IN ftree_sw_t * p_sw,
> -	    IN uint16_t lid_ho, IN uint8_t port_num,
> +	    IN uint16_t lid_ho,
> +	    IN ftree_port_group_t * p_group,
>  	    IN uint8_t hops)
>  {
> -	/* set local min hop table(LID) */
> -	return osm_switch_set_hops(p_sw->p_osm_sw, lid_ho, port_num, hops);
> +	ftree_port_t * p_port;
> +	uint8_t i, ports_num;
> +
> +	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 (osm_switch_set_hops(p_sw->p_osm_sw, lid_ho,
> +					p_port->port_num, hops))
> +			return -1;
> +	}
> +	return 0;
>  }
> 

Function is setting hops value for the origin ports of the given port group on the given switch
>  /***************************************************
> @@ -2027,19 +2040,11 @@ fabric_route_upgoing_by_going_down(IN ftree_fabric_t * p_ftree,
> 
>  			/* On the remote switch that is pointed by the p_group,
>  			   set hops for ALL the ports in the remote group. */
> -
> -			for (j = 0; j < ports_num; j++) {
> -				cl_ptr_vector_at(&p_group->ports, j,
> -						 (void *)&p_port);
> -
> -				sw_set_hops(p_remote_sw,
> -					cl_ntoh16(target_lid),
> -					p_port->remote_port_num,
> -					((target_rank - highest_rank_in_route) +
> -					 (p_remote_sw->rank - highest_rank_in_route)
> -					 + reverse_hops * 2));
> -			}
> -
> +			sw_set_hops(p_remote_sw,
> +				cl_ntoh16(target_lid), p_group,
> +				((target_rank - highest_rank_in_route) +
> +				 (p_remote_sw->rank - highest_rank_in_route) +
> +				 reverse_hops * 2));
>  		}

Here you call the function wth the remote switch and a local port group.
So if I'm write the port num you use on the function are the port num on p_sw and not p_remote_sw...

The remote port group should be pass to this function but I don't think we have an easy way to get it.
Other solution could be to factorize the code in another function which would call sw_set_hops and provide it the right port num.

Anyway, how is the hops table used?
In the ftree algorithm, it seems we only use the min value through osm_switch_get_least_hops so we don't use the fact that there can be multiple values.
If it has no use in the Ftree algorithm, is it worth spending time to set all the hop values ?


Nicolas