Skip to contentSkip to conversation

opensm/osm_sm_mad_ctrl.c: In sm_mad_ctrl_send_err_cb, revert setting of init failure on QoS initialization failures

PATCH Applied Hal Rosenstock 3 participants 7 messages Dec 24, 2009 – Dec 30, 2009 3 messages not in the archive InfiniBand

lore
Share & tools
lore

Hal Rosenstock

author
#0 lore raw

PATCH 1/1 opensm/osm_sm_mad_ctrl.c: In sm_mad_ctrl_send_err_cb, revert setting of init failure on QoS initialization failures

opensm/osm_sm_mad_ctrl.c: In sm_mad_ctrl_send_err_cb, revert setting of init failure on QoS attribute initialization failures

Revert the QoS related part of commit 254c2ef34fa0f475a93a63d805582bdd2efdeace

There are unexplained cases of status 0x1c that have been observed for SL to VL mapping table setting as documented in "Re: qos on service-id"
http://article.gmane.org/gmane.linux.drivers.rdma/1464

A subsequent patch will reintroduce this checking based on an additional option which will default to not do this.

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

---

opensm/opensm/osm_sm_mad_ctrl.c +1 -3
index 3ae1eb6..086f71c 100644
@@ -729,9 +729,7 @@ static void sm_mad_ctrl_send_err_cb(IN void *context, IN osm_madw_t * p_madw)
 	     p_smp->attr_id == IB_MAD_ATTR_MCAST_FWD_TBL ||
 	     p_smp->attr_id == IB_MAD_ATTR_SWITCH_INFO ||
 	     p_smp->attr_id == IB_MAD_ATTR_LIN_FWD_TBL ||
-	     p_smp->attr_id == IB_MAD_ATTR_P_KEY_TABLE ||
-	     p_smp->attr_id == IB_MAD_ATTR_SLVL_TABLE ||
-	     p_smp->attr_id == IB_MAD_ATTR_VL_ARBITRATION)) {
+	     p_smp->attr_id == IB_MAD_ATTR_P_KEY_TABLE)) {
 		OSM_LOG(p_ctrl->p_log, OSM_LOG_ERROR, "ERR 3119: "
 			"Set method failed for attribute 0x%X (%s)\n",
 			cl_ntoh16(p_smp->attr_id),

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

Applied
#2 lore raw
In reply to a message not in the archive
Quoted 5 lines · Hal Rosenstock

opensm/osm_sm_mad_ctrl.c: In sm_mad_ctrl_send_err_cb, revert setting of init failure on QoS attribute initialization failures

Revert the QoS related part of commit 254c2ef34fa0f475a93a63d805582bdd2efdeace

Quoted 8 lines

There are unexplained cases of status 0x1c that have been observed for SL to VL mapping table setting as documented in "Re: qos on service-id"
http://article.gmane.org/gmane.linux.drivers.rdma/1464

A subsequent patch will reintroduce this checking based on an additional option which will default to not do this.

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

Applied. Thanks.

Sasha
--
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

#3 lore raw
In reply to a message not in the archive

Hi Hal,

Quoted 7 lines · Hal Rosenstock

There are unexplained cases of status 0x1c that have been observed for SL to VL mapping table setting as documented in "Re: qos on service-id"
http://article.gmane.org/gmane.linux.drivers.rdma/1464

A subsequent patch will reintroduce this checking based on an additional option which will default to not do this.

I think that addition of such sort of options (eg "--workaround-bugX") should be avoided unless it is absolutely necessary. And even then more generic stuff would be better.

In this specific case more generic option like:
'--resweep-on-failed-attr=11,15' (with reasonable default) will be more useful for dealing with this and maybe another potential issues.

Sasha
--
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

#4 lore raw
Quoted 8 lines · Sasha Khapyorsky

Hi Hal,

Quoted 5 lines · Hal Rosenstock

There are unexplained cases of status 0x1c that have been observed for SL to VL mapping table setting as documented in "Re: qos on service-id"
http://article.gmane.org/gmane.linux.drivers.rdma/1464

Quoted 5 lines
Quoted 2 lines

A subsequent patch will reintroduce this checking based on an additional option which will default to not do this.

I think that addition of such sort of options (eg "--workaround-bugX") should be avoided unless it is absolutely necessary.

I was thinking something like --qos-init-error with a description.

Quoted 5 lines

And even then more generic stuff would be better.

In this specific case more generic option like:
'--resweep-on-failed-attr=11,15' (with reasonable default) will be more useful for dealing with this and maybe another potential issues.

Where else do you see this as being useful ?

Is such flexibility really needed or is this more "just in case" ?

-- Hal

Quoted 7 lines

Sasha
--
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

#6 lore raw
In reply to a message not in the archive
Quoted 9 lines · Hal Rosenstock
Quoted 6 lines
Quoted 3 lines

A subsequent patch will reintroduce this checking based on an additional option which will default to not do this.

I think that addition of such sort of options (eg "--workaround-bugX") should be avoided unless it is absolutely necessary.

I was thinking something like --qos-init-error with a description.

Quoted 7 lines
Quoted 5 lines

And even then more generic stuff would be better.

In this specific case more generic option like:
'--resweep-on-failed-attr=11,15' (with reasonable default) will be more useful for dealing with this and maybe another potential issues.

Where else do you see this as being useful ?

In this case at least. I had some questions in the past about how to ignore some sort of initialization errors. Another potential use is to extended this to SubnGet() responses errors handling too when requested.

Quoted 1 line

Is such flexibility really needed or is this more "just in case" ?

See above. Anyway it is better than introducing '--qos-init-error', ('--pkey-init-error', etc.) and not much harder to implement.

Sasha
--
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

#7 lore raw
Quoted 12 lines · Sasha Khapyorsky
Quoted 11 lines · Hal Rosenstock
Quoted 6 lines
Quoted 3 lines

A subsequent patch will reintroduce this checking based on an additional option which will default to not do this.

I think that addition of such sort of options (eg "--workaround-bugX") should be avoided unless it is absolutely necessary.

I was thinking something like --qos-init-error with a description.

Quoted 2 lines

And even then more generic stuff would be better.

Quoted 8 lines
Quoted 5 lines
Quoted 3 lines

In this specific case more generic option like:
'--resweep-on-failed-attr=11,15' (with reasonable default) will be more useful for dealing with this and maybe another potential issues.

Where else do you see this as being useful ?

In this case at least. I had some questions in the past about how to ignore some sort of initialization errors.

What (other) initialization errors should be ignored ? Isn't this dangerous and slippery slope ?

Quoted 3 lines

Another potential use is
to extended this to SubnGet() responses errors handling too when requested.

That would require additional changes beyond what is being discussed here and might not come from the same attribute list so it's unclear to me whether or not this is related (in terms of configuration).

Quoted 6 lines
Quoted 1 line

Is such flexibility really needed or is this more "just in case" ?

See above.

Anyway it is better than introducing '--qos-init-error', ('--pkey-init-error', etc.) and not much harder to implement.

Sure; the question was it's necessity and not it's implementation "difficulty.

-- Hal

Quoted 2 lines

Sasha

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

#9 lore raw
In reply to a message not in the archive
Quoted 10 lines · Hal Rosenstock
Quoted 7 lines
Quoted 4 lines
Quoted 2 lines

'--resweep-on-failed-attr=11,15' (with reasonable default) will be more useful for dealing with this and maybe another potential issues.

Where else do you see this as being useful ?

In this case at least. I had some questions in the past about how to ignore some sort of initialization errors.

What (other) initialization errors should be ignored ? Isn't this dangerous and slippery slope ?

This is what we have now:

	    (p_smp->attr_id == IB_MAD_ATTR_PORT_INFO ||
	     p_smp->attr_id == IB_MAD_ATTR_MCAST_FWD_TBL ||
	     p_smp->attr_id == IB_MAD_ATTR_SWITCH_INFO ||
	     p_smp->attr_id == IB_MAD_ATTR_LIN_FWD_TBL ||
	     p_smp->attr_id == IB_MAD_ATTR_P_KEY_TABLE)) {
		OSM_LOG(p_ctrl->p_log, OSM_LOG_ERROR, "ERR 3119: "
			"Set method failed for attribute 0x%X (%s)\n",
			cl_ntoh16(p_smp->attr_id),
			ib_get_sm_attr_str(p_smp->attr_id));
		p_ctrl->p_subn->subnet_initialization_error = TRUE;
	}

Of course ignoring an errors can be dangerous, and useful for debugging and sometimes to workaround buggy hardware issues.

Quoted 7 lines
Quoted 3 lines

Another potential use is
to extended this to SubnGet() responses errors handling too when requested.

That would require additional changes beyond what is being discussed here and might not come from the same attribute list so it's unclear to me whether or not this is related (in terms of configuration).

Correct, it would be yet another addition, but in the same direction. You can see this as not immediately related to the change in discussion.

Quoted 4 lines
Quoted 2 lines

Anyway it is better than introducing '--qos-init-error', ('--pkey-init-error', etc.) and not much harder to implement.

Sure; the question was it's necessity and not it's implementation "difficulty.

Whole point was to not introduce a new options like "--workaround-buggy-feature-X". But instead in cases when a quirks avoidance is impossible at least to make it in more generic fashion potentially useful for another things too.

Sasha
--
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