[ofa-general] Re: [PATCH] opensm: allow multiple scopes in a partition

2007-11-29 Thread Rolf Manderscheid


 
@@ -147,11 +162,13 @@ static int partition_add_flag(unsigned lineno, struct part_conf *conf,

"flag \'rate\' requires valid value"
" - skipped\n", lineno);
} else if (!strncmp(flag, "scope", len)) {
-   if (!val || (conf->scope = strtoul(val, NULL, 0)) == 0)
+   unsigned int scope;
+   if (!val || (scope = strtoul(val, NULL, 0)) == 0 || scope > 0xF)
osm_log(conf->p_log, OSM_LOG_VERBOSE,
"PARSE WARN: line %d: "
"flag \'scope\' requires valid value"
" - skipped\n", lineno);
+   conf->scope_mask |= (1<


In case when val is NULL scope will be non-initialized and
conf->scope_mask will get a wrong value. And in case of other errors
too...
  

Yes, there's obviously a missing else.  I'll repost right away.

   Rolf

___
general mailing list
[email protected]
http://lists.openfabrics.org/cgi-bin/mailman/listinfo/general

To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general


[ofa-general] Re: [PATCH] opensm: allow multiple scopes in a partition

2007-11-29 Thread Sasha Khapyorsky
Hi Rolf,

On 17:00 Wed 28 Nov , Rolf Manderscheid wrote:
> Hi Sasha,
> 
> This patch allows multiple scopes to be configured for a partition.
> This allows ipoib interfaces with different scopes to coexist in a
> partition.  The partition configuration file can now have multiple
> scope=N flags and they all take effect (instead of just the last one).
> 
> Signed-off-by: Rolf Manderscheid <[EMAIL PROTECTED]>

The idea looks good for me. Some comments about the patch are below.

> 
> --
> 
> diff --git a/opensm/man/opensm.8 b/opensm/man/opensm.8
> index efd6ff0..c51f386 100644
> --- a/opensm/man/opensm.8
> +++ b/opensm/man/opensm.8
> @@ -366,7 +366,8 @@ Currently recognized flags are:
>   sl=- specifies SL for this IPoIB MC group
> (default is 0)
>   scope= - specifies scope for this IPoIB MC group
> -   (default is 2 (link local))
> +   (default is 2 (link local)).  Multiple scope settings
> +   are permitted for a partition.
>  
>  Note that values for rate, mtu, and scope should be specified as
>  defined in the IBTA specification (for example, mtu=4 for 2048).
> diff --git a/opensm/opensm/osm_prtn_config.c b/opensm/opensm/osm_prtn_config.c
> index 1253031..646bf2a 100644
> --- a/opensm/opensm/osm_prtn_config.c
> +++ b/opensm/opensm/osm_prtn_config.c
> @@ -68,7 +68,7 @@ struct part_conf {
>   osm_log_t *p_log;
>   osm_subn_t *p_subn;
>   osm_prtn_t *p_prtn;
> - unsigned is_ipoib, mtu, rate, sl, scope;
> + unsigned is_ipoib, mtu, rate, sl, scope_mask;
>   boolean_t full;
>  };
>  
> @@ -89,6 +89,7 @@ static int partition_create(unsigned lineno, struct 
> part_conf *conf,
>   char *name, char *id, char *flag, char *flag_val)
>  {
>   uint16_t pkey;
> + unsigned int scope;
>  
>   if (!id && name && isdigit(*name)) {
>   id = name;
> @@ -119,12 +120,26 @@ static int partition_create(unsigned lineno, struct 
> part_conf *conf,
>   }
>   conf->p_prtn->sl = (uint8_t) conf->sl;
>  
> - if (conf->is_ipoib)
> + if (! conf->is_ipoib)

No need a blank after '!'.

> + return 0;
> +
> + if (! conf->scope_mask) {

Ditto.

>   osm_prtn_add_mcgroup(conf->p_log, conf->p_subn, conf->p_prtn,
>(uint8_t) conf->rate,
>(uint8_t) conf->mtu,
> -  (uint8_t) conf->scope);
> +  0);
> + return 0;
> + }
> +
> + for (scope = 0; scope < 16; scope++) {
> + if (((1 + continue;
>  
> + osm_prtn_add_mcgroup(conf->p_log, conf->p_subn, conf->p_prtn,
> +  (uint8_t) conf->rate,
> +  (uint8_t) conf->mtu,
> +  (uint8_t) scope);
> + }
>   return 0;
>  }
>  
> @@ -147,11 +162,13 @@ static int partition_add_flag(unsigned lineno, struct 
> part_conf *conf,
>   "flag \'rate\' requires valid value"
>   " - skipped\n", lineno);
>   } else if (!strncmp(flag, "scope", len)) {
> - if (!val || (conf->scope = strtoul(val, NULL, 0)) == 0)
> + unsigned int scope;
> + if (!val || (scope = strtoul(val, NULL, 0)) == 0 || scope > 0xF)
>   osm_log(conf->p_log, OSM_LOG_VERBOSE,
>   "PARSE WARN: line %d: "
>   "flag \'scope\' requires valid value"
>   " - skipped\n", lineno);
> + conf->scope_mask |= (1   } else if (!strncmp(flag, "sl", len)) {
>   unsigned sl;
>   char *end;
___
general mailing list
[email protected]
http://lists.openfabrics.org/cgi-bin/mailman/listinfo/general

To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general