[openib-general] [PATCH] opensm: mcast tables dump improvement

[Yevgeny Kliteynik]

Hi Sasha.

Looks good.

One remark though: all the static functions 
should have the "__osm_" prefix in their names.

--
Yevgeny

Sasha Khapyorsky wrote:
> This improves switch's mcast tables dumping and eliminates multiple file
> open/seek/close sequences. In one word - cleanup.
> 
> Signed-off-by: Sasha Khapyorsky <sashak at voltaire.com>
> ---
>  osm/opensm/osm_mcast_mgr.c |  108 +++++++++++++++++++++-----------------------
>  1 files changed, 52 insertions(+), 56 deletions(-)
> 
> diff --git a/osm/opensm/osm_mcast_mgr.c b/osm/opensm/osm_mcast_mgr.c
> index cb0ffb1..f4d6954 100644
> --- a/osm/opensm/osm_mcast_mgr.c
> +++ b/osm/opensm/osm_mcast_mgr.c
> @@ -53,6 +53,7 @@ #endif /* HAVE_CONFIG_H */
>  #include <unistd.h>
>  #include <stdlib.h>
>  #include <string.h>
> +#include <errno.h>
>  #include <iba/ib_types.h>
>  #include <complib/cl_debug.h>
>  #include <opensm/osm_mcast_mgr.h>
> @@ -1377,10 +1378,12 @@ osm_mcast_mgr_process_tree(
>  
>  /**********************************************************************
>   **********************************************************************/
> +
>  static void
> -osm_mcast_mgr_dump_mcast_routes(
> +mcast_mgr_dump_sw_routes(
>    IN const osm_mcast_mgr_t*   const p_mgr,
> -  IN const osm_switch_t*      const p_sw )
> +  IN const osm_switch_t*      const p_sw,
> +  IN FILE *p_mcfdbFile)
>  {
>    osm_mcast_tbl_t*      p_tbl;
>    int16_t               mlid_ho = 0;
> @@ -1390,35 +1393,14 @@ osm_mcast_mgr_dump_mcast_routes(
>    char                  line[OSM_REPORT_LINE_SIZE];
>    boolean_t             print_lid;
>    const osm_node_t*     p_node;
> -  FILE  *               p_mcfdbFile;
>    uint16_t              i, j;
>    uint16_t              mask_entry;
> -  char                 *file_name = NULL;
>  
> -  OSM_LOG_ENTER( p_mgr->p_log, osm_mcast_mgr_dump_mcast_routes );
> +  OSM_LOG_ENTER( p_mgr->p_log, mcast_mgr_dump_sw_routes );
>    
>    if( !osm_log_is_active( p_mgr->p_log, OSM_LOG_ROUTING ) )
>      goto Exit;
>  
> -  file_name = 
> -    (char*)malloc(strlen(p_mgr->p_subn->opt.dump_files_dir) + 12);
> -  
> -  CL_ASSERT(file_name);
> -  
> -  strcpy(file_name, p_mgr->p_subn->opt.dump_files_dir);
> -  strcat(file_name, "/osm.mcfdbs");
> -  
> -  /* Open the file or error */
> -  p_mcfdbFile = fopen(file_name, "a");
> -  if (! p_mcfdbFile)
> -  {
> -    osm_log( p_mgr->p_log, OSM_LOG_ERROR,
> -             "osm_mcast_mgr_dump_mcast_routes: ERR 0A23: "
> -             "Failed to open mcfdb file (%s)\n",
> -             file_name );
> -    goto Exit;
> -  }
> -
>    p_node = osm_switch_get_node_ptr( p_sw );
>  
>    p_tbl = osm_switch_get_mcast_tbl_ptr( p_sw );
> @@ -1459,30 +1441,56 @@ osm_mcast_mgr_dump_mcast_routes(
>      block_num++;
>    }
>  
> -  fclose(p_mcfdbFile);
> -
>   Exit:
> -  if (file_name)
> -    free(file_name);
>    OSM_LOG_EXIT( p_mgr->p_log );
>  }
>  
> +/**********************************************************************
> + **********************************************************************/
> +
> +struct mcast_mgr_dump_context {
> +	osm_mcast_mgr_t *p_mgr;
> +	FILE *file;
> +};
> +
>  static void
> -__unlink_mcast_fdb(IN osm_mcast_mgr_t* const p_mgr)
> +mcast_mgr_dump_table(cl_map_item_t *p_map_item, void *context)
>  {
> -  char *file_name = NULL;
> +	osm_switch_t *p_sw = (osm_switch_t *)p_map_item;
> +	struct mcast_mgr_dump_context *cxt = context;
>  
> -  /* remove the old fdb dump file: */
> -  file_name =
> -    (char*)malloc(strlen(p_mgr->p_subn->opt.dump_files_dir) + 12);
> +	mcast_mgr_dump_sw_routes(cxt->p_mgr, p_sw, cxt->file);
> +}
>  
> -  if( file_name )
> -  {
> -    strcpy(file_name, p_mgr->p_subn->opt.dump_files_dir);
> -    strcat(file_name, "/osm.mcfdbs");
> -    unlink(file_name);
> -    free(file_name);
> -  }
> +static void
> +mcast_mgr_dump_mcast_routes(osm_mcast_mgr_t *p_mgr)
> +{
> +	char file_name[1024];
> +	struct mcast_mgr_dump_context dump_context;
> +	FILE  *file;
> +
> +	if (!osm_log_is_active(p_mgr->p_log, OSM_LOG_ROUTING))
> +		return;
> +
> +	snprintf(file_name, sizeof(file_name), "%s/%s",
> +		 p_mgr->p_subn->opt.dump_files_dir, "osm.mcfdbs");
> + 
> + 	file = fopen(file_name, "w");
> +	if (!file) {
> +		osm_log(p_mgr->p_log, OSM_LOG_ERROR,
> +			"mcast_dump_mcast_routes: ERR 0A18: "
> +			"cannot create mcfdb file \'%s\': %s\n",
> +			file_name, strerror(errno));
> +		return;
> +	}
> +
> +	dump_context.p_mgr = p_mgr;
> +	dump_context.file = file;
> +
> +	cl_qmap_apply_func(&p_mgr->p_subn->sw_guid_tbl,
> +			   mcast_mgr_dump_table, &dump_context);
> +
> +	fclose(file);
>  }
>  
>  /**********************************************************************
> @@ -1518,12 +1526,6 @@ osm_mcast_mgr_process_mgrp(
>      goto Exit;
>    }
>  
> -  /* initialize the mc fdb dump file: */
> -  if( osm_log_is_active( p_mgr->p_log, OSM_LOG_ROUTING ) )
> -  {
> -    __unlink_mcast_fdb( p_mgr );
> -  }
> -
>    /*
>      Walk the switches and download the tables for each.
>    */
> @@ -1534,11 +1536,11 @@ osm_mcast_mgr_process_mgrp(
>      if( signal == OSM_SIGNAL_DONE_PENDING )
>        pending_transactions = TRUE;
>  
> -    osm_mcast_mgr_dump_mcast_routes( p_mgr, p_sw );
> -
>      p_sw = (osm_switch_t*)cl_qmap_next( &p_sw->map_item );
>    }
>  
> +  mcast_mgr_dump_mcast_routes( p_mgr );
> +
>   Exit:
>    OSM_LOG_EXIT( p_mgr->p_log );
>  
> @@ -1594,12 +1596,6 @@ osm_mcast_mgr_process(
>      p_mgrp = (osm_mgrp_t*)cl_qmap_next( &p_mgrp->map_item );
>    }
>  
> -  /* initialize the mc fdb dump file: */
> -  if( osm_log_is_active( p_mgr->p_log, OSM_LOG_ROUTING ) )
> -  {
> -    __unlink_mcast_fdb( p_mgr );
> -  }
> -
>    /*
>      Walk the switches and download the tables for each.
>    */
> @@ -1610,11 +1606,11 @@ osm_mcast_mgr_process(
>      if( signal == OSM_SIGNAL_DONE_PENDING )
>        pending_transactions = TRUE;
>  
> -    osm_mcast_mgr_dump_mcast_routes( p_mgr, p_sw );
> -
>      p_sw = (osm_switch_t*)cl_qmap_next( &p_sw->map_item );
>    }
>  
> +  mcast_mgr_dump_mcast_routes( p_mgr );
> +
>    CL_PLOCK_RELEASE( p_mgr->p_lock );
>  
>    OSM_LOG_EXIT( p_mgr->p_log );