| Message ID | 1494458598-6911-3-git-send-email-dasaratharaman.chandramouli@intel.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
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@intel.com> > Reviewed-by: Ira Weiny <ira.weiny@intel.com> > Signed-off-by: Dasaratharaman Chandramouli <dasaratharaman.chandramouli@intel.com> > --- > 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@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html
> > + 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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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
> > > > + 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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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;