[ofa-general] Re: [PATCH] osm: integer indexes in fat-tree

[Yevgeny Kliteynik]

Hi Hal,

Hal Rosenstock wrote:
> Hi Yevgeny,
> 
> On Mon, 2007-05-14 at 03:30, Yevgeny Kliteynik wrote:
>> Hi Hal,
>>
>> Enhancing integer indexes in fat-tree to 32 bits.
>> I'm not sure whether it's a bug - fat-tree routing makes indexing
>> not the same way as up/down. It marks rank on all the leaf switches,
>> and only then starts BFS (starting from all the leaf switches and not
>> from roots), so I don't think that it can really overflow the existing
>> indexes. But who knows...
>> Fixing this won't hurt anyway.
> 
> No, it won't.
> 
>> Please apply to master.
> 
> See comment/question below.
> 
> -- Hal
> 
>> -- Yevgeny
>>
>> Signed-off-by:  Yevgeny Kliteynik <kliteyn at dev.mellanox.co.il>
>> ---
>>  osm/opensm/osm_ucast_ftree.c |   12 ++++++------
>>  1 files changed, 6 insertions(+), 6 deletions(-)
>>
>> diff --git a/osm/opensm/osm_ucast_ftree.c b/osm/opensm/osm_ucast_ftree.c
>> index 7b6a6a5..ca51484 100644
>> --- a/osm/opensm/osm_ucast_ftree.c
>> +++ b/osm/opensm/osm_ucast_ftree.c
>> @@ -174,7 +174,7 @@ typedef struct ftree_sw_t_
>>  {
>>     cl_map_item_t          map_item;
>>     osm_switch_t         * p_osm_sw;
>> -   uint8_t                rank;
>> +   uint32_t               rank;
>>     ftree_tuple_t          tuple;
>>     ib_net16_t             base_lid;
>>     ftree_port_group_t  ** down_port_groups;
>> @@ -588,7 +588,7 @@ __osm_ftree_sw_create(
>>     memset(p_sw, 0, sizeof(ftree_sw_t));
>>  
>>     p_sw->p_osm_sw = p_osm_sw;
>> -   p_sw->rank = 0xFF;
>> +   p_sw->rank = 0xFFFFFFFF;
>>     __osm_ftree_tuple_init(p_sw->tuple);
>>  
>>     p_sw->base_lid = osm_node_get_base_lid(p_sw->p_osm_sw->p_node, 0);
>> @@ -678,7 +678,7 @@ static boolean_t
>>  __osm_ftree_sw_ranked(
>>     IN  ftree_sw_t * p_sw)
>>  {
>> -   return (p_sw->rank != 0xFF); 
>> +   return (p_sw->rank != 0xFFFFFFFF); 
>>  }
>>  
>>  /***************************************************/
>> @@ -1025,7 +1025,7 @@ __osm_ftree_fabric_destroy(ftree_fabric_
>>  /***************************************************/
>>  
>>  static void 
>> -__osm_ftree_fabric_set_rank(ftree_fabric_t * p_ftree, uint8_t rank)
>> +__osm_ftree_fabric_set_rank(ftree_fabric_t * p_ftree, uint32_t rank)
>>  {
>>     if (rank > p_ftree->tree_rank)
>>        p_ftree->tree_rank = rank;
>> @@ -1314,7 +1314,7 @@ __osm_ftree_fabric_assign_first_tuple(
>>     ftree_tuple_t new_tuple;
>>  
>>     __osm_ftree_tuple_init(new_tuple);
>> -   new_tuple[0] = p_sw->rank;
>> +   new_tuple[0] = (uint8_t)p_sw->rank;
> 
> Should the declaration of ftree_tuple_t change ?

Only if there's a chance that we will build a switch with more than 256 ports.
And when I say "switch" I mean a single unit, not the one that has internal
topology with several building blocks.
But in that case lots of things should be fixed.
For instance, ib_node_info_t.num_ports is 8 bits too.

-- Yevgeny

>>     for (i = 1; i <= p_sw->rank; i++)
>>        new_tuple[i] = 0;
>>  
>> @@ -1374,7 +1374,7 @@ __osm_ftree_fabric_calculate_rank(
>>  {
>>     ftree_sw_t   * p_sw;
>>     ftree_sw_t   * p_next_sw;
>> -   uint16_t       max_rank = 0;
>> +   uint32_t       max_rank = 0;
>>  
>>     /* go over all the switches and find maximal switch rank */
>>  
> 
>