| Message ID | 1431630106-28829-1-git-send-email-ira.weiny@intel.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
Hi, Le jeudi 14 mai 2015 à 15:01 -0400, ira.weiny@intel.com a écrit : > From: Ira Weiny <ira.weiny@intel.com> > > It has been decided that ROCE should be used within the kernel rather than IBOE > as we used before. Change iboe to roce on the new rdma_protocol_* functions. > Erk ... What's the usefulness of such patch ? IBoE is used throughout the IB/RDMA subsystem. Changing only these occurences is rather inconsistent. > Signed-off-by: Ira Weiny <ira.weiny@intel.com> > --- > drivers/infiniband/core/cma.c | 12 ++++++------ > drivers/infiniband/core/ucma.c | 2 +- > include/rdma/ib_verbs.h | 4 ++-- > net/sunrpc/xprtrdma/svc_rdma_transport.c | 2 +- > 4 files changed, 10 insertions(+), 10 deletions(-) > > diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c > index 1977f601a1ec..ea92a0daa61c 100644 > --- a/drivers/infiniband/core/cma.c > +++ b/drivers/infiniband/core/cma.c > @@ -391,7 +391,7 @@ static int cma_acquire_dev(struct rdma_id_private *id_priv, > if (listen_id_priv) { > cma_dev = listen_id_priv->cma_dev; > port = listen_id_priv->id.port_num; > - gidp = rdma_protocol_iboe(cma_dev->device, port) ? > + gidp = rdma_protocol_roce(cma_dev->device, port) ? > &iboe_gid : &gid; > > ret = cma_validate_port(cma_dev->device, port, gidp, > @@ -409,7 +409,7 @@ static int cma_acquire_dev(struct rdma_id_private *id_priv, > listen_id_priv->id.port_num == port) > continue; > > - gidp = rdma_protocol_iboe(cma_dev->device, port) ? > + gidp = rdma_protocol_roce(cma_dev->device, port) ? > &iboe_gid : &gid; > > ret = cma_validate_port(cma_dev->device, port, gidp, > @@ -647,7 +647,7 @@ static int cma_modify_qp_rtr(struct rdma_id_private *id_priv, > > BUG_ON(id_priv->cma_dev->device != id_priv->id.device); > > - if (rdma_protocol_iboe(id_priv->id.device, id_priv->id.port_num)) { > + if (rdma_protocol_roce(id_priv->id.device, id_priv->id.port_num)) { > ret = rdma_addr_find_smac_by_sgid(&sgid, qp_attr.smac, NULL); > > if (ret) > @@ -1966,7 +1966,7 @@ int rdma_resolve_route(struct rdma_cm_id *id, int timeout_ms) > atomic_inc(&id_priv->refcount); > if (rdma_cap_ib_sa(id->device, id->port_num)) > ret = cma_resolve_ib_route(id_priv, timeout_ms); > - else if (rdma_protocol_iboe(id->device, id->port_num)) > + else if (rdma_protocol_roce(id->device, id->port_num)) > ret = cma_resolve_iboe_route(id_priv); > else if (rdma_protocol_iwarp(id->device, id->port_num)) > ret = cma_resolve_iw_route(id_priv, timeout_ms); > @@ -3325,7 +3325,7 @@ int rdma_join_multicast(struct rdma_cm_id *id, struct sockaddr *addr, > list_add(&mc->list, &id_priv->mc_list); > spin_unlock(&id_priv->lock); > > - if (rdma_protocol_iboe(id->device, id->port_num)) { > + if (rdma_protocol_roce(id->device, id->port_num)) { > kref_init(&mc->mcref); > ret = cma_iboe_join_multicast(id_priv, mc); > } else if (rdma_cap_ib_mcast(id->device, id->port_num)) > @@ -3365,7 +3365,7 @@ void rdma_leave_multicast(struct rdma_cm_id *id, struct sockaddr *addr) > if (rdma_cap_ib_mcast(id->device, id->port_num)) { > ib_sa_free_multicast(mc->multicast.ib); > kfree(mc); > - } else if (rdma_protocol_iboe(id->device, id->port_num)) > + } else if (rdma_protocol_roce(id->device, id->port_num)) > kref_put(&mc->mcref, release_mc); > > return; > diff --git a/drivers/infiniband/core/ucma.c b/drivers/infiniband/core/ucma.c > index d42b816c781f..ad45469f7582 100644 > --- a/drivers/infiniband/core/ucma.c > +++ b/drivers/infiniband/core/ucma.c > @@ -725,7 +725,7 @@ static ssize_t ucma_query_route(struct ucma_file *file, > > if (rdma_cap_ib_sa(ctx->cm_id->device, ctx->cm_id->port_num)) > ucma_copy_ib_route(&resp, &ctx->cm_id->route); > - else if (rdma_protocol_iboe(ctx->cm_id->device, ctx->cm_id->port_num)) > + else if (rdma_protocol_roce(ctx->cm_id->device, ctx->cm_id->port_num)) > ucma_copy_iboe_route(&resp, &ctx->cm_id->route); > else if (rdma_protocol_iwarp(ctx->cm_id->device, ctx->cm_id->port_num)) > ucma_copy_iw_route(&resp, &ctx->cm_id->route); > diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h > index 29095fb6db5c..bcb87bf168c3 100644 > --- a/include/rdma/ib_verbs.h > +++ b/include/rdma/ib_verbs.h > @@ -1832,7 +1832,7 @@ static inline bool rdma_protocol_ib(struct ib_device *device, u8 port_num) > return device->port_immutable[port_num].core_cap_flags & RDMA_CORE_CAP_PROT_IB; > } > > -static inline bool rdma_protocol_iboe(struct ib_device *device, u8 port_num) > +static inline bool rdma_protocol_roce(struct ib_device *device, u8 port_num) > { > return device->port_immutable[port_num].core_cap_flags & RDMA_CORE_CAP_PROT_ROCE; > } > @@ -1842,7 +1842,7 @@ static inline bool rdma_protocol_iwarp(struct ib_device *device, u8 port_num) > return device->port_immutable[port_num].core_cap_flags & RDMA_CORE_CAP_PROT_IWARP; > } > > -static inline bool rdma_ib_or_iboe(struct ib_device *device, u8 port_num) > +static inline bool rdma_ib_or_roce(struct ib_device *device, u8 port_num) > { > return device->port_immutable[port_num].core_cap_flags & > (RDMA_CORE_CAP_PROT_IB | RDMA_CORE_CAP_PROT_ROCE); > diff --git a/net/sunrpc/xprtrdma/svc_rdma_transport.c b/net/sunrpc/xprtrdma/svc_rdma_transport.c > index 3df8320c6efe..3f5750cf187e 100644 > --- a/net/sunrpc/xprtrdma/svc_rdma_transport.c > +++ b/net/sunrpc/xprtrdma/svc_rdma_transport.c > @@ -987,7 +987,7 @@ static struct svc_xprt *svc_rdma_accept(struct svc_xprt *xprt) > */ > if (!rdma_protocol_iwarp(newxprt->sc_cm_id->device, > newxprt->sc_cm_id->port_num) && > - !rdma_ib_or_iboe(newxprt->sc_cm_id->device, > + !rdma_ib_or_roce(newxprt->sc_cm_id->device, > newxprt->sc_cm_id->port_num)) > goto errout; > Regards.
> > Hi, > > Le jeudi 14 mai 2015 à 15:01 -0400, ira.weiny@intel.com a écrit : > > From: Ira Weiny <ira.weiny@intel.com> > > > > It has been decided that ROCE should be used within the kernel rather > > than IBOE as we used before. Change iboe to roce on the new > rdma_protocol_* functions. > > > > Erk ... What's the usefulness of such patch ? Currently there is inconsistency in the naming of the RoCE technology. 12:27:35 > grep -r roce drivers/infiniband | grep -v Binary | grep -i -v proce | wc -l 85 All in the driver code. 12:27:48 > grep -r iboe drivers/infiniband | grep -v Binary | wc -l 130 Mainly in the core and mlx4 driver. This patch was to clean up the management helper function as a general move toward standardizing on roce rather than iboe. Most people in the community refer to this as "RoCE" so that name was chosen to move to. > > IBoE is used throughout the IB/RDMA subsystem. > > Changing only these occurences is rather inconsistent. > I asked about changing all the references and Doug mentioned he would make a patch to change the other references. Personally I don't want to see a massive rename patch but this could be done. Ira
Hi, Le vendredi 15 mai 2015 à 16:29 +0000, Weiny, Ira a écrit : > > Le jeudi 14 mai 2015 à 15:01 -0400, ira.weiny@intel.com a écrit : > > > From: Ira Weiny <ira.weiny@intel.com> > > > > > > It has been decided that ROCE should be used within the kernel rather > > > than IBOE as we used before. Change iboe to roce on the new > > rdma_protocol_* functions. > > > > > > > Erk ... What's the usefulness of such patch > > Currently there is inconsistency in the naming of the RoCE technology. > > 12:27:35 > grep -r roce drivers/infiniband | grep -v Binary | grep -i -v proce | wc -l > 85 > > All in the driver code. > > 12:27:48 > grep -r iboe drivers/infiniband | grep -v Binary | wc -l > 130 > > Mainly in the core and mlx4 driver. > I see thing a bit differently: $ grep -ri 'roce' include/rdma/ \ include/uapi/rdma \ include/uapi/linux/if_infiniband.h \ drivers/infiniband/core | grep -vi proce | wc -l 0 $ grep -ri 'iboe' include/rdma/ \ include/uapi/rdma \ include/uapi/linux/if_infiniband.h \ drivers/infiniband/core | grep -vi proce | wc -l 30 (On next-20150515). I believe the drivers can have the names they want, especially ocrdma, Emulex OneConnect RoCE. > This patch was to clean up the management helper function as a general > move toward standardizing on roce rather than iboe. Most people in > the community refer to this as "RoCE" so that name was chosen to move > to. > It's not the first time Linux use a name not matching the "vendor" one, amd64, arm64, etc. > > > > IBoE is used throughout the IB/RDMA subsystem. > > > > Changing only these occurences is rather inconsistent. > > > > I asked about changing all the references and Doug mentioned he would > make a patch to change the other references. Personally I don't want > to see a massive rename patch but this could be done. > That's my main concern: I dislike patch that change such large portion of code, for not well defined purpose. I think "It has been decided that ROCE should be used within" is not enough to justify the change. But "Most people in the community refer to this as "RoCE" so that name was chosen to move to." sound a bit better as an explanation. And now, I'm stopping bikeshedding :) Regards.
On Fri, 2015-05-15 at 23:32 +0200, Yann Droneaud wrote: > I think "It has been decided that ROCE should be used within" is not > enough to justify the change. > > But "Most people in the community refer to this as "RoCE" so that name > was chosen to move to." sound a bit better as an explanation. Which was more or less the conclusion of the discussion about the naming. It's not a big deal, but we did reach a general consensus to move to roce as the standard name instead of iboe.
Hi, At present we have chosen to call it RoCE. However I believe IBoE is more generic for the reason where, If the future RoCE versions may work on non-converged Ethernet, IBoE would be more appropriate name in long haul. Parav -----Original Message----- From: linux-rdma-owner@vger.kernel.org [mailto:linux-rdma-owner@vger.kernel.org] On Behalf Of Yann Droneaud Sent: Saturday, May 16, 2015 3:03 AM To: Weiny, Ira Cc: dledford@redhat.com; linux-rdma@vger.kernel.org Subject: Re: [PATCH] IB/core: Change rdma_protcol_iboe to roce Hi, Le vendredi 15 mai 2015 à 16:29 +0000, Weiny, Ira a écrit : > > Le jeudi 14 mai 2015 à 15:01 -0400, ira.weiny@intel.com a écrit : > > > From: Ira Weiny <ira.weiny@intel.com> > > > > > > It has been decided that ROCE should be used within the kernel > > > rather than IBOE as we used before. Change iboe to roce on the > > > new > > rdma_protocol_* functions. > > > > > > > Erk ... What's the usefulness of such patch > > Currently there is inconsistency in the naming of the RoCE technology. > > 12:27:35 > grep -r roce drivers/infiniband | grep -v Binary | grep -i > -v proce | wc -l > 85 > > All in the driver code. > > 12:27:48 > grep -r iboe drivers/infiniband | grep -v Binary | wc -l > 130 > > Mainly in the core and mlx4 driver. > I see thing a bit differently: $ grep -ri 'roce' include/rdma/ \ include/uapi/rdma \ include/uapi/linux/if_infiniband.h \ drivers/infiniband/core | grep -vi proce | wc -l 0 $ grep -ri 'iboe' include/rdma/ \ include/uapi/rdma \ include/uapi/linux/if_infiniband.h \ drivers/infiniband/core | grep -vi proce | wc -l 30 (On next-20150515). I believe the drivers can have the names they want, especially ocrdma, Emulex OneConnect RoCE. > This patch was to clean up the management helper function as a general > move toward standardizing on roce rather than iboe. Most people in > the community refer to this as "RoCE" so that name was chosen to move > to. > It's not the first time Linux use a name not matching the "vendor" one, amd64, arm64, etc. > > > > IBoE is used throughout the IB/RDMA subsystem. > > > > Changing only these occurences is rather inconsistent. > > > > I asked about changing all the references and Doug mentioned he would > make a patch to change the other references. Personally I don't want > to see a massive rename patch but this could be done. > That's my main concern: I dislike patch that change such large portion of code, for not well defined purpose. I think "It has been decided that ROCE should be used within" is not enough to justify the change. But "Most people in the community refer to this as "RoCE" so that name was chosen to move to." sound a bit better as an explanation. And now, I'm stopping bikeshedding :) Regards. -- Yann Droneaud OPTEYA -- 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 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, 2015-05-18 at 21:45 +0530, Parav Pandit wrote: > Hi, > > At present we have chosen to call it RoCE. However I believe IBoE is more > generic for the reason where, > If the future RoCE versions may work on non-converged Ethernet, IBoE would > be more appropriate name in long haul. I don't think that's much of a concern. If you change RoCE to work over non-converged ethernet (really meaning lossy ethernet), then you have to have an underlying transport that guarantees packet delivery in the face of losses...like TCP...and then you have iWARP ;-) Anyway, I picked this up for 4.2 Ira.
diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c index 1977f601a1ec..ea92a0daa61c 100644 --- a/drivers/infiniband/core/cma.c +++ b/drivers/infiniband/core/cma.c @@ -391,7 +391,7 @@ static int cma_acquire_dev(struct rdma_id_private *id_priv, if (listen_id_priv) { cma_dev = listen_id_priv->cma_dev; port = listen_id_priv->id.port_num; - gidp = rdma_protocol_iboe(cma_dev->device, port) ? + gidp = rdma_protocol_roce(cma_dev->device, port) ? &iboe_gid : &gid; ret = cma_validate_port(cma_dev->device, port, gidp, @@ -409,7 +409,7 @@ static int cma_acquire_dev(struct rdma_id_private *id_priv, listen_id_priv->id.port_num == port) continue; - gidp = rdma_protocol_iboe(cma_dev->device, port) ? + gidp = rdma_protocol_roce(cma_dev->device, port) ? &iboe_gid : &gid; ret = cma_validate_port(cma_dev->device, port, gidp, @@ -647,7 +647,7 @@ static int cma_modify_qp_rtr(struct rdma_id_private *id_priv, BUG_ON(id_priv->cma_dev->device != id_priv->id.device); - if (rdma_protocol_iboe(id_priv->id.device, id_priv->id.port_num)) { + if (rdma_protocol_roce(id_priv->id.device, id_priv->id.port_num)) { ret = rdma_addr_find_smac_by_sgid(&sgid, qp_attr.smac, NULL); if (ret) @@ -1966,7 +1966,7 @@ int rdma_resolve_route(struct rdma_cm_id *id, int timeout_ms) atomic_inc(&id_priv->refcount); if (rdma_cap_ib_sa(id->device, id->port_num)) ret = cma_resolve_ib_route(id_priv, timeout_ms); - else if (rdma_protocol_iboe(id->device, id->port_num)) + else if (rdma_protocol_roce(id->device, id->port_num)) ret = cma_resolve_iboe_route(id_priv); else if (rdma_protocol_iwarp(id->device, id->port_num)) ret = cma_resolve_iw_route(id_priv, timeout_ms); @@ -3325,7 +3325,7 @@ int rdma_join_multicast(struct rdma_cm_id *id, struct sockaddr *addr, list_add(&mc->list, &id_priv->mc_list); spin_unlock(&id_priv->lock); - if (rdma_protocol_iboe(id->device, id->port_num)) { + if (rdma_protocol_roce(id->device, id->port_num)) { kref_init(&mc->mcref); ret = cma_iboe_join_multicast(id_priv, mc); } else if (rdma_cap_ib_mcast(id->device, id->port_num)) @@ -3365,7 +3365,7 @@ void rdma_leave_multicast(struct rdma_cm_id *id, struct sockaddr *addr) if (rdma_cap_ib_mcast(id->device, id->port_num)) { ib_sa_free_multicast(mc->multicast.ib); kfree(mc); - } else if (rdma_protocol_iboe(id->device, id->port_num)) + } else if (rdma_protocol_roce(id->device, id->port_num)) kref_put(&mc->mcref, release_mc); return; diff --git a/drivers/infiniband/core/ucma.c b/drivers/infiniband/core/ucma.c index d42b816c781f..ad45469f7582 100644 --- a/drivers/infiniband/core/ucma.c +++ b/drivers/infiniband/core/ucma.c @@ -725,7 +725,7 @@ static ssize_t ucma_query_route(struct ucma_file *file, if (rdma_cap_ib_sa(ctx->cm_id->device, ctx->cm_id->port_num)) ucma_copy_ib_route(&resp, &ctx->cm_id->route); - else if (rdma_protocol_iboe(ctx->cm_id->device, ctx->cm_id->port_num)) + else if (rdma_protocol_roce(ctx->cm_id->device, ctx->cm_id->port_num)) ucma_copy_iboe_route(&resp, &ctx->cm_id->route); else if (rdma_protocol_iwarp(ctx->cm_id->device, ctx->cm_id->port_num)) ucma_copy_iw_route(&resp, &ctx->cm_id->route); diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h index 29095fb6db5c..bcb87bf168c3 100644 --- a/include/rdma/ib_verbs.h +++ b/include/rdma/ib_verbs.h @@ -1832,7 +1832,7 @@ static inline bool rdma_protocol_ib(struct ib_device *device, u8 port_num) return device->port_immutable[port_num].core_cap_flags & RDMA_CORE_CAP_PROT_IB; } -static inline bool rdma_protocol_iboe(struct ib_device *device, u8 port_num) +static inline bool rdma_protocol_roce(struct ib_device *device, u8 port_num) { return device->port_immutable[port_num].core_cap_flags & RDMA_CORE_CAP_PROT_ROCE; } @@ -1842,7 +1842,7 @@ static inline bool rdma_protocol_iwarp(struct ib_device *device, u8 port_num) return device->port_immutable[port_num].core_cap_flags & RDMA_CORE_CAP_PROT_IWARP; } -static inline bool rdma_ib_or_iboe(struct ib_device *device, u8 port_num) +static inline bool rdma_ib_or_roce(struct ib_device *device, u8 port_num) { return device->port_immutable[port_num].core_cap_flags & (RDMA_CORE_CAP_PROT_IB | RDMA_CORE_CAP_PROT_ROCE); diff --git a/net/sunrpc/xprtrdma/svc_rdma_transport.c b/net/sunrpc/xprtrdma/svc_rdma_transport.c index 3df8320c6efe..3f5750cf187e 100644 --- a/net/sunrpc/xprtrdma/svc_rdma_transport.c +++ b/net/sunrpc/xprtrdma/svc_rdma_transport.c @@ -987,7 +987,7 @@ static struct svc_xprt *svc_rdma_accept(struct svc_xprt *xprt) */ if (!rdma_protocol_iwarp(newxprt->sc_cm_id->device, newxprt->sc_cm_id->port_num) && - !rdma_ib_or_iboe(newxprt->sc_cm_id->device, + !rdma_ib_or_roce(newxprt->sc_cm_id->device, newxprt->sc_cm_id->port_num)) goto errout;