[PATCH rdma-next 0/4] Enhance IB CM to support OPA extended LIDs

[PATCH rdma-next 0/4] Enhance IB CM to support OPA extended LIDs

[Dasaratharaman Chandramouli]

This patch series adds enhancements to support extended LIDs in the
IB CM protocol. The core idea is to trasmit extended LIDs as OPA GIDs
on the wire in the primary_local_gid, primary_remote_gid,
alternate_local_gid and alternate_remote_gid fields of a CM request.
On the receiving side, the extended LID is retrieved from these
gids when processing the connection request.

The patches are organized as follows:

* Patch 1 converts an OPA path record to an IB path record before
  placing GIDs from the path record onto a CM reqeust message. If the
  OPA path records were to contain exteded LIDs, the converted IB path
  record would contain OPA GIDs in the SGID and DGID fields.
* Patch 2 creates OPA path records when handling a connection request
  if the request contained OPA GIDs.
* Patch 3 'extracts' the OPA extended LIDs from the GID fields when
  handling the connection request
* Patch 4 changes the sgid parameter back to an IB GID when handing
  a request thereby ensuring that underlying handers of the CM request
  message such as IPoIB continue to work when it compares an incoming
  GID with the GID assign to its interface.

Please note that these patches apply on top of the previously submmitted
patch set with subject: "[PATCH rdma-next 0/8] Add OPA extended LID support"

Dasaratharaman Chandramouli (4):
  IB/CM: Convert OPA Path record to IB during CM request
  IB/CM: Create appropriate path records when handling CM request
  IB/CM: Set appropriate slid and dlid when handling CM request
  IB/CM: Change sgid to IB GID when handling CM request

 drivers/infiniband/core/cm.c | 184 +++++++++++++++++++++++++++++++++++--------
 1 file changed, 152 insertions(+), 32 deletions(-)

-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH rdma-next 1/4] IB/CM: Convert OPA Path record to IB during CM request

[Dasaratharaman Chandramouli]

OPA path records need to be converted to IB path records
during a connection request. If the slid/dlid fields in the
OPA path records are extended, the resulting converted IB
path records will have the corresponding GIDs populated as
OPA GIDs

Reviewed-by: Don Hiatt <don.hiatt-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Reviewed-by: Ira Weiny <ira.weiny-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Signed-off-by: Dasaratharaman Chandramouli <dasaratharaman.chandramouli-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
 drivers/infiniband/core/cm.c | 49 +++++++++++++++++++++++++++++++++-----------
 1 file changed, 37 insertions(+), 12 deletions(-)

diff --git a/drivers/infiniband/core/cm.c b/drivers/infiniband/core/cm.c
index 6b3b0be..9a7f4be 100644
--- a/drivers/infiniband/core/cm.c
+++ b/drivers/infiniband/core/cm.c
@@ -1173,8 +1173,24 @@ static void cm_format_req(struct cm_req_msg *req_msg,
 			  struct cm_id_private *cm_id_priv,
 			  struct ib_cm_req_param *param)
 {
-	struct sa_path_rec *pri_path = param->primary_path;
-	struct sa_path_rec *alt_path = param->alternate_path;
+	struct sa_path_rec *pri_path;
+	struct sa_path_rec *alt_path;
+	struct sa_path_rec conv_path;
+
+	if (param->primary_path->rec_type == SA_PATH_REC_TYPE_OPA) {
+		sa_convert_path_opa_to_ib(&conv_path, param->primary_path);
+		pri_path = &conv_path;
+	} else {
+		pri_path = param->primary_path;
+	}
+
+	if ((param->alternate_path) &&
+	    (param->alternate_path->rec_type == SA_PATH_REC_TYPE_OPA)) {
+		sa_convert_path_opa_to_ib(&conv_path, param->alternate_path);
+		alt_path = &conv_path;
+	} else {
+		alt_path = param->alternate_path;
+	}
 
 	cm_format_mad_hdr(&req_msg->hdr, CM_REQ_ATTR_ID,
 			  cm_form_tid(cm_id_priv, CM_MSG_SEQUENCE_REQ));
@@ -2843,6 +2859,9 @@ static void cm_format_lap(struct cm_lap_msg *lap_msg,
 			  const void *private_data,
 			  u8 private_data_len)
 {
+	struct sa_path_rec conv_path;
+	struct sa_path_rec *path;
+
 	cm_format_mad_hdr(&lap_msg->hdr, CM_LAP_ATTR_ID,
 			  cm_form_tid(cm_id_priv, CM_MSG_SEQUENCE_LAP));
 	lap_msg->local_comm_id = cm_id_priv->id.local_id;
@@ -2850,21 +2869,27 @@ static void cm_format_lap(struct cm_lap_msg *lap_msg,
 	cm_lap_set_remote_qpn(lap_msg, cm_id_priv->remote_qpn);
 	/* todo: need remote CM response timeout */
 	cm_lap_set_remote_resp_timeout(lap_msg, 0x1F);
+	if (alternate_path->rec_type == SA_PATH_REC_TYPE_OPA) {
+		sa_convert_path_opa_to_ib(&conv_path, alternate_path);
+		path = &conv_path;
+	} else {
+		path = alternate_path;
+	}
 	lap_msg->alt_local_lid =
-		htons(ntohl(sa_path_get_slid(alternate_path)));
+		htons(ntohl(sa_path_get_slid(path)));
 	lap_msg->alt_remote_lid =
-		htons(ntohl(sa_path_get_dlid(alternate_path)));
-	lap_msg->alt_local_gid = alternate_path->sgid;
-	lap_msg->alt_remote_gid = alternate_path->dgid;
-	cm_lap_set_flow_label(lap_msg, alternate_path->flow_label);
-	cm_lap_set_traffic_class(lap_msg, alternate_path->traffic_class);
-	lap_msg->alt_hop_limit = alternate_path->hop_limit;
-	cm_lap_set_packet_rate(lap_msg, alternate_path->rate);
-	cm_lap_set_sl(lap_msg, alternate_path->sl);
+		htons(ntohl(sa_path_get_dlid(path)));
+	lap_msg->alt_local_gid = path->sgid;
+	lap_msg->alt_remote_gid = path->dgid;
+	cm_lap_set_flow_label(lap_msg, path->flow_label);
+	cm_lap_set_traffic_class(lap_msg, path->traffic_class);
+	lap_msg->alt_hop_limit = path->hop_limit;
+	cm_lap_set_packet_rate(lap_msg, path->rate);
+	cm_lap_set_sl(lap_msg, path->sl);
 	cm_lap_set_subnet_local(lap_msg, 1); /* local only... */
 	cm_lap_set_local_ack_timeout(lap_msg,
 		cm_ack_timeout(cm_id_priv->av.port->cm_dev->ack_delay,
-			       alternate_path->packet_life_time));
+			       path->packet_life_time));
 
 	if (private_data && private_data_len)
 		memcpy(lap_msg->private_data, private_data, private_data_len);
-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH rdma-next 2/4] IB/CM: Create appropriate path records when handling CM request

[Dasaratharaman Chandramouli]

When handling an incoming conection request, ib_cm creates
either an IB or an OPA path record based on the gid field
in the request.

Reviewed-by: Don Hiatt <don.hiatt-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Reviewed-by: Ira Weiny <ira.weiny-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Signed-off-by: Dasaratharaman Chandramouli <dasaratharaman.chandramouli-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
 drivers/infiniband/core/cm.c | 39 ++++++++++++++++++++++++++++++++-------
 1 file changed, 32 insertions(+), 7 deletions(-)

diff --git a/drivers/infiniband/core/cm.c b/drivers/infiniband/core/cm.c
index 9a7f4be..01cfa24 100644
--- a/drivers/infiniband/core/cm.c
+++ b/drivers/infiniband/core/cm.c
@@ -1421,6 +1421,12 @@ static inline int cm_is_active_peer(__be64 local_ca_guid, __be64 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) ||
+		(ib_is_opa_gid(&req_msg->alt_local_gid)));
+}
+
 static void cm_format_paths_from_req(struct cm_req_msg *req_msg,
 				     struct sa_path_rec *primary_path,
 				     struct sa_path_rec *alt_path)
@@ -1800,9 +1806,18 @@ static int cm_req_handler(struct cm_work *work)
 					 dev_net(gid_attr.ndev));
 			dev_put(gid_attr.ndev);
 		} else {
-			work->path[0].rec_type = SA_PATH_REC_TYPE_IB;
+			/*
+			 * If the gid in the request is an OPA GID,
+			 * create an OPA PR
+			 */
+			if (ib_is_opa_gid(&req_msg->primary_local_gid) &&
+			    rdma_cap_opa_ah(work->port->cm_dev->ib_device,
+					    work->port->port_num))
+				work->path[0].rec_type = SA_PATH_REC_TYPE_OPA;
+			else
+				work->path[0].rec_type = SA_PATH_REC_TYPE_IB;
 		}
-		if (req_msg->alt_local_lid)
+		if (cm_req_has_alt_path(req_msg))
 			work->path[1].rec_type = work->path[0].rec_type;
 		cm_format_paths_from_req(req_msg, &work->path[0],
 					 &work->path[1]);
@@ -1827,16 +1842,21 @@ static int cm_req_handler(struct cm_work *work)
 					 dev_net(gid_attr.ndev));
 			dev_put(gid_attr.ndev);
 		} else {
-			work->path[0].rec_type = SA_PATH_REC_TYPE_IB;
+			if (ib_is_opa_gid(&req_msg->primary_local_gid) &&
+			    rdma_cap_opa_ah(work->port->cm_dev->ib_device,
+					    work->port->port_num))
+				work->path[0].rec_type = SA_PATH_REC_TYPE_OPA;
+			else
+				work->path[0].rec_type = SA_PATH_REC_TYPE_IB;
 		}
-		if (req_msg->alt_local_lid)
+		if (cm_req_has_alt_path(req_msg))
 			work->path[1].rec_type = work->path[0].rec_type;
 		ib_send_cm_rej(cm_id, IB_CM_REJ_INVALID_GID,
 			       &work->path[0].sgid, sizeof work->path[0].sgid,
 			       NULL, 0);
 		goto rejected;
 	}
-	if (req_msg->alt_local_lid) {
+	if (cm_req_has_alt_path(req_msg)) {
 		ret = cm_init_av_by_path(&work->path[1], &cm_id_priv->alt_av,
 					 cm_id_priv);
 		if (ret) {
@@ -2953,8 +2973,6 @@ static void cm_format_path_from_lap(struct cm_id_private *cm_id_priv,
 				    struct sa_path_rec *path,
 				    struct cm_lap_msg *lap_msg)
 {
-	memset(path, 0, sizeof *path);
-	path->rec_type = SA_PATH_REC_TYPE_IB;
 	path->dgid = lap_msg->alt_local_gid;
 	path->sgid = lap_msg->alt_remote_gid;
 	sa_path_set_dlid(path, htonl(ntohs(lap_msg->alt_local_lid)));
@@ -2990,6 +3008,13 @@ static int cm_lap_handler(struct cm_work *work)
 		return -EINVAL;
 
 	param = &work->cm_event.param.lap_rcvd;
+	memset(&work->path[0], 0, sizeof(work->path[0]));
+	if (ib_is_opa_gid(&lap_msg->alt_local_gid) &&
+	    rdma_cap_opa_ah(work->port->cm_dev->ib_device,
+			    work->port->port_num))
+		work->path[0].rec_type = SA_PATH_REC_TYPE_OPA;
+	else
+		work->path[0].rec_type = SA_PATH_REC_TYPE_IB;
 	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;
-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH rdma-next 3/4] IB/CM: Set appropriate slid and dlid when handling CM request

[Dasaratharaman Chandramouli]

If extended LIDs are being used, a connection request contains
OPA GIDs in them. Extract the lids from the OPA gids and populate
slid/dlid fields in the path records that are created when handling
a connection request.

Reviewed-by: Don Hiatt <don.hiatt-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Reviewed-by: Ira Weiny <ira.weiny-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Signed-off-by: Dasaratharaman Chandramouli <dasaratharaman.chandramouli-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
 drivers/infiniband/core/cm.c | 67 ++++++++++++++++++++++++++++++++++++--------
 1 file changed, 56 insertions(+), 11 deletions(-)

diff --git a/drivers/infiniband/core/cm.c b/drivers/infiniband/core/cm.c
index 01cfa24..2d0a550 100644
--- a/drivers/infiniband/core/cm.c
+++ b/drivers/infiniband/core/cm.c
@@ -1427,16 +1427,48 @@ static bool cm_req_has_alt_path(struct cm_req_msg *req_msg)
 		(ib_is_opa_gid(&req_msg->alt_local_gid)));
 }
 
+static void cm_format_path_lid_from_req(struct cm_req_msg *req_msg,
+					struct sa_path_rec *primary_path,
+					struct sa_path_rec *alt_path)
+{
+	u32 lid;
+
+	if (primary_path->rec_type != SA_PATH_REC_TYPE_OPA) {
+		sa_path_set_dlid(primary_path,
+				 htonl(ntohs(req_msg->primary_local_lid)));
+		sa_path_set_slid(primary_path,
+				 htonl(ntohs(req_msg->primary_remote_lid)));
+	} else {
+		lid = opa_get_lid_from_gid(&req_msg->primary_local_gid);
+		sa_path_set_dlid(primary_path, cpu_to_be32(lid));
+
+		lid = opa_get_lid_from_gid(&req_msg->primary_remote_gid);
+		sa_path_set_slid(primary_path, cpu_to_be32(lid));
+	}
+
+	if (!cm_req_has_alt_path(req_msg))
+		return;
+
+	if (alt_path->rec_type != SA_PATH_REC_TYPE_OPA) {
+		sa_path_set_dlid(alt_path,
+				 htonl(ntohs(req_msg->alt_local_lid)));
+		sa_path_set_slid(alt_path,
+				 htonl(ntohs(req_msg->alt_remote_lid)));
+	} else {
+		lid = opa_get_lid_from_gid(&req_msg->alt_local_gid);
+		sa_path_set_dlid(alt_path, cpu_to_be32(lid));
+
+		lid = opa_get_lid_from_gid(&req_msg->alt_remote_gid);
+		sa_path_set_slid(alt_path, cpu_to_be32(lid));
+	}
+}
+
 static void cm_format_paths_from_req(struct cm_req_msg *req_msg,
 				     struct sa_path_rec *primary_path,
 				     struct sa_path_rec *alt_path)
 {
 	primary_path->dgid = req_msg->primary_local_gid;
 	primary_path->sgid = req_msg->primary_remote_gid;
-	sa_path_set_dlid(primary_path,
-			 htonl(ntohs(req_msg->primary_local_lid)));
-	sa_path_set_slid(primary_path,
-			 htonl(ntohs(req_msg->primary_remote_lid)));
 	primary_path->flow_label = cm_req_get_primary_flow_label(req_msg);
 	primary_path->hop_limit = req_msg->primary_hop_limit;
 	primary_path->traffic_class = req_msg->primary_traffic_class;
@@ -1453,13 +1485,9 @@ static void cm_format_paths_from_req(struct cm_req_msg *req_msg,
 	primary_path->packet_life_time -= (primary_path->packet_life_time > 0);
 	sa_path_set_service_id(primary_path, req_msg->service_id);
 
-	if (req_msg->alt_local_lid) {
+	if (cm_req_has_alt_path(req_msg)) {
 		alt_path->dgid = req_msg->alt_local_gid;
 		alt_path->sgid = req_msg->alt_remote_gid;
-		sa_path_set_dlid(alt_path,
-				 htonl(ntohs(req_msg->alt_local_lid)));
-		sa_path_set_slid(alt_path,
-				 htonl(ntohs(req_msg->alt_remote_lid)));
 		alt_path->flow_label = cm_req_get_alt_flow_label(req_msg);
 		alt_path->hop_limit = req_msg->alt_hop_limit;
 		alt_path->traffic_class = req_msg->alt_traffic_class;
@@ -1476,6 +1504,7 @@ static void cm_format_paths_from_req(struct cm_req_msg *req_msg,
 		alt_path->packet_life_time -= (alt_path->packet_life_time > 0);
 		sa_path_set_service_id(alt_path, req_msg->service_id);
 	}
+	cm_format_path_lid_from_req(req_msg, primary_path, alt_path);
 }
 
 static u16 cm_get_bth_pkey(struct cm_work *work)
@@ -2969,14 +2998,29 @@ int ib_send_cm_lap(struct ib_cm_id *cm_id,
 }
 EXPORT_SYMBOL(ib_send_cm_lap);
 
+static void cm_format_path_lid_from_lap(struct cm_lap_msg *lap_msg,
+					struct sa_path_rec *path)
+{
+	u32 lid;
+
+	if (path->rec_type != SA_PATH_REC_TYPE_OPA) {
+		sa_path_set_dlid(path, htonl(ntohs(lap_msg->alt_local_lid)));
+		sa_path_set_slid(path, htonl(ntohs(lap_msg->alt_remote_lid)));
+	} else {
+		lid = opa_get_lid_from_gid(&lap_msg->alt_local_gid);
+		sa_path_set_dlid(path, cpu_to_be32(lid));
+
+		lid = opa_get_lid_from_gid(&lap_msg->alt_remote_gid);
+		sa_path_set_slid(path, cpu_to_be32(lid));
+	}
+}
+
 static void cm_format_path_from_lap(struct cm_id_private *cm_id_priv,
 				    struct sa_path_rec *path,
 				    struct cm_lap_msg *lap_msg)
 {
 	path->dgid = lap_msg->alt_local_gid;
 	path->sgid = lap_msg->alt_remote_gid;
-	sa_path_set_dlid(path, htonl(ntohs(lap_msg->alt_local_lid)));
-	sa_path_set_slid(path, htonl(ntohs(lap_msg->alt_remote_lid)));
 	path->flow_label = cm_lap_get_flow_label(lap_msg);
 	path->hop_limit = lap_msg->alt_hop_limit;
 	path->traffic_class = cm_lap_get_traffic_class(lap_msg);
@@ -2990,6 +3034,7 @@ static void cm_format_path_from_lap(struct cm_id_private *cm_id_priv,
 	path->packet_life_time_selector = IB_SA_EQ;
 	path->packet_life_time = cm_lap_get_local_ack_timeout(lap_msg);
 	path->packet_life_time -= (path->packet_life_time > 0);
+	cm_format_path_lid_from_lap(lap_msg, path);
 }
 
 static int cm_lap_handler(struct cm_work *work)
-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH rdma-next 4/4] IB/CM: Change sgid to IB GID when handling CM request

[Dasaratharaman Chandramouli]

IPoIB uses the sgid to compare it with its local_gid and will
reject a CM request if they don't match. Change OPA GID back to
IB gid so the comparison succeeds.

Reviewed-by: Don Hiatt <don.hiatt-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Reviewed-by: Ira Weiny <ira.weiny-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Signed-off-by: Dasaratharaman Chandramouli <dasaratharaman.chandramouli-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
 drivers/infiniband/core/cm.c | 29 +++++++++++++++++++++++++++--
 1 file changed, 27 insertions(+), 2 deletions(-)

diff --git a/drivers/infiniband/core/cm.c b/drivers/infiniband/core/cm.c
index 2d0a550..4798572 100644
--- a/drivers/infiniband/core/cm.c
+++ b/drivers/infiniband/core/cm.c
@@ -1525,6 +1525,28 @@ static u16 cm_get_bth_pkey(struct cm_work *work)
 	return pkey;
 }
 
+static void cm_modify_sgid(struct cm_work *work,
+			   struct sa_path_rec *path)
+{
+	struct ib_device *dev = work->port->cm_dev->ib_device;
+	struct ib_gid_attr gid_attr;
+	u8 port_num = work->port->port_num;
+
+	if (rdma_cap_opa_ah(dev, port_num) &&
+	    (ib_is_opa_gid(&path->sgid))) {
+		union ib_gid sgid;
+
+		if (ib_get_cached_gid(dev, port_num, 0,
+				      &sgid, &gid_attr)) {
+			dev_warn(&dev->dev,
+				 "Error updating sgid in CM request\n");
+			return;
+		}
+
+		path->sgid = sgid;
+	}
+}
+
 static void cm_format_req_event(struct cm_work *work,
 				struct cm_id_private *cm_id_priv,
 				struct ib_cm_id *listen_id)
@@ -1538,10 +1560,13 @@ static void cm_format_req_event(struct cm_work *work,
 	param->bth_pkey = cm_get_bth_pkey(work);
 	param->port = cm_id_priv->av.port->port_num;
 	param->primary_path = &work->path[0];
-	if (req_msg->alt_local_lid)
+	cm_modify_sgid(work, param->primary_path);
+	if (cm_req_has_alt_path(req_msg)) {
 		param->alternate_path = &work->path[1];
-	else
+		cm_modify_sgid(work, param->alternate_path);
+	} else {
 		param->alternate_path = NULL;
+	}
 	param->remote_ca_guid = req_msg->local_ca_guid;
 	param->remote_qkey = be32_to_cpu(req_msg->local_qkey);
 	param->remote_qpn = be32_to_cpu(cm_req_get_local_qpn(req_msg));
-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH rdma-next 2/4] IB/CM: Create appropriate path records when handling CM request

[Leon Romanovsky]

[-- Attachment #1: Type: text/plain, Size: 4318 bytes --]

On Wed, May 10, 2017 at 07:23:16PM -0400, Dasaratharaman Chandramouli wrote:
> When handling an incoming conection request, ib_cm creates
> either an IB or an OPA path record based on the gid field
> in the request.
>
> Reviewed-by: Don Hiatt <don.hiatt-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> Reviewed-by: Ira Weiny <ira.weiny-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> Signed-off-by: Dasaratharaman Chandramouli <dasaratharaman.chandramouli-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> ---
>  drivers/infiniband/core/cm.c | 39 ++++++++++++++++++++++++++++++++-------
>  1 file changed, 32 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/infiniband/core/cm.c b/drivers/infiniband/core/cm.c
> index 9a7f4be..01cfa24 100644
> --- a/drivers/infiniband/core/cm.c
> +++ b/drivers/infiniband/core/cm.c
> @@ -1421,6 +1421,12 @@ static inline int cm_is_active_peer(__be64 local_ca_guid, __be64 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) ||
> +		(ib_is_opa_gid(&req_msg->alt_local_gid)));
> +}
> +
>  static void cm_format_paths_from_req(struct cm_req_msg *req_msg,
>  				     struct sa_path_rec *primary_path,
>  				     struct sa_path_rec *alt_path)
> @@ -1800,9 +1806,18 @@ static int cm_req_handler(struct cm_work *work)
>  					 dev_net(gid_attr.ndev));
>  			dev_put(gid_attr.ndev);
>  		} else {
> -			work->path[0].rec_type = SA_PATH_REC_TYPE_IB;
> +			/*
> +			 * If the gid in the request is an OPA GID,
> +			 * create an OPA PR
> +			 */
> +			if (ib_is_opa_gid(&req_msg->primary_local_gid) &&
> +			    rdma_cap_opa_ah(work->port->cm_dev->ib_device,
> +					    work->port->port_num))

Why isn't ib_is_opa_gid enough?

> +				work->path[0].rec_type = SA_PATH_REC_TYPE_OPA;
> +			else
> +				work->path[0].rec_type = SA_PATH_REC_TYPE_IB;
>  		}
> -		if (req_msg->alt_local_lid)
> +		if (cm_req_has_alt_path(req_msg))
>  			work->path[1].rec_type = work->path[0].rec_type;
>  		cm_format_paths_from_req(req_msg, &work->path[0],
>  					 &work->path[1]);
> @@ -1827,16 +1842,21 @@ static int cm_req_handler(struct cm_work *work)
>  					 dev_net(gid_attr.ndev));
>  			dev_put(gid_attr.ndev);
>  		} else {
> -			work->path[0].rec_type = SA_PATH_REC_TYPE_IB;
> +			if (ib_is_opa_gid(&req_msg->primary_local_gid) &&
> +			    rdma_cap_opa_ah(work->port->cm_dev->ib_device,
> +					    work->port->port_num))
> +				work->path[0].rec_type = SA_PATH_REC_TYPE_OPA;
> +			else
> +				work->path[0].rec_type = SA_PATH_REC_TYPE_IB;
>  		}
> -		if (req_msg->alt_local_lid)
> +		if (cm_req_has_alt_path(req_msg))
>  			work->path[1].rec_type = work->path[0].rec_type;
>  		ib_send_cm_rej(cm_id, IB_CM_REJ_INVALID_GID,
>  			       &work->path[0].sgid, sizeof work->path[0].sgid,
>  			       NULL, 0);
>  		goto rejected;
>  	}
> -	if (req_msg->alt_local_lid) {
> +	if (cm_req_has_alt_path(req_msg)) {
>  		ret = cm_init_av_by_path(&work->path[1], &cm_id_priv->alt_av,
>  					 cm_id_priv);
>  		if (ret) {
> @@ -2953,8 +2973,6 @@ static void cm_format_path_from_lap(struct cm_id_private *cm_id_priv,
>  				    struct sa_path_rec *path,
>  				    struct cm_lap_msg *lap_msg)
>  {
> -	memset(path, 0, sizeof *path);
> -	path->rec_type = SA_PATH_REC_TYPE_IB;
>  	path->dgid = lap_msg->alt_local_gid;
>  	path->sgid = lap_msg->alt_remote_gid;
>  	sa_path_set_dlid(path, htonl(ntohs(lap_msg->alt_local_lid)));
> @@ -2990,6 +3008,13 @@ static int cm_lap_handler(struct cm_work *work)
>  		return -EINVAL;
>
>  	param = &work->cm_event.param.lap_rcvd;
> +	memset(&work->path[0], 0, sizeof(work->path[0]));
> +	if (ib_is_opa_gid(&lap_msg->alt_local_gid) &&
> +	    rdma_cap_opa_ah(work->port->cm_dev->ib_device,
> +			    work->port->port_num))
> +		work->path[0].rec_type = SA_PATH_REC_TYPE_OPA;
> +	else
> +		work->path[0].rec_type = SA_PATH_REC_TYPE_IB;
>  	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;
> --
> 1.8.3.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

RE: [PATCH rdma-next 2/4] IB/CM: Create appropriate path records when handling CM request

[Hefty, Sean]

> > +			if (ib_is_opa_gid(&req_msg->primary_local_gid) &&
> > +			    rdma_cap_opa_ah(work->port->cm_dev->ib_device,
> > +					    work->port->port_num))
> 
> Why isn't ib_is_opa_gid enough?

It may be in reality, but that implies that IB and OPA are sharing GID space definitions.

It looks like rdma_cap_opa_ah() is basically being used as a check to see if the code is handling the OPA CM protocol or IB CM protocol.  (Even though the two protocols are nearly identical).
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH rdma-next 2/4] IB/CM: Create appropriate path records when handling CM request

[Leon Romanovsky]

[-- Attachment #1: Type: text/plain, Size: 815 bytes --]

On Mon, May 15, 2017 at 05:44:54PM +0000, Hefty, Sean wrote:
> > > +			if (ib_is_opa_gid(&req_msg->primary_local_gid) &&
> > > +			    rdma_cap_opa_ah(work->port->cm_dev->ib_device,
> > > +					    work->port->port_num))
> >
> > Why isn't ib_is_opa_gid enough?
>
> It may be in reality, but that implies that IB and OPA are sharing GID space definitions.

I understood from Ira's presentation [1] that it is the case.
And doesn't it need to be "||" and not "&&"?

>
> It looks like rdma_cap_opa_ah() is basically being used as a check to see if the code is handling the OPA CM protocol or IB CM protocol.  (Even though the two protocols are nearly identical).

Where can I read the difference between these protocols?

[1] https://www.openfabrics.org/images/eventpresos/2016presentations/104rdmaaltfabs.pdf

Thanks

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

RE: [PATCH rdma-next 2/4] IB/CM: Create appropriate path records when handling CM request

[Hefty, Sean]

> > > > +			if (ib_is_opa_gid(&req_msg->primary_local_gid)
> &&
> > > > +			    rdma_cap_opa_ah(work->port->cm_dev-
> >ib_device,
> > > > +					    work->port->port_num))
> > >
> > > Why isn't ib_is_opa_gid enough?
> >
> > It may be in reality, but that implies that IB and OPA are sharing
> GID space definitions.
> 
> I understood from Ira's presentation [1] that it is the case.

These are distinct addressing spaces.  OPA may be copying the IB spec GID definitions (similar to GIDs looking like IPv6 addresses), but there technically doesn't have to be any agreement between the IB spec and OPA.  I believe OPA has taken care not to use values that will or are likely to conflict with IB spec, so I think it comes down to what level of assumptions are we willing to accept in the code.  I'd like to see explicit checks, like what was done in the MAD area.

> And doesn't it need to be "||" and not "&&"?

&& should be correct.

> > It looks like rdma_cap_opa_ah() is basically being used as a check
> to see if the code is handling the OPA CM protocol or IB CM protocol.
> (Even though the two protocols are nearly identical).
> 
> Where can I read the difference between these protocols?

I don't know if the OPA CM (or MAD) protocol is published anywhere.  I haven't read the details myself.

- Sean
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH rdma-next 1/4] IB/CM: Convert OPA Path record to IB during CM request

[Hefty, Sean]

> OPA path records need to be converted to IB path records
> during a connection request. If the slid/dlid fields in the
> OPA path records are extended, the resulting converted IB
> path records will have the corresponding GIDs populated as
> OPA GIDs

I'm getting lost.  Why is a conversion from OPA PRs to IB PRs needed? 

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH rdma-next 1/4] IB/CM: Convert OPA Path record to IB during CM request

[Don Hiatt]

On 5/16/2017 1:34 PM, Hefty, Sean wrote:
>> OPA path records need to be converted to IB path records
>> during a connection request. If the slid/dlid fields in the
>> OPA path records are extended, the resulting converted IB
>> path records will have the corresponding GIDs populated as
>> OPA GIDs
> 
> I'm getting lost.  Why is a conversion from OPA PRs to IB PRs needed?
> 

If the OPA path record contained extended LIDS then the SGID/DGID fields 
of the IB path record would contain OPA GIDS.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH rdma-next 1/4] IB/CM: Convert OPA Path record to IB during CM request

[Hefty, Sean]

> >> OPA path records need to be converted to IB path records
> >> during a connection request. If the slid/dlid fields in the
> >> OPA path records are extended, the resulting converted IB
> >> path records will have the corresponding GIDs populated as
> >> OPA GIDs
> >
> > I'm getting lost.  Why is a conversion from OPA PRs to IB PRs
> needed?
> >
> 
> If the OPA path record contained extended LIDS then the SGID/DGID
> fields
> of the IB path record would contain OPA GIDS.

But *why* is the conversion necessary?  The CM doesn't send/receive PRs.  And the interface to the CM should be able to handle either. 
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH rdma-next 4/4] IB/CM: Change sgid to IB GID when handling CM request

[Hefty, Sean]

> IPoIB uses the sgid to compare it with its local_gid and will
> reject a CM request if they don't match. Change OPA GID back to
> IB gid so the comparison succeeds.

I have the same question in this patch as well.  Why are we converting GIDs and PRs back and forth?  Outside of user/kernel transitions for ABI compatibility, these conversions seem odd.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html