| Message ID | 552F6DEA.9080701@profitbricks.com (mailing list archive) |
|---|---|
| State | Rejected |
| Headers | show |
On 4/16/2015 4:08 AM, Michael Wang wrote: > > Use raw management helpers to reform cm related part in IB-core cma/ucm. > > These checks focus on the device cm type rather than the port capability, > directly pass port 1 works currently, but can't support mixing cm type > device in future. This is equivalent to today where the checks are per node rather than per port. Should all checks here be port 1 based or only certain ones like listen ? For example, in connect/reject/disconnect, don't we already have port ? Guess this can be dealt with later as this is not a regression from the current implementation. -- Hal > > Cc: Steve Wise <swise@opengridcomputing.com> > Cc: Tom Talpey <tom@talpey.com> > Cc: Jason Gunthorpe <jgunthorpe@obsidianresearch.com> > Cc: Doug Ledford <dledford@redhat.com> > Cc: Ira Weiny <ira.weiny@intel.com> > Cc: Sean Hefty <sean.hefty@intel.com> > Signed-off-by: Michael Wang <yun.wang@profitbricks.com> > --- > drivers/infiniband/core/cma.c | 81 +++++++++++++------------------------------ > drivers/infiniband/core/ucm.c | 3 +- > 2 files changed, 26 insertions(+), 58 deletions(-) > > diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c > index d570030..6b8a64d 100644 > --- a/drivers/infiniband/core/cma.c > +++ b/drivers/infiniband/core/cma.c > @@ -735,8 +735,7 @@ int rdma_init_qp_attr(struct rdma_cm_id *id, struct ib_qp_attr *qp_attr, > int ret = 0; > > id_priv = container_of(id, struct rdma_id_private, id); > - switch (rdma_node_get_transport(id_priv->id.device->node_type)) { > - case RDMA_TRANSPORT_IB: > + if (rdma_ib_or_iboe(id_priv->id.device, 1)) { > if (!id_priv->cm_id.ib || (id_priv->id.qp_type == IB_QPT_UD)) > ret = cma_ib_init_qp_attr(id_priv, qp_attr, qp_attr_mask); > else > @@ -745,19 +744,15 @@ int rdma_init_qp_attr(struct rdma_cm_id *id, struct ib_qp_attr *qp_attr, > > if (qp_attr->qp_state == IB_QPS_RTR) > qp_attr->rq_psn = id_priv->seq_num; > - break; > - case RDMA_TRANSPORT_IWARP: > + } else if (rdma_tech_iwarp(id_priv->id.device, 1)) { > if (!id_priv->cm_id.iw) { > qp_attr->qp_access_flags = 0; > *qp_attr_mask = IB_QP_STATE | IB_QP_ACCESS_FLAGS; > } else > ret = iw_cm_init_qp_attr(id_priv->cm_id.iw, qp_attr, > qp_attr_mask); > - break; > - default: > + } else > ret = -ENOSYS; > - break; > - } > > return ret; > } > @@ -1037,17 +1032,12 @@ void rdma_destroy_id(struct rdma_cm_id *id) > mutex_unlock(&id_priv->handler_mutex); > > if (id_priv->cma_dev) { > - switch (rdma_node_get_transport(id_priv->id.device->node_type)) { > - case RDMA_TRANSPORT_IB: > + if (rdma_ib_or_iboe(id_priv->id.device, 1)) { > if (id_priv->cm_id.ib) > ib_destroy_cm_id(id_priv->cm_id.ib); > - break; > - case RDMA_TRANSPORT_IWARP: > + } else if (rdma_tech_iwarp(id_priv->id.device, 1)) { > if (id_priv->cm_id.iw) > iw_destroy_cm_id(id_priv->cm_id.iw); > - break; > - default: > - break; > } > cma_leave_mc_groups(id_priv); > cma_release_dev(id_priv); > @@ -1626,7 +1616,7 @@ static void cma_listen_on_dev(struct rdma_id_private *id_priv, > int ret; > > if (cma_family(id_priv) == AF_IB && > - rdma_node_get_transport(cma_dev->device->node_type) != RDMA_TRANSPORT_IB) > + !rdma_ib_or_iboe(cma_dev->device, 1)) > return; > > id = rdma_create_id(cma_listen_handler, id_priv, id_priv->id.ps, > @@ -2028,7 +2018,7 @@ static int cma_bind_loopback(struct rdma_id_private *id_priv) > mutex_lock(&lock); > list_for_each_entry(cur_dev, &dev_list, list) { > if (cma_family(id_priv) == AF_IB && > - rdma_node_get_transport(cur_dev->device->node_type) != RDMA_TRANSPORT_IB) > + !rdma_ib_or_iboe(cur_dev->device, 1)) > continue; > > if (!cma_dev) > @@ -2060,7 +2050,7 @@ port_found: > goto out; > > id_priv->id.route.addr.dev_addr.dev_type = > - (rdma_port_get_link_layer(cma_dev->device, p) == IB_LINK_LAYER_INFINIBAND) ? > + (rdma_tech_ib(cma_dev->device, p)) ? > ARPHRD_INFINIBAND : ARPHRD_ETHER; > > rdma_addr_set_sgid(&id_priv->id.route.addr.dev_addr, &gid); > @@ -2537,18 +2527,15 @@ int rdma_listen(struct rdma_cm_id *id, int backlog) > > id_priv->backlog = backlog; > if (id->device) { > - switch (rdma_node_get_transport(id->device->node_type)) { > - case RDMA_TRANSPORT_IB: > + if (rdma_ib_or_iboe(id->device, 1)) { > ret = cma_ib_listen(id_priv); > if (ret) > goto err; > - break; > - case RDMA_TRANSPORT_IWARP: > + } else if (rdma_tech_iwarp(id->device, 1)) { > ret = cma_iw_listen(id_priv, backlog); > if (ret) > goto err; > - break; > - default: > + } else { > ret = -ENOSYS; > goto err; > } > @@ -2884,20 +2871,15 @@ int rdma_connect(struct rdma_cm_id *id, struct rdma_conn_param *conn_param) > id_priv->srq = conn_param->srq; > } > > - switch (rdma_node_get_transport(id->device->node_type)) { > - case RDMA_TRANSPORT_IB: > + if (rdma_ib_or_iboe(id->device, 1)) { > if (id->qp_type == IB_QPT_UD) > ret = cma_resolve_ib_udp(id_priv, conn_param); > else > ret = cma_connect_ib(id_priv, conn_param); > - break; > - case RDMA_TRANSPORT_IWARP: > + } else if (rdma_tech_iwarp(id->device, 1)) > ret = cma_connect_iw(id_priv, conn_param); > - break; > - default: > + else > ret = -ENOSYS; > - break; > - } > if (ret) > goto err; > > @@ -3000,8 +2982,7 @@ int rdma_accept(struct rdma_cm_id *id, struct rdma_conn_param *conn_param) > id_priv->srq = conn_param->srq; > } > > - switch (rdma_node_get_transport(id->device->node_type)) { > - case RDMA_TRANSPORT_IB: > + if (rdma_ib_or_iboe(id->device, 1)) { > if (id->qp_type == IB_QPT_UD) { > if (conn_param) > ret = cma_send_sidr_rep(id_priv, IB_SIDR_SUCCESS, > @@ -3017,14 +2998,10 @@ int rdma_accept(struct rdma_cm_id *id, struct rdma_conn_param *conn_param) > else > ret = cma_rep_recv(id_priv); > } > - break; > - case RDMA_TRANSPORT_IWARP: > + } else if (rdma_tech_iwarp(id->device, 1)) > ret = cma_accept_iw(id_priv, conn_param); > - break; > - default: > + else > ret = -ENOSYS; > - break; > - } > > if (ret) > goto reject; > @@ -3068,8 +3045,7 @@ int rdma_reject(struct rdma_cm_id *id, const void *private_data, > if (!id_priv->cm_id.ib) > return -EINVAL; > > - switch (rdma_node_get_transport(id->device->node_type)) { > - case RDMA_TRANSPORT_IB: > + if (rdma_ib_or_iboe(id->device, 1)) { > if (id->qp_type == IB_QPT_UD) > ret = cma_send_sidr_rep(id_priv, IB_SIDR_REJECT, 0, > private_data, private_data_len); > @@ -3077,15 +3053,12 @@ int rdma_reject(struct rdma_cm_id *id, const void *private_data, > ret = ib_send_cm_rej(id_priv->cm_id.ib, > IB_CM_REJ_CONSUMER_DEFINED, NULL, > 0, private_data, private_data_len); > - break; > - case RDMA_TRANSPORT_IWARP: > + } else if (rdma_tech_iwarp(id->device, 1)) { > ret = iw_cm_reject(id_priv->cm_id.iw, > private_data, private_data_len); > - break; > - default: > + } else > ret = -ENOSYS; > - break; > - } > + > return ret; > } > EXPORT_SYMBOL(rdma_reject); > @@ -3099,22 +3072,18 @@ int rdma_disconnect(struct rdma_cm_id *id) > if (!id_priv->cm_id.ib) > return -EINVAL; > > - switch (rdma_node_get_transport(id->device->node_type)) { > - case RDMA_TRANSPORT_IB: > + if (rdma_ib_or_iboe(id->device, 1)) { > ret = cma_modify_qp_err(id_priv); > if (ret) > goto out; > /* Initiate or respond to a disconnect. */ > if (ib_send_cm_dreq(id_priv->cm_id.ib, NULL, 0)) > ib_send_cm_drep(id_priv->cm_id.ib, NULL, 0); > - break; > - case RDMA_TRANSPORT_IWARP: > + } else if (rdma_tech_iwarp(id->device, 1)) { > ret = iw_cm_disconnect(id_priv->cm_id.iw, 0); > - break; > - default: > + } else > ret = -EINVAL; > - break; > - } > + > out: > return ret; > } > diff --git a/drivers/infiniband/core/ucm.c b/drivers/infiniband/core/ucm.c > index f2f6393..70e0ccb 100644 > --- a/drivers/infiniband/core/ucm.c > +++ b/drivers/infiniband/core/ucm.c > @@ -1253,8 +1253,7 @@ static void ib_ucm_add_one(struct ib_device *device) > dev_t base; > struct ib_ucm_device *ucm_dev; > > - if (!device->alloc_ucontext || > - rdma_node_get_transport(device->node_type) != RDMA_TRANSPORT_IB) > + if (!device->alloc_ucontext || !rdma_ib_or_iboe(device, 1)) > return; > > ucm_dev = kzalloc(sizeof *ucm_dev, GFP_KERNEL); -- 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 04/16/2015 03:10 PM, Hal Rosenstock wrote: > On 4/16/2015 4:08 AM, Michael Wang wrote: >> >> Use raw management helpers to reform cm related part in IB-core cma/ucm. >> >> These checks focus on the device cm type rather than the port capability, >> directly pass port 1 works currently, but can't support mixing cm type >> device in future. > > This is equivalent to today where the checks are per node rather than > per port. > > Should all checks here be port 1 based or only certain ones like listen > ? For example, in connect/reject/disconnect, don't we already have port > ? Guess this can be dealt with later as this is not a regression from > the current implementation. Yeah, these parts of cma may need more carve in future, like some new callback for different CM type as Sean suggested. Maybe directly using 1 could help to highlight the problem ;-) Regards, Michael Wanga > > -- Hal > >> >> Cc: Steve Wise <swise@opengridcomputing.com> >> Cc: Tom Talpey <tom@talpey.com> >> Cc: Jason Gunthorpe <jgunthorpe@obsidianresearch.com> >> Cc: Doug Ledford <dledford@redhat.com> >> Cc: Ira Weiny <ira.weiny@intel.com> >> Cc: Sean Hefty <sean.hefty@intel.com> >> Signed-off-by: Michael Wang <yun.wang@profitbricks.com> >> --- >> drivers/infiniband/core/cma.c | 81 +++++++++++++------------------------------ >> drivers/infiniband/core/ucm.c | 3 +- >> 2 files changed, 26 insertions(+), 58 deletions(-) >> >> diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c >> index d570030..6b8a64d 100644 >> --- a/drivers/infiniband/core/cma.c >> +++ b/drivers/infiniband/core/cma.c >> @@ -735,8 +735,7 @@ int rdma_init_qp_attr(struct rdma_cm_id *id, struct ib_qp_attr *qp_attr, >> int ret = 0; >> >> id_priv = container_of(id, struct rdma_id_private, id); >> - switch (rdma_node_get_transport(id_priv->id.device->node_type)) { >> - case RDMA_TRANSPORT_IB: >> + if (rdma_ib_or_iboe(id_priv->id.device, 1)) { >> if (!id_priv->cm_id.ib || (id_priv->id.qp_type == IB_QPT_UD)) >> ret = cma_ib_init_qp_attr(id_priv, qp_attr, qp_attr_mask); >> else >> @@ -745,19 +744,15 @@ int rdma_init_qp_attr(struct rdma_cm_id *id, struct ib_qp_attr *qp_attr, >> >> if (qp_attr->qp_state == IB_QPS_RTR) >> qp_attr->rq_psn = id_priv->seq_num; >> - break; >> - case RDMA_TRANSPORT_IWARP: >> + } else if (rdma_tech_iwarp(id_priv->id.device, 1)) { >> if (!id_priv->cm_id.iw) { >> qp_attr->qp_access_flags = 0; >> *qp_attr_mask = IB_QP_STATE | IB_QP_ACCESS_FLAGS; >> } else >> ret = iw_cm_init_qp_attr(id_priv->cm_id.iw, qp_attr, >> qp_attr_mask); >> - break; >> - default: >> + } else >> ret = -ENOSYS; >> - break; >> - } >> >> return ret; >> } >> @@ -1037,17 +1032,12 @@ void rdma_destroy_id(struct rdma_cm_id *id) >> mutex_unlock(&id_priv->handler_mutex); >> >> if (id_priv->cma_dev) { >> - switch (rdma_node_get_transport(id_priv->id.device->node_type)) { >> - case RDMA_TRANSPORT_IB: >> + if (rdma_ib_or_iboe(id_priv->id.device, 1)) { >> if (id_priv->cm_id.ib) >> ib_destroy_cm_id(id_priv->cm_id.ib); >> - break; >> - case RDMA_TRANSPORT_IWARP: >> + } else if (rdma_tech_iwarp(id_priv->id.device, 1)) { >> if (id_priv->cm_id.iw) >> iw_destroy_cm_id(id_priv->cm_id.iw); >> - break; >> - default: >> - break; >> } >> cma_leave_mc_groups(id_priv); >> cma_release_dev(id_priv); >> @@ -1626,7 +1616,7 @@ static void cma_listen_on_dev(struct rdma_id_private *id_priv, >> int ret; >> >> if (cma_family(id_priv) == AF_IB && >> - rdma_node_get_transport(cma_dev->device->node_type) != RDMA_TRANSPORT_IB) >> + !rdma_ib_or_iboe(cma_dev->device, 1)) >> return; >> >> id = rdma_create_id(cma_listen_handler, id_priv, id_priv->id.ps, >> @@ -2028,7 +2018,7 @@ static int cma_bind_loopback(struct rdma_id_private *id_priv) >> mutex_lock(&lock); >> list_for_each_entry(cur_dev, &dev_list, list) { >> if (cma_family(id_priv) == AF_IB && >> - rdma_node_get_transport(cur_dev->device->node_type) != RDMA_TRANSPORT_IB) >> + !rdma_ib_or_iboe(cur_dev->device, 1)) >> continue; >> >> if (!cma_dev) >> @@ -2060,7 +2050,7 @@ port_found: >> goto out; >> >> id_priv->id.route.addr.dev_addr.dev_type = >> - (rdma_port_get_link_layer(cma_dev->device, p) == IB_LINK_LAYER_INFINIBAND) ? >> + (rdma_tech_ib(cma_dev->device, p)) ? >> ARPHRD_INFINIBAND : ARPHRD_ETHER; >> >> rdma_addr_set_sgid(&id_priv->id.route.addr.dev_addr, &gid); >> @@ -2537,18 +2527,15 @@ int rdma_listen(struct rdma_cm_id *id, int backlog) >> >> id_priv->backlog = backlog; >> if (id->device) { >> - switch (rdma_node_get_transport(id->device->node_type)) { >> - case RDMA_TRANSPORT_IB: >> + if (rdma_ib_or_iboe(id->device, 1)) { >> ret = cma_ib_listen(id_priv); >> if (ret) >> goto err; >> - break; >> - case RDMA_TRANSPORT_IWARP: >> + } else if (rdma_tech_iwarp(id->device, 1)) { >> ret = cma_iw_listen(id_priv, backlog); >> if (ret) >> goto err; >> - break; >> - default: >> + } else { >> ret = -ENOSYS; >> goto err; >> } >> @@ -2884,20 +2871,15 @@ int rdma_connect(struct rdma_cm_id *id, struct rdma_conn_param *conn_param) >> id_priv->srq = conn_param->srq; >> } >> >> - switch (rdma_node_get_transport(id->device->node_type)) { >> - case RDMA_TRANSPORT_IB: >> + if (rdma_ib_or_iboe(id->device, 1)) { >> if (id->qp_type == IB_QPT_UD) >> ret = cma_resolve_ib_udp(id_priv, conn_param); >> else >> ret = cma_connect_ib(id_priv, conn_param); >> - break; >> - case RDMA_TRANSPORT_IWARP: >> + } else if (rdma_tech_iwarp(id->device, 1)) >> ret = cma_connect_iw(id_priv, conn_param); >> - break; >> - default: >> + else >> ret = -ENOSYS; >> - break; >> - } >> if (ret) >> goto err; >> >> @@ -3000,8 +2982,7 @@ int rdma_accept(struct rdma_cm_id *id, struct rdma_conn_param *conn_param) >> id_priv->srq = conn_param->srq; >> } >> >> - switch (rdma_node_get_transport(id->device->node_type)) { >> - case RDMA_TRANSPORT_IB: >> + if (rdma_ib_or_iboe(id->device, 1)) { >> if (id->qp_type == IB_QPT_UD) { >> if (conn_param) >> ret = cma_send_sidr_rep(id_priv, IB_SIDR_SUCCESS, >> @@ -3017,14 +2998,10 @@ int rdma_accept(struct rdma_cm_id *id, struct rdma_conn_param *conn_param) >> else >> ret = cma_rep_recv(id_priv); >> } >> - break; >> - case RDMA_TRANSPORT_IWARP: >> + } else if (rdma_tech_iwarp(id->device, 1)) >> ret = cma_accept_iw(id_priv, conn_param); >> - break; >> - default: >> + else >> ret = -ENOSYS; >> - break; >> - } >> >> if (ret) >> goto reject; >> @@ -3068,8 +3045,7 @@ int rdma_reject(struct rdma_cm_id *id, const void *private_data, >> if (!id_priv->cm_id.ib) >> return -EINVAL; >> >> - switch (rdma_node_get_transport(id->device->node_type)) { >> - case RDMA_TRANSPORT_IB: >> + if (rdma_ib_or_iboe(id->device, 1)) { >> if (id->qp_type == IB_QPT_UD) >> ret = cma_send_sidr_rep(id_priv, IB_SIDR_REJECT, 0, >> private_data, private_data_len); >> @@ -3077,15 +3053,12 @@ int rdma_reject(struct rdma_cm_id *id, const void *private_data, >> ret = ib_send_cm_rej(id_priv->cm_id.ib, >> IB_CM_REJ_CONSUMER_DEFINED, NULL, >> 0, private_data, private_data_len); >> - break; >> - case RDMA_TRANSPORT_IWARP: >> + } else if (rdma_tech_iwarp(id->device, 1)) { >> ret = iw_cm_reject(id_priv->cm_id.iw, >> private_data, private_data_len); >> - break; >> - default: >> + } else >> ret = -ENOSYS; >> - break; >> - } >> + >> return ret; >> } >> EXPORT_SYMBOL(rdma_reject); >> @@ -3099,22 +3072,18 @@ int rdma_disconnect(struct rdma_cm_id *id) >> if (!id_priv->cm_id.ib) >> return -EINVAL; >> >> - switch (rdma_node_get_transport(id->device->node_type)) { >> - case RDMA_TRANSPORT_IB: >> + if (rdma_ib_or_iboe(id->device, 1)) { >> ret = cma_modify_qp_err(id_priv); >> if (ret) >> goto out; >> /* Initiate or respond to a disconnect. */ >> if (ib_send_cm_dreq(id_priv->cm_id.ib, NULL, 0)) >> ib_send_cm_drep(id_priv->cm_id.ib, NULL, 0); >> - break; >> - case RDMA_TRANSPORT_IWARP: >> + } else if (rdma_tech_iwarp(id->device, 1)) { >> ret = iw_cm_disconnect(id_priv->cm_id.iw, 0); >> - break; >> - default: >> + } else >> ret = -EINVAL; >> - break; >> - } >> + >> out: >> return ret; >> } >> diff --git a/drivers/infiniband/core/ucm.c b/drivers/infiniband/core/ucm.c >> index f2f6393..70e0ccb 100644 >> --- a/drivers/infiniband/core/ucm.c >> +++ b/drivers/infiniband/core/ucm.c >> @@ -1253,8 +1253,7 @@ static void ib_ucm_add_one(struct ib_device *device) >> dev_t base; >> struct ib_ucm_device *ucm_dev; >> >> - if (!device->alloc_ucontext || >> - rdma_node_get_transport(device->node_type) != RDMA_TRANSPORT_IB) >> + if (!device->alloc_ucontext || !rdma_ib_or_iboe(device, 1)) >> return; >> >> ucm_dev = kzalloc(sizeof *ucm_dev, GFP_KERNEL); > -- 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
PiA+IFRoaXMgaXMgZXF1aXZhbGVudCB0byB0b2RheSB3aGVyZSB0aGUgY2hlY2tzIGFyZSBwZXIg bm9kZSByYXRoZXIgdGhhbg0KPiA+IHBlciBwb3J0Lg0KPiA+DQo+ID4gU2hvdWxkIGFsbCBjaGVj a3MgaGVyZSBiZSBwb3J0IDEgYmFzZWQgb3Igb25seSBjZXJ0YWluIG9uZXMgbGlrZSBsaXN0ZW4N Cj4gPiA/IEZvciBleGFtcGxlLCBpbiBjb25uZWN0L3JlamVjdC9kaXNjb25uZWN0LCBkb24ndCB3 ZSBhbHJlYWR5IGhhdmUgcG9ydA0KPiA+ID8gR3Vlc3MgdGhpcyBjYW4gYmUgZGVhbHQgd2l0aCBs YXRlciBhcyB0aGlzIGlzIG5vdCBhIHJlZ3Jlc3Npb24gZnJvbQ0KPiA+IHRoZSBjdXJyZW50IGlt cGxlbWVudGF0aW9uLg0KPiANCj4gWWVhaCwgdGhlc2UgcGFydHMgb2YgY21hIG1heSBuZWVkIG1v cmUgY2FydmUgaW4gZnV0dXJlLCBsaWtlIHNvbWUgbmV3DQo+IGNhbGxiYWNrDQo+IGZvciBkaWZm ZXJlbnQgQ00gdHlwZSBhcyBTZWFuIHN1Z2dlc3RlZC4NCj4gDQo+IE1heWJlIGRpcmVjdGx5IHVz aW5nIDEgY291bGQgaGVscCB0byBoaWdobGlnaHQgdGhlIHByb2JsZW0gOy0pDQoNCk9ubHkgYSBm ZXcgY2hlY2tzIG5lZWQgdG8gYmUgcGVyIGRldmljZS4gIEkgdGhpbmsgSSBwb2ludGVkIHRob3Nl IG91dCBwcmV2aW91c2x5LiAgVGVzdGluZyBzaG91bGQgc2hvdyBhbnl3aGVyZSB0aGF0IHdlIG1p c3MgZmFpcmx5IHF1aWNrbHksIHNpbmNlIHBvcnQgd291bGQgc3RpbGwgYmUgMC4gIEZvciB0aGUg Y2hlY2tzIHRoYXQgY2FuIGJlIHVwZGF0ZWQgdG8gYmUgcGVyIHBvcnQsIEkgd291bGQgcmF0aGVy IGdvIGFoZWFkIGFuZCBjb252ZXJ0IHRoZW0uDQoNCi0gU2Vhbg0K -- 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 04/16/2015 04:31 PM, Hefty, Sean wrote: >>> This is equivalent to today where the checks are per node rather than >>> per port. >>> >>> Should all checks here be port 1 based or only certain ones like listen >>> ? For example, in connect/reject/disconnect, don't we already have port >>> ? Guess this can be dealt with later as this is not a regression from >>> the current implementation. >> >> Yeah, these parts of cma may need more carve in future, like some new >> callback >> for different CM type as Sean suggested. >> >> Maybe directly using 1 could help to highlight the problem ;-) > > Only a few checks need to be per device. I think I pointed those out previously. Testing should show anywhere that we miss fairly quickly, since port would still be 0. For the checks that can be updated to be per port, I would rather go ahead and convert them. Got it, will be changed in next version :-) To be confirmed: PORT ASSIGNED rdma_init_qp_attr Y rdma_destroy_id unknown cma_listen_on_dev N cma_bind_loopback N rdma_listen N rdma_connect Y rdma_accept Y rdma_reject Y rdma_disconnect Y ib_ucm_add_one N Is this list correct? Regards, Michael Wang > > - 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
On Thu, Apr 16, 2015 at 10:08:10AM +0200, Michael Wang wrote: > > Use raw management helpers to reform cm related part in IB-core cma/ucm. > > These checks focus on the device cm type rather than the port capability, > directly pass port 1 works currently, but can't support mixing cm type > device in future. After the discussion settled, I ended up thinking that implementing explicit device checks, for use by CM, and the BUG_ON at register to require all ports have the same value was the best option. It also looks like hardwired 1 won't work on switch ports, so it is no-go. Jason -- 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
> After the discussion settled, I ended up thinking that implementing > explicit device checks, for use by CM, and the BUG_ON at register to > require all ports have the same value was the best option. Sure, but why not update the other areas anyway? This way when listens become per port, rather than per device, we only need to update that portion of the code. -- 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 Thu, Apr 16, 2015 at 04:55:10PM +0000, Hefty, Sean wrote: > > After the discussion settled, I ended up thinking that implementing > > explicit device checks, for use by CM, and the BUG_ON at register to > > require all ports have the same value was the best option. > > Sure, but why not update the other areas anyway? This way when > listens become per port, rather than per device, we only need to > update that portion of the code. I wasn't clear, I agree: Yes, update what is possible in the CM, but use an explicit device test for the areas we can't fix. Jason -- 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 4/16/2015 11:22 AM, Michael Wang wrote: > > > On 04/16/2015 04:31 PM, Hefty, Sean wrote: >>>> This is equivalent to today where the checks are per node rather than >>>> per port. >>>> >>>> Should all checks here be port 1 based or only certain ones like listen >>>> ? For example, in connect/reject/disconnect, don't we already have port >>>> ? Guess this can be dealt with later as this is not a regression from >>>> the current implementation. >>> >>> Yeah, these parts of cma may need more carve in future, like some new >>> callback >>> for different CM type as Sean suggested. >>> >>> Maybe directly using 1 could help to highlight the problem ;-) >> >> Only a few checks need to be per device. I think I pointed those out previously. Testing should show anywhere that we miss fairly quickly, since port would still be 0. For the checks that can be updated to be per port, I would rather go ahead and convert them. > > Got it, will be changed in next version :-) > > To be confirmed: > PORT ASSIGNED > rdma_init_qp_attr Y > rdma_destroy_id unknown > cma_listen_on_dev N > cma_bind_loopback N > rdma_listen N Why "N"? rdma_listen() can be constrained to a single port, right? And even if wildcarded, it needs to act on multiple ports, which is to say, it will fail only if no ports are eligible. Tom. > rdma_connect Y > rdma_accept Y > rdma_reject Y > rdma_disconnect Y > ib_ucm_add_one N > > Is this list correct? > > Regards, > Michael Wang > >> >> - 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
> > To be confirmed: > > PORT ASSIGNED > > rdma_init_qp_attr Y > > rdma_destroy_id unknown > > cma_listen_on_dev N > > cma_bind_loopback N Bind loopback will attach to a port, but the id does not have on entry. > > rdma_listen N > > Why "N"? rdma_listen() can be constrained to a single port, right? > And even if wildcarded, it needs to act on multiple ports, which is > to say, it will fail only if no ports are eligible. Rdma listen should be unknown. The id may be assigned to a port. It depends on the source address. > > rdma_connect Y > > rdma_accept Y > > rdma_reject Y > > rdma_disconnect Y > > ib_ucm_add_one N Others look correct. Btw, thanks for your work on this. I know this is becoming a much bigger change than you originally intended. :) - Sean
On 4/16/2015 11:58 AM, Jason Gunthorpe wrote:
> It also looks like hardwired 1 won't work on switch ports, so it is no-go.
AFAIK enhanced switch port 0 is not supported by CM/RDMA CM in the
current code. There is no need for CM/RDMA CM on base switch port 0.
--
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 4/16/2015 11:58 AM, Jason Gunthorpe wrote: > > It also looks like hardwired 1 won't work on switch ports, so it is no-go. > > AFAIK enhanced switch port 0 is not supported by CM/RDMA CM in the current > code. There is no need for CM/RDMA CM on base switch port 0. I concur and I thought I mentioned that before. Ira -- 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 Thu, Apr 16, 2015 at 01:38:07PM -0400, Hal Rosenstock wrote: > On 4/16/2015 11:58 AM, Jason Gunthorpe wrote: > > It also looks like hardwired 1 won't work on switch ports, so it is no-go. > > AFAIK enhanced switch port 0 is not supported by CM/RDMA CM in the > current code. There is no need for CM/RDMA CM on base switch port 0. Hurm, yet another reason to have a proper device level test to capture all this weirdness. I am surprised to hear this, actually. Jason -- 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 04/16/2015 07:21 PM, Tom Talpey wrote: > On 4/16/2015 11:22 AM, Michael Wang wrote: >> >> >> On 04/16/2015 04:31 PM, Hefty, Sean wrote: >>>>> This is equivalent to today where the checks are per node rather than >>>>> per port. >>>>> >>>>> Should all checks here be port 1 based or only certain ones like listen >>>>> ? For example, in connect/reject/disconnect, don't we already have port >>>>> ? Guess this can be dealt with later as this is not a regression from >>>>> the current implementation. >>>> >>>> Yeah, these parts of cma may need more carve in future, like some new >>>> callback >>>> for different CM type as Sean suggested. >>>> >>>> Maybe directly using 1 could help to highlight the problem ;-) >>> >>> Only a few checks need to be per device. I think I pointed those out previously. Testing should show anywhere that we miss fairly quickly, since port would still be 0. For the checks that can be updated to be per port, I would rather go ahead and convert them. >> >> Got it, will be changed in next version :-) >> >> To be confirmed: >> PORT ASSIGNED >> rdma_init_qp_attr Y >> rdma_destroy_id unknown >> cma_listen_on_dev N >> cma_bind_loopback N >> rdma_listen N > > Why "N"? rdma_listen() can be constrained to a single port, right? > And even if wildcarded, it needs to act on multiple ports, which is > to say, it will fail only if no ports are eligible. Yeah, it can or can't, maybe 'unknown' is better :-) Regards, Michael Wang > > Tom. > > >> rdma_connect Y >> rdma_accept Y >> rdma_reject Y >> rdma_disconnect Y >> ib_ucm_add_one N >> >> Is this list correct? >> >> Regards, >> Michael Wang >> >>> >>> - 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
On 04/16/2015 07:30 PM, Hefty, Sean wrote: >>> To be confirmed: >>> PORT ASSIGNED >>> rdma_init_qp_attr Y >>> rdma_destroy_id unknown >>> cma_listen_on_dev N >>> cma_bind_loopback N > > Bind loopback will attach to a port, but the id does not have on entry. > >>> rdma_listen N >> >> Why "N"? rdma_listen() can be constrained to a single port, right? >> And even if wildcarded, it needs to act on multiple ports, which is >> to say, it will fail only if no ports are eligible. > > Rdma listen should be unknown. The id may be assigned to a port. It depends on the source address. Agree, so for those 'N' or 'unknow', let's use port 1 directly. > >>> rdma_connect Y >>> rdma_accept Y >>> rdma_reject Y >>> rdma_disconnect Y >>> ib_ucm_add_one N > > Others look correct. > > Btw, thanks for your work on this. I know this is becoming a much bigger change than you originally intended. :) My pleasure :-) It do exceeded my expectations, fortunately I have you guys on my side, without your comments, it's impossible to make it so far :-) Regards, Michael Wang > > - 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
On 04/16/2015 05:58 PM, Jason Gunthorpe wrote: > On Thu, Apr 16, 2015 at 10:08:10AM +0200, Michael Wang wrote: >> >> Use raw management helpers to reform cm related part in IB-core cma/ucm. >> >> These checks focus on the device cm type rather than the port capability, >> directly pass port 1 works currently, but can't support mixing cm type >> device in future. > > After the discussion settled, I ended up thinking that implementing > explicit device checks, for use by CM, and the BUG_ON at register to > require all ports have the same value was the best option. > > It also looks like hardwired 1 won't work on switch ports, so it is no-go. AFAIK, the current HW won't trigger such Bug, actually only mlx4 using port_num to check the link-layer (but still ib cm anyway), others are just static whatever the port_num is. Thus as long as the check is still count on transport type and link-layer, the BUG may never be triggered... Regards, Michael Wang > > Jason > -- 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/cma.c b/drivers/infiniband/core/cma.c index d570030..6b8a64d 100644 --- a/drivers/infiniband/core/cma.c +++ b/drivers/infiniband/core/cma.c @@ -735,8 +735,7 @@ int rdma_init_qp_attr(struct rdma_cm_id *id, struct ib_qp_attr *qp_attr, int ret = 0; id_priv = container_of(id, struct rdma_id_private, id); - switch (rdma_node_get_transport(id_priv->id.device->node_type)) { - case RDMA_TRANSPORT_IB: + if (rdma_ib_or_iboe(id_priv->id.device, 1)) { if (!id_priv->cm_id.ib || (id_priv->id.qp_type == IB_QPT_UD)) ret = cma_ib_init_qp_attr(id_priv, qp_attr, qp_attr_mask); else @@ -745,19 +744,15 @@ int rdma_init_qp_attr(struct rdma_cm_id *id, struct ib_qp_attr *qp_attr, if (qp_attr->qp_state == IB_QPS_RTR) qp_attr->rq_psn = id_priv->seq_num; - break; - case RDMA_TRANSPORT_IWARP: + } else if (rdma_tech_iwarp(id_priv->id.device, 1)) { if (!id_priv->cm_id.iw) { qp_attr->qp_access_flags = 0; *qp_attr_mask = IB_QP_STATE | IB_QP_ACCESS_FLAGS; } else ret = iw_cm_init_qp_attr(id_priv->cm_id.iw, qp_attr, qp_attr_mask); - break; - default: + } else ret = -ENOSYS; - break; - } return ret; } @@ -1037,17 +1032,12 @@ void rdma_destroy_id(struct rdma_cm_id *id) mutex_unlock(&id_priv->handler_mutex); if (id_priv->cma_dev) { - switch (rdma_node_get_transport(id_priv->id.device->node_type)) { - case RDMA_TRANSPORT_IB: + if (rdma_ib_or_iboe(id_priv->id.device, 1)) { if (id_priv->cm_id.ib) ib_destroy_cm_id(id_priv->cm_id.ib); - break; - case RDMA_TRANSPORT_IWARP: + } else if (rdma_tech_iwarp(id_priv->id.device, 1)) { if (id_priv->cm_id.iw) iw_destroy_cm_id(id_priv->cm_id.iw); - break; - default: - break; } cma_leave_mc_groups(id_priv); cma_release_dev(id_priv); @@ -1626,7 +1616,7 @@ static void cma_listen_on_dev(struct rdma_id_private *id_priv, int ret; if (cma_family(id_priv) == AF_IB && - rdma_node_get_transport(cma_dev->device->node_type) != RDMA_TRANSPORT_IB) + !rdma_ib_or_iboe(cma_dev->device, 1)) return; id = rdma_create_id(cma_listen_handler, id_priv, id_priv->id.ps, @@ -2028,7 +2018,7 @@ static int cma_bind_loopback(struct rdma_id_private *id_priv) mutex_lock(&lock); list_for_each_entry(cur_dev, &dev_list, list) { if (cma_family(id_priv) == AF_IB && - rdma_node_get_transport(cur_dev->device->node_type) != RDMA_TRANSPORT_IB) + !rdma_ib_or_iboe(cur_dev->device, 1)) continue; if (!cma_dev) @@ -2060,7 +2050,7 @@ port_found: goto out; id_priv->id.route.addr.dev_addr.dev_type = - (rdma_port_get_link_layer(cma_dev->device, p) == IB_LINK_LAYER_INFINIBAND) ? + (rdma_tech_ib(cma_dev->device, p)) ? ARPHRD_INFINIBAND : ARPHRD_ETHER; rdma_addr_set_sgid(&id_priv->id.route.addr.dev_addr, &gid); @@ -2537,18 +2527,15 @@ int rdma_listen(struct rdma_cm_id *id, int backlog) id_priv->backlog = backlog; if (id->device) { - switch (rdma_node_get_transport(id->device->node_type)) { - case RDMA_TRANSPORT_IB: + if (rdma_ib_or_iboe(id->device, 1)) { ret = cma_ib_listen(id_priv); if (ret) goto err; - break; - case RDMA_TRANSPORT_IWARP: + } else if (rdma_tech_iwarp(id->device, 1)) { ret = cma_iw_listen(id_priv, backlog); if (ret) goto err; - break; - default: + } else { ret = -ENOSYS; goto err; } @@ -2884,20 +2871,15 @@ int rdma_connect(struct rdma_cm_id *id, struct rdma_conn_param *conn_param) id_priv->srq = conn_param->srq; } - switch (rdma_node_get_transport(id->device->node_type)) { - case RDMA_TRANSPORT_IB: + if (rdma_ib_or_iboe(id->device, 1)) { if (id->qp_type == IB_QPT_UD) ret = cma_resolve_ib_udp(id_priv, conn_param); else ret = cma_connect_ib(id_priv, conn_param); - break; - case RDMA_TRANSPORT_IWARP: + } else if (rdma_tech_iwarp(id->device, 1)) ret = cma_connect_iw(id_priv, conn_param); - break; - default: + else ret = -ENOSYS; - break; - } if (ret) goto err; @@ -3000,8 +2982,7 @@ int rdma_accept(struct rdma_cm_id *id, struct rdma_conn_param *conn_param) id_priv->srq = conn_param->srq; } - switch (rdma_node_get_transport(id->device->node_type)) { - case RDMA_TRANSPORT_IB: + if (rdma_ib_or_iboe(id->device, 1)) { if (id->qp_type == IB_QPT_UD) { if (conn_param) ret = cma_send_sidr_rep(id_priv, IB_SIDR_SUCCESS, @@ -3017,14 +2998,10 @@ int rdma_accept(struct rdma_cm_id *id, struct rdma_conn_param *conn_param) else ret = cma_rep_recv(id_priv); } - break; - case RDMA_TRANSPORT_IWARP: + } else if (rdma_tech_iwarp(id->device, 1)) ret = cma_accept_iw(id_priv, conn_param); - break; - default: + else ret = -ENOSYS; - break; - } if (ret) goto reject; @@ -3068,8 +3045,7 @@ int rdma_reject(struct rdma_cm_id *id, const void *private_data, if (!id_priv->cm_id.ib) return -EINVAL; - switch (rdma_node_get_transport(id->device->node_type)) { - case RDMA_TRANSPORT_IB: + if (rdma_ib_or_iboe(id->device, 1)) { if (id->qp_type == IB_QPT_UD) ret = cma_send_sidr_rep(id_priv, IB_SIDR_REJECT, 0, private_data, private_data_len); @@ -3077,15 +3053,12 @@ int rdma_reject(struct rdma_cm_id *id, const void *private_data, ret = ib_send_cm_rej(id_priv->cm_id.ib, IB_CM_REJ_CONSUMER_DEFINED, NULL, 0, private_data, private_data_len); - break; - case RDMA_TRANSPORT_IWARP: + } else if (rdma_tech_iwarp(id->device, 1)) { ret = iw_cm_reject(id_priv->cm_id.iw, private_data, private_data_len); - break; - default: + } else ret = -ENOSYS; - break; - } + return ret; } EXPORT_SYMBOL(rdma_reject); @@ -3099,22 +3072,18 @@ int rdma_disconnect(struct rdma_cm_id *id) if (!id_priv->cm_id.ib) return -EINVAL; - switch (rdma_node_get_transport(id->device->node_type)) { - case RDMA_TRANSPORT_IB: + if (rdma_ib_or_iboe(id->device, 1)) { ret = cma_modify_qp_err(id_priv); if (ret) goto out; /* Initiate or respond to a disconnect. */ if (ib_send_cm_dreq(id_priv->cm_id.ib, NULL, 0)) ib_send_cm_drep(id_priv->cm_id.ib, NULL, 0); - break; - case RDMA_TRANSPORT_IWARP: + } else if (rdma_tech_iwarp(id->device, 1)) { ret = iw_cm_disconnect(id_priv->cm_id.iw, 0); - break; - default: + } else ret = -EINVAL; - break; - } + out: return ret; } diff --git a/drivers/infiniband/core/ucm.c b/drivers/infiniband/core/ucm.c index f2f6393..70e0ccb 100644 --- a/drivers/infiniband/core/ucm.c +++ b/drivers/infiniband/core/ucm.c @@ -1253,8 +1253,7 @@ static void ib_ucm_add_one(struct ib_device *device) dev_t base; struct ib_ucm_device *ucm_dev; - if (!device->alloc_ucontext || - rdma_node_get_transport(device->node_type) != RDMA_TRANSPORT_IB) + if (!device->alloc_ucontext || !rdma_ib_or_iboe(device, 1)) return; ucm_dev = kzalloc(sizeof *ucm_dev, GFP_KERNEL);
Use raw management helpers to reform cm related part in IB-core cma/ucm. These checks focus on the device cm type rather than the port capability, directly pass port 1 works currently, but can't support mixing cm type device in future. Cc: Steve Wise <swise@opengridcomputing.com> Cc: Tom Talpey <tom@talpey.com> Cc: Jason Gunthorpe <jgunthorpe@obsidianresearch.com> Cc: Doug Ledford <dledford@redhat.com> Cc: Ira Weiny <ira.weiny@intel.com> Cc: Sean Hefty <sean.hefty@intel.com> Signed-off-by: Michael Wang <yun.wang@profitbricks.com> --- drivers/infiniband/core/cma.c | 81 +++++++++++++------------------------------ drivers/infiniband/core/ucm.c | 3 +- 2 files changed, 26 insertions(+), 58 deletions(-)