* [PATCH for-rc] RDMA/cma: Revert INIT-INIT patch
@ 2021-07-29 18:26 mike.marciniszyn
2021-08-03 15:57 ` Jason Gunthorpe
2021-08-09 12:20 ` Haakon Bugge
0 siblings, 2 replies; 3+ messages in thread
From: mike.marciniszyn @ 2021-07-29 18:26 UTC (permalink / raw)
To: dledford, jgg; +Cc: linux-rdma, Mike Marciniszyn, Haakon Bugge, Chuck Lever III
From: Mike Marciniszyn <mike.marciniszyn@cornelisnetworks.com>
The net/sunrpc/xprtrdma module creates its QP using rdma_create_qp() and
immediately post receives, implicitly assuming the QP is in the INIT
state and thus valid for ib_post_recv().
The patch noted in Fixes: removed the RESET->INIT modifiy from
rdma_create_qp(), breaking NFS rdma for verbs providers that fail the
ib_post_recv() for a bad state.
This situation was proven using kprobes in rvt_post_recv() and
rvt_modify_qp(). The traces showed that the rvt_post_recv() failed before
ANY modify QP and that the current state was RESET.
Fix by reverting the patch below.
Fixes: dc70f7c3ed34 ("RDMA/cma: Remove unnecessary INIT->INIT transition")
Cc: Haakon Bugge <haakon.bugge@oracle.com>
Cc: Chuck Lever III <chuck.lever@oracle.com>
Signed-off-by: Mike Marciniszyn <mike.marciniszyn@cornelisnetworks.com>
---
drivers/infiniband/core/cma.c | 17 ++++++++++++++++-
1 file changed, 16 insertions(+), 1 deletion(-)
diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
index 515a7e9..5d3b8b8 100644
--- a/drivers/infiniband/core/cma.c
+++ b/drivers/infiniband/core/cma.c
@@ -926,12 +926,25 @@ static int cma_init_ud_qp(struct rdma_id_private *id_priv, struct ib_qp *qp)
return ret;
}
+static int cma_init_conn_qp(struct rdma_id_private *id_priv, struct ib_qp *qp)
+{
+ struct ib_qp_attr qp_attr;
+ int qp_attr_mask, ret;
+
+ qp_attr.qp_state = IB_QPS_INIT;
+ ret = rdma_init_qp_attr(&id_priv->id, &qp_attr, &qp_attr_mask);
+ if (ret)
+ return ret;
+
+ return ib_modify_qp(qp, &qp_attr, qp_attr_mask);
+}
+
int rdma_create_qp(struct rdma_cm_id *id, struct ib_pd *pd,
struct ib_qp_init_attr *qp_init_attr)
{
struct rdma_id_private *id_priv;
struct ib_qp *qp;
- int ret = 0;
+ int ret;
id_priv = container_of(id, struct rdma_id_private, id);
if (id->device != pd->device) {
@@ -948,6 +961,8 @@ int rdma_create_qp(struct rdma_cm_id *id, struct ib_pd *pd,
if (id->qp_type == IB_QPT_UD)
ret = cma_init_ud_qp(id_priv, qp);
+ else
+ ret = cma_init_conn_qp(id_priv, qp);
if (ret)
goto out_destroy;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH for-rc] RDMA/cma: Revert INIT-INIT patch
2021-07-29 18:26 [PATCH for-rc] RDMA/cma: Revert INIT-INIT patch mike.marciniszyn
@ 2021-08-03 15:57 ` Jason Gunthorpe
2021-08-09 12:20 ` Haakon Bugge
1 sibling, 0 replies; 3+ messages in thread
From: Jason Gunthorpe @ 2021-08-03 15:57 UTC (permalink / raw)
To: mike.marciniszyn; +Cc: dledford, linux-rdma, Haakon Bugge, Chuck Lever III
On Thu, Jul 29, 2021 at 02:26:22PM -0400, mike.marciniszyn@cornelisnetworks.com wrote:
> From: Mike Marciniszyn <mike.marciniszyn@cornelisnetworks.com>
>
> The net/sunrpc/xprtrdma module creates its QP using rdma_create_qp() and
> immediately post receives, implicitly assuming the QP is in the INIT
> state and thus valid for ib_post_recv().
>
> The patch noted in Fixes: removed the RESET->INIT modifiy from
> rdma_create_qp(), breaking NFS rdma for verbs providers that fail the
> ib_post_recv() for a bad state.
>
> This situation was proven using kprobes in rvt_post_recv() and
> rvt_modify_qp(). The traces showed that the rvt_post_recv() failed before
> ANY modify QP and that the current state was RESET.
>
> Fix by reverting the patch below.
>
> Fixes: dc70f7c3ed34 ("RDMA/cma: Remove unnecessary INIT->INIT transition")
> Cc: Haakon Bugge <haakon.bugge@oracle.com>
> Cc: Chuck Lever III <chuck.lever@oracle.com>
> Signed-off-by: Mike Marciniszyn <mike.marciniszyn@cornelisnetworks.com>
> ---
> drivers/infiniband/core/cma.c | 17 ++++++++++++++++-
> 1 file changed, 16 insertions(+), 1 deletion(-)
Applied to for-rc, thanks
Jason
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH for-rc] RDMA/cma: Revert INIT-INIT patch
2021-07-29 18:26 [PATCH for-rc] RDMA/cma: Revert INIT-INIT patch mike.marciniszyn
2021-08-03 15:57 ` Jason Gunthorpe
@ 2021-08-09 12:20 ` Haakon Bugge
1 sibling, 0 replies; 3+ messages in thread
From: Haakon Bugge @ 2021-08-09 12:20 UTC (permalink / raw)
To: Mike Marciniszyn
Cc: Doug Ledford, Jason Gunthorpe, OFED mailing list, Chuck Lever III
> On 29 Jul 2021, at 20:26, mike.marciniszyn@cornelisnetworks.com wrote:
>
> From: Mike Marciniszyn <mike.marciniszyn@cornelisnetworks.com>
>
> The net/sunrpc/xprtrdma module creates its QP using rdma_create_qp() and
> immediately post receives, implicitly assuming the QP is in the INIT
> state and thus valid for ib_post_recv().
>
> The patch noted in Fixes: removed the RESET->INIT modifiy from
> rdma_create_qp(), breaking NFS rdma for verbs providers that fail the
> ib_post_recv() for a bad state.
>
> This situation was proven using kprobes in rvt_post_recv() and
> rvt_modify_qp(). The traces showed that the rvt_post_recv() failed before
> ANY modify QP and that the current state was RESET.
>
> Fix by reverting the patch below.
>
> Fixes: dc70f7c3ed34 ("RDMA/cma: Remove unnecessary INIT->INIT transition")
> Cc: Haakon Bugge <haakon.bugge@oracle.com>
> Cc: Chuck Lever III <chuck.lever@oracle.com>
> Signed-off-by: Mike Marciniszyn <mike.marciniszyn@cornelisnetworks.com>
Reviewed-by: Håkon Bugge <haakon.bugge@oracle.com>
Thxs, Håkon
> ---
> drivers/infiniband/core/cma.c | 17 ++++++++++++++++-
> 1 file changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
> index 515a7e9..5d3b8b8 100644
> --- a/drivers/infiniband/core/cma.c
> +++ b/drivers/infiniband/core/cma.c
> @@ -926,12 +926,25 @@ static int cma_init_ud_qp(struct rdma_id_private *id_priv, struct ib_qp *qp)
> return ret;
> }
>
> +static int cma_init_conn_qp(struct rdma_id_private *id_priv, struct ib_qp *qp)
> +{
> + struct ib_qp_attr qp_attr;
> + int qp_attr_mask, ret;
> +
> + qp_attr.qp_state = IB_QPS_INIT;
> + ret = rdma_init_qp_attr(&id_priv->id, &qp_attr, &qp_attr_mask);
> + if (ret)
> + return ret;
> +
> + return ib_modify_qp(qp, &qp_attr, qp_attr_mask);
> +}
> +
> int rdma_create_qp(struct rdma_cm_id *id, struct ib_pd *pd,
> struct ib_qp_init_attr *qp_init_attr)
> {
> struct rdma_id_private *id_priv;
> struct ib_qp *qp;
> - int ret = 0;
> + int ret;
>
> id_priv = container_of(id, struct rdma_id_private, id);
> if (id->device != pd->device) {
> @@ -948,6 +961,8 @@ int rdma_create_qp(struct rdma_cm_id *id, struct ib_pd *pd,
>
> if (id->qp_type == IB_QPT_UD)
> ret = cma_init_ud_qp(id_priv, qp);
> + else
> + ret = cma_init_conn_qp(id_priv, qp);
> if (ret)
> goto out_destroy;
>
> --
> 1.8.3.1
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-08-09 12:20 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-29 18:26 [PATCH for-rc] RDMA/cma: Revert INIT-INIT patch mike.marciniszyn
2021-08-03 15:57 ` Jason Gunthorpe
2021-08-09 12:20 ` Haakon Bugge
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).