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=<val> - specifies SL for this IPoIB MC group
> (default is 0)
> scope=<val> - 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<<scope) & conf->scope_mask) == 0)
> + 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<<scope);
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...
Sasha
> } 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
