Re: [PATCH] ib/cm: Change reject message type when destroying cm_id

[Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>]

[-- Attachment #1: Type: text/plain, Size: 3213 bytes --]

On Thu, 2015-05-14 at 12:49 -0700, sean.hefty-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org wrote:
> From: Ted Kim <ted.h.kim-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
> 
> Problem reported by: Ted Kim <ted.h.kim-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>:
> 
> 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.
> 
> Signed-off-by: Ted Kim <ted.h.kim-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
> Signed-off-by: Sean Hefty <sean.hefty-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>

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?

> ---
>  drivers/infiniband/core/cm.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/infiniband/core/cm.c b/drivers/infiniband/core/cm.c
> index e28a494..346243e 100644
> --- a/drivers/infiniband/core/cm.c
> +++ b/drivers/infiniband/core/cm.c
> @@ -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);


-- 
Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
              GPG KeyID: 0E572FDD


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]