| Message ID | b6d45ce73a6d67748723175b2e2a3cbedf2efe57.1620343860.git.metze@samba.org (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | rdma/siw: fix a lot of deadlocks and use after free bugs | expand |
-----"Stefan Metzmacher" <metze@samba.org> wrote: ----- >To: "Bernard Metzler" <bmt@zurich.ibm.com> >From: "Stefan Metzmacher" <metze@samba.org> >Date: 05/07/2021 01:38AM >Cc: linux-rdma@vger.kernel.org, "Stefan Metzmacher" <metze@samba.org> >Subject: [EXTERNAL] [PATCH 11/31] rdma/siw: introduce >SIW_EPSTATE_ACCEPTING/REJECTING for rdma_accept/rdma_reject > >When we received the MPA Request, we set SIW_EPSTATE_RECVD_MPAREQ >and port IW_CM_EVENT_CONNECT_REQUEST to the IWCM layer. > >In that state we expect the caller to reacted with rdma_accept() or >rdma_reject(), which will turn the connection into >SIW_EPSTATE_RDMA_MODE >or SIW_EPSTATE_CLOSED finally. > >I think it much saner that rdma_accept and rdma_reject change the >state >instead of keeping it as SIW_EPSTATE_RECVD_MPAREQ in order to make >the logic more understandable and allow more useful debug messages. > >In all cases we need to inform the IWCM layer about that error! >As it only allows IW_CM_EVENT_ESTABLISHED to be posted after >IW_CM_EVENT_CONNECT_REQUEST was posted, we need to go through >IW_CM_EVENT_ESTABLISHED via IW_CM_EVENT_DISCONNECT to >IW_CM_EVENT_CLOSE. > >Fixes: 6c52fdc244b5 ("rdma/siw: connection management") >Signed-off-by: Stefan Metzmacher <metze@samba.org> >Cc: Bernard Metzler <bmt@zurich.ibm.com> >Cc: linux-rdma@vger.kernel.org >--- > drivers/infiniband/sw/siw/siw_cm.c | 31 >++++++++++++++++++++++++++++-- > drivers/infiniband/sw/siw/siw_cm.h | 2 ++ > 2 files changed, 31 insertions(+), 2 deletions(-) > >diff --git a/drivers/infiniband/sw/siw/siw_cm.c >b/drivers/infiniband/sw/siw/siw_cm.c >index d03c7a66c6d1..3cc1d22fe232 100644 >--- a/drivers/infiniband/sw/siw/siw_cm.c >+++ b/drivers/infiniband/sw/siw/siw_cm.c >@@ -146,10 +146,35 @@ static void __siw_cep_terminate_upcall(struct >siw_cep *cep, > > case SIW_EPSTATE_RECVD_MPAREQ: > /* >- * Wait for the ulp/CM to call accept/reject >+ * Waited for the ulp/CM to call accept/reject > */ >- siw_dbg_cep(cep, "mpa req recvd, wait for ULP\n"); > WARN(!suspended, "SIW_EPSTATE_RECVD_MPAREQ called without >suspended\n"); Please remove all that WARN(!suspended, ..) >+ siw_dbg_cep(cep, "mpa req recvd, post >established/disconnect/close\n"); >+ siw_cm_upcall(cep, IW_CM_EVENT_ESTABLISHED, 0); >+ siw_cm_upcall(cep, IW_CM_EVENT_DISCONNECT, 0); >+ siw_cm_upcall(cep, IW_CM_EVENT_CLOSE, 0); >+ break; >+ >+ case SIW_EPSTATE_ACCEPTING: >+ /* >+ * We failed during the rdma_accept/siw_accept handling >+ */ >+ WARN(!suspended, "SIW_EPSTATE_ACCEPTING called without >suspended\n"); >+ siw_dbg_cep(cep, "accepting, post >established/disconnect/close\n"); >+ siw_cm_upcall(cep, IW_CM_EVENT_ESTABLISHED, 0); >+ siw_cm_upcall(cep, IW_CM_EVENT_DISCONNECT, 0); >+ siw_cm_upcall(cep, IW_CM_EVENT_CLOSE, 0); >+ break; >+ >+ case SIW_EPSTATE_REJECTING: >+ /* >+ * We failed during the rdma_reject/siw_reject handling >+ */ >+ WARN(!suspended, "SIW_EPSTATE_REJECTING called without >suspended\n"); >+ siw_dbg_cep(cep, "rejecting, post >established/disconnect/close\n"); >+ siw_cm_upcall(cep, IW_CM_EVENT_ESTABLISHED, 0); >+ siw_cm_upcall(cep, IW_CM_EVENT_DISCONNECT, 0); >+ siw_cm_upcall(cep, IW_CM_EVENT_CLOSE, 0); > break; > > case SIW_EPSTATE_AWAIT_MPAREQ: >@@ -1563,6 +1588,7 @@ int siw_accept(struct iw_cm_id *id, struct >iw_cm_conn_param *params) > > return -ECONNRESET; > } >+ cep->state = SIW_EPSTATE_ACCEPTING; > qp = siw_qp_id2obj(sdev, params->qpn); > if (!qp) { > WARN(1, "[QP %d] does not exist\n", params->qpn); >@@ -1743,6 +1769,7 @@ int siw_reject(struct iw_cm_id *id, const void >*pdata, u8 pd_len) > } > siw_dbg_cep(cep, "cep->state %d, pd_len %d\n", cep->state, > pd_len); >+ cep->state = SIW_EPSTATE_REJECTING; > > if (__mpa_rr_revision(cep->mpa.hdr.params.bits) >= MPA_REVISION_1) >{ > cep->mpa.hdr.params.bits |= MPA_RR_FLAG_REJECT; /* reject */ >diff --git a/drivers/infiniband/sw/siw/siw_cm.h >b/drivers/infiniband/sw/siw/siw_cm.h >index 8c59cb3e2868..4f6219bd746b 100644 >--- a/drivers/infiniband/sw/siw/siw_cm.h >+++ b/drivers/infiniband/sw/siw/siw_cm.h >@@ -20,6 +20,8 @@ enum siw_cep_state { > SIW_EPSTATE_AWAIT_MPAREQ, > SIW_EPSTATE_RECVD_MPAREQ, > SIW_EPSTATE_AWAIT_MPAREP, >+ SIW_EPSTATE_ACCEPTING, >+ SIW_EPSTATE_REJECTING, > SIW_EPSTATE_RDMA_MODE, > SIW_EPSTATE_CLOSED > }; >-- >2.25.1 > >
diff --git a/drivers/infiniband/sw/siw/siw_cm.c b/drivers/infiniband/sw/siw/siw_cm.c index d03c7a66c6d1..3cc1d22fe232 100644 --- a/drivers/infiniband/sw/siw/siw_cm.c +++ b/drivers/infiniband/sw/siw/siw_cm.c @@ -146,10 +146,35 @@ static void __siw_cep_terminate_upcall(struct siw_cep *cep, case SIW_EPSTATE_RECVD_MPAREQ: /* - * Wait for the ulp/CM to call accept/reject + * Waited for the ulp/CM to call accept/reject */ - siw_dbg_cep(cep, "mpa req recvd, wait for ULP\n"); WARN(!suspended, "SIW_EPSTATE_RECVD_MPAREQ called without suspended\n"); + siw_dbg_cep(cep, "mpa req recvd, post established/disconnect/close\n"); + siw_cm_upcall(cep, IW_CM_EVENT_ESTABLISHED, 0); + siw_cm_upcall(cep, IW_CM_EVENT_DISCONNECT, 0); + siw_cm_upcall(cep, IW_CM_EVENT_CLOSE, 0); + break; + + case SIW_EPSTATE_ACCEPTING: + /* + * We failed during the rdma_accept/siw_accept handling + */ + WARN(!suspended, "SIW_EPSTATE_ACCEPTING called without suspended\n"); + siw_dbg_cep(cep, "accepting, post established/disconnect/close\n"); + siw_cm_upcall(cep, IW_CM_EVENT_ESTABLISHED, 0); + siw_cm_upcall(cep, IW_CM_EVENT_DISCONNECT, 0); + siw_cm_upcall(cep, IW_CM_EVENT_CLOSE, 0); + break; + + case SIW_EPSTATE_REJECTING: + /* + * We failed during the rdma_reject/siw_reject handling + */ + WARN(!suspended, "SIW_EPSTATE_REJECTING called without suspended\n"); + siw_dbg_cep(cep, "rejecting, post established/disconnect/close\n"); + siw_cm_upcall(cep, IW_CM_EVENT_ESTABLISHED, 0); + siw_cm_upcall(cep, IW_CM_EVENT_DISCONNECT, 0); + siw_cm_upcall(cep, IW_CM_EVENT_CLOSE, 0); break; case SIW_EPSTATE_AWAIT_MPAREQ: @@ -1563,6 +1588,7 @@ int siw_accept(struct iw_cm_id *id, struct iw_cm_conn_param *params) return -ECONNRESET; } + cep->state = SIW_EPSTATE_ACCEPTING; qp = siw_qp_id2obj(sdev, params->qpn); if (!qp) { WARN(1, "[QP %d] does not exist\n", params->qpn); @@ -1743,6 +1769,7 @@ int siw_reject(struct iw_cm_id *id, const void *pdata, u8 pd_len) } siw_dbg_cep(cep, "cep->state %d, pd_len %d\n", cep->state, pd_len); + cep->state = SIW_EPSTATE_REJECTING; if (__mpa_rr_revision(cep->mpa.hdr.params.bits) >= MPA_REVISION_1) { cep->mpa.hdr.params.bits |= MPA_RR_FLAG_REJECT; /* reject */ diff --git a/drivers/infiniband/sw/siw/siw_cm.h b/drivers/infiniband/sw/siw/siw_cm.h index 8c59cb3e2868..4f6219bd746b 100644 --- a/drivers/infiniband/sw/siw/siw_cm.h +++ b/drivers/infiniband/sw/siw/siw_cm.h @@ -20,6 +20,8 @@ enum siw_cep_state { SIW_EPSTATE_AWAIT_MPAREQ, SIW_EPSTATE_RECVD_MPAREQ, SIW_EPSTATE_AWAIT_MPAREP, + SIW_EPSTATE_ACCEPTING, + SIW_EPSTATE_REJECTING, SIW_EPSTATE_RDMA_MODE, SIW_EPSTATE_CLOSED };
When we received the MPA Request, we set SIW_EPSTATE_RECVD_MPAREQ and port IW_CM_EVENT_CONNECT_REQUEST to the IWCM layer. In that state we expect the caller to reacted with rdma_accept() or rdma_reject(), which will turn the connection into SIW_EPSTATE_RDMA_MODE or SIW_EPSTATE_CLOSED finally. I think it much saner that rdma_accept and rdma_reject change the state instead of keeping it as SIW_EPSTATE_RECVD_MPAREQ in order to make the logic more understandable and allow more useful debug messages. In all cases we need to inform the IWCM layer about that error! As it only allows IW_CM_EVENT_ESTABLISHED to be posted after IW_CM_EVENT_CONNECT_REQUEST was posted, we need to go through IW_CM_EVENT_ESTABLISHED via IW_CM_EVENT_DISCONNECT to IW_CM_EVENT_CLOSE. Fixes: 6c52fdc244b5 ("rdma/siw: connection management") Signed-off-by: Stefan Metzmacher <metze@samba.org> Cc: Bernard Metzler <bmt@zurich.ibm.com> Cc: linux-rdma@vger.kernel.org --- drivers/infiniband/sw/siw/siw_cm.c | 31 ++++++++++++++++++++++++++++-- drivers/infiniband/sw/siw/siw_cm.h | 2 ++ 2 files changed, 31 insertions(+), 2 deletions(-)