| Message ID | 8cf78b479cca333caf82eca812d421c61675d776.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 09/31] rdma/siw: use >__siw_cep_terminate_upcall() for SIW_CM_WORK_PEER_CLOSE > >It's easier to have generic logic in just one place. > >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 | 89 >+++++++++++++++++------------- > 1 file changed, 50 insertions(+), 39 deletions(-) > >diff --git a/drivers/infiniband/sw/siw/siw_cm.c >b/drivers/infiniband/sw/siw/siw_cm.c >index b7e7f637bd03..5338be450285 100644 >--- a/drivers/infiniband/sw/siw/siw_cm.c >+++ b/drivers/infiniband/sw/siw/siw_cm.c >@@ -110,26 +110,67 @@ static void siw_socket_disassoc(struct socket >*s) > 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); >+ bool suspended = false; this is development debugging only. please remove it. >+ >+ if (cep->qp) { >+ struct siw_qp *qp = cep->qp; >+ >+ if (qp->term_info.valid) >+ siw_send_terminate(qp); >+ >+ if (qp->rx_stream.rx_suspend || qp->tx_ctx.tx_suspend) >+ suspended = true; >+ } else { >+ suspended = true; >+ } This block is not needed. > > switch (cep->state) { > case SIW_EPSTATE_AWAIT_MPAREP: >+ /* >+ * MPA reply not received, but connection drop, >+ * or timeout. >+ */ > siw_cm_upcall(cep, IW_CM_EVENT_CONNECT_REPLY, > reply_status); > break; > > case SIW_EPSTATE_RDMA_MODE: >+ /* >+ * NOTE: IW_CM_EVENT_DISCONNECT is given just >+ * to transition IWCM into CLOSING. >+ */ >+ WARN(!suspended, "SIW_EPSTATE_RDMA_MODE called without >suspended\n"); Please remove this development debugging. also below. >+ siw_cm_upcall(cep, IW_CM_EVENT_DISCONNECT, 0); > siw_cm_upcall(cep, IW_CM_EVENT_CLOSE, 0); > break; > >+ case SIW_EPSTATE_RECVD_MPAREQ: >+ /* >+ * Wait 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"); >+ break; >+ >+ case SIW_EPSTATE_AWAIT_MPAREQ: >+ /* >+ * Socket close before MPA request received. >+ */ >+ siw_dbg_cep(cep, "no mpareq: drop listener\n"); >+ if (cep->listen_cep) >+ siw_cep_put(cep->listen_cep); >+ cep->listen_cep = NULL; >+ 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: >+ /* >+ * for other states there is no connection >+ * known to the IWCM. >+ */ > break; > } > } >@@ -1077,41 +1118,11 @@ static void siw_cm_work_handler(struct >work_struct *w) > break; > > case SIW_CM_WORK_PEER_CLOSE: >- if (cep->cm_id) { >- if (cep->state == SIW_EPSTATE_AWAIT_MPAREP) { >- /* >- * MPA reply not received, but connection drop >- */ >- siw_cm_upcall(cep, IW_CM_EVENT_CONNECT_REPLY, >- -ECONNRESET); >- } else if (cep->state == SIW_EPSTATE_RDMA_MODE) { >- /* >- * NOTE: IW_CM_EVENT_DISCONNECT is given just >- * to transition IWCM into CLOSING. >- */ >- siw_cm_upcall(cep, IW_CM_EVENT_DISCONNECT, 0); >- siw_cm_upcall(cep, IW_CM_EVENT_CLOSE, 0); >- } >- /* >- * for other states there is no connection >- * known to the IWCM. >- */ >- } else { >- if (cep->state == SIW_EPSTATE_RECVD_MPAREQ) { >- /* >- * Wait for the ulp/CM to call accept/reject >- */ >- siw_dbg_cep(cep, >- "mpa req recvd, wait for ULP\n"); >- } else if (cep->state == SIW_EPSTATE_AWAIT_MPAREQ) { >- /* >- * Socket close before MPA request received. >- */ >- siw_dbg_cep(cep, "no mpareq: drop listener\n"); >- siw_cep_put(cep->listen_cep); >- cep->listen_cep = NULL; >- } >- } >+ /* >+ * Peer closed the connection: TCP_CLOSE* >+ */ >+ __siw_cep_terminate_upcall(cep, -ECONNRESET); >+ > release_cep = 1; > break; > >-- >2.25.1 > >
diff --git a/drivers/infiniband/sw/siw/siw_cm.c b/drivers/infiniband/sw/siw/siw_cm.c index b7e7f637bd03..5338be450285 100644 --- a/drivers/infiniband/sw/siw/siw_cm.c +++ b/drivers/infiniband/sw/siw/siw_cm.c @@ -110,26 +110,67 @@ static void siw_socket_disassoc(struct socket *s) 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); + bool suspended = false; + + if (cep->qp) { + struct siw_qp *qp = cep->qp; + + if (qp->term_info.valid) + siw_send_terminate(qp); + + if (qp->rx_stream.rx_suspend || qp->tx_ctx.tx_suspend) + suspended = true; + } else { + suspended = true; + } switch (cep->state) { case SIW_EPSTATE_AWAIT_MPAREP: + /* + * MPA reply not received, but connection drop, + * or timeout. + */ siw_cm_upcall(cep, IW_CM_EVENT_CONNECT_REPLY, reply_status); break; case SIW_EPSTATE_RDMA_MODE: + /* + * NOTE: IW_CM_EVENT_DISCONNECT is given just + * to transition IWCM into CLOSING. + */ + WARN(!suspended, "SIW_EPSTATE_RDMA_MODE called without suspended\n"); + siw_cm_upcall(cep, IW_CM_EVENT_DISCONNECT, 0); siw_cm_upcall(cep, IW_CM_EVENT_CLOSE, 0); break; + case SIW_EPSTATE_RECVD_MPAREQ: + /* + * Wait 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"); + break; + + case SIW_EPSTATE_AWAIT_MPAREQ: + /* + * Socket close before MPA request received. + */ + siw_dbg_cep(cep, "no mpareq: drop listener\n"); + if (cep->listen_cep) + siw_cep_put(cep->listen_cep); + cep->listen_cep = NULL; + 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: + /* + * for other states there is no connection + * known to the IWCM. + */ break; } } @@ -1077,41 +1118,11 @@ static void siw_cm_work_handler(struct work_struct *w) break; case SIW_CM_WORK_PEER_CLOSE: - if (cep->cm_id) { - if (cep->state == SIW_EPSTATE_AWAIT_MPAREP) { - /* - * MPA reply not received, but connection drop - */ - siw_cm_upcall(cep, IW_CM_EVENT_CONNECT_REPLY, - -ECONNRESET); - } else if (cep->state == SIW_EPSTATE_RDMA_MODE) { - /* - * NOTE: IW_CM_EVENT_DISCONNECT is given just - * to transition IWCM into CLOSING. - */ - siw_cm_upcall(cep, IW_CM_EVENT_DISCONNECT, 0); - siw_cm_upcall(cep, IW_CM_EVENT_CLOSE, 0); - } - /* - * for other states there is no connection - * known to the IWCM. - */ - } else { - if (cep->state == SIW_EPSTATE_RECVD_MPAREQ) { - /* - * Wait for the ulp/CM to call accept/reject - */ - siw_dbg_cep(cep, - "mpa req recvd, wait for ULP\n"); - } else if (cep->state == SIW_EPSTATE_AWAIT_MPAREQ) { - /* - * Socket close before MPA request received. - */ - siw_dbg_cep(cep, "no mpareq: drop listener\n"); - siw_cep_put(cep->listen_cep); - cep->listen_cep = NULL; - } - } + /* + * Peer closed the connection: TCP_CLOSE* + */ + __siw_cep_terminate_upcall(cep, -ECONNRESET); + release_cep = 1; break;
It's easier to have generic logic in just one place. 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 | 89 +++++++++++++++++------------- 1 file changed, 50 insertions(+), 39 deletions(-)