[openib-general] [PATCH] osm: added an option for providing dump function per routing engine

[Yevgeny Kliteynik]

Hi Hal,

Hal Rosenstock wrote:
> Hi Yevgeny,
> 
> On Tue, 2006-12-19 at 14:37, Yevgeny Kliteynik wrote:
>> Hi Hal
>>
>> As you suggested, added an option for providing dump 
>> function per routing engine.
>>
>> Signed-off-by: Yevgeny Kliteynik <kliteyn at dev.mellanox.co.il>
> 
> Thanks, Applied.
> 
> One minor question below:
> 
>>  osm/include/opensm/osm_opensm.h |    4 ++++
>>  osm/opensm/osm_ucast_mgr.c      |   23 ++++++++++++++---------
>>  2 files changed, 18 insertions(+), 9 deletions(-)
> 
> [snip...]
> 
>> diff --git a/osm/opensm/osm_ucast_mgr.c b/osm/opensm/osm_ucast_mgr.c
>> index e051c66..fcf6f72 100644
>> --- a/osm/opensm/osm_ucast_mgr.c
>> +++ b/osm/opensm/osm_ucast_mgr.c
> 
> [snip...]
> 
>> @@ -1256,16 +1257,20 @@ osm_ucast_mgr_process(
>>      build and download the switch forwarding tables.
>>    */
>>  
>> -  if (!p_routing_eng->ucast_build_fwd_tables ||
>> -      p_routing_eng->ucast_build_fwd_tables(p_routing_eng->context) != 0)
>> -  {
>> -    cl_qmap_apply_func( p_sw_guid_tbl,
>> -                        __osm_ucast_mgr_process_tbl, p_mgr );
>> -  }
>> +  if ( p_routing_eng->ucast_build_fwd_tables && 
>> +       (p_routing_eng->ucast_build_fwd_tables(p_routing_eng->context) == 0) )
>> +     default_routing = FALSE;
>> +  else
>> +     cl_qmap_apply_func( p_sw_guid_tbl, __osm_ucast_mgr_process_tbl, p_mgr );
>>  
>>    /* dump fdb into file: */
>>    if ( osm_log_is_active( p_mgr->p_log, OSM_LOG_ROUTING ) )
>> -    __osm_ucast_mgr_dump_tables( p_mgr );
>> +  {
>> +     if ( !default_routing && p_routing_eng->ucast_dump_tables )
>> +        p_routing_eng->ucast_dump_tables(p_routing_eng->context);
>> +     else
>> +        __osm_ucast_mgr_dump_tables( p_mgr );
>> +  }
> 
> Not sure if this is best going forward. Should it be like this:
> 
> 	if ( default_routing )
> 	  __osm_ucast_mgr_dump_tables( p_mgr );
> 	else
> 	{
> 	  if ( p_routing_eng->ucast_dump_tables != 0 )
> 	    p_routing_eng->ucast_dump_tables(p_routing_eng->context);
> 	}

But then what if I have some routing engine that wants to use 
default dump functions, like updn?

So in my approach is as follows:
 - If a routing engine wants to use default dump functions, 
   it should *not* define any dump function of its own.
 - If a routing engine does *not* want to dump anything, it 
   should define a dummy dump function of its own.

You're suggesting the following:
 - If a routing engine wants to use default dump functions, 
   it should define dump function that will call default function.
 - If a routing engine does *not* want to dump anything, it 
   should *not* define any dump function of its own.

I'm OK with both approaches - your call.

-- Yevgeny.

> -- Hal
>