* [PATCH rdma-next 1/6] RDMA/cm: Delete unused cm_is_active_peer function
2019-10-20 7:15 [PATCH rdma-next 0/6] CM cleanup Leon Romanovsky
@ 2019-10-20 7:15 ` Leon Romanovsky
2019-10-20 7:15 ` [PATCH rdma-next 2/6] RDMA/cm: Use specific keyword to check define Leon Romanovsky
` (5 subsequent siblings)
6 siblings, 0 replies; 20+ messages in thread
From: Leon Romanovsky @ 2019-10-20 7:15 UTC (permalink / raw)
To: Doug Ledford, Jason Gunthorpe
Cc: Leon Romanovsky, RDMA mailing list, Bart Van Assche
From: Leon Romanovsky <leonro@mellanox.com>
Function cm_is_active_peer is not used, delete it.
Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
---
drivers/infiniband/core/cm.c | 8 --------
1 file changed, 8 deletions(-)
diff --git a/drivers/infiniband/core/cm.c b/drivers/infiniband/core/cm.c
index c0aa3a4b4cfd..bc4abb05dbd4 100644
--- a/drivers/infiniband/core/cm.c
+++ b/drivers/infiniband/core/cm.c
@@ -1525,14 +1525,6 @@ static int cm_issue_rej(struct cm_port *port,
return ret;
}
-static inline int cm_is_active_peer(__be64 local_ca_guid, __be64 remote_ca_guid,
- __be32 local_qpn, __be32 remote_qpn)
-{
- return (be64_to_cpu(local_ca_guid) > be64_to_cpu(remote_ca_guid) ||
- ((local_ca_guid == remote_ca_guid) &&
- (be32_to_cpu(local_qpn) > be32_to_cpu(remote_qpn))));
-}
-
static bool cm_req_has_alt_path(struct cm_req_msg *req_msg)
{
return ((req_msg->alt_local_lid) ||
--
2.20.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH rdma-next 2/6] RDMA/cm: Use specific keyword to check define
2019-10-20 7:15 [PATCH rdma-next 0/6] CM cleanup Leon Romanovsky
2019-10-20 7:15 ` [PATCH rdma-next 1/6] RDMA/cm: Delete unused cm_is_active_peer function Leon Romanovsky
@ 2019-10-20 7:15 ` Leon Romanovsky
2019-10-20 7:15 ` [PATCH rdma-next 3/6] RDMA/cm: Update copyright together with SPDX tag Leon Romanovsky
` (4 subsequent siblings)
6 siblings, 0 replies; 20+ messages in thread
From: Leon Romanovsky @ 2019-10-20 7:15 UTC (permalink / raw)
To: Doug Ledford, Jason Gunthorpe
Cc: Leon Romanovsky, RDMA mailing list, Bart Van Assche
From: Leon Romanovsky <leonro@mellanox.com>
There is a specific define keyword to check if define exists or not,
let's use it instead of open-coded variant.
Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
---
drivers/infiniband/core/cm_msgs.h | 2 +-
include/rdma/ib_cm.h | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/infiniband/core/cm_msgs.h b/drivers/infiniband/core/cm_msgs.h
index 3d16d614aff6..0a9e2d3fb9df 100644
--- a/drivers/infiniband/core/cm_msgs.h
+++ b/drivers/infiniband/core/cm_msgs.h
@@ -31,7 +31,7 @@
* CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS THE
* SOFTWARE.
*/
-#if !defined(CM_MSGS_H)
+#ifndef CM_MSGS_H
#define CM_MSGS_H
#include <rdma/ib_mad.h>
diff --git a/include/rdma/ib_cm.h b/include/rdma/ib_cm.h
index 49f4f75499b3..5dc4ec986527 100644
--- a/include/rdma/ib_cm.h
+++ b/include/rdma/ib_cm.h
@@ -32,7 +32,7 @@
* CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
* SOFTWARE.
*/
-#if !defined(IB_CM_H)
+#ifndef IB_CM_H
#define IB_CM_H
#include <rdma/ib_mad.h>
--
2.20.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH rdma-next 3/6] RDMA/cm: Update copyright together with SPDX tag
2019-10-20 7:15 [PATCH rdma-next 0/6] CM cleanup Leon Romanovsky
2019-10-20 7:15 ` [PATCH rdma-next 1/6] RDMA/cm: Delete unused cm_is_active_peer function Leon Romanovsky
2019-10-20 7:15 ` [PATCH rdma-next 2/6] RDMA/cm: Use specific keyword to check define Leon Romanovsky
@ 2019-10-20 7:15 ` Leon Romanovsky
2019-10-20 7:15 ` [PATCH rdma-next 4/6] RDMA/cm: Delete useless QPN masking Leon Romanovsky
` (3 subsequent siblings)
6 siblings, 0 replies; 20+ messages in thread
From: Leon Romanovsky @ 2019-10-20 7:15 UTC (permalink / raw)
To: Doug Ledford, Jason Gunthorpe
Cc: Leon Romanovsky, RDMA mailing list, Bart Van Assche
From: Leon Romanovsky <leonro@mellanox.com>
Add Mellanox to lust of copyright holders and replace copyright
boilerplate with relevant SPDX tag.
Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
---
drivers/infiniband/core/cm.c | 30 ++----------------------------
drivers/infiniband/core/cm_msgs.h | 30 ++----------------------------
drivers/infiniband/core/cma.c | 31 ++-----------------------------
include/rdma/ib_cm.h | 30 ++----------------------------
4 files changed, 8 insertions(+), 113 deletions(-)
diff --git a/drivers/infiniband/core/cm.c b/drivers/infiniband/core/cm.c
index bc4abb05dbd4..7ffa16ea5fe3 100644
--- a/drivers/infiniband/core/cm.c
+++ b/drivers/infiniband/core/cm.c
@@ -1,36 +1,10 @@
+// SPDX-License-Identifier: GPL-2.0 OR Linux-OpenIB
/*
* Copyright (c) 2004-2007 Intel Corporation. All rights reserved.
* Copyright (c) 2004 Topspin Corporation. All rights reserved.
* Copyright (c) 2004, 2005 Voltaire Corporation. All rights reserved.
* Copyright (c) 2005 Sun Microsystems, Inc. All rights reserved.
- *
- * This software is available to you under a choice of one of two
- * licenses. You may choose to be licensed under the terms of the GNU
- * General Public License (GPL) Version 2, available from the file
- * COPYING in the main directory of this source tree, or the
- * OpenIB.org BSD license below:
- *
- * Redistribution and use in source and binary forms, with or
- * without modification, are permitted provided that the following
- * conditions are met:
- *
- * - Redistributions of source code must retain the above
- * copyright notice, this list of conditions and the following
- * disclaimer.
- *
- * - Redistributions in binary form must reproduce the above
- * copyright notice, this list of conditions and the following
- * disclaimer in the documentation and/or other materials
- * provided with the distribution.
- *
- * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
- * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
- * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
- * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
- * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
- * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
- * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
- * SOFTWARE.
+ * Copyright (c) 2019, Mellanox Technologies inc. All rights reserved.
*/
#include <linux/completion.h>
diff --git a/drivers/infiniband/core/cm_msgs.h b/drivers/infiniband/core/cm_msgs.h
index 0a9e2d3fb9df..92d7260ac913 100644
--- a/drivers/infiniband/core/cm_msgs.h
+++ b/drivers/infiniband/core/cm_msgs.h
@@ -1,35 +1,9 @@
+/* SPDX-License-Identifier: GPL-2.0 OR Linux-OpenIB */
/*
* Copyright (c) 2004, 2011 Intel Corporation. All rights reserved.
* Copyright (c) 2004 Topspin Corporation. All rights reserved.
* Copyright (c) 2004 Voltaire Corporation. All rights reserved.
- *
- * This software is available to you under a choice of one of two
- * licenses. You may choose to be licensed under the terms of the GNU
- * General Public License (GPL) Version 2, available from the file
- * COPYING the madirectory of this source tree, or the
- * OpenIB.org BSD license below:
- *
- * Redistribution and use source and binary forms, with or
- * withmodification, are permitted provided that the following
- * conditions are met:
- *
- * - Redistributions of source code must retathe above
- * copyright notice, this list of conditions and the following
- * disclaimer.
- *
- * - Redistributions binary form must reproduce the above
- * copyright notice, this list of conditions and the following
- * disclaimer the documentation and/or other materials
- * provided with the distribution.
- *
- * THE SOFTWARE IS PROVIDED "AS IS", WITHWARRANTY OF ANY KIND,
- * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
- * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
- * NONINFRINGEMENT. NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
- * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER AN
- * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OF OR IN
- * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS THE
- * SOFTWARE.
+ * Copyright (c) 2019, Mellanox Technologies inc. All rights reserved.
*/
#ifndef CM_MSGS_H
#define CM_MSGS_H
diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
index c8566a423719..8f318928f29d 100644
--- a/drivers/infiniband/core/cma.c
+++ b/drivers/infiniband/core/cma.c
@@ -1,36 +1,9 @@
+// SPDX-License-Identifier: GPL-2.0 OR Linux-OpenIB
/*
* Copyright (c) 2005 Voltaire Inc. All rights reserved.
* Copyright (c) 2002-2005, Network Appliance, Inc. All rights reserved.
- * Copyright (c) 1999-2005, Mellanox Technologies, Inc. All rights reserved.
+ * Copyright (c) 1999-2019, Mellanox Technologies, Inc. All rights reserved.
* Copyright (c) 2005-2006 Intel Corporation. All rights reserved.
- *
- * This software is available to you under a choice of one of two
- * licenses. You may choose to be licensed under the terms of the GNU
- * General Public License (GPL) Version 2, available from the file
- * COPYING in the main directory of this source tree, or the
- * OpenIB.org BSD license below:
- *
- * Redistribution and use in source and binary forms, with or
- * without modification, are permitted provided that the following
- * conditions are met:
- *
- * - Redistributions of source code must retain the above
- * copyright notice, this list of conditions and the following
- * disclaimer.
- *
- * - Redistributions in binary form must reproduce the above
- * copyright notice, this list of conditions and the following
- * disclaimer in the documentation and/or other materials
- * provided with the distribution.
- *
- * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
- * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
- * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
- * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
- * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
- * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
- * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
- * SOFTWARE.
*/
#include <linux/completion.h>
diff --git a/include/rdma/ib_cm.h b/include/rdma/ib_cm.h
index 5dc4ec986527..b01a8a8d4de9 100644
--- a/include/rdma/ib_cm.h
+++ b/include/rdma/ib_cm.h
@@ -1,36 +1,10 @@
+/* SPDX-License-Identifier: GPL-2.0 OR Linux-OpenIB */
/*
* Copyright (c) 2004, 2005 Intel Corporation. All rights reserved.
* Copyright (c) 2004 Topspin Corporation. All rights reserved.
* Copyright (c) 2004 Voltaire Corporation. All rights reserved.
* Copyright (c) 2005 Sun Microsystems, Inc. All rights reserved.
- *
- * This software is available to you under a choice of one of two
- * licenses. You may choose to be licensed under the terms of the GNU
- * General Public License (GPL) Version 2, available from the file
- * COPYING in the main directory of this source tree, or the
- * OpenIB.org BSD license below:
- *
- * Redistribution and use in source and binary forms, with or
- * without modification, are permitted provided that the following
- * conditions are met:
- *
- * - Redistributions of source code must retain the above
- * copyright notice, this list of conditions and the following
- * disclaimer.
- *
- * - Redistributions in binary form must reproduce the above
- * copyright notice, this list of conditions and the following
- * disclaimer in the documentation and/or other materials
- * provided with the distribution.
- *
- * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
- * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
- * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
- * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
- * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
- * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
- * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
- * SOFTWARE.
+ * Copyright (c) 2019, Mellanox Technologies inc. All rights reserved.
*/
#ifndef IB_CM_H
#define IB_CM_H
--
2.20.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH rdma-next 4/6] RDMA/cm: Delete useless QPN masking
2019-10-20 7:15 [PATCH rdma-next 0/6] CM cleanup Leon Romanovsky
` (2 preceding siblings ...)
2019-10-20 7:15 ` [PATCH rdma-next 3/6] RDMA/cm: Update copyright together with SPDX tag Leon Romanovsky
@ 2019-10-20 7:15 ` Leon Romanovsky
2019-10-20 8:48 ` Or Gerlitz
2019-10-28 12:52 ` Jason Gunthorpe
2019-10-20 7:15 ` [PATCH rdma-next 5/6] RDMA/cm: Provide private data size to CM users Leon Romanovsky
` (2 subsequent siblings)
6 siblings, 2 replies; 20+ messages in thread
From: Leon Romanovsky @ 2019-10-20 7:15 UTC (permalink / raw)
To: Doug Ledford, Jason Gunthorpe
Cc: Leon Romanovsky, RDMA mailing list, Bart Van Assche
From: Leon Romanovsky <leonro@mellanox.com>
QPN is supplied by kernel users who controls and creates valid QPs,
such flow ensures that QPN is limited to 24bits and no need to mask
already valid QPN.
Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
---
drivers/infiniband/core/cm.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/infiniband/core/cm.c b/drivers/infiniband/core/cm.c
index 7ffa16ea5fe3..2eb8e1fab962 100644
--- a/drivers/infiniband/core/cm.c
+++ b/drivers/infiniband/core/cm.c
@@ -2101,7 +2101,7 @@ int ib_send_cm_rep(struct ib_cm_id *cm_id,
cm_id_priv->initiator_depth = param->initiator_depth;
cm_id_priv->responder_resources = param->responder_resources;
cm_id_priv->rq_psn = cm_rep_get_starting_psn(rep_msg);
- cm_id_priv->local_qpn = cpu_to_be32(param->qp_num & 0xFFFFFF);
+ cm_id_priv->local_qpn = cpu_to_be32(param->qp_num);
out: spin_unlock_irqrestore(&cm_id_priv->lock, flags);
return ret;
--
2.20.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH rdma-next 4/6] RDMA/cm: Delete useless QPN masking
2019-10-20 7:15 ` [PATCH rdma-next 4/6] RDMA/cm: Delete useless QPN masking Leon Romanovsky
@ 2019-10-20 8:48 ` Or Gerlitz
2019-10-20 8:54 ` Leon Romanovsky
2019-10-28 12:52 ` Jason Gunthorpe
1 sibling, 1 reply; 20+ messages in thread
From: Or Gerlitz @ 2019-10-20 8:48 UTC (permalink / raw)
To: Leon Romanovsky
Cc: Doug Ledford, Jason Gunthorpe, Leon Romanovsky,
RDMA mailing list, Bart Van Assche
On Sun, Oct 20, 2019 at 10:18 AM Leon Romanovsky <leon@kernel.org> wrote:
>
> From: Leon Romanovsky <leonro@mellanox.com>
>
> QPN is supplied by kernel users who controls and creates valid QPs,
AFAIK this can also arrive from user-space, agree?
> such flow ensures that QPN is limited to 24bits and no need to mask
> already valid QPN.
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH rdma-next 4/6] RDMA/cm: Delete useless QPN masking
2019-10-20 8:48 ` Or Gerlitz
@ 2019-10-20 8:54 ` Leon Romanovsky
0 siblings, 0 replies; 20+ messages in thread
From: Leon Romanovsky @ 2019-10-20 8:54 UTC (permalink / raw)
To: Or Gerlitz
Cc: Doug Ledford, Jason Gunthorpe, RDMA mailing list, Bart Van Assche
On Sun, Oct 20, 2019 at 11:48:39AM +0300, Or Gerlitz wrote:
> On Sun, Oct 20, 2019 at 10:18 AM Leon Romanovsky <leon@kernel.org> wrote:
> >
> > From: Leon Romanovsky <leonro@mellanox.com>
> >
> > QPN is supplied by kernel users who controls and creates valid QPs,
>
> AFAIK this can also arrive from user-space, agree?
Not in this path, anyway we are masking it again while writing
to the spec struct,
>
> > such flow ensures that QPN is limited to 24bits and no need to mask
> > already valid QPN.
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH rdma-next 4/6] RDMA/cm: Delete useless QPN masking
2019-10-20 7:15 ` [PATCH rdma-next 4/6] RDMA/cm: Delete useless QPN masking Leon Romanovsky
2019-10-20 8:48 ` Or Gerlitz
@ 2019-10-28 12:52 ` Jason Gunthorpe
2019-10-28 13:13 ` Leon Romanovsky
1 sibling, 1 reply; 20+ messages in thread
From: Jason Gunthorpe @ 2019-10-28 12:52 UTC (permalink / raw)
To: Leon Romanovsky
Cc: Doug Ledford, Leon Romanovsky, RDMA mailing list, Bart Van Assche
On Sun, Oct 20, 2019 at 10:15:57AM +0300, Leon Romanovsky wrote:
> From: Leon Romanovsky <leonro@mellanox.com>
>
> QPN is supplied by kernel users who controls and creates valid QPs,
> such flow ensures that QPN is limited to 24bits and no need to mask
> already valid QPN.
>
> Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
> ---
> drivers/infiniband/core/cm.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/infiniband/core/cm.c b/drivers/infiniband/core/cm.c
> index 7ffa16ea5fe3..2eb8e1fab962 100644
> --- a/drivers/infiniband/core/cm.c
> +++ b/drivers/infiniband/core/cm.c
> @@ -2101,7 +2101,7 @@ int ib_send_cm_rep(struct ib_cm_id *cm_id,
> cm_id_priv->initiator_depth = param->initiator_depth;
> cm_id_priv->responder_resources = param->responder_resources;
> cm_id_priv->rq_psn = cm_rep_get_starting_psn(rep_msg);
> - cm_id_priv->local_qpn = cpu_to_be32(param->qp_num & 0xFFFFFF);
> + cm_id_priv->local_qpn = cpu_to_be32(param->qp_num);
It does seem like this value comes from userspace:
ucma_connect()
ucma_copy_conn_param()
dst->qp_num = src->qp_num
rdma_connect(.., &dst)
if (!id->qp) {
id_priv->qp_num = conn_param->qp_num;
vs
cma_accept_ib()
rep.qp_num = id_priv->qp_num;
Maybe this needs to add some masking to ucma_copy_conn_param()?
Jason
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH rdma-next 4/6] RDMA/cm: Delete useless QPN masking
2019-10-28 12:52 ` Jason Gunthorpe
@ 2019-10-28 13:13 ` Leon Romanovsky
2019-10-28 13:44 ` Jason Gunthorpe
0 siblings, 1 reply; 20+ messages in thread
From: Leon Romanovsky @ 2019-10-28 13:13 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: Doug Ledford, RDMA mailing list, Bart Van Assche, Or Gerlitz
On Mon, Oct 28, 2019 at 09:52:33AM -0300, Jason Gunthorpe wrote:
> On Sun, Oct 20, 2019 at 10:15:57AM +0300, Leon Romanovsky wrote:
> > From: Leon Romanovsky <leonro@mellanox.com>
> >
> > QPN is supplied by kernel users who controls and creates valid QPs,
> > such flow ensures that QPN is limited to 24bits and no need to mask
> > already valid QPN.
> >
> > Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
> > ---
> > drivers/infiniband/core/cm.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/infiniband/core/cm.c b/drivers/infiniband/core/cm.c
> > index 7ffa16ea5fe3..2eb8e1fab962 100644
> > --- a/drivers/infiniband/core/cm.c
> > +++ b/drivers/infiniband/core/cm.c
> > @@ -2101,7 +2101,7 @@ int ib_send_cm_rep(struct ib_cm_id *cm_id,
> > cm_id_priv->initiator_depth = param->initiator_depth;
> > cm_id_priv->responder_resources = param->responder_resources;
> > cm_id_priv->rq_psn = cm_rep_get_starting_psn(rep_msg);
> > - cm_id_priv->local_qpn = cpu_to_be32(param->qp_num & 0xFFFFFF);
> > + cm_id_priv->local_qpn = cpu_to_be32(param->qp_num);
>
> It does seem like this value comes from userspace:
>
> ucma_connect()
> ucma_copy_conn_param()
> dst->qp_num = src->qp_num
> rdma_connect(.., &dst)
> if (!id->qp) {
> id_priv->qp_num = conn_param->qp_num;
>
> vs
>
> cma_accept_ib()
> rep.qp_num = id_priv->qp_num;
>
> Maybe this needs to add some masking to ucma_copy_conn_param()?
Thanks for the callstack, Or pointed it to me too, but I missed this flow.
Let's create a pre-patch with QPN masking.
Thanks
>
> Jason
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH rdma-next 4/6] RDMA/cm: Delete useless QPN masking
2019-10-28 13:13 ` Leon Romanovsky
@ 2019-10-28 13:44 ` Jason Gunthorpe
2019-10-28 14:02 ` Leon Romanovsky
0 siblings, 1 reply; 20+ messages in thread
From: Jason Gunthorpe @ 2019-10-28 13:44 UTC (permalink / raw)
To: Leon Romanovsky
Cc: Doug Ledford, RDMA mailing list, Bart Van Assche, Or Gerlitz
On Mon, Oct 28, 2019 at 03:13:33PM +0200, Leon Romanovsky wrote:
> On Mon, Oct 28, 2019 at 09:52:33AM -0300, Jason Gunthorpe wrote:
> > On Sun, Oct 20, 2019 at 10:15:57AM +0300, Leon Romanovsky wrote:
> > > From: Leon Romanovsky <leonro@mellanox.com>
> > >
> > > QPN is supplied by kernel users who controls and creates valid QPs,
> > > such flow ensures that QPN is limited to 24bits and no need to mask
> > > already valid QPN.
> > >
> > > Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
> > > drivers/infiniband/core/cm.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/infiniband/core/cm.c b/drivers/infiniband/core/cm.c
> > > index 7ffa16ea5fe3..2eb8e1fab962 100644
> > > +++ b/drivers/infiniband/core/cm.c
> > > @@ -2101,7 +2101,7 @@ int ib_send_cm_rep(struct ib_cm_id *cm_id,
> > > cm_id_priv->initiator_depth = param->initiator_depth;
> > > cm_id_priv->responder_resources = param->responder_resources;
> > > cm_id_priv->rq_psn = cm_rep_get_starting_psn(rep_msg);
> > > - cm_id_priv->local_qpn = cpu_to_be32(param->qp_num & 0xFFFFFF);
> > > + cm_id_priv->local_qpn = cpu_to_be32(param->qp_num);
> >
> > It does seem like this value comes from userspace:
> >
> > ucma_connect()
> > ucma_copy_conn_param()
> > dst->qp_num = src->qp_num
> > rdma_connect(.., &dst)
> > if (!id->qp) {
> > id_priv->qp_num = conn_param->qp_num;
> >
> > vs
> >
> > cma_accept_ib()
> > rep.qp_num = id_priv->qp_num;
> >
> > Maybe this needs to add some masking to ucma_copy_conn_param()?
>
> Thanks for the callstack, Or pointed it to me too, but I missed this flow.
> Let's create a pre-patch with QPN masking.
You'll need to check all the id_priv->qp_num users, I stopped when I
found the above
Jason
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH rdma-next 4/6] RDMA/cm: Delete useless QPN masking
2019-10-28 13:44 ` Jason Gunthorpe
@ 2019-10-28 14:02 ` Leon Romanovsky
0 siblings, 0 replies; 20+ messages in thread
From: Leon Romanovsky @ 2019-10-28 14:02 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: Doug Ledford, RDMA mailing list, Bart Van Assche, Or Gerlitz
On Mon, Oct 28, 2019 at 10:44:57AM -0300, Jason Gunthorpe wrote:
> On Mon, Oct 28, 2019 at 03:13:33PM +0200, Leon Romanovsky wrote:
> > On Mon, Oct 28, 2019 at 09:52:33AM -0300, Jason Gunthorpe wrote:
> > > On Sun, Oct 20, 2019 at 10:15:57AM +0300, Leon Romanovsky wrote:
> > > > From: Leon Romanovsky <leonro@mellanox.com>
> > > >
> > > > QPN is supplied by kernel users who controls and creates valid QPs,
> > > > such flow ensures that QPN is limited to 24bits and no need to mask
> > > > already valid QPN.
> > > >
> > > > Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
> > > > drivers/infiniband/core/cm.c | 2 +-
> > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/infiniband/core/cm.c b/drivers/infiniband/core/cm.c
> > > > index 7ffa16ea5fe3..2eb8e1fab962 100644
> > > > +++ b/drivers/infiniband/core/cm.c
> > > > @@ -2101,7 +2101,7 @@ int ib_send_cm_rep(struct ib_cm_id *cm_id,
> > > > cm_id_priv->initiator_depth = param->initiator_depth;
> > > > cm_id_priv->responder_resources = param->responder_resources;
> > > > cm_id_priv->rq_psn = cm_rep_get_starting_psn(rep_msg);
> > > > - cm_id_priv->local_qpn = cpu_to_be32(param->qp_num & 0xFFFFFF);
> > > > + cm_id_priv->local_qpn = cpu_to_be32(param->qp_num);
> > >
> > > It does seem like this value comes from userspace:
> > >
> > > ucma_connect()
> > > ucma_copy_conn_param()
> > > dst->qp_num = src->qp_num
> > > rdma_connect(.., &dst)
> > > if (!id->qp) {
> > > id_priv->qp_num = conn_param->qp_num;
> > >
> > > vs
> > >
> > > cma_accept_ib()
> > > rep.qp_num = id_priv->qp_num;
> > >
> > > Maybe this needs to add some masking to ucma_copy_conn_param()?
> >
> > Thanks for the callstack, Or pointed it to me too, but I missed this flow.
> > Let's create a pre-patch with QPN masking.
>
> You'll need to check all the id_priv->qp_num users, I stopped when I
> found the above
Initially, I wanted to add masking in rdma_connect(), but decided that
it is not the cleanest approach, so I grepped to see rdma_conn_param
users. I'll continue to grep.
Thanks
>
> Jason
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH rdma-next 5/6] RDMA/cm: Provide private data size to CM users
2019-10-20 7:15 [PATCH rdma-next 0/6] CM cleanup Leon Romanovsky
` (3 preceding siblings ...)
2019-10-20 7:15 ` [PATCH rdma-next 4/6] RDMA/cm: Delete useless QPN masking Leon Romanovsky
@ 2019-10-20 7:15 ` Leon Romanovsky
2019-10-20 7:15 ` [PATCH rdma-next 6/6] RDMA/srpt: Use private_data_len instead of hardcoded value Leon Romanovsky
2019-10-28 13:17 ` [PATCH rdma-next 0/6] CM cleanup Jason Gunthorpe
6 siblings, 0 replies; 20+ messages in thread
From: Leon Romanovsky @ 2019-10-20 7:15 UTC (permalink / raw)
To: Doug Ledford, Jason Gunthorpe
Cc: Leon Romanovsky, RDMA mailing list, Bart Van Assche
From: Leon Romanovsky <leonro@mellanox.com>
Prepare code to removal IB_CM_*_PRIVATE_DATA_SIZE enum so we will store
such size in adjacent to actual data.
Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
---
drivers/infiniband/core/cm.c | 11 +++++++++++
include/rdma/ib_cm.h | 1 +
2 files changed, 12 insertions(+)
diff --git a/drivers/infiniband/core/cm.c b/drivers/infiniband/core/cm.c
index 2eb8e1fab962..ecd868954958 100644
--- a/drivers/infiniband/core/cm.c
+++ b/drivers/infiniband/core/cm.c
@@ -1681,6 +1681,7 @@ static void cm_format_req_event(struct cm_work *work,
param->srq = cm_req_get_srq(req_msg);
param->ppath_sgid_attr = cm_id_priv->av.ah_attr.grh.sgid_attr;
work->cm_event.private_data = &req_msg->private_data;
+ work->cm_event.private_data_len = IB_CM_REQ_PRIVATE_DATA_SIZE;
}
static void cm_process_work(struct cm_id_private *cm_id_priv,
@@ -2193,6 +2194,7 @@ static void cm_format_rep_event(struct cm_work *work, enum ib_qp_type qp_type)
param->rnr_retry_count = cm_rep_get_rnr_retry_count(rep_msg);
param->srq = cm_rep_get_srq(rep_msg);
work->cm_event.private_data = &rep_msg->private_data;
+ work->cm_event.private_data_len = IB_CM_REP_PRIVATE_DATA_SIZE;
}
static void cm_dup_rep_handler(struct cm_work *work)
@@ -2395,6 +2397,7 @@ static int cm_rtu_handler(struct cm_work *work)
return -EINVAL;
work->cm_event.private_data = &rtu_msg->private_data;
+ work->cm_event.private_data_len = IB_CM_RTU_PRIVATE_DATA_SIZE;
spin_lock_irq(&cm_id_priv->lock);
if (cm_id_priv->id.state != IB_CM_REP_SENT &&
@@ -2597,6 +2600,7 @@ static int cm_dreq_handler(struct cm_work *work)
}
work->cm_event.private_data = &dreq_msg->private_data;
+ work->cm_event.private_data_len = IB_CM_DREQ_PRIVATE_DATA_SIZE;
spin_lock_irq(&cm_id_priv->lock);
if (cm_id_priv->local_qpn != cm_dreq_get_remote_qpn(dreq_msg))
@@ -2671,6 +2675,7 @@ static int cm_drep_handler(struct cm_work *work)
return -EINVAL;
work->cm_event.private_data = &drep_msg->private_data;
+ work->cm_event.private_data_len = IB_CM_DREP_PRIVATE_DATA_SIZE;
spin_lock_irq(&cm_id_priv->lock);
if (cm_id_priv->id.state != IB_CM_DREQ_SENT &&
@@ -2770,6 +2775,7 @@ static void cm_format_rej_event(struct cm_work *work)
param->ari_length = cm_rej_get_reject_info_len(rej_msg);
param->reason = __be16_to_cpu(rej_msg->reason);
work->cm_event.private_data = &rej_msg->private_data;
+ work->cm_event.private_data_len = IB_CM_REJ_PRIVATE_DATA_SIZE;
}
static struct cm_id_private * cm_acquire_rejected_id(struct cm_rej_msg *rej_msg)
@@ -2982,6 +2988,7 @@ static int cm_mra_handler(struct cm_work *work)
return -EINVAL;
work->cm_event.private_data = &mra_msg->private_data;
+ work->cm_event.private_data_len = IB_CM_MRA_PRIVATE_DATA_SIZE;
work->cm_event.param.mra_rcvd.service_timeout =
cm_mra_get_service_timeout(mra_msg);
timeout = cm_convert_to_ms(cm_mra_get_service_timeout(mra_msg)) +
@@ -3214,6 +3221,7 @@ static int cm_lap_handler(struct cm_work *work)
param->alternate_path = &work->path[0];
cm_format_path_from_lap(cm_id_priv, param->alternate_path, lap_msg);
work->cm_event.private_data = &lap_msg->private_data;
+ work->cm_event.private_data_len = IB_CM_LAP_PRIVATE_DATA_SIZE;
spin_lock_irq(&cm_id_priv->lock);
if (cm_id_priv->id.state != IB_CM_ESTABLISHED)
@@ -3367,6 +3375,7 @@ static int cm_apr_handler(struct cm_work *work)
work->cm_event.param.apr_rcvd.apr_info = &apr_msg->info;
work->cm_event.param.apr_rcvd.info_len = apr_msg->info_length;
work->cm_event.private_data = &apr_msg->private_data;
+ work->cm_event.private_data_len = IB_CM_APR_PRIVATE_DATA_SIZE;
spin_lock_irq(&cm_id_priv->lock);
if (cm_id_priv->id.state != IB_CM_ESTABLISHED ||
@@ -3515,6 +3524,7 @@ static void cm_format_sidr_req_event(struct cm_work *work,
param->port = work->port->port_num;
param->sgid_attr = rx_cm_id->av.ah_attr.grh.sgid_attr;
work->cm_event.private_data = &sidr_req_msg->private_data;
+ work->cm_event.private_data_len = IB_CM_SIDR_REQ_PRIVATE_DATA_SIZE;
}
static int cm_sidr_req_handler(struct cm_work *work)
@@ -3664,6 +3674,7 @@ static void cm_format_sidr_rep_event(struct cm_work *work,
param->info_len = sidr_rep_msg->info_length;
param->sgid_attr = cm_id_priv->av.ah_attr.grh.sgid_attr;
work->cm_event.private_data = &sidr_rep_msg->private_data;
+ work->cm_event.private_data_len = IB_CM_SIDR_REP_PRIVATE_DATA_SIZE;
}
static int cm_sidr_rep_handler(struct cm_work *work)
diff --git a/include/rdma/ib_cm.h b/include/rdma/ib_cm.h
index b01a8a8d4de9..b476e0e27ec9 100644
--- a/include/rdma/ib_cm.h
+++ b/include/rdma/ib_cm.h
@@ -254,6 +254,7 @@ struct ib_cm_event {
} param;
void *private_data;
+ u8 private_data_len;
};
#define CM_REQ_ATTR_ID cpu_to_be16(0x0010)
--
2.20.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH rdma-next 6/6] RDMA/srpt: Use private_data_len instead of hardcoded value
2019-10-20 7:15 [PATCH rdma-next 0/6] CM cleanup Leon Romanovsky
` (4 preceding siblings ...)
2019-10-20 7:15 ` [PATCH rdma-next 5/6] RDMA/cm: Provide private data size to CM users Leon Romanovsky
@ 2019-10-20 7:15 ` Leon Romanovsky
2019-10-24 17:01 ` Bart Van Assche
2019-10-28 13:13 ` Jason Gunthorpe
2019-10-28 13:17 ` [PATCH rdma-next 0/6] CM cleanup Jason Gunthorpe
6 siblings, 2 replies; 20+ messages in thread
From: Leon Romanovsky @ 2019-10-20 7:15 UTC (permalink / raw)
To: Doug Ledford, Jason Gunthorpe
Cc: Leon Romanovsky, RDMA mailing list, Bart Van Assche
From: Leon Romanovsky <leonro@mellanox.com>
Reuse recently introduced private_data_len to get IBTA REJ message
private data size.
Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
---
drivers/infiniband/ulp/srpt/ib_srpt.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/infiniband/ulp/srpt/ib_srpt.c b/drivers/infiniband/ulp/srpt/ib_srpt.c
index daf811abf40a..e66366de11e9 100644
--- a/drivers/infiniband/ulp/srpt/ib_srpt.c
+++ b/drivers/infiniband/ulp/srpt/ib_srpt.c
@@ -2609,7 +2609,7 @@ static int srpt_cm_handler(struct ib_cm_id *cm_id,
case IB_CM_REJ_RECEIVED:
srpt_cm_rej_recv(ch, event->param.rej_rcvd.reason,
event->private_data,
- IB_CM_REJ_PRIVATE_DATA_SIZE);
+ event->private_data_len);
break;
case IB_CM_RTU_RECEIVED:
case IB_CM_USER_ESTABLISHED:
--
2.20.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH rdma-next 6/6] RDMA/srpt: Use private_data_len instead of hardcoded value
2019-10-20 7:15 ` [PATCH rdma-next 6/6] RDMA/srpt: Use private_data_len instead of hardcoded value Leon Romanovsky
@ 2019-10-24 17:01 ` Bart Van Assche
2019-10-28 13:13 ` Jason Gunthorpe
1 sibling, 0 replies; 20+ messages in thread
From: Bart Van Assche @ 2019-10-24 17:01 UTC (permalink / raw)
To: Leon Romanovsky, Doug Ledford, Jason Gunthorpe
Cc: Leon Romanovsky, RDMA mailing list
On 10/20/19 12:15 AM, Leon Romanovsky wrote:
> From: Leon Romanovsky <leonro@mellanox.com>
>
> Reuse recently introduced private_data_len to get IBTA REJ message
> private data size.
>
> Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
> ---
> drivers/infiniband/ulp/srpt/ib_srpt.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/infiniband/ulp/srpt/ib_srpt.c b/drivers/infiniband/ulp/srpt/ib_srpt.c
> index daf811abf40a..e66366de11e9 100644
> --- a/drivers/infiniband/ulp/srpt/ib_srpt.c
> +++ b/drivers/infiniband/ulp/srpt/ib_srpt.c
> @@ -2609,7 +2609,7 @@ static int srpt_cm_handler(struct ib_cm_id *cm_id,
> case IB_CM_REJ_RECEIVED:
> srpt_cm_rej_recv(ch, event->param.rej_rcvd.reason,
> event->private_data,
> - IB_CM_REJ_PRIVATE_DATA_SIZE);
> + event->private_data_len);
> break;
> case IB_CM_RTU_RECEIVED:
> case IB_CM_USER_ESTABLISHED:
Reviewed-by: Bart Van Assche <bvanassche@acm.org>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH rdma-next 6/6] RDMA/srpt: Use private_data_len instead of hardcoded value
2019-10-20 7:15 ` [PATCH rdma-next 6/6] RDMA/srpt: Use private_data_len instead of hardcoded value Leon Romanovsky
2019-10-24 17:01 ` Bart Van Assche
@ 2019-10-28 13:13 ` Jason Gunthorpe
2019-10-28 13:19 ` Leon Romanovsky
1 sibling, 1 reply; 20+ messages in thread
From: Jason Gunthorpe @ 2019-10-28 13:13 UTC (permalink / raw)
To: Leon Romanovsky
Cc: Doug Ledford, Leon Romanovsky, RDMA mailing list, Bart Van Assche
On Sun, Oct 20, 2019 at 10:15:59AM +0300, Leon Romanovsky wrote:
> diff --git a/drivers/infiniband/ulp/srpt/ib_srpt.c b/drivers/infiniband/ulp/srpt/ib_srpt.c
> index daf811abf40a..e66366de11e9 100644
> --- a/drivers/infiniband/ulp/srpt/ib_srpt.c
> +++ b/drivers/infiniband/ulp/srpt/ib_srpt.c
> @@ -2609,7 +2609,7 @@ static int srpt_cm_handler(struct ib_cm_id *cm_id,
> case IB_CM_REJ_RECEIVED:
> srpt_cm_rej_recv(ch, event->param.rej_rcvd.reason,
> event->private_data,
> - IB_CM_REJ_PRIVATE_DATA_SIZE);
> + event->private_data_len);
So, I took a look and found a heck of a lot more places assuming the
size of private data that really should be checked if we are going to
introduce a buffer length here.
This is the first couple I noticed, but there were many many more and
they all should be handled..
--- a/drivers/infiniband/ulp/srp/ib_srp.c
+++ b/drivers/infiniband/ulp/srp/ib_srp.c
@@ -2677,9 +2677,14 @@ static void srp_ib_cm_rej_handler(struct ib_cm_id *cm_id,
break;
case IB_CM_REJ_CONSUMER_DEFINED:
+ if (event->private_data_len < sizeof(struct srp_login_rej)) {
+ ch->status = -ECONNRESET;
+ break;
+ }
+
--- a/drivers/infiniband/ulp/ipoib/ipoib_cm.c
+++ b/drivers/infiniband/ulp/ipoib/ipoib_cm.c
@@ -985,12 +985,15 @@ static int ipoib_cm_rep_handler(struct ib_cm_id *cm_id,
{
struct ipoib_cm_tx *p = cm_id->context;
struct ipoib_dev_priv *priv = ipoib_priv(p->dev);
struct ipoib_cm_data *data = event->private_data;
struct sk_buff_head skqueue;
struct ib_qp_attr qp_attr;
int qp_attr_mask, ret;
struct sk_buff *skb;
+ if (event->private_data_len < sizeof(*data))
+ return -EINVAL;
+
Thanks,
Jason
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH rdma-next 6/6] RDMA/srpt: Use private_data_len instead of hardcoded value
2019-10-28 13:13 ` Jason Gunthorpe
@ 2019-10-28 13:19 ` Leon Romanovsky
2019-10-28 13:43 ` Jason Gunthorpe
0 siblings, 1 reply; 20+ messages in thread
From: Leon Romanovsky @ 2019-10-28 13:19 UTC (permalink / raw)
To: Jason Gunthorpe; +Cc: Doug Ledford, RDMA mailing list, Bart Van Assche
On Mon, Oct 28, 2019 at 10:13:19AM -0300, Jason Gunthorpe wrote:
> On Sun, Oct 20, 2019 at 10:15:59AM +0300, Leon Romanovsky wrote:
> > diff --git a/drivers/infiniband/ulp/srpt/ib_srpt.c b/drivers/infiniband/ulp/srpt/ib_srpt.c
> > index daf811abf40a..e66366de11e9 100644
> > --- a/drivers/infiniband/ulp/srpt/ib_srpt.c
> > +++ b/drivers/infiniband/ulp/srpt/ib_srpt.c
> > @@ -2609,7 +2609,7 @@ static int srpt_cm_handler(struct ib_cm_id *cm_id,
> > case IB_CM_REJ_RECEIVED:
> > srpt_cm_rej_recv(ch, event->param.rej_rcvd.reason,
> > event->private_data,
> > - IB_CM_REJ_PRIVATE_DATA_SIZE);
> > + event->private_data_len);
>
> So, I took a look and found a heck of a lot more places assuming the
> size of private data that really should be checked if we are going to
> introduce a buffer length here.
But we are not interested to make it dynamic, "private_data_len" has
constant size according to IBTA spec, I just don't want users to be
aware of this.
Why should we add the below checks if it wasn't before?
>
> This is the first couple I noticed, but there were many many more and
> they all should be handled..
>
> --- a/drivers/infiniband/ulp/srp/ib_srp.c
> +++ b/drivers/infiniband/ulp/srp/ib_srp.c
> @@ -2677,9 +2677,14 @@ static void srp_ib_cm_rej_handler(struct ib_cm_id *cm_id,
> break;
>
> case IB_CM_REJ_CONSUMER_DEFINED:
> + if (event->private_data_len < sizeof(struct srp_login_rej)) {
> + ch->status = -ECONNRESET;
> + break;
> + }
> +
>
> --- a/drivers/infiniband/ulp/ipoib/ipoib_cm.c
> +++ b/drivers/infiniband/ulp/ipoib/ipoib_cm.c
> @@ -985,12 +985,15 @@ static int ipoib_cm_rep_handler(struct ib_cm_id *cm_id,
> {
> struct ipoib_cm_tx *p = cm_id->context;
> struct ipoib_dev_priv *priv = ipoib_priv(p->dev);
> struct ipoib_cm_data *data = event->private_data;
> struct sk_buff_head skqueue;
> struct ib_qp_attr qp_attr;
> int qp_attr_mask, ret;
> struct sk_buff *skb;
>
> + if (event->private_data_len < sizeof(*data))
> + return -EINVAL;
> +
>
>
> Thanks,
> Jason
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH rdma-next 6/6] RDMA/srpt: Use private_data_len instead of hardcoded value
2019-10-28 13:19 ` Leon Romanovsky
@ 2019-10-28 13:43 ` Jason Gunthorpe
2019-10-28 14:06 ` Leon Romanovsky
0 siblings, 1 reply; 20+ messages in thread
From: Jason Gunthorpe @ 2019-10-28 13:43 UTC (permalink / raw)
To: Leon Romanovsky; +Cc: Doug Ledford, RDMA mailing list, Bart Van Assche
On Mon, Oct 28, 2019 at 03:19:17PM +0200, Leon Romanovsky wrote:
> On Mon, Oct 28, 2019 at 10:13:19AM -0300, Jason Gunthorpe wrote:
> > On Sun, Oct 20, 2019 at 10:15:59AM +0300, Leon Romanovsky wrote:
> > > diff --git a/drivers/infiniband/ulp/srpt/ib_srpt.c b/drivers/infiniband/ulp/srpt/ib_srpt.c
> > > index daf811abf40a..e66366de11e9 100644
> > > +++ b/drivers/infiniband/ulp/srpt/ib_srpt.c
> > > @@ -2609,7 +2609,7 @@ static int srpt_cm_handler(struct ib_cm_id *cm_id,
> > > case IB_CM_REJ_RECEIVED:
> > > srpt_cm_rej_recv(ch, event->param.rej_rcvd.reason,
> > > event->private_data,
> > > - IB_CM_REJ_PRIVATE_DATA_SIZE);
> > > + event->private_data_len);
> >
> > So, I took a look and found a heck of a lot more places assuming the
> > size of private data that really should be checked if we are going to
> > introduce a buffer length here.
>
> But we are not interested to make it dynamic, "private_data_len" has
> constant size according to IBTA spec, I just don't want users to be
> aware of this.
>
> Why should we add the below checks if it wasn't before?
Why are we doing any of this then?
Jason
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH rdma-next 6/6] RDMA/srpt: Use private_data_len instead of hardcoded value
2019-10-28 13:43 ` Jason Gunthorpe
@ 2019-10-28 14:06 ` Leon Romanovsky
0 siblings, 0 replies; 20+ messages in thread
From: Leon Romanovsky @ 2019-10-28 14:06 UTC (permalink / raw)
To: Jason Gunthorpe; +Cc: Doug Ledford, RDMA mailing list, Bart Van Assche
On Mon, Oct 28, 2019 at 10:43:51AM -0300, Jason Gunthorpe wrote:
> On Mon, Oct 28, 2019 at 03:19:17PM +0200, Leon Romanovsky wrote:
> > On Mon, Oct 28, 2019 at 10:13:19AM -0300, Jason Gunthorpe wrote:
> > > On Sun, Oct 20, 2019 at 10:15:59AM +0300, Leon Romanovsky wrote:
> > > > diff --git a/drivers/infiniband/ulp/srpt/ib_srpt.c b/drivers/infiniband/ulp/srpt/ib_srpt.c
> > > > index daf811abf40a..e66366de11e9 100644
> > > > +++ b/drivers/infiniband/ulp/srpt/ib_srpt.c
> > > > @@ -2609,7 +2609,7 @@ static int srpt_cm_handler(struct ib_cm_id *cm_id,
> > > > case IB_CM_REJ_RECEIVED:
> > > > srpt_cm_rej_recv(ch, event->param.rej_rcvd.reason,
> > > > event->private_data,
> > > > - IB_CM_REJ_PRIVATE_DATA_SIZE);
> > > > + event->private_data_len);
> > >
> > > So, I took a look and found a heck of a lot more places assuming the
> > > size of private data that really should be checked if we are going to
> > > introduce a buffer length here.
> >
> > But we are not interested to make it dynamic, "private_data_len" has
> > constant size according to IBTA spec, I just don't want users to be
> > aware of this.
> >
> > Why should we add the below checks if it wasn't before?
>
> Why are we doing any of this then?
The final goal will be to present all CM messages as binary blobs with
access through specific GET/SET helpers, those including access to
private data. It is needed to completely eliminate be32 madness in
IB/core code.
Thanks
>
> Jason
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH rdma-next 0/6] CM cleanup
2019-10-20 7:15 [PATCH rdma-next 0/6] CM cleanup Leon Romanovsky
` (5 preceding siblings ...)
2019-10-20 7:15 ` [PATCH rdma-next 6/6] RDMA/srpt: Use private_data_len instead of hardcoded value Leon Romanovsky
@ 2019-10-28 13:17 ` Jason Gunthorpe
2019-10-28 13:20 ` Leon Romanovsky
6 siblings, 1 reply; 20+ messages in thread
From: Jason Gunthorpe @ 2019-10-28 13:17 UTC (permalink / raw)
To: Leon Romanovsky
Cc: Doug Ledford, Leon Romanovsky, RDMA mailing list, Bart Van Assche
On Sun, Oct 20, 2019 at 10:15:53AM +0300, Leon Romanovsky wrote:
> From: Leon Romanovsky <leonro@mellanox.com>
>
> Hi,
>
> This is first part of CM cleanup series needed to effectively
> add IBTA Enhanced Connection Establishment (ECE) spec changes.
>
> In this series, we don't do anything major, just small code cleanup
> with small change in srpt. This change will allow us to provide
> general getter and setter for all CM structures.
>
> Thanks
>
> Leon Romanovsky (6):
> RDMA/cm: Delete unused cm_is_active_peer function
> RDMA/cm: Use specific keyword to check define
> RDMA/cm: Update copyright together with SPDX tag
Applied to for-next
> RDMA/cm: Delete useless QPN masking
> RDMA/cm: Provide private data size to CM users
> RDMA/srpt: Use private_data_len instead of hardcoded value
These ones probably need a bit more work
Thanks,
Jason
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH rdma-next 0/6] CM cleanup
2019-10-28 13:17 ` [PATCH rdma-next 0/6] CM cleanup Jason Gunthorpe
@ 2019-10-28 13:20 ` Leon Romanovsky
0 siblings, 0 replies; 20+ messages in thread
From: Leon Romanovsky @ 2019-10-28 13:20 UTC (permalink / raw)
To: Jason Gunthorpe; +Cc: Doug Ledford, RDMA mailing list, Bart Van Assche
On Mon, Oct 28, 2019 at 10:17:25AM -0300, Jason Gunthorpe wrote:
> On Sun, Oct 20, 2019 at 10:15:53AM +0300, Leon Romanovsky wrote:
> > From: Leon Romanovsky <leonro@mellanox.com>
> >
> > Hi,
> >
> > This is first part of CM cleanup series needed to effectively
> > add IBTA Enhanced Connection Establishment (ECE) spec changes.
> >
> > In this series, we don't do anything major, just small code cleanup
> > with small change in srpt. This change will allow us to provide
> > general getter and setter for all CM structures.
> >
> > Thanks
> >
> > Leon Romanovsky (6):
> > RDMA/cm: Delete unused cm_is_active_peer function
> > RDMA/cm: Use specific keyword to check define
> > RDMA/cm: Update copyright together with SPDX tag
>
> Applied to for-next
Thanks
>
> > RDMA/cm: Delete useless QPN masking
> > RDMA/cm: Provide private data size to CM users
> > RDMA/srpt: Use private_data_len instead of hardcoded value
>
> These ones probably need a bit more work
I disagree about that last patch, we don't want to make private_data_len dynamic.
Thanks
>
> Thanks,
> Jason
^ permalink raw reply [flat|nested] 20+ messages in thread