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

Signed-off-by Hal Rosenstock <[email protected]>

---

opensm/libvendor/osm_vendor_ibumad.c +15 -4

index 8d3c680..01250fb 100644
@@ -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 [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

Quoted 1 line · Hal Rosenstock

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

Quoted 39 lines

Signed-off-by Hal Rosenstock <[email protected]>

---

opensm/libvendor/osm_vendor_ibumad.c +11 -0

index 8d3c680..01250fb 100644
@@ -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);

Quoted 7 lines

      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 [email protected] 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 [email protected] More majordomo info at  http://vger.kernel.org/majordomo-info.html

Quoted 2 lines

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.

Quoted 2 lines

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 ?

Quoted 5 lines · Hal Rosenstock

Quoted 2 lines

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).

Quoted 7 lines

and control over the number of retries helps and there is no vendor API support for this.

Quoted 2 lines

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.

Quoted 2 lines

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

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

Quoted 3 lines · Hal Rosenstock

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

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.