[PATCH rdma-next v2 0/7] Add Enhanced Connection Established (ECE)

[PATCH rdma-next v2 0/7] Add Enhanced Connection Established (ECE)

[Leon Romanovsky]

From: Leon Romanovsky <leonro@mellanox.com>

Changelog:
 v2:
 * Rebased on latest rdma-next and removed already accepted patches.
 * Updated all rdma_reject in-kernel users to provide reject reason.
 v1: Dropped field_avail patch in favor of mass conversion to use function
     which already exists in the kernel code.
 https://lore.kernel.org/lkml/20200310091438.248429-1-leon@kernel.org
 v0: https://lore.kernel.org/lkml/20200305150105.207959-1-leon@kernel.org

Enhanced Connection Established or ECE is new negotiation scheme
introduced in IBTA v1.4 to exchange extra information about nodes
capabilities and later negotiate them at the connection establishment
phase.

The RDMA-CM messages (REQ, REP, SIDR_REQ and SIDR_REP) were extended
to carry two fields, one new and another gained new functionality:
 * VendorID is a new field that indicates that common subset of vendor
   option bits are supported as indicated by that VendorID.
 * AttributeModifier already exists, but overloaded to indicate which
   vendor options are supported by this VendorID.

This is kernel part of such functionality which is responsible to get data
from librdmacm and properly create and handle RDMA-CM messages.

Thanks

Leon Romanovsky (7):
  RDMA/cm: Add Enhanced Connection Establishment (ECE) bits
  RDMA/uapi: Add ECE definitions to UCMA
  RDMA/ucma: Extend ucma_connect to receive ECE parameters
  RDMA/ucma: Deliver ECE parameters through UCMA events
  RDMA/cm: Send and receive ECE parameter over the wire
  RDMA/cma: Connect ECE to rdma_accept
  RDMA/cma: Provide ECE reject reason

 drivers/infiniband/core/cm.c            | 41 ++++++++++++++++---
 drivers/infiniband/core/cma.c           | 52 ++++++++++++++++++++++---
 drivers/infiniband/core/cma_priv.h      |  1 +
 drivers/infiniband/core/ucma.c          | 40 +++++++++++++++----
 drivers/infiniband/ulp/isert/ib_isert.c |  4 +-
 drivers/infiniband/ulp/srpt/ib_srpt.c   |  2 +-
 drivers/nvme/target/rdma.c              |  2 +-
 include/rdma/ib_cm.h                    | 10 ++++-
 include/rdma/ibta_vol1_c12.h            |  6 +++
 include/rdma/rdma_cm.h                  | 18 ++++++++-
 include/uapi/rdma/rdma_user_cm.h        | 15 ++++++-
 net/rds/ib_cm.c                         |  2 +-
 12 files changed, 167 insertions(+), 26 deletions(-)

--
2.25.2

[PATCH rdma-next v2 1/7] RDMA/cm: Add Enhanced Connection Establishment (ECE) bits

[Leon Romanovsky]

From: Leon Romanovsky <leonro@mellanox.com>

Extend REQ (request for communications), REP (reply to request
for communication), rejected reason and SIDR_REP (service ID
resolution response) structures with hardware vendor ID bits
according to IBTA v1.4.

Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
---
 include/rdma/ibta_vol1_c12.h | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/include/rdma/ibta_vol1_c12.h b/include/rdma/ibta_vol1_c12.h
index 269904425d3f..960c86bec76c 100644
--- a/include/rdma/ibta_vol1_c12.h
+++ b/include/rdma/ibta_vol1_c12.h
@@ -38,6 +38,7 @@
 
 /* Table 106 REQ Message Contents */
 #define CM_REQ_LOCAL_COMM_ID CM_FIELD32_LOC(struct cm_req_msg, 0, 32)
+#define CM_REQ_VENDOR_ID CM_FIELD32_LOC(struct cm_req_msg, 5, 24)
 #define CM_REQ_SERVICE_ID CM_FIELD64_LOC(struct cm_req_msg, 8)
 #define CM_REQ_LOCAL_CA_GUID CM_FIELD64_LOC(struct cm_req_msg, 16)
 #define CM_REQ_LOCAL_Q_KEY CM_FIELD32_LOC(struct cm_req_msg, 28, 32)
@@ -119,8 +120,11 @@ CM_STRUCT(struct cm_rej_msg, 84 * 8 + 1184);
 #define CM_REP_REMOTE_COMM_ID CM_FIELD32_LOC(struct cm_rep_msg, 4, 32)
 #define CM_REP_LOCAL_Q_KEY CM_FIELD32_LOC(struct cm_rep_msg, 8, 32)
 #define CM_REP_LOCAL_QPN CM_FIELD32_LOC(struct cm_rep_msg, 12, 24)
+#define CM_REP_VENDOR_ID_H CM_FIELD8_LOC(struct cm_rep_msg, 15, 8)
 #define CM_REP_LOCAL_EE_CONTEXT_NUMBER CM_FIELD32_LOC(struct cm_rep_msg, 16, 24)
+#define CM_REP_VENDOR_ID_M CM_FIELD8_LOC(struct cm_rep_msg, 19, 8)
 #define CM_REP_STARTING_PSN CM_FIELD32_LOC(struct cm_rep_msg, 20, 24)
+#define CM_REP_VENDOR_ID_L CM_FIELD8_LOC(struct cm_rep_msg, 23, 8)
 #define CM_REP_RESPONDER_RESOURCES CM_FIELD8_LOC(struct cm_rep_msg, 24, 8)
 #define CM_REP_INITIATOR_DEPTH CM_FIELD8_LOC(struct cm_rep_msg, 25, 8)
 #define CM_REP_TARGET_ACK_DELAY CM_FIELD8_LOC(struct cm_rep_msg, 26, 5)
@@ -201,7 +205,9 @@ CM_STRUCT(struct cm_sidr_req_msg, 16 * 8 + 1728);
 #define CM_SIDR_REP_STATUS CM_FIELD8_LOC(struct cm_sidr_rep_msg, 4, 8)
 #define CM_SIDR_REP_ADDITIONAL_INFORMATION_LENGTH                              \
 	CM_FIELD8_LOC(struct cm_sidr_rep_msg, 5, 8)
+#define CM_SIDR_REP_VENDOR_ID_H CM_FIELD16_LOC(struct cm_sidr_rep_msg, 6, 16)
 #define CM_SIDR_REP_QPN CM_FIELD32_LOC(struct cm_sidr_rep_msg, 8, 24)
+#define CM_SIDR_REP_VENDOR_ID_L CM_FIELD8_LOC(struct cm_sidr_rep_msg, 11, 8)
 #define CM_SIDR_REP_SERVICEID CM_FIELD64_LOC(struct cm_sidr_rep_msg, 12)
 #define CM_SIDR_REP_Q_KEY CM_FIELD32_LOC(struct cm_sidr_rep_msg, 20, 32)
 #define CM_SIDR_REP_ADDITIONAL_INFORMATION                                     \
-- 
2.25.2

[PATCH rdma-next v2 2/7] RDMA/uapi: Add ECE definitions to UCMA

[Leon Romanovsky]

From: Leon Romanovsky <leonro@mellanox.com>

ECE parameters are used to perform handshake between different
CMID nodes in order to allow extra connection setup supported
by those two nodes. The data is provided by rdma_connect()
for the client and rdma_get_events() for the server.

Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
---
 include/uapi/rdma/rdma_user_cm.h | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/include/uapi/rdma/rdma_user_cm.h b/include/uapi/rdma/rdma_user_cm.h
index e42940a215a3..150b3f075f99 100644
--- a/include/uapi/rdma/rdma_user_cm.h
+++ b/include/uapi/rdma/rdma_user_cm.h
@@ -206,10 +206,16 @@ struct rdma_ucm_ud_param {
 	__u8  reserved[7];
 };
 
+struct rdma_ucm_ece {
+	__u32 vendor_id;
+	__u32 attr_mod;
+};
+
 struct rdma_ucm_connect {
 	struct rdma_ucm_conn_param conn_param;
 	__u32 id;
 	__u32 reserved;
+	struct rdma_ucm_ece ece;
 };
 
 struct rdma_ucm_listen {
@@ -287,6 +293,7 @@ struct rdma_ucm_event_resp {
 		struct rdma_ucm_ud_param   ud;
 	} param;
 	__u32 reserved;
+	struct rdma_ucm_ece ece;
 };
 
 /* Option levels */
-- 
2.25.2

[PATCH rdma-next v2 3/7] RDMA/ucma: Extend ucma_connect to receive ECE parameters

[Leon Romanovsky]

From: Leon Romanovsky <leonro@mellanox.com>

Active side of CMID initiates connection through librdmacm's rdma_connect()
and kernel's ucma_connect(). Extend UCMA interface to handle those new
parameters.

Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
---
 drivers/infiniband/core/cma.c      | 13 +++++++++++++
 drivers/infiniband/core/cma_priv.h |  1 +
 drivers/infiniband/core/ucma.c     | 14 +++++++++++---
 include/rdma/rdma_cm.h             | 11 +++++++++++
 4 files changed, 36 insertions(+), 3 deletions(-)

diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
index 22df46933d60..6ded5d3ecc1d 100644
--- a/drivers/infiniband/core/cma.c
+++ b/drivers/infiniband/core/cma.c
@@ -4031,6 +4031,19 @@ int rdma_connect(struct rdma_cm_id *id, struct rdma_conn_param *conn_param)
 }
 EXPORT_SYMBOL(rdma_connect);
 
+int rdma_connect_ece(struct rdma_cm_id *id, struct rdma_conn_param *conn_param,
+		     struct rdma_ucm_ece *ece)
+{
+	struct rdma_id_private *id_priv =
+		container_of(id, struct rdma_id_private, id);
+
+	id_priv->ece.vendor_id = ece->vendor_id;
+	id_priv->ece.attr_mod = ece->attr_mod;
+
+	return rdma_connect(id, conn_param);
+}
+EXPORT_SYMBOL(rdma_connect_ece);
+
 static int cma_accept_ib(struct rdma_id_private *id_priv,
 			 struct rdma_conn_param *conn_param)
 {
diff --git a/drivers/infiniband/core/cma_priv.h b/drivers/infiniband/core/cma_priv.h
index 5edcf44a9307..caece96ebcf5 100644
--- a/drivers/infiniband/core/cma_priv.h
+++ b/drivers/infiniband/core/cma_priv.h
@@ -95,6 +95,7 @@ struct rdma_id_private {
 	 * Internal to RDMA/core, don't use in the drivers
 	 */
 	struct rdma_restrack_entry     res;
+	struct rdma_ucm_ece ece;
 };
 
 #if IS_ENABLED(CONFIG_INFINIBAND_ADDR_TRANS_CONFIGFS)
diff --git a/drivers/infiniband/core/ucma.c b/drivers/infiniband/core/ucma.c
index 16b6cf57fa85..f7415d8d2140 100644
--- a/drivers/infiniband/core/ucma.c
+++ b/drivers/infiniband/core/ucma.c
@@ -1070,12 +1070,15 @@ static void ucma_copy_conn_param(struct rdma_cm_id *id,
 static ssize_t ucma_connect(struct ucma_file *file, const char __user *inbuf,
 			    int in_len, int out_len)
 {
-	struct rdma_ucm_connect cmd;
 	struct rdma_conn_param conn_param;
+	struct rdma_ucm_ece ece = {};
+	struct rdma_ucm_connect cmd;
 	struct ucma_context *ctx;
+	size_t in_size;
 	int ret;
 
-	if (copy_from_user(&cmd, inbuf, sizeof(cmd)))
+	in_size = min_t(size_t, in_len, sizeof(cmd));
+	if (copy_from_user(&cmd, inbuf, in_size))
 		return -EFAULT;
 
 	if (!cmd.conn_param.valid)
@@ -1086,8 +1089,13 @@ static ssize_t ucma_connect(struct ucma_file *file, const char __user *inbuf,
 		return PTR_ERR(ctx);
 
 	ucma_copy_conn_param(ctx->cm_id, &conn_param, &cmd.conn_param);
+	if (offsetofend(typeof(cmd), ece) <= in_size) {
+		ece.vendor_id = cmd.ece.vendor_id;
+		ece.attr_mod = cmd.ece.attr_mod;
+	}
+
 	mutex_lock(&ctx->mutex);
-	ret = rdma_connect(ctx->cm_id, &conn_param);
+	ret = rdma_connect_ece(ctx->cm_id, &conn_param, &ece);
 	mutex_unlock(&ctx->mutex);
 	ucma_put_ctx(ctx);
 	return ret;
diff --git a/include/rdma/rdma_cm.h b/include/rdma/rdma_cm.h
index 71f48cfdc24c..86a849214c84 100644
--- a/include/rdma/rdma_cm.h
+++ b/include/rdma/rdma_cm.h
@@ -264,6 +264,17 @@ int rdma_init_qp_attr(struct rdma_cm_id *id, struct ib_qp_attr *qp_attr,
  */
 int rdma_connect(struct rdma_cm_id *id, struct rdma_conn_param *conn_param);
 
+/**
+ * rdma_connect_ece - Initiate an active connection request with ECE data.
+ * @id: Connection identifier to connect.
+ * @conn_param: Connection information used for connected QPs.
+ * @ece: ECE parameters
+ *
+ * See rdma_connect() explanation.
+ */
+int rdma_connect_ece(struct rdma_cm_id *id, struct rdma_conn_param *conn_param,
+		     struct rdma_ucm_ece *ece);
+
 /**
  * rdma_listen - This function is called by the passive side to
  *   listen for incoming connection requests.
-- 
2.25.2

[PATCH rdma-next v2 4/7] RDMA/ucma: Deliver ECE parameters through UCMA events

[Leon Romanovsky]

From: Leon Romanovsky <leonro@mellanox.com>

Passive side of CMID connection receives ECE request through
REQ message and needs to respond with relevant REP message which
will be forwarded to active side.

The UCMA events interface is responsible for such communication
with the user space (librdmacm). Extend it to provide ECE wire
data.

Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
---
 drivers/infiniband/core/ucma.c | 6 +++++-
 include/rdma/rdma_cm.h         | 1 +
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/infiniband/core/ucma.c b/drivers/infiniband/core/ucma.c
index f7415d8d2140..ed2c17046ee1 100644
--- a/drivers/infiniband/core/ucma.c
+++ b/drivers/infiniband/core/ucma.c
@@ -360,6 +360,9 @@ static int ucma_event_handler(struct rdma_cm_id *cm_id,
 		ucma_copy_conn_event(&uevent->resp.param.conn,
 				     &event->param.conn);
 
+	uevent->resp.ece.vendor_id = event->ece.vendor_id;
+	uevent->resp.ece.attr_mod = event->ece.attr_mod;
+
 	if (event->event == RDMA_CM_EVENT_CONNECT_REQUEST) {
 		if (!ctx->backlog) {
 			ret = -ENOMEM;
@@ -404,7 +407,8 @@ static ssize_t ucma_get_event(struct ucma_file *file, const char __user *inbuf,
 	 * Old 32 bit user space does not send the 4 byte padding in the
 	 * reserved field. We don't care, allow it to keep working.
 	 */
-	if (out_len < sizeof(uevent->resp) - sizeof(uevent->resp.reserved))
+	if (out_len < sizeof(uevent->resp) - sizeof(uevent->resp.reserved) -
+			      sizeof(uevent->resp.ece))
 		return -ENOSPC;
 
 	if (copy_from_user(&cmd, inbuf, sizeof(cmd)))
diff --git a/include/rdma/rdma_cm.h b/include/rdma/rdma_cm.h
index 86a849214c84..761168c41848 100644
--- a/include/rdma/rdma_cm.h
+++ b/include/rdma/rdma_cm.h
@@ -111,6 +111,7 @@ struct rdma_cm_event {
 		struct rdma_conn_param	conn;
 		struct rdma_ud_param	ud;
 	} param;
+	struct rdma_ucm_ece ece;
 };
 
 struct rdma_cm_id;
-- 
2.25.2

[PATCH rdma-next v2 5/7] RDMA/cm: Send and receive ECE parameter over the wire

[Leon Romanovsky]

From: Leon Romanovsky <leonro@mellanox.com>

ECE parameters are exchanged through REQ->REP/SIDR_REP messages,
this patch adds the data to provide to other side of CMID communication
channel.

Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
---
 drivers/infiniband/core/cm.c   | 41 +++++++++++++++++++++++++++++-----
 drivers/infiniband/core/cma.c  |  8 +++++++
 drivers/infiniband/core/ucma.c |  1 -
 include/rdma/ib_cm.h           |  9 +++++++-
 4 files changed, 52 insertions(+), 7 deletions(-)

diff --git a/drivers/infiniband/core/cm.c b/drivers/infiniband/core/cm.c
index 68e1a9bba027..e77e01b03622 100644
--- a/drivers/infiniband/core/cm.c
+++ b/drivers/infiniband/core/cm.c
@@ -66,6 +66,8 @@ static const char * const ibcm_rej_reason_strs[] = {
 	[IB_CM_REJ_INVALID_CLASS_VERSION]	= "invalid class version",
 	[IB_CM_REJ_INVALID_FLOW_LABEL]		= "invalid flow label",
 	[IB_CM_REJ_INVALID_ALT_FLOW_LABEL]	= "invalid alt flow label",
+	[IB_CM_REJ_VENDOR_OPTION_NOT_SUPPORTED] =
+		"vendor option is not supported",
 };
 
 const char *__attribute_const__ ibcm_reject_msg(int reason)
@@ -287,6 +289,8 @@ struct cm_id_private {
 
 	struct list_head work_list;
 	atomic_t work_count;
+
+	struct rdma_ucm_ece ece;
 };
 
 static void cm_work_handler(struct work_struct *work);
@@ -1287,6 +1291,13 @@ static void cm_format_mad_hdr(struct ib_mad_hdr *hdr,
 	hdr->tid	   = tid;
 }
 
+static void cm_format_mad_ece_hdr(struct ib_mad_hdr *hdr, __be16 attr_id,
+				  __be64 tid, u32 attr_mod)
+{
+	cm_format_mad_hdr(hdr, attr_id, tid);
+	hdr->attr_mod = cpu_to_be32(attr_mod);
+}
+
 static void cm_format_req(struct cm_req_msg *req_msg,
 			  struct cm_id_private *cm_id_priv,
 			  struct ib_cm_req_param *param)
@@ -1299,8 +1310,8 @@ static void cm_format_req(struct cm_req_msg *req_msg,
 		pri_ext = opa_is_extended_lid(pri_path->opa.dlid,
 					      pri_path->opa.slid);
 
-	cm_format_mad_hdr(&req_msg->hdr, CM_REQ_ATTR_ID,
-			  cm_form_tid(cm_id_priv));
+	cm_format_mad_ece_hdr(&req_msg->hdr, CM_REQ_ATTR_ID,
+			      cm_form_tid(cm_id_priv), param->ece.attr_mod);
 
 	IBA_SET(CM_REQ_LOCAL_COMM_ID, req_msg,
 		be32_to_cpu(cm_id_priv->id.local_id));
@@ -1423,6 +1434,7 @@ static void cm_format_req(struct cm_req_msg *req_msg,
 			cm_ack_timeout(cm_id_priv->av.port->cm_dev->ack_delay,
 				       alt_path->packet_life_time));
 	}
+	IBA_SET(CM_REQ_VENDOR_ID, req_msg, param->ece.vendor_id);
 
 	if (param->private_data && param->private_data_len)
 		IBA_SET_MEM(CM_REQ_PRIVATE_DATA, req_msg, param->private_data,
@@ -1779,6 +1791,9 @@ static void cm_format_req_event(struct cm_work *work,
 	param->rnr_retry_count = IBA_GET(CM_REQ_RNR_RETRY_COUNT, req_msg);
 	param->srq = IBA_GET(CM_REQ_SRQ, req_msg);
 	param->ppath_sgid_attr = cm_id_priv->av.ah_attr.grh.sgid_attr;
+	param->ece.vendor_id = IBA_GET(CM_REQ_VENDOR_ID, req_msg);
+	param->ece.attr_mod = be32_to_cpu(req_msg->hdr.attr_mod);
+
 	work->cm_event.private_data =
 		IBA_GET_MEM_PTR(CM_REQ_PRIVATE_DATA, req_msg);
 }
@@ -2177,7 +2192,8 @@ static void cm_format_rep(struct cm_rep_msg *rep_msg,
 			  struct cm_id_private *cm_id_priv,
 			  struct ib_cm_rep_param *param)
 {
-	cm_format_mad_hdr(&rep_msg->hdr, CM_REP_ATTR_ID, cm_id_priv->tid);
+	cm_format_mad_ece_hdr(&rep_msg->hdr, CM_REP_ATTR_ID, cm_id_priv->tid,
+			      param->ece.attr_mod);
 	IBA_SET(CM_REP_LOCAL_COMM_ID, rep_msg,
 		be32_to_cpu(cm_id_priv->id.local_id));
 	IBA_SET(CM_REP_REMOTE_COMM_ID, rep_msg,
@@ -2204,6 +2220,12 @@ static void cm_format_rep(struct cm_rep_msg *rep_msg,
 		IBA_SET(CM_REP_LOCAL_EE_CONTEXT_NUMBER, rep_msg, param->qp_num);
 	}
 
+	IBA_SET(CM_REP_VENDOR_ID_L, rep_msg, param->ece.vendor_id & 0xFF);
+	IBA_SET(CM_REP_VENDOR_ID_M, rep_msg,
+		(param->ece.vendor_id >> 8) & 0xFF);
+	IBA_SET(CM_REP_VENDOR_ID_H, rep_msg,
+		(param->ece.vendor_id >> 16) & 0xFF);
+
 	if (param->private_data && param->private_data_len)
 		IBA_SET_MEM(CM_REP_PRIVATE_DATA, rep_msg, param->private_data,
 			    param->private_data_len);
@@ -2351,6 +2373,11 @@ static void cm_format_rep_event(struct cm_work *work, enum ib_qp_type qp_type)
 	param->flow_control = IBA_GET(CM_REP_END_TO_END_FLOW_CONTROL, rep_msg);
 	param->rnr_retry_count = IBA_GET(CM_REP_RNR_RETRY_COUNT, rep_msg);
 	param->srq = IBA_GET(CM_REP_SRQ, rep_msg);
+	param->ece.vendor_id = IBA_GET(CM_REP_VENDOR_ID_H, rep_msg) << 16;
+	param->ece.vendor_id |= IBA_GET(CM_REP_VENDOR_ID_M, rep_msg) << 8;
+	param->ece.vendor_id |= IBA_GET(CM_REP_VENDOR_ID_L, rep_msg);
+	param->ece.attr_mod = be32_to_cpu(rep_msg->hdr.attr_mod);
+
 	work->cm_event.private_data =
 		IBA_GET_MEM_PTR(CM_REP_PRIVATE_DATA, rep_msg);
 }
@@ -3672,8 +3699,8 @@ static void cm_format_sidr_rep(struct cm_sidr_rep_msg *sidr_rep_msg,
 			       struct cm_id_private *cm_id_priv,
 			       struct ib_cm_sidr_rep_param *param)
 {
-	cm_format_mad_hdr(&sidr_rep_msg->hdr, CM_SIDR_REP_ATTR_ID,
-			  cm_id_priv->tid);
+	cm_format_mad_ece_hdr(&sidr_rep_msg->hdr, CM_SIDR_REP_ATTR_ID,
+			      cm_id_priv->tid, param->ece.attr_mod);
 	IBA_SET(CM_SIDR_REP_REQUESTID, sidr_rep_msg,
 		be32_to_cpu(cm_id_priv->id.remote_id));
 	IBA_SET(CM_SIDR_REP_STATUS, sidr_rep_msg, param->status);
@@ -3681,6 +3708,10 @@ static void cm_format_sidr_rep(struct cm_sidr_rep_msg *sidr_rep_msg,
 	IBA_SET(CM_SIDR_REP_SERVICEID, sidr_rep_msg,
 		be64_to_cpu(cm_id_priv->id.service_id));
 	IBA_SET(CM_SIDR_REP_Q_KEY, sidr_rep_msg, param->qkey);
+	IBA_SET(CM_SIDR_REP_VENDOR_ID_L, sidr_rep_msg,
+		param->ece.vendor_id & 0xFF);
+	IBA_SET(CM_SIDR_REP_VENDOR_ID_H, sidr_rep_msg,
+		(param->ece.vendor_id >> 8) & 0xFF);
 
 	if (param->info && param->info_length)
 		IBA_SET_MEM(CM_SIDR_REP_ADDITIONAL_INFORMATION, sidr_rep_msg,
diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
index 6ded5d3ecc1d..e33e47caab08 100644
--- a/drivers/infiniband/core/cma.c
+++ b/drivers/infiniband/core/cma.c
@@ -1906,6 +1906,9 @@ static void cma_set_rep_event_data(struct rdma_cm_event *event,
 	event->param.conn.rnr_retry_count = rep_data->rnr_retry_count;
 	event->param.conn.srq = rep_data->srq;
 	event->param.conn.qp_num = rep_data->remote_qpn;
+
+	event->ece.vendor_id = rep_data->ece.vendor_id;
+	event->ece.attr_mod = rep_data->ece.attr_mod;
 }
 
 static int cma_cm_event_handler(struct rdma_id_private *id_priv,
@@ -2124,6 +2127,9 @@ static void cma_set_req_event_data(struct rdma_cm_event *event,
 	event->param.conn.rnr_retry_count = req_data->rnr_retry_count;
 	event->param.conn.srq = req_data->srq;
 	event->param.conn.qp_num = req_data->remote_qpn;
+
+	event->ece.vendor_id = req_data->ece.vendor_id;
+	event->ece.attr_mod = req_data->ece.attr_mod;
 }
 
 static int cma_ib_check_req_qp_type(const struct rdma_cm_id *id,
@@ -3942,6 +3948,8 @@ static int cma_connect_ib(struct rdma_id_private *id_priv,
 	req.local_cm_response_timeout = CMA_CM_RESPONSE_TIMEOUT;
 	req.max_cm_retries = CMA_MAX_CM_RETRIES;
 	req.srq = id_priv->srq ? 1 : 0;
+	req.ece.vendor_id = id_priv->ece.vendor_id;
+	req.ece.attr_mod = id_priv->ece.attr_mod;
 
 	trace_cm_send_req(id_priv);
 	ret = ib_send_cm_req(id_priv->cm_id.ib, &req);
diff --git a/drivers/infiniband/core/ucma.c b/drivers/infiniband/core/ucma.c
index ed2c17046ee1..b67cdd2ef187 100644
--- a/drivers/infiniband/core/ucma.c
+++ b/drivers/infiniband/core/ucma.c
@@ -362,7 +362,6 @@ static int ucma_event_handler(struct rdma_cm_id *cm_id,
 
 	uevent->resp.ece.vendor_id = event->ece.vendor_id;
 	uevent->resp.ece.attr_mod = event->ece.attr_mod;
-
 	if (event->event == RDMA_CM_EVENT_CONNECT_REQUEST) {
 		if (!ctx->backlog) {
 			ret = -ENOMEM;
diff --git a/include/rdma/ib_cm.h b/include/rdma/ib_cm.h
index 058cfbc2b37f..0f1ea5f2d01c 100644
--- a/include/rdma/ib_cm.h
+++ b/include/rdma/ib_cm.h
@@ -11,6 +11,7 @@
 
 #include <rdma/ib_mad.h>
 #include <rdma/ib_sa.h>
+#include <rdma/rdma_cm.h>
 
 /* ib_cm and ib_user_cm modules share /sys/class/infiniband_cm */
 extern struct class cm_class;
@@ -115,6 +116,7 @@ struct ib_cm_req_event_param {
 	unsigned int		retry_count:3;
 	unsigned int		rnr_retry_count:3;
 	unsigned int		srq:1;
+	struct rdma_ucm_ece	ece;
 };
 
 struct ib_cm_rep_event_param {
@@ -129,6 +131,7 @@ struct ib_cm_rep_event_param {
 	unsigned int		flow_control:1;
 	unsigned int		rnr_retry_count:3;
 	unsigned int		srq:1;
+	struct rdma_ucm_ece	ece;
 };
 
 enum ib_cm_rej_reason {
@@ -164,7 +167,8 @@ enum ib_cm_rej_reason {
 	IB_CM_REJ_DUPLICATE_LOCAL_COMM_ID	= 30,
 	IB_CM_REJ_INVALID_CLASS_VERSION		= 31,
 	IB_CM_REJ_INVALID_FLOW_LABEL		= 32,
-	IB_CM_REJ_INVALID_ALT_FLOW_LABEL	= 33
+	IB_CM_REJ_INVALID_ALT_FLOW_LABEL	= 33,
+	IB_CM_REJ_VENDOR_OPTION_NOT_SUPPORTED	= 35,
 };
 
 struct ib_cm_rej_event_param {
@@ -369,6 +373,7 @@ struct ib_cm_req_param {
 	u8			rnr_retry_count;
 	u8			max_cm_retries;
 	u8			srq;
+	struct rdma_ucm_ece	ece;
 };
 
 /**
@@ -392,6 +397,7 @@ struct ib_cm_rep_param {
 	u8		flow_control;
 	u8		rnr_retry_count;
 	u8		srq;
+	struct rdma_ucm_ece ece;
 };
 
 /**
@@ -546,6 +552,7 @@ struct ib_cm_sidr_rep_param {
 	u8			info_length;
 	const void		*private_data;
 	u8			private_data_len;
+	struct rdma_ucm_ece	ece;
 };
 
 /**
-- 
2.25.2

[PATCH rdma-next v2 6/7] RDMA/cma: Connect ECE to rdma_accept

[Leon Romanovsky]

From: Leon Romanovsky <leonro@mellanox.com>

The rdma_accept() is called by both passive and active
sides of CMID connection to mark readiness to start data
transfer. For passive side, this is called explicitly,
for active side, it is called implicitly while receiving
REP message.

Provide ECE data to rdma_accept function needed for passive
side to send that REP message.

Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
---
 drivers/infiniband/core/cma.c    | 19 +++++++++++++++++++
 drivers/infiniband/core/ucma.c   | 14 +++++++++++---
 include/rdma/rdma_cm.h           |  3 +++
 include/uapi/rdma/rdma_user_cm.h |  1 +
 4 files changed, 34 insertions(+), 3 deletions(-)

diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
index e33e47caab08..409f752f40ae 100644
--- a/drivers/infiniband/core/cma.c
+++ b/drivers/infiniband/core/cma.c
@@ -4077,6 +4077,8 @@ static int cma_accept_ib(struct rdma_id_private *id_priv,
 	rep.flow_control = conn_param->flow_control;
 	rep.rnr_retry_count = min_t(u8, 7, conn_param->rnr_retry_count);
 	rep.srq = id_priv->srq ? 1 : 0;
+	rep.ece.vendor_id = id_priv->ece.vendor_id;
+	rep.ece.attr_mod = id_priv->ece.attr_mod;
 
 	trace_cm_send_rep(id_priv);
 	ret = ib_send_cm_rep(id_priv->cm_id.ib, &rep);
@@ -4124,7 +4126,11 @@ static int cma_send_sidr_rep(struct rdma_id_private *id_priv,
 			return ret;
 		rep.qp_num = id_priv->qp_num;
 		rep.qkey = id_priv->qkey;
+
+		rep.ece.vendor_id = id_priv->ece.vendor_id;
+		rep.ece.attr_mod = id_priv->ece.attr_mod;
 	}
+
 	rep.private_data = private_data;
 	rep.private_data_len = private_data_len;
 
@@ -4182,6 +4188,19 @@ int __rdma_accept(struct rdma_cm_id *id, struct rdma_conn_param *conn_param,
 }
 EXPORT_SYMBOL(__rdma_accept);
 
+int __rdma_accept_ece(struct rdma_cm_id *id, struct rdma_conn_param *conn_param,
+		      const char *caller, struct rdma_ucm_ece *ece)
+{
+	struct rdma_id_private *id_priv =
+		container_of(id, struct rdma_id_private, id);
+
+	id_priv->ece.vendor_id = ece->vendor_id;
+	id_priv->ece.attr_mod = ece->attr_mod;
+
+	return __rdma_accept(id, conn_param, caller);
+}
+EXPORT_SYMBOL(__rdma_accept_ece);
+
 int rdma_notify(struct rdma_cm_id *id, enum ib_event_type event)
 {
 	struct rdma_id_private *id_priv;
diff --git a/drivers/infiniband/core/ucma.c b/drivers/infiniband/core/ucma.c
index b67cdd2ef187..d41598954cc4 100644
--- a/drivers/infiniband/core/ucma.c
+++ b/drivers/infiniband/core/ucma.c
@@ -1132,28 +1132,36 @@ static ssize_t ucma_accept(struct ucma_file *file, const char __user *inbuf,
 {
 	struct rdma_ucm_accept cmd;
 	struct rdma_conn_param conn_param;
+	struct rdma_ucm_ece ece = {};
 	struct ucma_context *ctx;
+	size_t in_size;
 	int ret;
 
-	if (copy_from_user(&cmd, inbuf, sizeof(cmd)))
+	in_size = min_t(size_t, in_len, sizeof(cmd));
+	if (copy_from_user(&cmd, inbuf, in_size))
 		return -EFAULT;
 
 	ctx = ucma_get_ctx_dev(file, cmd.id);
 	if (IS_ERR(ctx))
 		return PTR_ERR(ctx);
 
+	if (offsetofend(typeof(cmd), ece) <= in_size) {
+		ece.vendor_id = cmd.ece.vendor_id;
+		ece.attr_mod = cmd.ece.attr_mod;
+	}
+
 	if (cmd.conn_param.valid) {
 		ucma_copy_conn_param(ctx->cm_id, &conn_param, &cmd.conn_param);
 		mutex_lock(&file->mut);
 		mutex_lock(&ctx->mutex);
-		ret = __rdma_accept(ctx->cm_id, &conn_param, NULL);
+		ret = __rdma_accept_ece(ctx->cm_id, &conn_param, NULL, &ece);
 		mutex_unlock(&ctx->mutex);
 		if (!ret)
 			ctx->uid = cmd.uid;
 		mutex_unlock(&file->mut);
 	} else {
 		mutex_lock(&ctx->mutex);
-		ret = __rdma_accept(ctx->cm_id, NULL, NULL);
+		ret = __rdma_accept_ece(ctx->cm_id, NULL, NULL, &ece);
 		mutex_unlock(&ctx->mutex);
 	}
 	ucma_put_ctx(ctx);
diff --git a/include/rdma/rdma_cm.h b/include/rdma/rdma_cm.h
index 761168c41848..8d961d8b7cdb 100644
--- a/include/rdma/rdma_cm.h
+++ b/include/rdma/rdma_cm.h
@@ -288,6 +288,9 @@ int rdma_listen(struct rdma_cm_id *id, int backlog);
 int __rdma_accept(struct rdma_cm_id *id, struct rdma_conn_param *conn_param,
 		  const char *caller);
 
+int __rdma_accept_ece(struct rdma_cm_id *id, struct rdma_conn_param *conn_param,
+		      const char *caller, struct rdma_ucm_ece *ece);
+
 /**
  * rdma_accept - Called to accept a connection request or response.
  * @id: Connection identifier associated with the request.
diff --git a/include/uapi/rdma/rdma_user_cm.h b/include/uapi/rdma/rdma_user_cm.h
index 150b3f075f99..c4ca1412bcf9 100644
--- a/include/uapi/rdma/rdma_user_cm.h
+++ b/include/uapi/rdma/rdma_user_cm.h
@@ -228,6 +228,7 @@ struct rdma_ucm_accept {
 	struct rdma_ucm_conn_param conn_param;
 	__u32 id;
 	__u32 reserved;
+	struct rdma_ucm_ece ece;
 };
 
 struct rdma_ucm_reject {
-- 
2.25.2

[PATCH rdma-next v2 7/7] RDMA/cma: Provide ECE reject reason

[Leon Romanovsky]

From: Leon Romanovsky <leonro@mellanox.com>

IBTA declares "vendor option not supported" reject reason in REJ
messages if passive side doesn't want to accept proposed ECE options.

Due to the fact that ECE is managed by userspace, there is a need to let
users to provide such rejected reason.

Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
---
 drivers/infiniband/core/cma.c           | 12 +++++++-----
 drivers/infiniband/core/ucma.c          |  7 ++++++-
 drivers/infiniband/ulp/isert/ib_isert.c |  4 ++--
 drivers/infiniband/ulp/srpt/ib_srpt.c   |  2 +-
 drivers/nvme/target/rdma.c              |  2 +-
 include/rdma/ib_cm.h                    |  3 ++-
 include/rdma/rdma_cm.h                  |  3 ++-
 include/uapi/rdma/rdma_user_cm.h        |  7 ++++++-
 net/rds/ib_cm.c                         |  2 +-
 9 files changed, 28 insertions(+), 14 deletions(-)

diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
index 409f752f40ae..b79f71b10593 100644
--- a/drivers/infiniband/core/cma.c
+++ b/drivers/infiniband/core/cma.c
@@ -4183,7 +4183,7 @@ int __rdma_accept(struct rdma_cm_id *id, struct rdma_conn_param *conn_param,
 	return 0;
 reject:
 	cma_modify_qp_err(id_priv);
-	rdma_reject(id, NULL, 0);
+	rdma_reject(id, NULL, 0, 0);
 	return ret;
 }
 EXPORT_SYMBOL(__rdma_accept);
@@ -4223,7 +4223,7 @@ int rdma_notify(struct rdma_cm_id *id, enum ib_event_type event)
 EXPORT_SYMBOL(rdma_notify);
 
 int rdma_reject(struct rdma_cm_id *id, const void *private_data,
-		u8 private_data_len)
+		u8 private_data_len, enum rdma_ucm_reject_reason reason)
 {
 	struct rdma_id_private *id_priv;
 	int ret;
@@ -4237,10 +4237,12 @@ int rdma_reject(struct rdma_cm_id *id, const void *private_data,
 			ret = cma_send_sidr_rep(id_priv, IB_SIDR_REJECT, 0,
 						private_data, private_data_len);
 		} else {
+			enum ib_cm_rej_reason r =
+				(reason) ?: IB_CM_REJ_CONSUMER_DEFINED;
+
 			trace_cm_send_rej(id_priv);
-			ret = ib_send_cm_rej(id_priv->cm_id.ib,
-					     IB_CM_REJ_CONSUMER_DEFINED, NULL,
-					     0, private_data, private_data_len);
+			ret = ib_send_cm_rej(id_priv->cm_id.ib, r, NULL, 0,
+					     private_data, private_data_len);
 		}
 	} else if (rdma_cap_iw_cm(id->device, id->port_num)) {
 		ret = iw_cm_reject(id_priv->cm_id.iw,
diff --git a/drivers/infiniband/core/ucma.c b/drivers/infiniband/core/ucma.c
index d41598954cc4..99482dc5934b 100644
--- a/drivers/infiniband/core/ucma.c
+++ b/drivers/infiniband/core/ucma.c
@@ -1178,12 +1178,17 @@ static ssize_t ucma_reject(struct ucma_file *file, const char __user *inbuf,
 	if (copy_from_user(&cmd, inbuf, sizeof(cmd)))
 		return -EFAULT;
 
+	if (cmd.reason &&
+	    cmd.reason != RDMA_USER_CM_REJ_VENDOR_OPTION_NOT_SUPPORTED)
+		return -EINVAL;
+
 	ctx = ucma_get_ctx_dev(file, cmd.id);
 	if (IS_ERR(ctx))
 		return PTR_ERR(ctx);
 
 	mutex_lock(&ctx->mutex);
-	ret = rdma_reject(ctx->cm_id, cmd.private_data, cmd.private_data_len);
+	ret = rdma_reject(ctx->cm_id, cmd.private_data, cmd.private_data_len,
+			  cmd.reason);
 	mutex_unlock(&ctx->mutex);
 	ucma_put_ctx(ctx);
 	return ret;
diff --git a/drivers/infiniband/ulp/isert/ib_isert.c b/drivers/infiniband/ulp/isert/ib_isert.c
index a1a035270cab..42f78d0b61cc 100644
--- a/drivers/infiniband/ulp/isert/ib_isert.c
+++ b/drivers/infiniband/ulp/isert/ib_isert.c
@@ -502,7 +502,7 @@ isert_connect_request(struct rdma_cm_id *cma_id, struct rdma_cm_event *event)
 	if (!np->enabled) {
 		spin_unlock_bh(&np->np_thread_lock);
 		isert_dbg("iscsi_np is not enabled, reject connect request\n");
-		return rdma_reject(cma_id, NULL, 0);
+		return rdma_reject(cma_id, NULL, 0, 0);
 	}
 	spin_unlock_bh(&np->np_thread_lock);
 
@@ -553,7 +553,7 @@ isert_connect_request(struct rdma_cm_id *cma_id, struct rdma_cm_event *event)
 	isert_free_login_buf(isert_conn);
 out:
 	kfree(isert_conn);
-	rdma_reject(cma_id, NULL, 0);
+	rdma_reject(cma_id, NULL, 0, 0);
 	return ret;
 }
 
diff --git a/drivers/infiniband/ulp/srpt/ib_srpt.c b/drivers/infiniband/ulp/srpt/ib_srpt.c
index 7047f6a5d0a3..44eefa83bfc6 100644
--- a/drivers/infiniband/ulp/srpt/ib_srpt.c
+++ b/drivers/infiniband/ulp/srpt/ib_srpt.c
@@ -2496,7 +2496,7 @@ static int srpt_cm_req_recv(struct srpt_device *const sdev,
 				   SRP_BUF_FORMAT_INDIRECT);
 
 	if (rdma_cm_id)
-		rdma_reject(rdma_cm_id, rej, sizeof(*rej));
+		rdma_reject(rdma_cm_id, rej, sizeof(*rej), 0);
 	else
 		ib_send_cm_rej(ib_cm_id, IB_CM_REJ_CONSUMER_DEFINED, NULL, 0,
 			       rej, sizeof(*rej));
diff --git a/drivers/nvme/target/rdma.c b/drivers/nvme/target/rdma.c
index fd47de0e4e4e..9f3fea89b254 100644
--- a/drivers/nvme/target/rdma.c
+++ b/drivers/nvme/target/rdma.c
@@ -1138,7 +1138,7 @@ static int nvmet_rdma_cm_reject(struct rdma_cm_id *cm_id,
 	rej.recfmt = cpu_to_le16(NVME_RDMA_CM_FMT_1_0);
 	rej.sts = cpu_to_le16(status);
 
-	return rdma_reject(cm_id, (void *)&rej, sizeof(rej));
+	return rdma_reject(cm_id, (void *)&rej, sizeof(rej), 0);
 }
 
 static struct nvmet_rdma_queue *
diff --git a/include/rdma/ib_cm.h b/include/rdma/ib_cm.h
index 0f1ea5f2d01c..ed328a99ed0a 100644
--- a/include/rdma/ib_cm.h
+++ b/include/rdma/ib_cm.h
@@ -168,7 +168,8 @@ enum ib_cm_rej_reason {
 	IB_CM_REJ_INVALID_CLASS_VERSION		= 31,
 	IB_CM_REJ_INVALID_FLOW_LABEL		= 32,
 	IB_CM_REJ_INVALID_ALT_FLOW_LABEL	= 33,
-	IB_CM_REJ_VENDOR_OPTION_NOT_SUPPORTED	= 35,
+	IB_CM_REJ_VENDOR_OPTION_NOT_SUPPORTED	=
+		RDMA_USER_CM_REJ_VENDOR_OPTION_NOT_SUPPORTED,
 };
 
 struct ib_cm_rej_event_param {
diff --git a/include/rdma/rdma_cm.h b/include/rdma/rdma_cm.h
index 8d961d8b7cdb..f8781b132f62 100644
--- a/include/rdma/rdma_cm.h
+++ b/include/rdma/rdma_cm.h
@@ -324,11 +324,12 @@ int __rdma_accept_ece(struct rdma_cm_id *id, struct rdma_conn_param *conn_param,
  */
 int rdma_notify(struct rdma_cm_id *id, enum ib_event_type event);
 
+
 /**
  * rdma_reject - Called to reject a connection request or response.
  */
 int rdma_reject(struct rdma_cm_id *id, const void *private_data,
-		u8 private_data_len);
+		u8 private_data_len, enum rdma_ucm_reject_reason reason);
 
 /**
  * rdma_disconnect - This function disconnects the associated QP and
diff --git a/include/uapi/rdma/rdma_user_cm.h b/include/uapi/rdma/rdma_user_cm.h
index c4ca1412bcf9..e545f2de1e13 100644
--- a/include/uapi/rdma/rdma_user_cm.h
+++ b/include/uapi/rdma/rdma_user_cm.h
@@ -78,6 +78,10 @@ enum rdma_ucm_port_space {
 	RDMA_PS_UDP   = 0x0111,
 };
 
+enum rdma_ucm_reject_reason {
+	RDMA_USER_CM_REJ_VENDOR_OPTION_NOT_SUPPORTED = 35
+};
+
 /*
  * command ABI structures.
  */
@@ -234,7 +238,8 @@ struct rdma_ucm_accept {
 struct rdma_ucm_reject {
 	__u32 id;
 	__u8  private_data_len;
-	__u8  reserved[3];
+	__u8  reason; /* enum rdma_ucm_reject_reason */
+	__u8  reserved[2];
 	__u8  private_data[RDMA_MAX_PRIVATE_DATA];
 };
 
diff --git a/net/rds/ib_cm.c b/net/rds/ib_cm.c
index c71f4328d138..bac8c68df66c 100644
--- a/net/rds/ib_cm.c
+++ b/net/rds/ib_cm.c
@@ -927,7 +927,7 @@ int rds_ib_cm_handle_connect(struct rdma_cm_id *cm_id,
 	if (conn)
 		mutex_unlock(&conn->c_cm_lock);
 	if (err)
-		rdma_reject(cm_id, &err, sizeof(int));
+		rdma_reject(cm_id, &err, sizeof(int), 0);
 	return destroy;
 }
 
-- 
2.25.2

Re: [PATCH rdma-next v2 0/7] Add Enhanced Connection Established (ECE)

[Leon Romanovsky]

On Mon, Apr 13, 2020 at 05:15:31PM +0300, Leon Romanovsky wrote:
> From: Leon Romanovsky <leonro@mellanox.com>
>
> Changelog:
>  v2:
>  * Rebased on latest rdma-next and removed already accepted patches.
>  * Updated all rdma_reject in-kernel users to provide reject reason.
>  v1: Dropped field_avail patch in favor of mass conversion to use function
>      which already exists in the kernel code.
>  https://lore.kernel.org/lkml/20200310091438.248429-1-leon@kernel.org
>  v0: https://lore.kernel.org/lkml/20200305150105.207959-1-leon@kernel.org
>
> Enhanced Connection Established or ECE is new negotiation scheme
> introduced in IBTA v1.4 to exchange extra information about nodes
> capabilities and later negotiate them at the connection establishment
> phase.
>
> The RDMA-CM messages (REQ, REP, SIDR_REQ and SIDR_REP) were extended
> to carry two fields, one new and another gained new functionality:
>  * VendorID is a new field that indicates that common subset of vendor
>    option bits are supported as indicated by that VendorID.
>  * AttributeModifier already exists, but overloaded to indicate which
>    vendor options are supported by this VendorID.
>
> This is kernel part of such functionality which is responsible to get data
> from librdmacm and properly create and handle RDMA-CM messages.
>
> Thanks
>
> Leon Romanovsky (7):
>   RDMA/cm: Add Enhanced Connection Establishment (ECE) bits
>   RDMA/uapi: Add ECE definitions to UCMA
>   RDMA/ucma: Extend ucma_connect to receive ECE parameters
>   RDMA/ucma: Deliver ECE parameters through UCMA events
>   RDMA/cm: Send and receive ECE parameter over the wire
>   RDMA/cma: Connect ECE to rdma_accept
>   RDMA/cma: Provide ECE reject reason

PR: https://github.com/linux-rdma/rdma-core/pull/745

Thanks

Re: [PATCH rdma-next v2 3/7] RDMA/ucma: Extend ucma_connect to receive ECE parameters

[Jason Gunthorpe]

On Mon, Apr 13, 2020 at 05:15:34PM +0300, Leon Romanovsky wrote:

> -	if (copy_from_user(&cmd, inbuf, sizeof(cmd)))
> +	in_size = min_t(size_t, in_len, sizeof(cmd));
> +	if (copy_from_user(&cmd, inbuf, in_size))
>  		return -EFAULT;
>  
>  	if (!cmd.conn_param.valid)
> @@ -1086,8 +1089,13 @@ static ssize_t ucma_connect(struct ucma_file *file, const char __user *inbuf,
>  		return PTR_ERR(ctx);
>  
>  	ucma_copy_conn_param(ctx->cm_id, &conn_param, &cmd.conn_param);
> +	if (offsetofend(typeof(cmd), ece) <= in_size) {
> +		ece.vendor_id = cmd.ece.vendor_id;
> +		ece.attr_mod = cmd.ece.attr_mod;
> +	}

The uapi changes in the prior patch should be placed in the patches
that actually implement them, eg one here..

> diff --git a/include/rdma/rdma_cm.h b/include/rdma/rdma_cm.h
> index 71f48cfdc24c..86a849214c84 100644
> --- a/include/rdma/rdma_cm.h
> +++ b/include/rdma/rdma_cm.h
> @@ -264,6 +264,17 @@ int rdma_init_qp_attr(struct rdma_cm_id *id, struct ib_qp_attr *qp_attr,
>   */
>  int rdma_connect(struct rdma_cm_id *id, struct rdma_conn_param *conn_param);
>  
> +/**
> + * rdma_connect_ece - Initiate an active connection request with ECE data.
> + * @id: Connection identifier to connect.
> + * @conn_param: Connection information used for connected QPs.
> + * @ece: ECE parameters
> + *
> + * See rdma_connect() explanation.
> + */
> +int rdma_connect_ece(struct rdma_cm_id *id, struct rdma_conn_param *conn_param,
> +		     struct rdma_ucm_ece *ece);

kdoc's go in the C files

Jason

Re: [PATCH rdma-next v2 3/7] RDMA/ucma: Extend ucma_connect to receive ECE parameters

[Leon Romanovsky]

On Mon, May 25, 2020 at 02:41:41PM -0300, Jason Gunthorpe wrote:
> On Mon, Apr 13, 2020 at 05:15:34PM +0300, Leon Romanovsky wrote:
>
> > -	if (copy_from_user(&cmd, inbuf, sizeof(cmd)))
> > +	in_size = min_t(size_t, in_len, sizeof(cmd));
> > +	if (copy_from_user(&cmd, inbuf, in_size))
> >  		return -EFAULT;
> >
> >  	if (!cmd.conn_param.valid)
> > @@ -1086,8 +1089,13 @@ static ssize_t ucma_connect(struct ucma_file *file, const char __user *inbuf,
> >  		return PTR_ERR(ctx);
> >
> >  	ucma_copy_conn_param(ctx->cm_id, &conn_param, &cmd.conn_param);
> > +	if (offsetofend(typeof(cmd), ece) <= in_size) {
> > +		ece.vendor_id = cmd.ece.vendor_id;
> > +		ece.attr_mod = cmd.ece.attr_mod;
> > +	}
>
> The uapi changes in the prior patch should be placed in the patches
> that actually implement them, eg one here..

I wanted to simplify the series and keep its bisectable at the same
time. Should I squash them?

>
> > diff --git a/include/rdma/rdma_cm.h b/include/rdma/rdma_cm.h
> > index 71f48cfdc24c..86a849214c84 100644
> > --- a/include/rdma/rdma_cm.h
> > +++ b/include/rdma/rdma_cm.h
> > @@ -264,6 +264,17 @@ int rdma_init_qp_attr(struct rdma_cm_id *id, struct ib_qp_attr *qp_attr,
> >   */
> >  int rdma_connect(struct rdma_cm_id *id, struct rdma_conn_param *conn_param);
> >
> > +/**
> > + * rdma_connect_ece - Initiate an active connection request with ECE data.
> > + * @id: Connection identifier to connect.
> > + * @conn_param: Connection information used for connected QPs.
> > + * @ece: ECE parameters
> > + *
> > + * See rdma_connect() explanation.
> > + */
> > +int rdma_connect_ece(struct rdma_cm_id *id, struct rdma_conn_param *conn_param,
> > +		     struct rdma_ucm_ece *ece);
>
> kdoc's go in the C files

I know, but didn't know if to follow existing pattern or not.

>
> Jason

Re: [PATCH rdma-next v2 5/7] RDMA/cm: Send and receive ECE parameter over the wire

[Jason Gunthorpe]

On Mon, Apr 13, 2020 at 05:15:36PM +0300, Leon Romanovsky wrote:
> @@ -2204,6 +2220,12 @@ static void cm_format_rep(struct cm_rep_msg *rep_msg,
>  		IBA_SET(CM_REP_LOCAL_EE_CONTEXT_NUMBER, rep_msg, param->qp_num);
>  	}
>  
> +	IBA_SET(CM_REP_VENDOR_ID_L, rep_msg, param->ece.vendor_id & 0xFF);
> +	IBA_SET(CM_REP_VENDOR_ID_M, rep_msg,
> +		(param->ece.vendor_id >> 8) & 0xFF);
> +	IBA_SET(CM_REP_VENDOR_ID_H, rep_msg,
> +		(param->ece.vendor_id >> 16) & 0xFF);

I'm pretty sure the & 0xFF isn't needed?

> diff --git a/drivers/infiniband/core/ucma.c b/drivers/infiniband/core/ucma.c
> index ed2c17046ee1..b67cdd2ef187 100644
> +++ b/drivers/infiniband/core/ucma.c
> @@ -362,7 +362,6 @@ static int ucma_event_handler(struct rdma_cm_id *cm_id,
>  
>  	uevent->resp.ece.vendor_id = event->ece.vendor_id;
>  	uevent->resp.ece.attr_mod = event->ece.attr_mod;
> -
>  	if (event->event == RDMA_CM_EVENT_CONNECT_REQUEST) {
>  		if (!ctx->backlog) {
>  			ret = -ENOMEM;

Extra hunk?

Jason

Re: [PATCH rdma-next v2 5/7] RDMA/cm: Send and receive ECE parameter over the wire

[Leon Romanovsky]

On Mon, May 25, 2020 at 02:58:12PM -0300, Jason Gunthorpe wrote:
> On Mon, Apr 13, 2020 at 05:15:36PM +0300, Leon Romanovsky wrote:
> > @@ -2204,6 +2220,12 @@ static void cm_format_rep(struct cm_rep_msg *rep_msg,
> >  		IBA_SET(CM_REP_LOCAL_EE_CONTEXT_NUMBER, rep_msg, param->qp_num);
> >  	}
> >
> > +	IBA_SET(CM_REP_VENDOR_ID_L, rep_msg, param->ece.vendor_id & 0xFF);
> > +	IBA_SET(CM_REP_VENDOR_ID_M, rep_msg,
> > +		(param->ece.vendor_id >> 8) & 0xFF);
> > +	IBA_SET(CM_REP_VENDOR_ID_H, rep_msg,
> > +		(param->ece.vendor_id >> 16) & 0xFF);
>
> I'm pretty sure the & 0xFF isn't needed?

vendor_id is 32 bits, but IBTA spec uses only 24 bits. I preferred to
write it implicitly, but of course IBA_SET() will mask it properly.

>
> > diff --git a/drivers/infiniband/core/ucma.c b/drivers/infiniband/core/ucma.c
> > index ed2c17046ee1..b67cdd2ef187 100644
> > +++ b/drivers/infiniband/core/ucma.c
> > @@ -362,7 +362,6 @@ static int ucma_event_handler(struct rdma_cm_id *cm_id,
> >
> >  	uevent->resp.ece.vendor_id = event->ece.vendor_id;
> >  	uevent->resp.ece.attr_mod = event->ece.attr_mod;
> > -
> >  	if (event->event == RDMA_CM_EVENT_CONNECT_REQUEST) {
> >  		if (!ctx->backlog) {
> >  			ret = -ENOMEM;
>
> Extra hunk?

Right

>
> Jason

Re: [PATCH rdma-next v2 7/7] RDMA/cma: Provide ECE reject reason

[Jason Gunthorpe]

On Mon, Apr 13, 2020 at 05:15:38PM +0300, Leon Romanovsky wrote:
> @@ -4223,7 +4223,7 @@ int rdma_notify(struct rdma_cm_id *id, enum ib_event_type event)
>  EXPORT_SYMBOL(rdma_notify);
>  
>  int rdma_reject(struct rdma_cm_id *id, const void *private_data,
> -		u8 private_data_len)
> +		u8 private_data_len, enum rdma_ucm_reject_reason reason)
>  {
>  	struct rdma_id_private *id_priv;
>  	int ret;
> @@ -4237,10 +4237,12 @@ int rdma_reject(struct rdma_cm_id *id, const void *private_data,
>  			ret = cma_send_sidr_rep(id_priv, IB_SIDR_REJECT, 0,
>  						private_data, private_data_len);
>  		} else {
> +			enum ib_cm_rej_reason r =
> +				(reason) ?: IB_CM_REJ_CONSUMER_DEFINED;
> +
>  			trace_cm_send_rej(id_priv);
> -			ret = ib_send_cm_rej(id_priv->cm_id.ib,
> -					     IB_CM_REJ_CONSUMER_DEFINED, NULL,
> -					     0, private_data, private_data_len);
> +			ret = ib_send_cm_rej(id_priv->cm_id.ib, r, NULL, 0,
> +					     private_data, private_data_len);
>  		}
>  	} else if (rdma_cap_iw_cm(id->device, id->port_num)) {
>  		ret = iw_cm_reject(id_priv->cm_id.iw,
> diff --git a/drivers/infiniband/core/ucma.c b/drivers/infiniband/core/ucma.c
> index d41598954cc4..99482dc5934b 100644
> +++ b/drivers/infiniband/core/ucma.c
> @@ -1178,12 +1178,17 @@ static ssize_t ucma_reject(struct ucma_file *file, const char __user *inbuf,
>  	if (copy_from_user(&cmd, inbuf, sizeof(cmd)))
>  		return -EFAULT;
>  
> +	if (cmd.reason &&
> +	    cmd.reason != RDMA_USER_CM_REJ_VENDOR_OPTION_NOT_SUPPORTED)
> +		return -EINVAL;

It would be clearer to set cmd.reason to IB_CM_REJ_CONSUMER_DEFINED at
this point.. 

if (!cmd.reason)
   cmd.reason = IB_CM_REJ_CONSUMER_DEFINED

if (cmd.reason != IB_CM_REJ_CONSUMER_DEFINED && cmd.reason !=
    RDMA_USER_CM_REJ_VENDOR_OPTION_NOT_SUPPORTED)
   return -EINVAL

Esaier to follow and no reason userspace shouldn't be able to
explicitly specifiy the reason's that it is allowed to use.


> index 8d961d8b7cdb..f8781b132f62 100644
> +++ b/include/rdma/rdma_cm.h
> @@ -324,11 +324,12 @@ int __rdma_accept_ece(struct rdma_cm_id *id, struct rdma_conn_param *conn_param,
>   */
>  int rdma_notify(struct rdma_cm_id *id, enum ib_event_type event);
>  
> +
>  /**

Extra hunk?

>   * rdma_reject - Called to reject a connection request or response.
>   */
>  int rdma_reject(struct rdma_cm_id *id, const void *private_data,
> -		u8 private_data_len);
> +		u8 private_data_len, enum rdma_ucm_reject_reason reason);
>  
>  /**
>   * rdma_disconnect - This function disconnects the associated QP and
> diff --git a/include/uapi/rdma/rdma_user_cm.h b/include/uapi/rdma/rdma_user_cm.h
> index c4ca1412bcf9..e545f2de1e13 100644
> +++ b/include/uapi/rdma/rdma_user_cm.h
> @@ -78,6 +78,10 @@ enum rdma_ucm_port_space {
>  	RDMA_PS_UDP   = 0x0111,
>  };
>  
> +enum rdma_ucm_reject_reason {
> +	RDMA_USER_CM_REJ_VENDOR_OPTION_NOT_SUPPORTED = 35
> +};

not sure we need ABI defines for IBTA constants?

Jason

Re: [PATCH rdma-next v2 3/7] RDMA/ucma: Extend ucma_connect to receive ECE parameters

[Jason Gunthorpe]

On Mon, May 25, 2020 at 08:47:39PM +0300, Leon Romanovsky wrote:
> On Mon, May 25, 2020 at 02:41:41PM -0300, Jason Gunthorpe wrote:
> > On Mon, Apr 13, 2020 at 05:15:34PM +0300, Leon Romanovsky wrote:
> >
> > > -	if (copy_from_user(&cmd, inbuf, sizeof(cmd)))
> > > +	in_size = min_t(size_t, in_len, sizeof(cmd));
> > > +	if (copy_from_user(&cmd, inbuf, in_size))
> > >  		return -EFAULT;
> > >
> > >  	if (!cmd.conn_param.valid)
> > > @@ -1086,8 +1089,13 @@ static ssize_t ucma_connect(struct ucma_file *file, const char __user *inbuf,
> > >  		return PTR_ERR(ctx);
> > >
> > >  	ucma_copy_conn_param(ctx->cm_id, &conn_param, &cmd.conn_param);
> > > +	if (offsetofend(typeof(cmd), ece) <= in_size) {
> > > +		ece.vendor_id = cmd.ece.vendor_id;
> > > +		ece.attr_mod = cmd.ece.attr_mod;
> > > +	}
> >
> > The uapi changes in the prior patch should be placed in the patches
> > that actually implement them, eg one here..
> 
> I wanted to simplify the series and keep its bisectable at the same
> time. Should I squash them?

Yes, the two structs should be introduced in the patches that populate
them, that is the usual wy.

> > > diff --git a/include/rdma/rdma_cm.h b/include/rdma/rdma_cm.h
> > > index 71f48cfdc24c..86a849214c84 100644
> > > +++ b/include/rdma/rdma_cm.h
> > > @@ -264,6 +264,17 @@ int rdma_init_qp_attr(struct rdma_cm_id *id, struct ib_qp_attr *qp_attr,
> > >   */
> > >  int rdma_connect(struct rdma_cm_id *id, struct rdma_conn_param *conn_param);
> > >
> > > +/**
> > > + * rdma_connect_ece - Initiate an active connection request with ECE data.
> > > + * @id: Connection identifier to connect.
> > > + * @conn_param: Connection information used for connected QPs.
> > > + * @ece: ECE parameters
> > > + *
> > > + * See rdma_connect() explanation.
> > > + */
> > > +int rdma_connect_ece(struct rdma_cm_id *id, struct rdma_conn_param *conn_param,
> > > +		     struct rdma_ucm_ece *ece);
> >
> > kdoc's go in the C files
> 
> I know, but didn't know if to follow existing pattern or not.

I've been of the opinion that new code should follow the expected
style.

Trying to keep some huge matrix of certain files have certain legacy
exceptions is too hard on my brain.

Jason

Re: [PATCH rdma-next v2 7/7] RDMA/cma: Provide ECE reject reason

[Leon Romanovsky]

On Mon, May 25, 2020 at 03:14:17PM -0300, Jason Gunthorpe wrote:
> On Mon, Apr 13, 2020 at 05:15:38PM +0300, Leon Romanovsky wrote:
> > @@ -4223,7 +4223,7 @@ int rdma_notify(struct rdma_cm_id *id, enum ib_event_type event)
> >  EXPORT_SYMBOL(rdma_notify);
> >
> >  int rdma_reject(struct rdma_cm_id *id, const void *private_data,
> > -		u8 private_data_len)
> > +		u8 private_data_len, enum rdma_ucm_reject_reason reason)
> >  {
> >  	struct rdma_id_private *id_priv;
> >  	int ret;
> > @@ -4237,10 +4237,12 @@ int rdma_reject(struct rdma_cm_id *id, const void *private_data,
> >  			ret = cma_send_sidr_rep(id_priv, IB_SIDR_REJECT, 0,
> >  						private_data, private_data_len);
> >  		} else {
> > +			enum ib_cm_rej_reason r =
> > +				(reason) ?: IB_CM_REJ_CONSUMER_DEFINED;
> > +
> >  			trace_cm_send_rej(id_priv);
> > -			ret = ib_send_cm_rej(id_priv->cm_id.ib,
> > -					     IB_CM_REJ_CONSUMER_DEFINED, NULL,
> > -					     0, private_data, private_data_len);
> > +			ret = ib_send_cm_rej(id_priv->cm_id.ib, r, NULL, 0,
> > +					     private_data, private_data_len);
> >  		}
> >  	} else if (rdma_cap_iw_cm(id->device, id->port_num)) {
> >  		ret = iw_cm_reject(id_priv->cm_id.iw,
> > diff --git a/drivers/infiniband/core/ucma.c b/drivers/infiniband/core/ucma.c
> > index d41598954cc4..99482dc5934b 100644
> > +++ b/drivers/infiniband/core/ucma.c
> > @@ -1178,12 +1178,17 @@ static ssize_t ucma_reject(struct ucma_file *file, const char __user *inbuf,
> >  	if (copy_from_user(&cmd, inbuf, sizeof(cmd)))
> >  		return -EFAULT;
> >
> > +	if (cmd.reason &&
> > +	    cmd.reason != RDMA_USER_CM_REJ_VENDOR_OPTION_NOT_SUPPORTED)
> > +		return -EINVAL;
>
> It would be clearer to set cmd.reason to IB_CM_REJ_CONSUMER_DEFINED at
> this point..
>
> if (!cmd.reason)
>    cmd.reason = IB_CM_REJ_CONSUMER_DEFINED
>
> if (cmd.reason != IB_CM_REJ_CONSUMER_DEFINED && cmd.reason !=
>     RDMA_USER_CM_REJ_VENDOR_OPTION_NOT_SUPPORTED)
>    return -EINVAL
>
> Esaier to follow and no reason userspace shouldn't be able to
> explicitly specifiy the reason's that it is allowed to use.
>
>
> > index 8d961d8b7cdb..f8781b132f62 100644
> > +++ b/include/rdma/rdma_cm.h
> > @@ -324,11 +324,12 @@ int __rdma_accept_ece(struct rdma_cm_id *id, struct rdma_conn_param *conn_param,
> >   */
> >  int rdma_notify(struct rdma_cm_id *id, enum ib_event_type event);
> >
> > +
> >  /**
>
> Extra hunk?
>
> >   * rdma_reject - Called to reject a connection request or response.
> >   */
> >  int rdma_reject(struct rdma_cm_id *id, const void *private_data,
> > -		u8 private_data_len);
> > +		u8 private_data_len, enum rdma_ucm_reject_reason reason);
> >
> >  /**
> >   * rdma_disconnect - This function disconnects the associated QP and
> > diff --git a/include/uapi/rdma/rdma_user_cm.h b/include/uapi/rdma/rdma_user_cm.h
> > index c4ca1412bcf9..e545f2de1e13 100644
> > +++ b/include/uapi/rdma/rdma_user_cm.h
> > @@ -78,6 +78,10 @@ enum rdma_ucm_port_space {
> >  	RDMA_PS_UDP   = 0x0111,
> >  };
> >
> > +enum rdma_ucm_reject_reason {
> > +	RDMA_USER_CM_REJ_VENDOR_OPTION_NOT_SUPPORTED = 35
> > +};
>
> not sure we need ABI defines for IBTA constants?

Do you want to give an option to write any number?
Right now, I'm enforcing only allowed by IBTA reason
and which is used in user space.

Thanks

>
> Jason

Re: [PATCH rdma-next v2 3/7] RDMA/ucma: Extend ucma_connect to receive ECE parameters

[Leon Romanovsky]

On Mon, May 25, 2020 at 03:15:34PM -0300, Jason Gunthorpe wrote:
> On Mon, May 25, 2020 at 08:47:39PM +0300, Leon Romanovsky wrote:
> > On Mon, May 25, 2020 at 02:41:41PM -0300, Jason Gunthorpe wrote:
> > > On Mon, Apr 13, 2020 at 05:15:34PM +0300, Leon Romanovsky wrote:
> > >
> > > > -	if (copy_from_user(&cmd, inbuf, sizeof(cmd)))
> > > > +	in_size = min_t(size_t, in_len, sizeof(cmd));
> > > > +	if (copy_from_user(&cmd, inbuf, in_size))
> > > >  		return -EFAULT;
> > > >
> > > >  	if (!cmd.conn_param.valid)
> > > > @@ -1086,8 +1089,13 @@ static ssize_t ucma_connect(struct ucma_file *file, const char __user *inbuf,
> > > >  		return PTR_ERR(ctx);
> > > >
> > > >  	ucma_copy_conn_param(ctx->cm_id, &conn_param, &cmd.conn_param);
> > > > +	if (offsetofend(typeof(cmd), ece) <= in_size) {
> > > > +		ece.vendor_id = cmd.ece.vendor_id;
> > > > +		ece.attr_mod = cmd.ece.attr_mod;
> > > > +	}
> > >
> > > The uapi changes in the prior patch should be placed in the patches
> > > that actually implement them, eg one here..
> >
> > I wanted to simplify the series and keep its bisectable at the same
> > time. Should I squash them?
>
> Yes, the two structs should be introduced in the patches that populate
> them, that is the usual wy.
>
> > > > diff --git a/include/rdma/rdma_cm.h b/include/rdma/rdma_cm.h
> > > > index 71f48cfdc24c..86a849214c84 100644
> > > > +++ b/include/rdma/rdma_cm.h
> > > > @@ -264,6 +264,17 @@ int rdma_init_qp_attr(struct rdma_cm_id *id, struct ib_qp_attr *qp_attr,
> > > >   */
> > > >  int rdma_connect(struct rdma_cm_id *id, struct rdma_conn_param *conn_param);
> > > >
> > > > +/**
> > > > + * rdma_connect_ece - Initiate an active connection request with ECE data.
> > > > + * @id: Connection identifier to connect.
> > > > + * @conn_param: Connection information used for connected QPs.
> > > > + * @ece: ECE parameters
> > > > + *
> > > > + * See rdma_connect() explanation.
> > > > + */
> > > > +int rdma_connect_ece(struct rdma_cm_id *id, struct rdma_conn_param *conn_param,
> > > > +		     struct rdma_ucm_ece *ece);
> > >
> > > kdoc's go in the C files
> >
> > I know, but didn't know if to follow existing pattern or not.
>
> I've been of the opinion that new code should follow the expected
> style.
>
> Trying to keep some huge matrix of certain files have certain legacy
> exceptions is too hard on my brain.

No problem I'll sent new version tomorrow.

Thanks

>
> Jason

Re: [PATCH rdma-next v2 7/7] RDMA/cma: Provide ECE reject reason

[Jason Gunthorpe]

On Mon, May 25, 2020 at 09:22:42PM +0300, Leon Romanovsky wrote:
> On Mon, May 25, 2020 at 03:14:17PM -0300, Jason Gunthorpe wrote:
> > On Mon, Apr 13, 2020 at 05:15:38PM +0300, Leon Romanovsky wrote:
> > > @@ -4223,7 +4223,7 @@ int rdma_notify(struct rdma_cm_id *id, enum ib_event_type event)
> > >  EXPORT_SYMBOL(rdma_notify);
> > >
> > >  int rdma_reject(struct rdma_cm_id *id, const void *private_data,
> > > -		u8 private_data_len)
> > > +		u8 private_data_len, enum rdma_ucm_reject_reason reason)
> > >  {
> > >  	struct rdma_id_private *id_priv;
> > >  	int ret;
> > > @@ -4237,10 +4237,12 @@ int rdma_reject(struct rdma_cm_id *id, const void *private_data,
> > >  			ret = cma_send_sidr_rep(id_priv, IB_SIDR_REJECT, 0,
> > >  						private_data, private_data_len);
> > >  		} else {
> > > +			enum ib_cm_rej_reason r =
> > > +				(reason) ?: IB_CM_REJ_CONSUMER_DEFINED;
> > > +
> > >  			trace_cm_send_rej(id_priv);
> > > -			ret = ib_send_cm_rej(id_priv->cm_id.ib,
> > > -					     IB_CM_REJ_CONSUMER_DEFINED, NULL,
> > > -					     0, private_data, private_data_len);
> > > +			ret = ib_send_cm_rej(id_priv->cm_id.ib, r, NULL, 0,
> > > +					     private_data, private_data_len);
> > >  		}
> > >  	} else if (rdma_cap_iw_cm(id->device, id->port_num)) {
> > >  		ret = iw_cm_reject(id_priv->cm_id.iw,
> > > diff --git a/drivers/infiniband/core/ucma.c b/drivers/infiniband/core/ucma.c
> > > index d41598954cc4..99482dc5934b 100644
> > > +++ b/drivers/infiniband/core/ucma.c
> > > @@ -1178,12 +1178,17 @@ static ssize_t ucma_reject(struct ucma_file *file, const char __user *inbuf,
> > >  	if (copy_from_user(&cmd, inbuf, sizeof(cmd)))
> > >  		return -EFAULT;
> > >
> > > +	if (cmd.reason &&
> > > +	    cmd.reason != RDMA_USER_CM_REJ_VENDOR_OPTION_NOT_SUPPORTED)
> > > +		return -EINVAL;
> >
> > It would be clearer to set cmd.reason to IB_CM_REJ_CONSUMER_DEFINED at
> > this point..
> >
> > if (!cmd.reason)
> >    cmd.reason = IB_CM_REJ_CONSUMER_DEFINED
> >
> > if (cmd.reason != IB_CM_REJ_CONSUMER_DEFINED && cmd.reason !=
> >     RDMA_USER_CM_REJ_VENDOR_OPTION_NOT_SUPPORTED)
> >    return -EINVAL
> >
> > Esaier to follow and no reason userspace shouldn't be able to
> > explicitly specifiy the reason's that it is allowed to use.
> >
> >
> > > index 8d961d8b7cdb..f8781b132f62 100644
> > > +++ b/include/rdma/rdma_cm.h
> > > @@ -324,11 +324,12 @@ int __rdma_accept_ece(struct rdma_cm_id *id, struct rdma_conn_param *conn_param,
> > >   */
> > >  int rdma_notify(struct rdma_cm_id *id, enum ib_event_type event);
> > >
> > > +
> > >  /**
> >
> > Extra hunk?
> >
> > >   * rdma_reject - Called to reject a connection request or response.
> > >   */
> > >  int rdma_reject(struct rdma_cm_id *id, const void *private_data,
> > > -		u8 private_data_len);
> > > +		u8 private_data_len, enum rdma_ucm_reject_reason reason);
> > >
> > >  /**
> > >   * rdma_disconnect - This function disconnects the associated QP and
> > > diff --git a/include/uapi/rdma/rdma_user_cm.h b/include/uapi/rdma/rdma_user_cm.h
> > > index c4ca1412bcf9..e545f2de1e13 100644
> > > +++ b/include/uapi/rdma/rdma_user_cm.h
> > > @@ -78,6 +78,10 @@ enum rdma_ucm_port_space {
> > >  	RDMA_PS_UDP   = 0x0111,
> > >  };
> > >
> > > +enum rdma_ucm_reject_reason {
> > > +	RDMA_USER_CM_REJ_VENDOR_OPTION_NOT_SUPPORTED = 35
> > > +};
> >
> > not sure we need ABI defines for IBTA constants?
> 
> Do you want to give an option to write any number?
> Right now, I'm enforcing only allowed by IBTA reason
> and which is used in user space.

no, just the allowed numbers, just wondering if we need constants for
fixed IBTA values .. 

Jason

Re: [PATCH rdma-next v2 7/7] RDMA/cma: Provide ECE reject reason

[Leon Romanovsky]

On Mon, May 25, 2020 at 03:36:47PM -0300, Jason Gunthorpe wrote:
> On Mon, May 25, 2020 at 09:22:42PM +0300, Leon Romanovsky wrote:
> > On Mon, May 25, 2020 at 03:14:17PM -0300, Jason Gunthorpe wrote:
> > > On Mon, Apr 13, 2020 at 05:15:38PM +0300, Leon Romanovsky wrote:
> > > > @@ -4223,7 +4223,7 @@ int rdma_notify(struct rdma_cm_id *id, enum ib_event_type event)
> > > >  EXPORT_SYMBOL(rdma_notify);
> > > >
> > > >  int rdma_reject(struct rdma_cm_id *id, const void *private_data,
> > > > -		u8 private_data_len)
> > > > +		u8 private_data_len, enum rdma_ucm_reject_reason reason)
> > > >  {
> > > >  	struct rdma_id_private *id_priv;
> > > >  	int ret;
> > > > @@ -4237,10 +4237,12 @@ int rdma_reject(struct rdma_cm_id *id, const void *private_data,
> > > >  			ret = cma_send_sidr_rep(id_priv, IB_SIDR_REJECT, 0,
> > > >  						private_data, private_data_len);
> > > >  		} else {
> > > > +			enum ib_cm_rej_reason r =
> > > > +				(reason) ?: IB_CM_REJ_CONSUMER_DEFINED;
> > > > +
> > > >  			trace_cm_send_rej(id_priv);
> > > > -			ret = ib_send_cm_rej(id_priv->cm_id.ib,
> > > > -					     IB_CM_REJ_CONSUMER_DEFINED, NULL,
> > > > -					     0, private_data, private_data_len);
> > > > +			ret = ib_send_cm_rej(id_priv->cm_id.ib, r, NULL, 0,
> > > > +					     private_data, private_data_len);
> > > >  		}
> > > >  	} else if (rdma_cap_iw_cm(id->device, id->port_num)) {
> > > >  		ret = iw_cm_reject(id_priv->cm_id.iw,
> > > > diff --git a/drivers/infiniband/core/ucma.c b/drivers/infiniband/core/ucma.c
> > > > index d41598954cc4..99482dc5934b 100644
> > > > +++ b/drivers/infiniband/core/ucma.c
> > > > @@ -1178,12 +1178,17 @@ static ssize_t ucma_reject(struct ucma_file *file, const char __user *inbuf,
> > > >  	if (copy_from_user(&cmd, inbuf, sizeof(cmd)))
> > > >  		return -EFAULT;
> > > >
> > > > +	if (cmd.reason &&
> > > > +	    cmd.reason != RDMA_USER_CM_REJ_VENDOR_OPTION_NOT_SUPPORTED)
> > > > +		return -EINVAL;
> > >
> > > It would be clearer to set cmd.reason to IB_CM_REJ_CONSUMER_DEFINED at
> > > this point..
> > >
> > > if (!cmd.reason)
> > >    cmd.reason = IB_CM_REJ_CONSUMER_DEFINED
> > >
> > > if (cmd.reason != IB_CM_REJ_CONSUMER_DEFINED && cmd.reason !=
> > >     RDMA_USER_CM_REJ_VENDOR_OPTION_NOT_SUPPORTED)
> > >    return -EINVAL
> > >
> > > Esaier to follow and no reason userspace shouldn't be able to
> > > explicitly specifiy the reason's that it is allowed to use.
> > >
> > >
> > > > index 8d961d8b7cdb..f8781b132f62 100644
> > > > +++ b/include/rdma/rdma_cm.h
> > > > @@ -324,11 +324,12 @@ int __rdma_accept_ece(struct rdma_cm_id *id, struct rdma_conn_param *conn_param,
> > > >   */
> > > >  int rdma_notify(struct rdma_cm_id *id, enum ib_event_type event);
> > > >
> > > > +
> > > >  /**
> > >
> > > Extra hunk?
> > >
> > > >   * rdma_reject - Called to reject a connection request or response.
> > > >   */
> > > >  int rdma_reject(struct rdma_cm_id *id, const void *private_data,
> > > > -		u8 private_data_len);
> > > > +		u8 private_data_len, enum rdma_ucm_reject_reason reason);
> > > >
> > > >  /**
> > > >   * rdma_disconnect - This function disconnects the associated QP and
> > > > diff --git a/include/uapi/rdma/rdma_user_cm.h b/include/uapi/rdma/rdma_user_cm.h
> > > > index c4ca1412bcf9..e545f2de1e13 100644
> > > > +++ b/include/uapi/rdma/rdma_user_cm.h
> > > > @@ -78,6 +78,10 @@ enum rdma_ucm_port_space {
> > > >  	RDMA_PS_UDP   = 0x0111,
> > > >  };
> > > >
> > > > +enum rdma_ucm_reject_reason {
> > > > +	RDMA_USER_CM_REJ_VENDOR_OPTION_NOT_SUPPORTED = 35
> > > > +};
> > >
> > > not sure we need ABI defines for IBTA constants?
> >
> > Do you want to give an option to write any number?
> > Right now, I'm enforcing only allowed by IBTA reason
> > and which is used in user space.
>
> no, just the allowed numbers, just wondering if we need constants for
> fixed IBTA values ..

I will take a look.

Thanks

>
> Jason