[PATCH] opensm/osm_vendor_ibumad.c: Add environment variable to change max retries

[PATCH] opensm/osm_vendor_ibumad.c: Add environment variable to change max retries

[Hal Rosenstock]

Signed-off-by: Hal Rosenstock <hal.rosenstock-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
diff --git a/opensm/libvendor/osm_vendor_ibumad.c b/opensm/libvendor/osm_vendor_ibumad.c
index 8d3c680..01250fb 100644
--- a/opensm/libvendor/osm_vendor_ibumad.c
+++ b/opensm/libvendor/osm_vendor_ibumad.c
@@ -446,6 +446,17 @@ osm_vendor_init(IN osm_vendor_t * const p_vend,
 	p_vend->p_log = p_log;
 	p_vend->timeout = timeout;
 	p_vend->max_retries = OSM_DEFAULT_RETRY_COUNT;
+
+	if ((max = getenv("OSM_UMAD_MAX_RETRIES")) != NULL) {
+		int tmp = strtol(max, NULL, 0);
+		if (tmp > 0)
+			p_vend->max_retries = tmp;
+		else
+			OSM_LOG(p_vend->p_log, OSM_LOG_ERROR, "Error:"
+				"OSM_UMAD_MAX_RETRIES=%d is invalid\n",
+				tmp);
+        }
+
 	pthread_mutex_init(&p_vend->cb_mutex, NULL);
 	pthread_mutex_init(&p_vend->match_tbl_mutex, NULL);
 	p_vend->umad_port_id = -1;
@@ -476,12 +487,13 @@ osm_vendor_init(IN osm_vendor_t * const p_vend,
 			p_vend->mtbl.max = tmp;
 		else
 			OSM_LOG(p_vend->p_log, OSM_LOG_ERROR, "Error:"
-				"OSM_UMAD_MAX_PENDING=%d is invalid",
+				"OSM_UMAD_MAX_PENDING=%d is invalid\n",
 				tmp);
 	}
 
-	OSM_LOG(p_vend->p_log, OSM_LOG_INFO, "%d pending umads specified\n",
-		p_vend->mtbl.max);
+	OSM_LOG(p_vend->p_log, OSM_LOG_INFO,
+		"%d max retries %d max pending umads specified\n",
+		p_vend->max_retries, p_vend->mtbl.max);
 
 	p_vend->mtbl.tbl = calloc(p_vend->mtbl.max, sizeof(*(p_vend->mtbl.tbl)));
 	if (!p_vend->mtbl.tbl) {
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] opensm/osm_vendor_ibumad.c: Add environment variable to change max retries

[Sasha Khapyorsky]

On 14:33 Mon 19 Oct     , Hal Rosenstock wrote:
> 

Why do you need this?

And why did you choose to use environment variable and not command
line/config option? I think that it is less obvious.

Sasha

> Signed-off-by: Hal Rosenstock <hal.rosenstock-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
> diff --git a/opensm/libvendor/osm_vendor_ibumad.c b/opensm/libvendor/osm_vendor_ibumad.c
> index 8d3c680..01250fb 100644
> --- a/opensm/libvendor/osm_vendor_ibumad.c
> +++ b/opensm/libvendor/osm_vendor_ibumad.c
> @@ -446,6 +446,17 @@ osm_vendor_init(IN osm_vendor_t * const p_vend,
>  	p_vend->p_log = p_log;
>  	p_vend->timeout = timeout;
>  	p_vend->max_retries = OSM_DEFAULT_RETRY_COUNT;
> +
> +	if ((max = getenv("OSM_UMAD_MAX_RETRIES")) != NULL) {
> +		int tmp = strtol(max, NULL, 0);
> +		if (tmp > 0)
> +			p_vend->max_retries = tmp;
> +		else
> +			OSM_LOG(p_vend->p_log, OSM_LOG_ERROR, "Error:"
> +				"OSM_UMAD_MAX_RETRIES=%d is invalid\n",
> +				tmp);
> +        }
> +
>  	pthread_mutex_init(&p_vend->cb_mutex, NULL);
>  	pthread_mutex_init(&p_vend->match_tbl_mutex, NULL);
>  	p_vend->umad_port_id = -1;
> @@ -476,12 +487,13 @@ osm_vendor_init(IN osm_vendor_t * const p_vend,
>  			p_vend->mtbl.max = tmp;
>  		else
>  			OSM_LOG(p_vend->p_log, OSM_LOG_ERROR, "Error:"
> -				"OSM_UMAD_MAX_PENDING=%d is invalid",
> +				"OSM_UMAD_MAX_PENDING=%d is invalid\n",
>  				tmp);
>  	}
>  
> -	OSM_LOG(p_vend->p_log, OSM_LOG_INFO, "%d pending umads specified\n",
> -		p_vend->mtbl.max);
> +	OSM_LOG(p_vend->p_log, OSM_LOG_INFO,
> +		"%d max retries %d max pending umads specified\n",
> +		p_vend->max_retries, p_vend->mtbl.max);
>  
>  	p_vend->mtbl.tbl = calloc(p_vend->mtbl.max, sizeof(*(p_vend->mtbl.tbl)));
>  	if (!p_vend->mtbl.tbl) {
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] opensm/osm_vendor_ibumad.c: Add environment variable to change max retries

[Hal Rosenstock]

On Thu, Oct 22, 2009 at 8:24 AM, Sasha Khapyorsky <sashak-smomgflXvOZWk0Htik3J/w@public.gmane.org> wrote:
> On 14:33 Mon 19 Oct     , Hal Rosenstock wrote:
>>
>
> Why do you need this?

It is useful in some clusters where the default behavior is
insufficient and control over the number of retries helps and there is
no vendor API support for this.

> And why did you choose to use environment variable and not command
> line/config option? I think that it is less obvious.

It was modeled after OSM_UMAD_MAX_PENDING. It could be made a command
line/options file item. Is that your preference ?

-- Hal

> Sasha
>
>> Signed-off-by: Hal Rosenstock <hal.rosenstock-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> ---
>> diff --git a/opensm/libvendor/osm_vendor_ibumad.c b/opensm/libvendor/osm_vendor_ibumad.c
>> index 8d3c680..01250fb 100644
>> --- a/opensm/libvendor/osm_vendor_ibumad.c
>> +++ b/opensm/libvendor/osm_vendor_ibumad.c
>> @@ -446,6 +446,17 @@ osm_vendor_init(IN osm_vendor_t * const p_vend,
>>       p_vend->p_log = p_log;
>>       p_vend->timeout = timeout;
>>       p_vend->max_retries = OSM_DEFAULT_RETRY_COUNT;
>> +
>> +     if ((max = getenv("OSM_UMAD_MAX_RETRIES")) != NULL) {
>> +             int tmp = strtol(max, NULL, 0);
>> +             if (tmp > 0)
>> +                     p_vend->max_retries = tmp;
>> +             else
>> +                     OSM_LOG(p_vend->p_log, OSM_LOG_ERROR, "Error:"
>> +                             "OSM_UMAD_MAX_RETRIES=%d is invalid\n",
>> +                             tmp);
>> +        }
>> +
>>       pthread_mutex_init(&p_vend->cb_mutex, NULL);
>>       pthread_mutex_init(&p_vend->match_tbl_mutex, NULL);
>>       p_vend->umad_port_id = -1;
>> @@ -476,12 +487,13 @@ osm_vendor_init(IN osm_vendor_t * const p_vend,
>>                       p_vend->mtbl.max = tmp;
>>               else
>>                       OSM_LOG(p_vend->p_log, OSM_LOG_ERROR, "Error:"
>> -                             "OSM_UMAD_MAX_PENDING=%d is invalid",
>> +                             "OSM_UMAD_MAX_PENDING=%d is invalid\n",
>>                               tmp);
>>       }
>>
>> -     OSM_LOG(p_vend->p_log, OSM_LOG_INFO, "%d pending umads specified\n",
>> -             p_vend->mtbl.max);
>> +     OSM_LOG(p_vend->p_log, OSM_LOG_INFO,
>> +             "%d max retries %d max pending umads specified\n",
>> +             p_vend->max_retries, p_vend->mtbl.max);
>>
>>       p_vend->mtbl.tbl = calloc(p_vend->mtbl.max, sizeof(*(p_vend->mtbl.tbl)));
>>       if (!p_vend->mtbl.tbl) {
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
>> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] opensm/osm_vendor_ibumad.c: Add environment variable to change max retries

[Sasha Khapyorsky]

On 08:56 Thu 22 Oct     , Hal Rosenstock wrote:
> >
> > Why do you need this?
> 
> It is useful in some clusters where the default behavior is
> insufficient

I would suppose that by itself this may indicate some external issues
like bad connectivity (cabling) or similar and some investigation would
be helpful there (regardless to number of retries).

> and control over the number of retries helps and there is
> no vendor API support for this.
> 
> > And why did you choose to use environment variable and not command
> > line/config option? I think that it is less obvious.
> 
> It was modeled after OSM_UMAD_MAX_PENDING.

It was internal libvendor hack (which is also not 100% clean :))
controlling ibumad implementation specific parameter. Unlike this a
number of MAD retries (similar to MAD response timeout value) has more
generic meaning.

> It could be made a command
> line/options file item. Is that your preference ?

I think command line/config option is a way to do it.

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

Re: [PATCH] opensm/osm_vendor_ibumad.c: Add environment variable to change max retries

[Hal Rosenstock]

On Thu, Oct 22, 2009 at 9:50 AM, Sasha Khapyorsky <sashak-smomgflXvOZWk0Htik3J/w@public.gmane.org> wrote:
> On 08:56 Thu 22 Oct     , Hal Rosenstock wrote:
>> >
>> > Why do you need this?
>>
>> It is useful in some clusters where the default behavior is
>> insufficient
>
> I would suppose that by itself this may indicate some external issues
> like bad connectivity (cabling) or similar and some investigation would
> be helpful there (regardless to number of retries).
>
>> and control over the number of retries helps and there is
>> no vendor API support for this.
>>
>> > And why did you choose to use environment variable and not command
>> > line/config option? I think that it is less obvious.
>>
>> It was modeled after OSM_UMAD_MAX_PENDING.
>
> It was internal libvendor hack (which is also not 100% clean :))
> controlling ibumad implementation specific parameter. Unlike this a
> number of MAD retries (similar to MAD response timeout value) has more
> generic meaning.
>
>> It could be made a command
>> line/options file item. Is that your preference ?
>
> I think command line/config option is a way to do it.

Do you see a way to do this without changing the vendor API ? I was
trying to add this without doing that.

-- Hal

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

Re: [PATCH] opensm/osm_vendor_ibumad.c: Add environment variable to change max retries

[Sasha Khapyorsky]

On 11:41 Thu 22 Oct     , Hal Rosenstock wrote:
> 
> Do you see a way to do this without changing the vendor API ? I was
> trying to add this without doing that.

What is the problem exactly?

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

Re: [PATCH] opensm/osm_vendor_ibumad.c: Add environment variable to change max retries

[Hal Rosenstock]

On Fri, Oct 23, 2009 at 5:33 PM, Sasha Khapyorsky <sashak-smomgflXvOZWk0Htik3J/w@public.gmane.org> wrote:
> On 11:41 Thu 22 Oct     , Hal Rosenstock wrote:
>>
>> Do you see a way to do this without changing the vendor API ? I was
>> trying to add this without doing that.
>
> What is the problem exactly?

Somehow passing retries in to the vendor layer (either as an
additional API or parameter to existing API) would change the API and
I see no way to get to the subnet options where retries is held from
the vendor layer. That was the reason the environment variable hack
was used.

-- Hal

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

Re: [PATCH] opensm/osm_vendor_ibumad.c: Add environment variable to change max retries

[Sasha Khapyorsky]

On 18:02 Fri 23 Oct     , Hal Rosenstock wrote:
> 
> Somehow passing retries in to the vendor layer (either as an
> additional API or parameter to existing API) would change the API and
> I see no way to get to the subnet options where retries is held from
> the vendor layer.

You can pass it (as well as timeout) as part of bind_info struct, and
even make it per umad agent.

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