[PATCH rdma-next v1 0/4] Fix returning correct GIDs for RoCE

[PATCH rdma-next v1 0/4] Fix returning correct GIDs for RoCE

[Leon Romanovsky]

Changelog:
 v0 -> v1:
  * Returned zeros when device is not yet bound to cm_id.
  * Reordered patches.
  * Moved checks of being not-NULL for the sgid, dgid into rdma_read_gids()
    and made that function ugly like hell.

--------------------------------------------------------------------
>From Parav:

Currently there are few issues with rdma_addr_get_dgid() and
rdma_addr_get_sgid() APIs. They are described below.

1. rdma_addr_get_dgid() returns MAC address for RoCE connections.
2. rdma_addr_get_sgid() returns SGID based on IPv4 address regardless
of connection of AF_INET or AF_INET6 type; resulting into returning
incorrect SGID.
3. rdma_addr_get_sgid() returns SGID based on the first IP address of
the netdevice interface. netdevice can have multiple IP adresses. When
connection is established by non first IP address, this results into
returning incorrect SGID.
4. rdma_addr_get_sgid() is not actually used in connection establishment
APIs such as rdma_resolve_addr(), rdma_resolve_route(), rdma_accept()
for RoCE transport.
RoCE specific rdma cm APIs have used ip2gid() API to derive SGID.

Additionally, ULP module such as net/rds accesses some of the internal
data structures of the rdma_cm_id, such as rdma_route and rdma_addr
structure which might not be the best way to access GIDs.

rdma_ucm module provides GIDs to user space via ucma_query_gid() API.
In certain above conditions it returns incorrect GIDs.

Therefore, this patchset introduces a transport agnostic API for
existing ULP such as rds and rdma_ucm to query GIDs for a given
connection referred by rdma_cm_id.

Since rdma_addr_get_sgid() is not used for RoCE transport during
connection establishment phase, iboe_addr_get_sgid() is removed.
rdma_addr_get_sgid() is refactored for such simplicity.

rdma_read_gids() new API is introduced that allows to read the SGID
and/or DGID of the rdma_cm_id for client and server side connections.

Therefore, this patchset introduces a transport agnostic API for
existing ULP such as rds and rdma_ucm to query GIDs for a given
connection referred by rdma_cm_id.

Since RoCE support was incrementally added, there is no specific commit
id that can be qualified as breaking rdma_addr_get_sgid() API.
rdma_addr_get_dgid() was written initially without RoCE transport in
consideration. When RoCE support was added incrementally, this API
remained broken.

Thanks

Parav Pandit (4):
  RDMA/cma: Introduce API to read GIDs for multiple transports
  RDMA/ucma: Use rdma cm API to query GID
  net/rds: Use rdma_read_gids to read connection GIDs
  RDMA/core: Simplify rdma_addr_get_sgid() to not support RoCE

 drivers/infiniband/core/cma.c  | 27 +++++++++++++++++++++++++++
 drivers/infiniband/core/ucma.c |  8 ++++----
 include/rdma/ib_addr.h         | 33 +++++++--------------------------
 include/rdma/rdma_cm.h         | 19 +++++++++++++++++++
 net/rds/ib.c                   |  6 ++----
 5 files changed, 59 insertions(+), 34 deletions(-)

--
2.15.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 v1 1/4] RDMA/cma: Introduce API to read GIDs for multiple transports

[Leon Romanovsky]

From: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

This patch introduces an API that allows legacy applications to query
GIDs for a rdma_cm_id which is used during connection establishment.

GIDs are stored and created differently for iWarp, IB and RoCE transports.
Therefore rdma_read_gids() returns GID for all the transports hiding
such internal details to caller.
It is usable for client side and server side connections.

rdma_read_gids() should not be used by any new ULPs.

Signed-off-by: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Reviewed-by: Daniel Jurgens <danielj-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 drivers/infiniband/core/cma.c | 27 +++++++++++++++++++++++++++
 include/rdma/rdma_cm.h        | 19 +++++++++++++++++++
 2 files changed, 46 insertions(+)

diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
index 3810716ea65e..a4c010e7b82b 100644
--- a/drivers/infiniband/core/cma.c
+++ b/drivers/infiniband/core/cma.c
@@ -2037,6 +2037,33 @@ __be64 rdma_get_service_id(struct rdma_cm_id *id, struct sockaddr *addr)
 }
 EXPORT_SYMBOL(rdma_get_service_id);
 
+void rdma_read_gids(struct rdma_cm_id *cm_id, union ib_gid *sgid,
+		    union ib_gid *dgid)
+{
+	struct rdma_addr *addr = &cm_id->route.addr;
+
+	if (!cm_id->device) {
+		if (sgid)
+			memset(sgid, 0, sizeof(*sgid));
+		if (dgid)
+			memset(dgid, 0, sizeof(*dgid));
+		return;
+	}
+
+	if (rdma_protocol_roce(cm_id->device, cm_id->port_num)) {
+		if (sgid)
+			rdma_ip2gid((struct sockaddr *)&addr->src_addr, sgid);
+		if (dgid)
+			rdma_ip2gid((struct sockaddr *)&addr->dst_addr, dgid);
+	} else {
+		if (sgid)
+			rdma_addr_get_sgid(&addr->dev_addr, sgid);
+		if (dgid)
+			rdma_addr_get_dgid(&addr->dev_addr, dgid);
+	}
+}
+EXPORT_SYMBOL(rdma_read_gids);
+
 static int cma_iw_handler(struct iw_cm_id *iw_id, struct iw_cm_event *iw_event)
 {
 	struct rdma_id_private *id_priv = iw_id->context;
diff --git a/include/rdma/rdma_cm.h b/include/rdma/rdma_cm.h
index 3d2eed3c4e75..6538a5cc27b6 100644
--- a/include/rdma/rdma_cm.h
+++ b/include/rdma/rdma_cm.h
@@ -413,4 +413,23 @@ bool rdma_is_consumer_reject(struct rdma_cm_id *id, int reason);
 const void *rdma_consumer_reject_data(struct rdma_cm_id *id,
 				      struct rdma_cm_event *ev, u8 *data_len);
 
+/**
+ * rdma_read_gids - Return the SGID and DGID used for establishing
+ *                  connection. This can be used after rdma_resolve_addr()
+ *                  on client side. This can be use on new connection
+ *                  on server side. This is applicable to IB, RoCE, iWarp.
+ *                  If cm_id is not bound yet to the RDMA device, it doesn't
+ *                  copy and SGID or DGID to the given pointers.
+ * @id: Communication identifier whose GIDs are queried.
+ * @sgid: Pointer to SGID where SGID will be returned. It is optional.
+ * @dgid: Pointer to DGID where DGID will be returned. It is optional.
+ * Note: This API should not be used by any new ULPs or new code.
+ * Instead, users interested in querying GIDs should refer to path record
+ * of the rdma_cm_id to query the GIDs.
+ * This API is provided for compatibility for existing users.
+ */
+
+void rdma_read_gids(struct rdma_cm_id *cm_id, union ib_gid *sgid,
+		    union ib_gid *dgid);
+
 #endif /* RDMA_CM_H */
-- 
2.15.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 v1 2/4] RDMA/ucma: Use rdma cm API to query GID

[Leon Romanovsky]

From: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

Make use of rdma_read_gids() API to read SGID and DGID which returns
correct GIDs for RoCE and other transports.

rdma_addr_get_dgid() for RoCE for client side connections returns MAC
address, instead of DGID.
rdma_addr_get_sgid() for RoCE doesn't return correct SGID for IPv6 and
when more than one IP address is assigned to the netdevice.

Therefore use transport agnostic rdma_read_gids() API provided by rdma_cm
module.

Signed-off-by: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Reviewed-by: Daniel Jurgens <danielj-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 drivers/infiniband/core/ucma.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/infiniband/core/ucma.c b/drivers/infiniband/core/ucma.c
index 2ec0f89035fb..d67219db99be 100644
--- a/drivers/infiniband/core/ucma.c
+++ b/drivers/infiniband/core/ucma.c
@@ -944,8 +944,8 @@ static ssize_t ucma_query_gid(struct ucma_context *ctx,
 	} else {
 		addr->sib_family = AF_IB;
 		addr->sib_pkey = (__force __be16) resp.pkey;
-		rdma_addr_get_sgid(&ctx->cm_id->route.addr.dev_addr,
-				   (union ib_gid *) &addr->sib_addr);
+		rdma_read_gids(ctx->cm_id, (union ib_gid *)&addr->sib_addr,
+			       NULL);
 		addr->sib_sid = rdma_get_service_id(ctx->cm_id, (struct sockaddr *)
 						    &ctx->cm_id->route.addr.src_addr);
 	}
@@ -957,8 +957,8 @@ static ssize_t ucma_query_gid(struct ucma_context *ctx,
 	} else {
 		addr->sib_family = AF_IB;
 		addr->sib_pkey = (__force __be16) resp.pkey;
-		rdma_addr_get_dgid(&ctx->cm_id->route.addr.dev_addr,
-				   (union ib_gid *) &addr->sib_addr);
+		rdma_read_gids(ctx->cm_id, NULL,
+			       (union ib_gid *)&addr->sib_addr);
 		addr->sib_sid = rdma_get_service_id(ctx->cm_id, (struct sockaddr *)
 						    &ctx->cm_id->route.addr.dst_addr);
 	}
-- 
2.15.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 v1 3/4] net/rds: Use rdma_read_gids to read connection GIDs

[Leon Romanovsky]

From: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

Use newly introduced API rdma_read_gids to read the SGID and DGID for
the connection which returns GID correctly for RoCE transport as well.

rdma_addr_get_dgid() for RoCE for client side connections returns MAC
address, instead of DGID.
rdma_addr_get_sgid() for RoCE doesn't return correct SGID for IPv6 and
when more than one IP address is assigned to the netdevice.

Therefore use transport agnostic rdma_read_gids() API provided by rdma_cm
module.

Signed-off-by: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Reviewed-by: Daniel Jurgens <danielj-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Acked-by: Santosh Shilimkar <santosh.shilimkar-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 net/rds/ib.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/net/rds/ib.c b/net/rds/ib.c
index 36dd2099048a..b2a5067b4afe 100644
--- a/net/rds/ib.c
+++ b/net/rds/ib.c
@@ -301,13 +301,11 @@ static int rds_ib_conn_info_visitor(struct rds_connection *conn,
 	memset(&iinfo->dst_gid, 0, sizeof(iinfo->dst_gid));
 	if (rds_conn_state(conn) == RDS_CONN_UP) {
 		struct rds_ib_device *rds_ibdev;
-		struct rdma_dev_addr *dev_addr;

 		ic = conn->c_transport_data;
-		dev_addr = &ic->i_cm_id->route.addr.dev_addr;

-		rdma_addr_get_sgid(dev_addr, (union ib_gid *) &iinfo->src_gid);
-		rdma_addr_get_dgid(dev_addr, (union ib_gid *) &iinfo->dst_gid);
+		rdma_read_gids(ic->i_cm_id, (union ib_gid *)&iinfo->src_gid,
+			       (union ib_gid *)&iinfo->dst_gid);

 		rds_ibdev = ic->rds_ibdev;
 		iinfo->max_send_wr = ic->i_send_ring.w_nr;
--
2.15.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 v1 4/4] RDMA/core: Simplify rdma_addr_get_sgid() to not support RoCE

[Leon Romanovsky]

From: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

rdma_addr_get_sgid() for RoCE has few issues as below.
1. With IP based GIDs, SGID derived using iboe_addr_get_sgid() was
not used in connection establishment process.
SGID was always derived using ip2gid() in various routines such as
cma_resolve_iboe_route(), cma_iboe_join_multicast(), cma_acquire_dev().

2. It returns the first IP address of the netdevice, while netdevice
can have multiple IP addresses. Connection could have been established
using non first IP address, resulting into returning wrong SGID.

3. It returns SGID based on IPv4 address. It doesn't cover IPv6
addresses, which means it returns wrong SGID for IPv6 based connections.

4. As the comment of rdma_dev_addr src_dev_addr and dst_dev_addr suggest,
such members of rdma_dev_addr stores the mac address. Therefore it is not
the best place to store the RoCE GIDs. RoCE GIDs are derived from the IP
addresses and they are already available in cm_id->route->addr.***_addr.

It was only used to return SGID to rds/ib.c and ucma.c modules, where it
returned incorrect SGID in above matching conditions.
Therefore code is simplified to not support RoCE.

Signed-off-by: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Reviewed-by: Daniel Jurgens <danielj-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 include/rdma/ib_addr.h | 33 +++++++--------------------------
 1 file changed, 7 insertions(+), 26 deletions(-)

diff --git a/include/rdma/ib_addr.h b/include/rdma/ib_addr.h
index dc0b642e0175..d656809f1217 100644
--- a/include/rdma/ib_addr.h
+++ b/include/rdma/ib_addr.h
@@ -197,34 +197,15 @@ static inline void rdma_gid2ip(struct sockaddr *out, const union ib_gid *gid)
 	}
 }
 
-static inline void iboe_addr_get_sgid(struct rdma_dev_addr *dev_addr,
-				      union ib_gid *gid)
-{
-	struct net_device *dev;
-	struct in_device *ip4;
-
-	dev = dev_get_by_index(&init_net, dev_addr->bound_dev_if);
-	if (dev) {
-		ip4 = in_dev_get(dev);
-		if (ip4 && ip4->ifa_list && ip4->ifa_list->ifa_address)
-			ipv6_addr_set_v4mapped(ip4->ifa_list->ifa_address,
-					       (struct in6_addr *)gid);
-
-		if (ip4)
-			in_dev_put(ip4);
-
-		dev_put(dev);
-	}
-}
-
+/*
+ * rdma_get/set_sgid/dgid() APIs are applicable to IB, and iWarp.
+ * They are not applicable to RoCE.
+ * RoCE GIDs are derived from the IP addresses.
+ */
 static inline void rdma_addr_get_sgid(struct rdma_dev_addr *dev_addr, union ib_gid *gid)
 {
-	if (dev_addr->transport == RDMA_TRANSPORT_IB &&
-	    dev_addr->dev_type != ARPHRD_INFINIBAND)
-		iboe_addr_get_sgid(dev_addr, gid);
-	else
-		memcpy(gid, dev_addr->src_dev_addr +
-		       rdma_addr_gid_offset(dev_addr), sizeof *gid);
+	memcpy(gid, dev_addr->src_dev_addr + rdma_addr_gid_offset(dev_addr),
+	       sizeof(*gid));
 }
 
 static inline void rdma_addr_set_sgid(struct rdma_dev_addr *dev_addr, union ib_gid *gid)
-- 
2.15.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 v1 0/4] Fix returning correct GIDs for RoCE

[Jason Gunthorpe]

On Thu, Jan 18, 2018 at 10:11:15AM +0200, Leon Romanovsky wrote:
> Changelog:
>  v0 -> v1:
>   * Returned zeros when device is not yet bound to cm_id.
>   * Reordered patches.
>   * Moved checks of being not-NULL for the sgid, dgid into rdma_read_gids()
>     and made that function ugly like hell.
>
> >From Parav:
> 
> Currently there are few issues with rdma_addr_get_dgid() and
> rdma_addr_get_sgid() APIs. They are described below.
> 
> 1. rdma_addr_get_dgid() returns MAC address for RoCE connections.
> 2. rdma_addr_get_sgid() returns SGID based on IPv4 address regardless
> of connection of AF_INET or AF_INET6 type; resulting into returning
> incorrect SGID.
> 3. rdma_addr_get_sgid() returns SGID based on the first IP address of
> the netdevice interface. netdevice can have multiple IP adresses. When
> connection is established by non first IP address, this results into
> returning incorrect SGID.
> 4. rdma_addr_get_sgid() is not actually used in connection establishment
> APIs such as rdma_resolve_addr(), rdma_resolve_route(), rdma_accept()
> for RoCE transport.
> RoCE specific rdma cm APIs have used ip2gid() API to derive SGID.
> 
> Additionally, ULP module such as net/rds accesses some of the internal
> data structures of the rdma_cm_id, such as rdma_route and rdma_addr
> structure which might not be the best way to access GIDs.
> 
> rdma_ucm module provides GIDs to user space via ucma_query_gid() API.
> In certain above conditions it returns incorrect GIDs.
> 
> Therefore, this patchset introduces a transport agnostic API for
> existing ULP such as rds and rdma_ucm to query GIDs for a given
> connection referred by rdma_cm_id.
> 
> Since rdma_addr_get_sgid() is not used for RoCE transport during
> connection establishment phase, iboe_addr_get_sgid() is removed.
> rdma_addr_get_sgid() is refactored for such simplicity.
> 
> rdma_read_gids() new API is introduced that allows to read the SGID
> and/or DGID of the rdma_cm_id for client and server side connections.
> 
> Therefore, this patchset introduces a transport agnostic API for
> existing ULP such as rds and rdma_ucm to query GIDs for a given
> connection referred by rdma_cm_id.
> 
> Since RoCE support was incrementally added, there is no specific commit
> id that can be qualified as breaking rdma_addr_get_sgid() API.
> rdma_addr_get_dgid() was written initially without RoCE transport in
> consideration. When RoCE support was added incrementally, this API
> remained broken.

Applied to for-next thanks

Jason
--
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