ib/cm: Change reject message type when destroying cm_id
PATCH 1/1 ib/cm: Change reject message type when destroying cm_id
From: Ted Kim <[email protected]>
Problem reported by: Ted Kim <[email protected]>:
We have a case where a Linux system and a non-Linux system are trying to interoperate. The Linux host is the active side and starts the connection establishment, but later decides to not go through with the connection setup and does rdma_destroy_id().
The rdma_destroy_id() eventually works its way down to cm_destroy_id() in core/cm.c, where a REJ is sent. The non-Linux system
has some trouble recognizing the REJ because of:
A. CM states which can't receive the REJ
B. Some issues about REJ formatting (missing comm ID)
ISSUE A: That part of the spec says, a Consumer Reject REJ can be sent for a connection abort, but it goes further
and says: can send a REJ message with a "Consumer Reject" Reason code if they are in a CM state (i.e. REP
Rcvd, MRA(REP) Sent, REQ Rcvd, MRA Sent) that allows
a REJ to be sent (lines 35-38).
Of the states listed there in that sentence, it would seem to limit the active side to using the Consumer Reject (for the abort case) in just the REP-Rcvd and MRA-REP-Sent states. That is basically only after the active side sees a REP (or alternatively goes down the state transitions to timeout in which case a Timeout REJ is sent).
As a fix, in cm-destroy-id() move the IB-CM-MRA-REQ-RCVD case to the same as REQ-SENT. Essentially, make a REJ sent after getting an MRA on active side a timeout rather than Consumer- Reject, which is arguably more correct with the CM state diagrams previous to getting a REP.
--- drivers/infiniband/core/cm.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
drivers/infiniband/core/cm.c +1 -1
@@ -862,6 +862,7 @@ retest:cm_reject_sidr_req (cm_id_priv,IB_SIDR_REJECT );break ;case IB_CM_REQ_SENT : +case IB_CM_MRA_REQ_RCVD :ib_cancel_mad (cm_id_priv->av.port->mad_agent, cm_id_priv->msg);spin_unlock_irq (&cm_id_priv->lock); ib_send_cm_rej(cm_id, IB_CM_REJ_TIMEOUT,
@@ -880,7 +881,6 @@ retest: NULL, 0, NULL, 0); }break ; -case IB_CM_MRA_REQ_RCVD :case IB_CM_REP_SENT :case IB_CM_MRA_REP_RCVD :ib_cancel_mad (cm_id_priv->av.port->mad_agent, cm_id_priv->msg);
Signature · 6 lines
1.7.3 -- 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): 1431632941-3430-1-git-send-email-sean.hefty-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org
Quoted 30 lines · On Thu, 2015-05-14 at 12:49 -0700, [email protected]…
From: Ted Kim <[email protected]>
Problem reported by: Ted Kim <[email protected]>:
We have a case where a Linux system and a non-Linux system are trying to interoperate. The Linux host is the active side and starts the connection establishment, but later decides to not go through with the connection setup and does
rdma_destroy_id().The
rdma_destroy_id()eventually works its way down tocm_destroy_id()incore/cm.c, where a REJ is sent. The non-Linux system
has some trouble recognizing the REJ because of:A. CM states which can't receive the REJ
B. Some issues about REJ formatting (missing comm ID)ISSUE A: That part of the spec says, a Consumer Reject REJ can be sent for a connection abort, but it goes further
and says: can send a REJ message with a "Consumer Reject" Reason code if they are in a CM state (i.e. REP
Rcvd, MRA(REP) Sent, REQ Rcvd, MRA Sent) that allows
a REJ to be sent (lines 35-38).Of the states listed there in that sentence, it would seem to limit the active side to using the Consumer Reject (for the abort case) in just the REP-Rcvd and MRA-REP-Sent states. That is basically only after the active side sees a REP (or alternatively goes down the state transitions to timeout in which case a Timeout REJ is sent).
Quoted 8 lines
As a fix, in cm-destroy-id() move the IB-CM-MRA-REQ-RCVD case to the same as REQ-SENT. Essentially, make a REJ sent after getting an MRA on active side a timeout rather than Consumer- Reject, which is arguably more correct with the CM state diagrams previous to getting a REP.
Signed-off-by Ted Kim <[email protected]>Signed-off-by Sean Hefty <[email protected]>
Sean, a few questions:
Are you proposing for 4.1-rc?
I assume this passed testing on the linux/non-linux setup? Was it tested in a similar situation on linux/linux setup? How do we handle this specific rej scenario?
Did you intend to add an ISSUE B: section to the commit message?
Quoted 24 lines
--- drivers/infiniband/core/cm.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)drivers/infiniband/core/cm.c +1
-1@@ -862,6 +862,7 @@ retest:cm_reject_sidr_req (cm_id_priv,IB_SIDR_REJECT );break ;case IB_CM_REQ_SENT : +case IB_CM_MRA_REQ_RCVD :ib_cancel_mad (cm_id_priv->av.port->mad_agent, cm_id_priv->msg);spin_unlock_irq (&cm_id_priv->lock); ib_send_cm_rej(cm_id, IB_CM_REJ_TIMEOUT,@@ -880,7 +881,6 @@ retest: NULL, 0, NULL, 0); }break ; -case IB_CM_MRA_REQ_RCVD :case IB_CM_REP_SENT :case IB_CM_MRA_REP_RCVD :ib_cancel_mad (cm_id_priv->av.port->mad_agent, cm_id_priv->msg);
Signature · 2 lines
Doug Ledford <[email protected]> GPG KeyID: 0E572FDD
A message that is not in the archive (sent off-list or not yet received): 1431708034.29187.14.camel-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org
PiBBcmUgeW91IHByb3Bvc2luZyBmb3IgNC4xLXJjPw0KDQpUaGUgY29kZSBoYXMgYmVlbiB0aGlz IHdheSBmb3JldmVyLCBzbyBJTU8gNC4yIHNob3VsZCBiZSBmaW5lLg0KDQo+IEkgYXNzdW1lIHRo aXMgcGFzc2VkIHRlc3Rpbmcgb24gdGhlIGxpbnV4L25vbi1saW51eCBzZXR1cD8NCg0KVGhlIHBh dGNoIGNhbWUgZnJvbSBUZWQuICBJJ20gYXNzdW1pbmcgdGhhdCBoZSB0ZXN0ZWQgd2l0aCB0aGlz Lg0KDQo+IFdhcyBpdCB0ZXN0ZWQgaW4gYSBzaW1pbGFyIHNpdHVhdGlvbiBvbiBsaW51eC9saW51 eCBzZXR1cD8gIEhvdyBkbyB3ZQ0KPiBoYW5kbGUgdGhpcyBzcGVjaWZpYyByZWogc2NlbmFyaW8/ DQoNClRoaXMgY2hhbmdlIGVmZmVjdGl2ZWx5IGNoYW5nZXMgdGhlIHJlamVjdCBjb2RlIHRoYXQg aXMgcmVwb3J0ZWQgaW4gdGhlIHJlamVjdCBtZXNzYWdlIGZyb20gJ2NvbnN1bWVyIHJlamVjdCcg dG8gJ3RpbWVvdXQnLiAgVGhlIGNvZGUgYWxyZWFkeSBoYW5kbGVzIHRoaXMuDQoNCj4gRGlkIHlv dSBpbnRlbmQgdG8gYWRkIGFuIElTU1VFIEI6IHNlY3Rpb24gdG8gdGhlIGNvbW1pdCBtZXNzYWdl Pw0KDQpUaGUgaXNzdWUgaXMgbGlzdGVkIGFzIHRoZSBtaXNzaW5nIGNvbW0gaWQuICBUaG91Z2gs IHRoaW5raW5nIGFib3V0IGl0IGFnYWluLCB0aGUgaXNzdWUgbWF5IGhhdmUgYmVlbiBhIG1pc3Np bmcgbm9kZSBHVUlELiAgSSBjYW4ndCByZWNhbGwgbm93LiAgTWF5YmUgVGVkIGRvZXMuDQoNClRo ZSBJQiBzcGVjIGlzIGdvb2Z5IGhlcmUgaW4gdGhhdCBpdCBvbmx5IGFsbG93cyBhIENNIFJFUSwg b25jZSBzZW50LCB0byB0aW1lb3V0LiAgSXQgZG9lc24ndCBkZWZpbmUgYW4gJ2Fib3J0JyBjYXNl LiAgVGhlIHNwZWMgYWxzbyB0cmVhdHMgdGhlIGNvbm5lY3Rpb24gaXMgYmVpbmcgaW4gYSBzaW5n bGUgJ1JFUCB3YWl0JyBzdGF0ZSwgcmVnYXJkbGVzcyBvZiB3aGV0aGVyIGFuIE1SQSBoYXMgYmVl biByZWNlaXZlZCBvciBub3QuICBUaGlzIGNoYW5nZSByZXN1bHRzIGluIHRoZSAnSSBnb3QgYW4g TVJBIGJ1dCBJJ20gd2FpdGluZyBmb3IgYSBSRVAnIHRvIGJlIGhhbmRsZWQgdGhlIHNhbWUgYXMg J0knbSB3YWl0aW5nIGZvciBhIFJFUCcsIGF0IGxlYXN0IGZvciB0aGUgZGVzdHJ1Y3Rpb24gY2Fz ZS4NCg0KLSBTZWFuDQo=
--
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): 1828884A29C6694DAF28B7E6B8A82373A8FDBF8D-P5GAC/[email protected]
Quoted 5 lines · Sean
Quoted 1 line
Did you intend to add an ISSUE B: section to the commit message?
The issue is listed as the missing comm id. Though, thinking about it again, the issue may have been a missing node GUID. I can't recall now. Maybe Ted does.
Imagine the case of the Linux active side sending REQ with it's CM communication id. When it gets the REP back, the other side's communication ID will be received and internal CM state updated. So any REJ from this point on can send both communication IDs.
Now imagine a slightly different case. The active side sends the REQ, and the passive side responds with MRA. At this point the CM should update the state with the second communication ID coming in the MRA. But it does not do so. This is "ISSUE B". This is will require a different fix from the patch in question to add the communication ID updating when getting an MRA.
So as a side effect, a REJ issued at this point by the active side has only its own communication ID. The problem with only one communication ID REJs is that they are all generated by remote systems from the one being sent the REJ and therefore might not be unique. The way that TIMEOUT REJ addresses this is to have a GUID in the ARI to make it unique. But other REJ codes don't have this.
-ted
Quoted 6 lines
The IB spec is goofy here in that it only allows a CM REQ, once sent, to timeout. It doesn't define an 'abort' case. The spec also treats the connection is being in a single 'REP wait' state, regardless of whether an MRA has been received or not. This change results in the 'I got an MRA but I'm waiting for a REP' to be handled the same as 'I'm waiting for a REP', at least for the destruction case.
Signature · 8 lines
Ted H. Kim, PhD Sun/Oracle America [email protected] 5750 Hannum Drive, #200 +1 310-258-7536 Culver City, CA 90230 -- 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 3 lines · Sean
Quoted 1 line
Are you proposing for 4.1-rc?
The code has been this way forever, so IMO 4.2 should be fine.
Yes, but surely we want this backported to -stable and distros? It isn't tagged for that..
Jason
--
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): 20150519181457.GD18675-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org
Quoted 2 lines
Yes, but surely we want this backported to -stable and distros? It isn't tagged for that..
I don't think that the problem justifies backporting the patch. The passive side drops the reject, which could have happened anyway, and gets a reject message when sending a REP.
- Sean
--
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 30 lines · On Thu, 2015-05-14 at 12:49 -0700, [email protected]…
From: Ted Kim <[email protected]>
Problem reported by: Ted Kim <[email protected]>:
We have a case where a Linux system and a non-Linux system are trying to interoperate. The Linux host is the active side and starts the connection establishment, but later decides to not go through with the connection setup and does
rdma_destroy_id().The
rdma_destroy_id()eventually works its way down tocm_destroy_id()incore/cm.c, where a REJ is sent. The non-Linux system
has some trouble recognizing the REJ because of:A. CM states which can't receive the REJ
B. Some issues about REJ formatting (missing comm ID)ISSUE A: That part of the spec says, a Consumer Reject REJ can be sent for a connection abort, but it goes further
and says: can send a REJ message with a "Consumer Reject" Reason code if they are in a CM state (i.e. REP
Rcvd, MRA(REP) Sent, REQ Rcvd, MRA Sent) that allows
a REJ to be sent (lines 35-38).Of the states listed there in that sentence, it would seem to limit the active side to using the Consumer Reject (for the abort case) in just the REP-Rcvd and MRA-REP-Sent states. That is basically only after the active side sees a REP (or alternatively goes down the state transitions to timeout in which case a Timeout REJ is sent).
Quoted 8 lines
As a fix, in cm-destroy-id() move the IB-CM-MRA-REQ-RCVD case to the same as REQ-SENT. Essentially, make a REJ sent after getting an MRA on active side a timeout rather than Consumer- Reject, which is arguably more correct with the CM state diagrams previous to getting a REP.
Signed-off-by Ted Kim <[email protected]>Signed-off-by Sean Hefty <[email protected]>
I've picked this up for 4.2.
Signature · 2 lines
Doug Ledford <[email protected]> GPG KeyID: 0E572FDD