Re: [PATCH v3 01/17] opensm: Prepare for routing engine input to path record SL lookup and SL2VL map setup.

2010-07-07 Thread Sasha Khapyorsky
Hi Jim,

On 13:53 Tue 15 Jun , Jim Schutt wrote:
 diff --git a/opensm/opensm/osm_opensm.c b/opensm/opensm/osm_opensm.c
 index d3dc02e..5614240 100644
 --- a/opensm/opensm/osm_opensm.c
 +++ b/opensm/opensm/osm_opensm.c
 @@ -147,7 +147,8 @@ static void append_routing_engine(osm_opensm_t *osm,
   r-next = routing_engine;
  }
  
 -static void setup_routing_engine(osm_opensm_t *osm, const char *name)
 +static struct osm_routing_engine *setup_routing_engine(osm_opensm_t *osm,
 +const char *name)
  {
   struct osm_routing_engine *re;
   const struct routing_engine_module *m;
 @@ -158,47 +159,53 @@ static void setup_routing_engine(osm_opensm_t *osm, 
 const char *name)
   if (!re) {
   OSM_LOG(osm-log, OSM_LOG_VERBOSE,
   memory allocation failed\n);
 - return;
 + return NULL;
   }
   memset(re, 0, sizeof(struct osm_routing_engine));
  
   re-name = m-name;
 + re-type = osm_routing_engine_type(m-name);
   if (m-setup(re, osm)) {
   OSM_LOG(osm-log, OSM_LOG_VERBOSE,
   setup of routing
engine \'%s\' failed\n, name);
 - return;
 + free(re);
 + return NULL;
   }
   OSM_LOG(osm-log, OSM_LOG_DEBUG,
   \'%s\' routing engine set up\n, re-name);
 - append_routing_engine(osm, re);
 - return;
 + if (re-type == OSM_ROUTING_ENGINE_TYPE_MINHOP)
 + osm-default_routing_engine = re;
 + return re;
   }
   }
  
   OSM_LOG(osm-log, OSM_LOG_ERROR,
   cannot find or setup routing engine \'%s\'\n, name);
 + return NULL;
  }
  
  static void setup_routing_engines(osm_opensm_t *osm, const char 
 *engine_names)
  {
   char *name, *str, *p;
 + struct osm_routing_engine *re;
  
 - if (!engine_names || !*engine_names) {
 - setup_routing_engine(osm, minhop);
 - return;
 + if (engine_names  *engine_names) {
 + str = strdup(engine_names);
 + name = strtok_r(str, , \t\n, p);
 + while (name  *name) {
 + re = setup_routing_engine(osm, name);
 + if (re)
 + append_routing_engine(osm, re);
 + name = strtok_r(NULL, , \t\n, p);
 + }
 + free(str);
   }
 -
 - str = strdup(engine_names);
 - name = strtok_r(str, , \t\n, p);
 - while (name  *name) {
 - setup_routing_engine(osm, name);
 - name = strtok_r(NULL, , \t\n, p);
 + if (!osm-default_routing_engine) {
 + re = setup_routing_engine(osm, minhop);
 + if (!osm-routing_engine_list  re)
 + append_routing_engine(osm, re);

Shouldn't here be:

osm-default_routing_engine = re;

too?


   }
 - free(str);
 -
 - if (!osm-routing_engine_list)
 - setup_routing_engine(osm, minhop);
  }
  
  void osm_opensm_construct(IN osm_opensm_t * p_osm)


So that this chunk in osm_ucast_mgr_process() (below) will not break
over NULL pointer?

 - if (p_osm-routing_engine_used == OSM_ROUTING_ENGINE_TYPE_NONE) {
 + if (!p_osm-routing_engine_used) {
   /* If configured routing algorithm failed, use default MinHop */
 - osm_ucast_mgr_build_lid_matrices(p_mgr);
 - ucast_mgr_build_lfts(p_mgr);
 + struct osm_routing_engine *r = p_osm-default_routing_engine;
 +
 + r-build_lid_matrices(r-context);
 + r-ucast_build_fwd_tables(r-context);
 + p_osm-routing_engine_used = r;
   osm_ucast_mgr_set_fwd_tables(p_mgr);
 - p_osm-routing_engine_used = OSM_ROUTING_ENGINE_TYPE_MINHOP;
   }

Sasha
--
To unsubscribe from this list: send the line unsubscribe linux-rdma in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3 01/17] opensm: Prepare for routing engine input to path record SL lookup and SL2VL map setup.

2010-07-07 Thread Jim Schutt
Hi Sasha:

On Wed, 2010-07-07 at 11:06 -0600, Sasha Khapyorsky wrote:
 Hi Jim,
 
 On 13:53 Tue 15 Jun , Jim Schutt wrote:
  diff --git a/opensm/opensm/osm_opensm.c b/opensm/opensm/osm_opensm.c
  index d3dc02e..5614240 100644
  --- a/opensm/opensm/osm_opensm.c
  +++ b/opensm/opensm/osm_opensm.c
  @@ -147,7 +147,8 @@ static void append_routing_engine(osm_opensm_t *osm,
  r-next = routing_engine;
   }
   
  -static void setup_routing_engine(osm_opensm_t *osm, const char *name)
  +static struct osm_routing_engine *setup_routing_engine(osm_opensm_t *osm,
  +  const char *name)
   {
  struct osm_routing_engine *re;
  const struct routing_engine_module *m;
  @@ -158,47 +159,53 @@ static void setup_routing_engine(osm_opensm_t *osm, 
  const char *name)
  if (!re) {
  OSM_LOG(osm-log, OSM_LOG_VERBOSE,
  memory allocation failed\n);
  -   return;
  +   return NULL;
  }
  memset(re, 0, sizeof(struct osm_routing_engine));
   
  re-name = m-name;
  +   re-type = osm_routing_engine_type(m-name);
  if (m-setup(re, osm)) {
  OSM_LOG(osm-log, OSM_LOG_VERBOSE,
  setup of routing
   engine \'%s\' failed\n, name);
  -   return;
  +   free(re);
  +   return NULL;
  }
  OSM_LOG(osm-log, OSM_LOG_DEBUG,
  \'%s\' routing engine set up\n, re-name);
  -   append_routing_engine(osm, re);
  -   return;
  +   if (re-type == OSM_ROUTING_ENGINE_TYPE_MINHOP)
  +   osm-default_routing_engine = re;
  +   return re;
  }
  }
   
  OSM_LOG(osm-log, OSM_LOG_ERROR,
  cannot find or setup routing engine \'%s\'\n, name);
  +   return NULL;
   }
   
   static void setup_routing_engines(osm_opensm_t *osm, const char 
  *engine_names)
   {
  char *name, *str, *p;
  +   struct osm_routing_engine *re;
   
  -   if (!engine_names || !*engine_names) {
  -   setup_routing_engine(osm, minhop);
  -   return;
  +   if (engine_names  *engine_names) {
  +   str = strdup(engine_names);
  +   name = strtok_r(str, , \t\n, p);
  +   while (name  *name) {
  +   re = setup_routing_engine(osm, name);
  +   if (re)
  +   append_routing_engine(osm, re);
  +   name = strtok_r(NULL, , \t\n, p);
  +   }
  +   free(str);
  }
  -
  -   str = strdup(engine_names);
  -   name = strtok_r(str, , \t\n, p);
  -   while (name  *name) {
  -   setup_routing_engine(osm, name);
  -   name = strtok_r(NULL, , \t\n, p);
  +   if (!osm-default_routing_engine) {
  +   re = setup_routing_engine(osm, minhop);
  +   if (!osm-routing_engine_list  re)
  +   append_routing_engine(osm, re);
 
 Shouldn't here be:
 
   osm-default_routing_engine = re;
 
 too?

I think above call to setup_routing_engine(osm, minhop)
does that, because we're explicitly calling it for minhop?

But now that I look at this again, I'm confused why I
thought I needed to append a minhop routing engine to
the routing engine list when the list was empty and there 
was no default routing engine.

I was trying to exactly duplicate old functionality, where
minhop is only in the routing engine list if explicitly
configured, but always called if no routing engines are
configured or all configured engines fail.
   
So I think the end of the above chunk only needs to be

-
-   str = strdup(engine_names);
-   name = strtok_r(str, , \t\n, p);
-   while (name  *name) {
-   setup_routing_engine(osm, name);
-   name = strtok_r(NULL, , \t\n, p);
-   }
+   if (!osm-default_routing_engine)
+   setup_routing_engine(osm, minhop);

-- Jim

 
 
  }
  -   free(str);
  -
  -   if (!osm-routing_engine_list)
  -   setup_routing_engine(osm, minhop);
   }
   
   void osm_opensm_construct(IN osm_opensm_t * p_osm)
 
 
 So that this chunk in osm_ucast_mgr_process() (below) will not break
 over NULL pointer?
 
  -   if (p_osm-routing_engine_used == OSM_ROUTING_ENGINE_TYPE_NONE) {
  +   if (!p_osm-routing_engine_used) {
  /* If configured routing algorithm failed, use default MinHop */
  -   osm_ucast_mgr_build_lid_matrices(p_mgr);
  -   ucast_mgr_build_lfts(p_mgr);
  +   struct osm_routing_engine *r = p_osm-default_routing_engine;
  +
  +   r-build_lid_matrices(r-context);
  +   

Re: [PATCH v3 01/17] opensm: Prepare for routing engine input to path record SL lookup and SL2VL map setup.

2010-07-07 Thread Sasha Khapyorsky
On 11:57 Wed 07 Jul , Jim Schutt wrote:

 So I think the end of the above chunk only needs to be
 
 -
 - str = strdup(engine_names);
 - name = strtok_r(str, , \t\n, p);
 - while (name  *name) {
 - setup_routing_engine(osm, name);
 - name = strtok_r(NULL, , \t\n, p);
 - }
 + if (!osm-default_routing_engine)
 + setup_routing_engine(osm, minhop);

This makes sense. Don't need to resubmit the patch, I will fix.

Sasha
--
To unsubscribe from this list: send the line unsubscribe linux-rdma in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html