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

[Hal Rosenstock]

Hi Yevgeny,

On Wed, 2006-12-20 at 08:49, Yevgeny Kliteynik wrote:
> 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.

You're right. It's 6 of one half a dozen of another. Let's leave it
alone.

-- Hal

> -- Yevgeny.
> 
> > -- Hal
> > 
>