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

2009-12-24 11:20 UTC #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

A message that is not in the archive (sent off-list or not yet received): [email protected]

Sasha Khapyorsky

Applied

2009-12-24 12:37 UTC #2 lore raw

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

Sasha Khapyorsky

2009-12-24 12:56 UTC #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

Hal Rosenstock

2009-12-28 16:08 UTC #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

A message that is not in the archive (sent off-list or not yet received): f0e08f230912280808s31c09d46ndeb3586007507e40-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org

Sasha Khapyorsky

2009-12-29 14:54 UTC #6 lore raw

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

Hal Rosenstock

2009-12-30 16:14 UTC #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

A message that is not in the archive (sent off-list or not yet received): f0e08f230912300814h1c6ad213j2b8a343403e2744e-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org

Sasha Khapyorsky

2009-12-30 19:37 UTC #9 lore raw

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