| Message ID | 8d6af83bbaf4647290d25c7ba0017a5a8059f107.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:37AM >Cc: linux-rdma@vger.kernel.org, "Stefan Metzmacher" <metze@samba.org> >Subject: [EXTERNAL] [PATCH 07/31] rdma/siw: split out a >__siw_cep_terminate_upcall() function > >There are multiple places where should have the same logic. >Having one helper function to be used in all places >makes it easier to extended the logic. > >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 | 53 >++++++++++++++++++------------ > 1 file changed, 32 insertions(+), 21 deletions(-) > >diff --git a/drivers/infiniband/sw/siw/siw_cm.c >b/drivers/infiniband/sw/siw/siw_cm.c >index 2cc2863bd427..c91a74271b9b 100644 >--- a/drivers/infiniband/sw/siw/siw_cm.c >+++ b/drivers/infiniband/sw/siw/siw_cm.c >@@ -103,6 +103,37 @@ static void siw_socket_disassoc(struct socket >*s) > } > } > >+/* >+ * The caller needs to deal with siw_cep_set_inuse() >+ * and siw_cep_set_free() >+ */ >+static void __siw_cep_terminate_upcall(struct siw_cep *cep, >+ int reply_status) >+{ >+ if (cep->qp && cep->qp->term_info.valid) >+ siw_send_terminate(cep->qp); >+ While I see some merits of having consolidated the updates to the IWCM state machine on close, I don't think we should mix this with sending TERMINATE messages on the wire. Just keep that where it currently is. Call that new thing different - 'siw_upcall_llp_close' or some such? >+ switch (cep->state) { >+ case SIW_EPSTATE_AWAIT_MPAREP: >+ siw_cm_upcall(cep, IW_CM_EVENT_CONNECT_REPLY, >+ reply_status); >+ break; >+ >+ case SIW_EPSTATE_RDMA_MODE: >+ siw_cm_upcall(cep, IW_CM_EVENT_CLOSE, 0); >+ break; >+ >+ case SIW_EPSTATE_IDLE: >+ case SIW_EPSTATE_LISTENING: >+ case SIW_EPSTATE_CONNECTING: >+ case SIW_EPSTATE_AWAIT_MPAREQ: >+ case SIW_EPSTATE_RECVD_MPAREQ: >+ case SIW_EPSTATE_CLOSED: >+ default: >+ break; >+ } >+} >+ > static void siw_rtr_data_ready(struct sock *sk) > { > struct siw_cep *cep; >@@ -395,29 +426,9 @@ void siw_qp_cm_drop(struct siw_qp *qp, int >schedule) > } > siw_dbg_cep(cep, "immediate close, state %d\n", cep->state); > >- if (qp->term_info.valid) >- siw_send_terminate(qp); >+ __siw_cep_terminate_upcall(cep, -EINVAL); > > if (cep->cm_id) { >- switch (cep->state) { >- case SIW_EPSTATE_AWAIT_MPAREP: >- siw_cm_upcall(cep, IW_CM_EVENT_CONNECT_REPLY, >- -EINVAL); >- break; >- >- case SIW_EPSTATE_RDMA_MODE: >- siw_cm_upcall(cep, IW_CM_EVENT_CLOSE, 0); >- break; >- >- case SIW_EPSTATE_IDLE: >- case SIW_EPSTATE_LISTENING: >- case SIW_EPSTATE_CONNECTING: >- case SIW_EPSTATE_AWAIT_MPAREQ: >- case SIW_EPSTATE_RECVD_MPAREQ: >- case SIW_EPSTATE_CLOSED: >- default: >- break; >- } > cep->cm_id->rem_ref(cep->cm_id); > cep->cm_id = NULL; > siw_cep_put(cep); >-- >2.25.1 > >
diff --git a/drivers/infiniband/sw/siw/siw_cm.c b/drivers/infiniband/sw/siw/siw_cm.c index 2cc2863bd427..c91a74271b9b 100644 --- a/drivers/infiniband/sw/siw/siw_cm.c +++ b/drivers/infiniband/sw/siw/siw_cm.c @@ -103,6 +103,37 @@ static void siw_socket_disassoc(struct socket *s) } } +/* + * The caller needs to deal with siw_cep_set_inuse() + * and siw_cep_set_free() + */ +static void __siw_cep_terminate_upcall(struct siw_cep *cep, + int reply_status) +{ + if (cep->qp && cep->qp->term_info.valid) + siw_send_terminate(cep->qp); + + switch (cep->state) { + case SIW_EPSTATE_AWAIT_MPAREP: + siw_cm_upcall(cep, IW_CM_EVENT_CONNECT_REPLY, + reply_status); + break; + + case SIW_EPSTATE_RDMA_MODE: + siw_cm_upcall(cep, IW_CM_EVENT_CLOSE, 0); + break; + + case SIW_EPSTATE_IDLE: + case SIW_EPSTATE_LISTENING: + case SIW_EPSTATE_CONNECTING: + case SIW_EPSTATE_AWAIT_MPAREQ: + case SIW_EPSTATE_RECVD_MPAREQ: + case SIW_EPSTATE_CLOSED: + default: + break; + } +} + static void siw_rtr_data_ready(struct sock *sk) { struct siw_cep *cep; @@ -395,29 +426,9 @@ void siw_qp_cm_drop(struct siw_qp *qp, int schedule) } siw_dbg_cep(cep, "immediate close, state %d\n", cep->state); - if (qp->term_info.valid) - siw_send_terminate(qp); + __siw_cep_terminate_upcall(cep, -EINVAL); if (cep->cm_id) { - switch (cep->state) { - case SIW_EPSTATE_AWAIT_MPAREP: - siw_cm_upcall(cep, IW_CM_EVENT_CONNECT_REPLY, - -EINVAL); - break; - - case SIW_EPSTATE_RDMA_MODE: - siw_cm_upcall(cep, IW_CM_EVENT_CLOSE, 0); - break; - - case SIW_EPSTATE_IDLE: - case SIW_EPSTATE_LISTENING: - case SIW_EPSTATE_CONNECTING: - case SIW_EPSTATE_AWAIT_MPAREQ: - case SIW_EPSTATE_RECVD_MPAREQ: - case SIW_EPSTATE_CLOSED: - default: - break; - } cep->cm_id->rem_ref(cep->cm_id); cep->cm_id = NULL; siw_cep_put(cep);
There are multiple places where should have the same logic. Having one helper function to be used in all places makes it easier to extended the logic. 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 | 53 ++++++++++++++++++------------ 1 file changed, 32 insertions(+), 21 deletions(-)