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

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

[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 

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 <hal.rosenstock-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
diff --git a/opensm/opensm/osm_sm_mad_ctrl.c b/opensm/opensm/osm_sm_mad_ctrl.c
index 3ae1eb6..086f71c 100644
--- a/opensm/opensm/osm_sm_mad_ctrl.c
+++ b/opensm/opensm/osm_sm_mad_ctrl.c
@@ -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 majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Sasha Khapyorsky]

On 06:20 Thu 24 Dec     , Hal Rosenstock wrote:
> 
> 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 <hal.rosenstock-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

Applied. Thanks.

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_sm_mad_ctrl.c: In sm_mad_ctrl_send_err_cb, revert setting of init failure on QoS initialization failures

[Sasha Khapyorsky]

Hi Hal,

On 06:20 Thu 24 Dec     , Hal Rosenstock wrote:
> 
> 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 majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Hal Rosenstock]

On Thu, Dec 24, 2009 at 7:56 AM, Sasha Khapyorsky <sashak-smomgflXvOZWk0Htik3J/w@public.gmane.org> wrote:
> Hi Hal,
>
> On 06:20 Thu 24 Dec     , Hal Rosenstock wrote:
>>
>> 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.

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

> 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

>
> 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
>
--
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_sm_mad_ctrl.c: In sm_mad_ctrl_send_err_cb, revert setting of init failure on QoS initialization failures

[Sasha Khapyorsky]

On 11:08 Mon 28 Dec     , Hal Rosenstock wrote:
> >>
> >> 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.
> 
> > 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.

> 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 majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Hal Rosenstock]

On Tue, Dec 29, 2009 at 9:54 AM, Sasha Khapyorsky <sashak-smomgflXvOZWk0Htik3J/w@public.gmane.org> wrote:
> On 11:08 Mon 28 Dec     , Hal Rosenstock wrote:
>> >>
>> >> 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.
>>
>> > 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.

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

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

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

> 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_sm_mad_ctrl.c: In sm_mad_ctrl_send_err_cb, revert setting of init failure on QoS initialization failures

[Sasha Khapyorsky]

On 11:14 Wed 30 Dec     , Hal Rosenstock wrote:
> >> > '--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.

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

> > 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 majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html