[PATCH 0/2] Setup response for MADs as appropriate

[PATCH 0/2] Setup response for MADs as appropriate

[Swapna Thete]

The issue is observed when a query command is issued
for any agent that is not implemented (CCA, BMA etc).

The devices in the OFED will not respond to queries,
thus tools making query to these devices will un-necessarily
take long time waiting for these commands to time out. 
So it would be better for OFED to respond to such a query
for all these devices by setting up a response message. 
For a large fabric consisting of these devices the 
issue would be further aggravated.

Setting up a response with appropriate error message
would be the efficient way to handle such messages
so that devices do not appear un-responsive to such
query commands and respond faster. 

Problem reported-by: Jenny Reick <jenny.reick-h88ZbnxC6KDQT0dZR+AlfA@public.gmane.org>
Reviewed-by: Mike Marciniszyn <mike.marciniszyn-h88ZbnxC6KDQT0dZR+AlfA@public.gmane.org>
Signed-off-by: Swapna Thete <swapna.thete-h88ZbnxC6KDQT0dZR+AlfA@public.gmane.org>

---

Swapna Thete (2):
      IB/mad: Add MAD error codes per IBTA spec
      IB/mad: Return unsupported for MADs as appropriate


 drivers/infiniband/core/mad.c |   10 ++++++++++
 include/rdma/ib_mad.h         |    9 +++++++++
 2 files changed, 19 insertions(+), 0 deletions(-)

-- 

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

[PATCH 1/2] IB/mad: Add MAD error codes per IBTA spec

[Swapna Thete]

Add appropriate MAD error codes so that
they can be used while returning appropriate
error responses.
Signed-off-by: Swapna Thete <swapna.thete-h88ZbnxC6KDQT0dZR+AlfA@public.gmane.org>
---
 include/rdma/ib_mad.h |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/include/rdma/ib_mad.h b/include/rdma/ib_mad.h
index d3b9401..d9751b7 100644
--- a/include/rdma/ib_mad.h
+++ b/include/rdma/ib_mad.h
@@ -77,6 +77,15 @@
 
 #define IB_MGMT_MAX_METHODS			128
 
+/* MAD Status field bit masks */
+#define IB_MGMT_MAD_STATUS_SUCCESS					0x0000
+#define IB_MGMT_MAD_STATUS_BUSY						0x0001
+#define IB_MGMT_MAD_STATUS_REDIRECT_REQD				0x0002
+#define IB_MGMT_MAD_STATUS_BAD_VERSION					0x0004
+#define IB_MGMT_MAD_STATUS_UNSUPPORTED_METHOD			0x0008
+#define IB_MGMT_MAD_STATUS_UNSUPPORTED_METHOD_ATTRIB	0x000c
+#define IB_MGMT_MAD_STATUS_INVALID_ATTRIB_VALUE			0x001c
+
 /* RMPP information */
 #define IB_MGMT_RMPP_VERSION			1
 


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

[PATCH 2/2] IB/mad: Return unsupported for MADs as appropriate

[Swapna Thete]

Setup a response with appropriate error status and
send it for the MADs that are not supported by a
specific class/version.
Signed-off-by: Swapna Thete <swapna.thete-h88ZbnxC6KDQT0dZR+AlfA@public.gmane.org>
---
 drivers/infiniband/core/mad.c |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/drivers/infiniband/core/mad.c b/drivers/infiniband/core/mad.c
index 2fe428b..734d846 100644
--- a/drivers/infiniband/core/mad.c
+++ b/drivers/infiniband/core/mad.c
@@ -1963,6 +1963,16 @@ local:
 		 * or via recv_handler in ib_mad_complete_recv()
 		 */
 		recv = NULL;
+	} else {
+		memcpy(response, recv, sizeof(*response));
+		response->header.recv_wc.wc = &response->header.wc;
+		response->header.recv_wc.recv_buf.mad = &response->mad.mad;
+		response->header.recv_wc.recv_buf.grh = &response->grh;
+		response->mad.mad.mad_hdr.method = IB_MGMT_METHOD_GET_RESP;
+		response->mad.mad.mad_hdr.status =
+				__be16_to_cpu(IB_MGMT_MAD_STATUS_BAD_VERSION);
+		agent_send_response(&response->mad.mad, &recv->grh, wc,
+			port_priv->device, port_num, qp_info->qp->qp_num);
 	}
 
 out:


--
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 2/2] IB/mad: Return unsupported for MADs as appropriate

[Or Gerlitz]

On 1/18/2012 10:43 AM, Swapna Thete wrote:
> @@ -1963,6 +1963,16 @@ local:
> +	} else {
> +		memcpy(response, recv, sizeof(*response));
> +		response->header.recv_wc.wc =&response->header.wc;
> +		response->header.recv_wc.recv_buf.mad =&response->mad.mad;
> +		response->header.recv_wc.recv_buf.grh =&response->grh;
> +		response->mad.mad.mad_hdr.method = IB_MGMT_METHOD_GET_RESP;
> +		response->mad.mad.mad_hdr.status =
> +				__be16_to_cpu(IB_MGMT_MAD_STATUS_BAD_VERSION);
Don't we want here IB_MGMT_MAD_STATUS_UNSUPPORTED_METHOD_ATTRIB ?

Or.
> +		agent_send_response(&response->mad.mad,&recv->grh, wc,
> +			port_priv->device, port_num, qp_info->qp->qp_num);
>   	}
>
>   out:
>
>
> --
> 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 2/2] IB/mad: Return unsupported for MADs as appropriate

[Swapna Thete]

I did explore that option, however,
Bad Version essentially gives message as

“MAD returned with Bad Status: Unsupported Class or Version”

which seems better and appropriate than IB_MGMT_MAD_STATUS_UNSUPPORTED_METHOD_ATTRIB

“MAD returned with Bad Status: Unsupported Method/Attribute Combination”.

-----Original Message-----
From: Or Gerlitz [mailto:ogerlitz@mellanox.com]
Sent: Wednesday, January 18, 2012 3:35 PM
To: Swapna Thete
Cc: roland@kernel.org; linux-rdma@vger.kernel.org; Jack Morgenstein
Subject: Re: [PATCH 2/2] IB/mad: Return unsupported for MADs as appropriate

On 1/18/2012 10:43 AM, Swapna Thete wrote:
> @@ -1963,6 +1963,16 @@ local:
> +     } else {
> +             memcpy(response, recv, sizeof(*response));
> +             response->header.recv_wc.wc =&response->header.wc;
> +             response->header.recv_wc.recv_buf.mad =&response->mad.mad;
> +             response->header.recv_wc.recv_buf.grh =&response->grh;
> +             response->mad.mad.mad_hdr.method = IB_MGMT_METHOD_GET_RESP;
> +             response->mad.mad.mad_hdr.status =
> +                             __be16_to_cpu(IB_MGMT_MAD_STATUS_BAD_VERSION);
Don't we want here IB_MGMT_MAD_STATUS_UNSUPPORTED_METHOD_ATTRIB ?

Or.
> +             agent_send_response(&response->mad.mad,&recv->grh, wc,
> +                     port_priv->device, port_num, qp_info->qp->qp_num);
>       }
>
>   out:
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



This message and any attached documents contain information from QLogic Corporation or its wholly-owned subsidiaries that may be confidential. If you are not the intended recipient, you may not read, copy, distribute, or use this information. If you have received this transmission in error, please notify the sender immediately by reply e-mail and then delete this message.

Re: [PATCH 2/2] IB/mad: Return unsupported for MADs as appropriate

[Roland Dreier]

On Wed, Jan 18, 2012 at 2:14 AM, Swapna Thete <swapna.thete-h88ZbnxC6KDQT0dZR+AlfA@public.gmane.org> wrote:
> I did explore that option, however,
> Bad Version essentially gives message as
>
> “MAD returned with Bad Status: Unsupported Class or Version”
>
> which seems better and appropriate than IB_MGMT_MAD_STATUS_UNSUPPORTED_METHOD_ATTRIB
>
> “MAD returned with Bad Status: Unsupported Method/Attribute Combination”.

Surely we should be picking the response status based on the IB spec
rather than on the error message that some arbitrary tool prints?

 - R.
--
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 2/2] IB/mad: Return unsupported for MADs as appropriate

[Hefty, Sean]

> Surely we should be picking the response status based on the IB spec
> rather than on the error message that some arbitrary tool prints?

agreed - these are the basic choices:

Code for invalid field
0 - no invalid fields
1 - Bad version. Either the base version, or the class version, or the
combination of the two is not supported.
2 - The method specified is not supported
3 - The method/attribute combination is not supported

none of which seem that great...  IMO, unsupported class version seems as good as any.

- Sean
--
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 2/2] IB/mad: Return unsupported for MADs as appropriate

[Jason Gunthorpe]

On Wed, Jan 18, 2012 at 05:20:13PM +0000, Hefty, Sean wrote:
> > Surely we should be picking the response status based on the IB spec
> > rather than on the error message that some arbitrary tool prints?
> 
> agreed - these are the basic choices:
> 
> Code for invalid field
> 0 - no invalid fields
> 1 - Bad version. Either the base version, or the class version, or the
> combination of the two is not supported.
> 2 - The method specified is not supported
> 3 - The method/attribute combination is not supported
> 
> none of which seem that great...  IMO, unsupported class version
> seems as good as any.

Figure 169 specifies the validation flow. 1 is the correct reply for
'Management class is not supported on this port'.

Jason
--
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 2/2] IB/mad: Return unsupported for MADs as appropriate

[Jack Morgenstein]

I see a problem here.  If a node receives an SMInfo MAD (which is SMI -- LID-Route or Direct-Route classes), it would look bizarre to indicate that an entire SMI class is not supported, when fact OpenSM is simply not answering a "get" on the SMInfo **attribute** .  In this case, #3 is correct (and #1 is NOT correct -- the HCA's SMA would still support SMI mads!).

In other cases, where an entire class is not supported (e.g., dev_mgt or SNMP), we can still probably return #3, since the attributes tend not to overlap between classes (i.e., each class has a mostly unique set of attributes).  Since it would be a nightmare to differentiate between returning #1 and #3 (if this is at all possible -- I believe that in real time it is impossible to tell if an entire class has just gone missing (or was never supported), or just the specific attribute, such as SM_INFO, for a given class), I suggest we return #3.

-Jack

-----Original Message-----
From: Jason Gunthorpe [mailto:jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org]
Sent: Wednesday, January 18, 2012 8:57 PM
To: Hefty, Sean
Cc: Roland Dreier; Swapna Thete; Or Gerlitz; linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Jack Morgenstein
Subject: Re: [PATCH 2/2] IB/mad: Return unsupported for MADs as appropriate

On Wed, Jan 18, 2012 at 05:20:13PM +0000, Hefty, Sean wrote:
> > Surely we should be picking the response status based on the IB spec
> > rather than on the error message that some arbitrary tool prints?
>
> agreed - these are the basic choices:
>
> Code for invalid field
> 0 - no invalid fields
> 1 - Bad version. Either the base version, or the class version, or the
> combination of the two is not supported.
> 2 - The method specified is not supported
> 3 - The method/attribute combination is not supported
>
> none of which seem that great...  IMO, unsupported class version seems
> as good as any.

Figure 169 specifies the validation flow. 1 is the correct reply for 'Management class is not supported on this port'.

Jason
--
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 2/2] IB/mad: Return unsupported for MADs as appropriate

[Hefty, Sean]

> +	} else {
> +		memcpy(response, recv, sizeof(*response));
> +		response->header.recv_wc.wc = &response->header.wc;
> +		response->header.recv_wc.recv_buf.mad = &response->mad.mad;
> +		response->header.recv_wc.recv_buf.grh = &response->grh;
> +		response->mad.mad.mad_hdr.method = IB_MGMT_METHOD_GET_RESP;
> +		response->mad.mad.mad_hdr.status =
> +				__be16_to_cpu(IB_MGMT_MAD_STATUS_BAD_VERSION);
> +		agent_send_response(&response->mad.mad, &recv->grh, wc,
> +			port_priv->device, port_num, qp_info->qp->qp_num);
>  	}

I think we need to do more here than just generate a GetResp for an unmatched receive.  The received MAD in question could itself be a GetResp or a Send, Trap, Report, etc.  Maybe add a check:

} else if generate_unmatched_resp(recv, ..) {
	agent_send_response(...)
}

and let generate_unmatched_resp() determine if an actual response should be generated and what the contents should be, if there are any differences in the response, such as different methods.  The initial submission could limit which MADs a response is generated for.

- Sean

Re: [PATCH 2/2] IB/mad: Return unsupported for MADs as appropriate

[Jason Gunthorpe]

On Wed, Jan 18, 2012 at 06:06:11PM +0000, Hefty, Sean wrote:
> > +	} else {
> > +		memcpy(response, recv, sizeof(*response));
> > +		response->header.recv_wc.wc = &response->header.wc;
> > +		response->header.recv_wc.recv_buf.mad = &response->mad.mad;
> > +		response->header.recv_wc.recv_buf.grh = &response->grh;
> > +		response->mad.mad.mad_hdr.method = IB_MGMT_METHOD_GET_RESP;
> > +		response->mad.mad.mad_hdr.status =
> > +				__be16_to_cpu(IB_MGMT_MAD_STATUS_BAD_VERSION);
> > +		agent_send_response(&response->mad.mad, &recv->grh, wc,
> > +			port_priv->device, port_num, qp_info->qp->qp_num);
> >  	}
> 
> I think we need to do more here than just generate a GetResp for an
> unmatched receive.  The received MAD in question could itself be a
> GetResp or a Send, Trap, Report, etc.  Maybe add a check:

No, refer to Figure 169. GetResp is the correct response.

The method cannot be parsed outside the context of the
(baseVersion,manamgentClass,classVersion) tuple. A agent that does not
understand the managementClass must return GetResp.

I feel if the base version is not supported then a GetResp should be
returned without a memcpy. The base version set to the highest
supported value and the TID copied over from the reply but everything
else 0'd. This allows base version discovery which might be important
some day.

Otherwise this looks right..

Jason
--
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 2/2] IB/mad: Return unsupported for MADs as appropriate

[Hefty, Sean]

> -----Original Message-----
> From: linux-rdma-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org [mailto:linux-rdma-
> owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org] On Behalf Of Jason Gunthorpe
> Sent: Wednesday, January 18, 2012 11:02 AM
> To: Hefty, Sean
> Cc: Swapna Thete; roland-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org; linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Subject: Re: [PATCH 2/2] IB/mad: Return unsupported for MADs as appropriate
> 
> On Wed, Jan 18, 2012 at 06:06:11PM +0000, Hefty, Sean wrote:
> > > +	} else {
> > > +		memcpy(response, recv, sizeof(*response));
> > > +		response->header.recv_wc.wc = &response->header.wc;
> > > +		response->header.recv_wc.recv_buf.mad = &response->mad.mad;
> > > +		response->header.recv_wc.recv_buf.grh = &response->grh;
> > > +		response->mad.mad.mad_hdr.method = IB_MGMT_METHOD_GET_RESP;
> > > +		response->mad.mad.mad_hdr.status =
> > > +				__be16_to_cpu(IB_MGMT_MAD_STATUS_BAD_VERSION);
> > > +		agent_send_response(&response->mad.mad, &recv->grh, wc,
> > > +			port_priv->device, port_num, qp_info->qp->qp_num);
> > >  	}
> >
> > I think we need to do more here than just generate a GetResp for an
> > unmatched receive.  The received MAD in question could itself be a
> > GetResp or a Send, Trap, Report, etc.  Maybe add a check:
> 
> No, refer to Figure 169. GetResp is the correct response.
> 
> The method cannot be parsed outside the context of the
> (baseVersion,manamgentClass,classVersion) tuple. A agent that does not
> understand the managementClass must return GetResp.
> 
> I feel if the base version is not supported then a GetResp should be
> returned without a memcpy. The base version set to the highest
> supported value and the TID copied over from the reply but everything
> else 0'd. This allows base version discovery which might be important
> some day.

See C13-47 - "Packets failing one or more of these checks are discarded and no action is taken in response unless the method is a Get() or a Set()."

This patch would send a GetResp() to any method.
--
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 2/2] IB/mad: Return unsupported for MADs as appropriate

[Jason Gunthorpe]

On Wed, Jan 18, 2012 at 07:06:40PM +0000, Hefty, Sean wrote:

> > I feel if the base version is not supported then a GetResp should
> > be returned without a memcpy. The base version set to the highest
> > supported value and the TID copied over from the reply but
> > everything else 0'd. This allows base version discovery which
> > might be important some day.
> 
> See C13-47 - "Packets failing one or more of these checks are
> discarded and no action is taken in response unless the method is a
> Get() or a Set()."

Okay, agree.

Jason
--
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 2/2] IB/mad: Return unsupported for MADs as appropriate

[Hefty, Sean]

> The method cannot be parsed outside the context of the
> (baseVersion,manamgentClass,classVersion) tuple. A agent that does not
> understand the managementClass must return GetResp.
>
> I feel if the base version is not supported then a GetResp should be
> returned without a memcpy. The base version set to the highest
> supported value and the TID copied over from the reply but everything
> else 0'd. This allows base version discovery which might be important
> some day.

If the BaseVersion isn't supported, then I don't know that the code should assume anything about the rest of the MAD format, including the location or size of any other fields.  If we at least know that BaseVersion = 1, then we should be safe checking for common methods.  If BaseVersion > 1, then maybe we're better off just dropping the MAD still.

- Sean
--
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 2/2] IB/mad: Return unsupported for MADs as appropriate

[Jason Gunthorpe]

On Wed, Jan 18, 2012 at 07:30:03PM +0000, Hefty, Sean wrote:

> > I feel if the base version is not supported then a GetResp should be
> > returned without a memcpy. The base version set to the highest
> > supported value and the TID copied over from the reply but everything
> > else 0'd. This allows base version discovery which might be important
> > some day.
> 
> If the BaseVersion isn't supported, then I don't know that the code
> should assume anything about the rest of the MAD format, including
> the location or size of any other fields.  If we at least know that
> BaseVersion = 1, then we should be safe checking for common methods.
> If BaseVersion > 1, then maybe we're better off just dropping the
> MAD still.

Yes, that is why the compliance statement is sort of short sighted..
The flowchart makes more sense.

Jason
--
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 2/2] IB/mad: Return unsupported for MADs as appropriate

[Swapna Thete]

This is taken care of in validate_mad() function before
code in my patch gets called. So it should work as Sean
points out, basically if Baseversion is not 1, MAD
would get dropped.

Thanks,
Swapna

-----Original Message-----
From: linux-rdma-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org [mailto:linux-rdma-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org] On Behalf Of Jason Gunthorpe
Sent: Thursday, January 19, 2012 2:49 AM
To: Hefty, Sean
Cc: Swapna Thete; roland-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org; linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Subject: Re: [PATCH 2/2] IB/mad: Return unsupported for MADs as appropriate

On Wed, Jan 18, 2012 at 07:30:03PM +0000, Hefty, Sean wrote:

> > I feel if the base version is not supported then a GetResp should be
> > returned without a memcpy. The base version set to the highest
> > supported value and the TID copied over from the reply but everything
> > else 0'd. This allows base version discovery which might be important
> > some day.
>
> If the BaseVersion isn't supported, then I don't know that the code
> should assume anything about the rest of the MAD format, including
> the location or size of any other fields.  If we at least know that
> BaseVersion = 1, then we should be safe checking for common methods.
> If BaseVersion > 1, then maybe we're better off just dropping the
> MAD still.

Yes, that is why the compliance statement is sort of short sighted..
The flowchart makes more sense.

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


This message and any attached documents contain information from QLogic Corporation or its wholly-owned subsidiaries that may be confidential. If you are not the intended recipient, you may not read, copy, distribute, or use this information. If you have received this transmission in error, please notify the sender immediately by reply e-mail and then delete this message.

--
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 2/2] IB/mad: Return unsupported for MADs as appropriate

[Swapna Thete]

Yes. I will look into this and incorporate the change
And get back.

Thanks,
Swapna

-----Original Message-----
From: linux-rdma-owner@vger.kernel.org [mailto:linux-rdma-owner@vger.kernel.org] On Behalf Of Hefty, Sean
Sent: Wednesday, January 18, 2012 11:36 PM
To: Swapna Thete; roland@kernel.org
Cc: linux-rdma@vger.kernel.org
Subject: RE: [PATCH 2/2] IB/mad: Return unsupported for MADs as appropriate

> +     } else {
> +             memcpy(response, recv, sizeof(*response));
> +             response->header.recv_wc.wc = &response->header.wc;
> +             response->header.recv_wc.recv_buf.mad = &response->mad.mad;
> +             response->header.recv_wc.recv_buf.grh = &response->grh;
> +             response->mad.mad.mad_hdr.method = IB_MGMT_METHOD_GET_RESP;
> +             response->mad.mad.mad_hdr.status =
> +                             __be16_to_cpu(IB_MGMT_MAD_STATUS_BAD_VERSION);
> +             agent_send_response(&response->mad.mad, &recv->grh, wc,
> +                     port_priv->device, port_num, qp_info->qp->qp_num);
>       }

I think we need to do more here than just generate a GetResp for an unmatched receive.  The received MAD in question could itself be a GetResp or a Send, Trap, Report, etc.  Maybe add a check:

} else if generate_unmatched_resp(recv, ..) {
        agent_send_response(...)
}

and let generate_unmatched_resp() determine if an actual response should be generated and what the contents should be, if there are any differences in the response, such as different methods.  The initial submission could limit which MADs a response is generated for.

- Sean
\x13��칻\x1c�&�~�&�\x18��+-��ݶ\x17��w��˛���m�b��kvf���^n�r���z�\x1a��h����&��\x1e�G���h�\x03(�階�ݢj"��\x1a�^[m�����z�ޖ���f���h���~�m�

This message and any attached documents contain information from QLogic Corporation or its wholly-owned subsidiaries that may be confidential. If you are not the intended recipient, you may not read, copy, distribute, or use this information. If you have received this transmission in error, please notify the sender immediately by reply e-mail and then delete this message.

Re: [PATCH 2/2] IB/mad: Return unsupported for MADs as appropriate

[Hal Rosenstock]

On 1/18/2012 3:43 AM, Swapna Thete wrote:
> Setup a response with appropriate error status and
> send it for the MADs that are not supported by a
> specific class/version.
> Signed-off-by: Swapna Thete <swapna.thete-h88ZbnxC6KDQT0dZR+AlfA@public.gmane.org>
> ---
>  drivers/infiniband/core/mad.c |   10 ++++++++++
>  1 files changed, 10 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/infiniband/core/mad.c b/drivers/infiniband/core/mad.c
> index 2fe428b..734d846 100644
> --- a/drivers/infiniband/core/mad.c
> +++ b/drivers/infiniband/core/mad.c
> @@ -1963,6 +1963,16 @@ local:
>  		 * or via recv_handler in ib_mad_complete_recv()
>  		 */
>  		recv = NULL;
> +	} else {
> +		memcpy(response, recv, sizeof(*response));

Isn't this overkill as the bad MAD status precludes looking at the MAD
data ?

> +		response->header.recv_wc.wc = &response->header.wc;
> +		response->header.recv_wc.recv_buf.mad = &response->mad.mad;
> +		response->header.recv_wc.recv_buf.grh = &response->grh;
> +		response->mad.mad.mad_hdr.method = IB_MGMT_METHOD_GET_RESP;
> +		response->mad.mad.mad_hdr.status =
> +				__be16_to_cpu(IB_MGMT_MAD_STATUS_BAD_VERSION);

While this is the best status for class not supported, that's not all
the cases that get to here. Attribute not supported (in a supported
class) could also occur here for which unsupported method/attribute
combination is more appropriate as a MAD status. I'm not sure it's worth
the effort to discern that (as any invalid MAD status is treated the
same) but I think it could be done if we want to be more precise about
the error.

-- Hal

> +		agent_send_response(&response->mad.mad, &recv->grh, wc,
> +			port_priv->device, port_num, qp_info->qp->qp_num);
>  	}
>  
>  out:
> 
> 
> --
> 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 2/2] IB/mad: Return unsupported for MADs as appropriate

[Hal Rosenstock]

On 1/18/2012 5:30 PM, Hal Rosenstock wrote:
> On 1/18/2012 3:43 AM, Swapna Thete wrote:
>> Setup a response with appropriate error status and
>> send it for the MADs that are not supported by a
>> specific class/version.
>> Signed-off-by: Swapna Thete <swapna.thete-h88ZbnxC6KDQT0dZR+AlfA@public.gmane.org>
>> ---
>>  drivers/infiniband/core/mad.c |   10 ++++++++++
>>  1 files changed, 10 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/infiniband/core/mad.c b/drivers/infiniband/core/mad.c
>> index 2fe428b..734d846 100644
>> --- a/drivers/infiniband/core/mad.c
>> +++ b/drivers/infiniband/core/mad.c
>> @@ -1963,6 +1963,16 @@ local:
>>  		 * or via recv_handler in ib_mad_complete_recv()
>>  		 */
>>  		recv = NULL;
>> +	} else {
>> +		memcpy(response, recv, sizeof(*response));
> 
> Isn't this overkill as the bad MAD status precludes looking at the MAD
> data ?
> 
>> +		response->header.recv_wc.wc = &response->header.wc;
>> +		response->header.recv_wc.recv_buf.mad = &response->mad.mad;
>> +		response->header.recv_wc.recv_buf.grh = &response->grh;
>> +		response->mad.mad.mad_hdr.method = IB_MGMT_METHOD_GET_RESP;
>> +		response->mad.mad.mad_hdr.status =
>> +				__be16_to_cpu(IB_MGMT_MAD_STATUS_BAD_VERSION);
> 
> While this is the best status for class not supported, that's not all
> the cases that get to here. Attribute not supported (in a supported
> class) could also occur here for which unsupported method/attribute
> combination is more appropriate as a MAD status. I'm not sure it's worth
> the effort to discern that (as any invalid MAD status is treated the
> same) but I think it could be done if we want to be more precise about
> the error.

The other alternative is to just return method/attribute combination not
supported (STATUS_FIELD[4:2] = 0x3 (MAD status 0xc)) for all this as
Jack and Or have indicated. FWIW that's my preference.

-- Hal

> 
> -- Hal
> 
>> +		agent_send_response(&response->mad.mad, &recv->grh, wc,
>> +			port_priv->device, port_num, qp_info->qp->qp_num);
>>  	}
>>  
>>  out:
>>
>>
>> --
>> 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 2/2] IB/mad: Return unsupported for MADs as appropriate

[Swapna Thete]

Thank you all for your suggestions.
However, I just wanted to understand that given this code is for the
case of an entire class not supported, isn't this error code
IB_MGMT_MAD_STATUS_BAD_VERSION (MAD returned with Bad Status: Unsupported Class or Version) more appropriate? Also this is
Part of IB spec error codes.

Also I did not make any changes to the handling of SMA Get(SmInfo
attribute). Let me know if I am missing something.

Thanks,
Swapna
-----Original Message-----
From: Hal Rosenstock [mailto:hal@dev.mellanox.co.il]
Sent: Thursday, January 19, 2012 6:28 PM
To: Swapna Thete
Cc: roland@kernel.org; linux-rdma@vger.kernel.org; Jack Morgenstein
Subject: Re: [PATCH 2/2] IB/mad: Return unsupported for MADs as appropriate

On 1/18/2012 5:30 PM, Hal Rosenstock wrote:
> On 1/18/2012 3:43 AM, Swapna Thete wrote:
>> Setup a response with appropriate error status and
>> send it for the MADs that are not supported by a
>> specific class/version.
>> Signed-off-by: Swapna Thete <swapna.thete@qlogic.com>
>> ---
>>  drivers/infiniband/core/mad.c |   10 ++++++++++
>>  1 files changed, 10 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/infiniband/core/mad.c b/drivers/infiniband/core/mad.c
>> index 2fe428b..734d846 100644
>> --- a/drivers/infiniband/core/mad.c
>> +++ b/drivers/infiniband/core/mad.c
>> @@ -1963,6 +1963,16 @@ local:
>>               * or via recv_handler in ib_mad_complete_recv()
>>               */
>>              recv = NULL;
>> +    } else {
>> +            memcpy(response, recv, sizeof(*response));
>
> Isn't this overkill as the bad MAD status precludes looking at the MAD
> data ?
>
>> +            response->header.recv_wc.wc = &response->header.wc;
>> +            response->header.recv_wc.recv_buf.mad = &response->mad.mad;
>> +            response->header.recv_wc.recv_buf.grh = &response->grh;
>> +            response->mad.mad.mad_hdr.method = IB_MGMT_METHOD_GET_RESP;
>> +            response->mad.mad.mad_hdr.status =
>> +                            __be16_to_cpu(IB_MGMT_MAD_STATUS_BAD_VERSION);
>
> While this is the best status for class not supported, that's not all
> the cases that get to here. Attribute not supported (in a supported
> class) could also occur here for which unsupported method/attribute
> combination is more appropriate as a MAD status. I'm not sure it's worth
> the effort to discern that (as any invalid MAD status is treated the
> same) but I think it could be done if we want to be more precise about
> the error.

The other alternative is to just return method/attribute combination not
supported (STATUS_FIELD[4:2] = 0x3 (MAD status 0xc)) for all this as
Jack and Or have indicated. FWIW that's my preference.

-- Hal

>
> -- Hal
>
>> +            agent_send_response(&response->mad.mad, &recv->grh, wc,
>> +                    port_priv->device, port_num, qp_info->qp->qp_num);
>>      }
>>
>>  out:
>>
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
>



This message and any attached documents contain information from QLogic Corporation or its wholly-owned subsidiaries that may be confidential. If you are not the intended recipient, you may not read, copy, distribute, or use this information. If you have received this transmission in error, please notify the sender immediately by reply e-mail and then delete this message.

Re: [PATCH 2/2] IB/mad: Return unsupported for MADs as appropriate

[Hal Rosenstock]

On 1/20/2012 7:12 AM, Swapna Thete wrote:
> Thank you all for your suggestions.
> However, I just wanted to understand that given this code is for the
> case of an entire class not supported, 

There's more than just "class not supported" which gets to that point in
the MAD code flow. For example, SMInfo can and that is more appropriate
as "method/attribute not supported" rather than "class not supported".
The one example cited so far is SMInfo is part of SM class and that is
required to be supported by SMA in every node.

> isn't this error code
> IB_MGMT_MAD_STATUS_BAD_VERSION (MAD returned with Bad Status: Unsupported Class or Version) more appropriate? Also this is
> Part of IB spec error codes.

Yes, class not supported is indicated by this code.

We can either choose to discern which case it is and return the correct
error code (preferable if not too much overhead) or pick one of the two
(in the case where it's an incoming get/set). If we pick one, which one
causes less confusion when it comes to answering why that MAD status was
returned ? I don't think it would/should cause any behavioral difference
as any bad MAD status is/should be treated as an error.

-- Hal

> Also I did not make any changes to the handling of SMA Get(SmInfo
> attribute). Let me know if I am missing something.
> 
> Thanks,
> Swapna
> -----Original Message-----
> From: Hal Rosenstock [mailto:hal-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org]
> Sent: Thursday, January 19, 2012 6:28 PM
> To: Swapna Thete
> Cc: roland-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org; linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Jack Morgenstein
> Subject: Re: [PATCH 2/2] IB/mad: Return unsupported for MADs as appropriate
> 
> On 1/18/2012 5:30 PM, Hal Rosenstock wrote:
>> On 1/18/2012 3:43 AM, Swapna Thete wrote:
>>> Setup a response with appropriate error status and
>>> send it for the MADs that are not supported by a
>>> specific class/version.
>>> Signed-off-by: Swapna Thete <swapna.thete-h88ZbnxC6KDQT0dZR+AlfA@public.gmane.org>
>>> ---
>>>  drivers/infiniband/core/mad.c |   10 ++++++++++
>>>  1 files changed, 10 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/drivers/infiniband/core/mad.c b/drivers/infiniband/core/mad.c
>>> index 2fe428b..734d846 100644
>>> --- a/drivers/infiniband/core/mad.c
>>> +++ b/drivers/infiniband/core/mad.c
>>> @@ -1963,6 +1963,16 @@ local:
>>>               * or via recv_handler in ib_mad_complete_recv()
>>>               */
>>>              recv = NULL;
>>> +    } else {
>>> +            memcpy(response, recv, sizeof(*response));
>>
>> Isn't this overkill as the bad MAD status precludes looking at the MAD
>> data ?
>>
>>> +            response->header.recv_wc.wc = &response->header.wc;
>>> +            response->header.recv_wc.recv_buf.mad = &response->mad.mad;
>>> +            response->header.recv_wc.recv_buf.grh = &response->grh;
>>> +            response->mad.mad.mad_hdr.method = IB_MGMT_METHOD_GET_RESP;
>>> +            response->mad.mad.mad_hdr.status =
>>> +                            __be16_to_cpu(IB_MGMT_MAD_STATUS_BAD_VERSION);
>>
>> While this is the best status for class not supported, that's not all
>> the cases that get to here. Attribute not supported (in a supported
>> class) could also occur here for which unsupported method/attribute
>> combination is more appropriate as a MAD status. I'm not sure it's worth
>> the effort to discern that (as any invalid MAD status is treated the
>> same) but I think it could be done if we want to be more precise about
>> the error.
> 
> The other alternative is to just return method/attribute combination not
> supported (STATUS_FIELD[4:2] = 0x3 (MAD status 0xc)) for all this as
> Jack and Or have indicated. FWIW that's my preference.
> 
> -- Hal
> 
>>
>> -- Hal
>>
>>> +            agent_send_response(&response->mad.mad, &recv->grh, wc,
>>> +                    port_priv->device, port_num, qp_info->qp->qp_num);
>>>      }
>>>
>>>  out:
>>>
>>>
>>> --
>>> 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
>>>
>>
> 
> 
> 
> This message and any attached documents contain information from QLogic Corporation or its wholly-owned subsidiaries that may be confidential. If you are not the intended recipient, you may not read, copy, distribute, or use this information. If you have received this transmission in error, please notify the sender immediately by reply e-mail and then delete this message.

--
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 2/2] IB/mad: Return unsupported for MADs as appropriate

[Hefty, Sean]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 1039 bytes --]

> Thank you all for your suggestions.
> However, I just wanted to understand that given this code is for the
> case of an entire class not supported, isn't this error code
> IB_MGMT_MAD_STATUS_BAD_VERSION (MAD returned with Bad Status: Unsupported
> Class or Version) more appropriate? Also this is
> Part of IB spec error codes.

As Jason pointed out, see figure 169, but, yes, this is the correct value for your case.  What I was suggesting is moving these checks out into another function, dealing with the specific case(s) that you are interested in, and deferring work on other cases.  The function I'm referring to would use positive checks to determine if a reply should be generated and what type of reply it needs to be, with the default case to simply discard the MAD.  Silently dropping these MADs is permissible under the spec, automatically generating a canned reply is not.

- Sean
N‹§²æìr¸›yúèšØb²X¬¶Ç§vØ^–)Þº{.nÇ+‰·¥Š{±­ÙšŠ{ayº\x1dʇڙë,j\a­¢f£¢·hš‹»öì\x17/oSc¾™Ú³9˜uÀ¦æå‰È&jw¨®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿïêäz¹Þ–Šàþf£¢·hšˆ§~ˆmš

Re: [PATCH 2/2] IB/mad: Return unsupported for MADs as appropriate

[Hal Rosenstock]

On 1/20/2012 10:27 AM, Hefty, Sean wrote:
> As Jason pointed out, see figure 169, but, yes, this is the correct value for your case.

Figure 169 is GMP Check. SMInfo is SM class so it's a different case.
When SMInfo is not handled due to no SM present, method/attribute not
supported is most appropriate as a MAD status as SMA handles SM class so
it's always present. I think there are other similar cases (when GS
class is partially supported) too. The only way to return the optimal
MAD status is to look at the port's MAD registrations to really figure
this out.

-- Hal
--
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 2/2] IB/mad: Return unsupported for MADs as appropriate

[Swapna Thete]

Yes Sean. I got your point. As I mentioned earlier, there is already a function called validate_mad(), which checks for
Base version and class version as per figure 169 that Jason
Pointed out. If it's not matching, no response will be generated
And packet will be dropped. I will add a function like generate_unmatched_resp() the way you suggested to check for method get/set and return response as IB_MGMT_MAD_STATUS_BAD_VERSION only
Then. For other methods, again the packet would be dropped.

However, Hal and Jack were still concerned that this is not the correct Status code and it should instead be "Method/attribute unsupported". If there is a consensus on this status code, I can
Include that as well along with the change for generate_unmatched_resp().

Thanks,
Swapna

-----Original Message-----
From: Hefty, Sean [mailto:sean.hefty@intel.com]
Sent: Friday, January 20, 2012 8:58 PM
To: Swapna Thete; Hal Rosenstock
Cc: roland@kernel.org; linux-rdma@vger.kernel.org; Jack Morgenstein
Subject: RE: [PATCH 2/2] IB/mad: Return unsupported for MADs as appropriate

> Thank you all for your suggestions.
> However, I just wanted to understand that given this code is for the
> case of an entire class not supported, isn't this error code
> IB_MGMT_MAD_STATUS_BAD_VERSION (MAD returned with Bad Status: Unsupported
> Class or Version) more appropriate? Also this is
> Part of IB spec error codes.

As Jason pointed out, see figure 169, but, yes, this is the correct value for your case.  What I was suggesting is moving these checks out into another function, dealing with the specific case(s) that you are interested in, and deferring work on other cases.  The function I'm referring to would use positive checks to determine if a reply should be generated and what type of reply it needs to be, with the default case to simply discard the MAD.  Silently dropping these MADs is permissible under the spec, automatically generating a canned reply is not.

- Sean

This message and any attached documents contain information from QLogic Corporation or its wholly-owned subsidiaries that may be confidential. If you are not the intended recipient, you may not read, copy, distribute, or use this information. If you have received this transmission in error, please notify the sender immediately by reply e-mail and then delete this message.

[PATCH 0/2] Setup response for MADs as appropriate

[Swapna Thete]

Changes suggested by Sean to include a function
generate_unmatched_resp() to construct a response
if method is get/set is incorporated. Also hdr
status has been changed to 
IB_MGMT_MAD_STATUS_UNSUPPORTED_METHOD_ATTRIB as
suggested by Hal and Jack.

======================================================
The issue is observed when a query command is issued
for any agent that is not implemented (CCA, BMA etc).

The devices in the OFED will not respond to queries,
thus tools making query to these devices will un-necessarily
take long time waiting for these commands to time out. 
So it would be better for OFED to respond to such a query
for all these devices by setting up a response message. 
For a large fabric consisting of these devices the 
issue would be further aggravated.

Setting up a response with appropriate error message
would be the efficient way to handle such messages
so that devices do not appear un-responsive to such
query commands and respond faster. 

Problem reported-by: Jenny Reick <jenny.reick-h88ZbnxC6KDQT0dZR+AlfA@public.gmane.org>
Reviewed-by: Mike Marciniszyn <mike.marciniszyn-h88ZbnxC6KDQT0dZR+AlfA@public.gmane.org>
Signed-off-by: Swapna Thete <swapna.thete-h88ZbnxC6KDQT0dZR+AlfA@public.gmane.org>

---

Swapna Thete (2):
      IB/mad: Add MAD error codes per IBTA spec
      IB/mad: Return unsupported for MADs as appropriate


 drivers/infiniband/core/mad.c |   10 ++++++++++
 include/rdma/ib_mad.h         |    9 +++++++++
 2 files changed, 19 insertions(+), 0 deletions(-)

-- 

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

[PATCH 2/2] IB/mad: Return unsupported for MADs as appropriate

[Swapna Thete]

Setup a response with appropriate error status and send
it for the MADs that are not supported by a specific
class/version.

Signed-off-by: Swapna Thete <swapna.thete-h88ZbnxC6KDQT0dZR+AlfA@public.gmane.org>
---
 drivers/infiniband/core/mad.c |   22 ++++++++++++++++++++++
 1 files changed, 22 insertions(+), 0 deletions(-)

diff --git a/drivers/infiniband/core/mad.c b/drivers/infiniband/core/mad.c
index 2fe428b..c1c7617 100644
--- a/drivers/infiniband/core/mad.c
+++ b/drivers/infiniband/core/mad.c
@@ -1842,6 +1842,25 @@ static void ib_mad_complete_recv(struct ib_mad_agent_private *mad_agent_priv,
 	}
 }
 
+static int generate_unmatched_resp(struct ib_mad_private *recv,
+				   struct ib_mad_private *response)
+{
+	int matched = 0;
+
+	if ((recv->mad.mad.mad_hdr.method == IB_MGMT_METHOD_GET) ||
+		(recv->mad.mad.mad_hdr.method == IB_MGMT_METHOD_SET)) {
+		memcpy(response, recv, sizeof(*response));
+		response->header.recv_wc.wc = &response->header.wc;
+		response->header.recv_wc.recv_buf.mad = &response->mad.mad;
+		response->header.recv_wc.recv_buf.grh = &response->grh;
+		response->mad.mad.mad_hdr.method = IB_MGMT_METHOD_GET_RESP;
+		response->mad.mad.mad_hdr.status =
+		   __be16_to_cpu(IB_MGMT_MAD_STATUS_UNSUPPORTED_METHOD_ATTRIB);
+		matched = 1;
+	}
+
+	return matched;
+}
 static void ib_mad_recv_done_handler(struct ib_mad_port_private *port_priv,
 				     struct ib_wc *wc)
 {
@@ -1963,6 +1982,9 @@ local:
 		 * or via recv_handler in ib_mad_complete_recv()
 		 */
 		recv = NULL;
+	} else if (generate_unmatched_resp(recv, response)) {
+		agent_send_response(&response->mad.mad, &recv->grh, wc,
+			port_priv->device, port_num, qp_info->qp->qp_num);
 	}
 
 out:


--
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 2/2] IB/mad: Return unsupported for MADs as appropriate

[Hal Rosenstock]

On 1/31/2012 3:11 AM, Swapna Thete wrote:
> Setup a response with appropriate error status and send
> it for the MADs that are not supported by a specific
> class/version.
> 
> Signed-off-by: Swapna Thete <swapna.thete-h88ZbnxC6KDQT0dZR+AlfA@public.gmane.org>

Reviewed-by: Hal Rosenstock <hal-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

> ---
>  drivers/infiniband/core/mad.c |   22 ++++++++++++++++++++++
>  1 files changed, 22 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/infiniband/core/mad.c b/drivers/infiniband/core/mad.c
> index 2fe428b..c1c7617 100644
> --- a/drivers/infiniband/core/mad.c
> +++ b/drivers/infiniband/core/mad.c
> @@ -1842,6 +1842,25 @@ static void ib_mad_complete_recv(struct ib_mad_agent_private *mad_agent_priv,
>  	}
>  }
>  
> +static int generate_unmatched_resp(struct ib_mad_private *recv,
> +				   struct ib_mad_private *response)
> +{
> +	int matched = 0;
> +
> +	if ((recv->mad.mad.mad_hdr.method == IB_MGMT_METHOD_GET) ||
> +		(recv->mad.mad.mad_hdr.method == IB_MGMT_METHOD_SET)) {
> +		memcpy(response, recv, sizeof(*response));

When returning bad MAD status, the entire MAD need not be copied. Just a
minor inefficiency.

-- Hal

> +		response->header.recv_wc.wc = &response->header.wc;
> +		response->header.recv_wc.recv_buf.mad = &response->mad.mad;
> +		response->header.recv_wc.recv_buf.grh = &response->grh;
> +		response->mad.mad.mad_hdr.method = IB_MGMT_METHOD_GET_RESP;
> +		response->mad.mad.mad_hdr.status =
> +		   __be16_to_cpu(IB_MGMT_MAD_STATUS_UNSUPPORTED_METHOD_ATTRIB);
> +		matched = 1;
> +	}
> +
> +	return matched;
> +}
>  static void ib_mad_recv_done_handler(struct ib_mad_port_private *port_priv,
>  				     struct ib_wc *wc)
>  {
> @@ -1963,6 +1982,9 @@ local:
>  		 * or via recv_handler in ib_mad_complete_recv()
>  		 */
>  		recv = NULL;
> +	} else if (generate_unmatched_resp(recv, response)) {
> +		agent_send_response(&response->mad.mad, &recv->grh, wc,
> +			port_priv->device, port_num, qp_info->qp->qp_num);
>  	}
>  
>  out:
> 
> 
> --
> 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