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

[Nicolas Morey-Chaisemartin]

Le 16/04/2009 12:52, Yevgeny Kliteynik a écrit :
> Hi Nicolas,
>
> Nicolas Morey Chaisemartin wrote:
>> This patch enhances the use of the min hop table done in the Fat-Tree
>> algorithm.
....
>> /***************************************************/
>>
>> 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
>
>
>
>

Actually hops count to HCA is set in the local hop table, not in the OpenSM one.
As in the local hop table we have only one entry per switch, we need to set it once only which is done here:
>> CL_ASSERT(p_group->remote_node_type == IB_NODE_TYPE_SWITCH);
>> + p_remote_sw->hops[target_lid] = hops;

Not returning 0 would only do the same thing multiple times in the local table and still not set the OpenSM table.

So actual behaviour seems OK to me.

Nicolas