Skip to contentSkip to conversation

ib_core: IBoE CMA device binding

PATCH In review Eli Cohen 3 participants 8 messages Feb 18, 2010 – May 14, 2010 4 messages not in the archive InfiniBand

lore
Share & tools
lore

Eli Cohen

author
#0 lore raw

PATCH v8 4/11 ib_core: IBoE CMA device binding

Add support for IBoE device binding and IP --> GID resolution. Path resolving and multicast joining are implemented within cma.c by filling the responses and pushing the callbacks to the cma work queue. IP->GID resolution always yields IPv6 link local addresses - remote GIDs are derived from the destination MAC address of the remote port. Multicast GIDs are always mapped to multicast MACs as is done in IPv6. Some helper functions are added to ib_addr.h. IPv4 multicast is enabled by translating IPv4 multicast addresses to IPv6 multicast as described in
http://www.mail-archive.com/[email protected]/msg02134.html.

---

Chages from v7:

1. Add force_grh flag to ib_init_ah_from_path() to request IB_AH_GRH for IB_LINK_LAYER_ETHERNET ports thus allowing to use hop limit 1 in path records.
2. cma_acquire_dev() finds the cma_dev by first assuming an iboe type device for none ARPHRD_INFINIBAND dev type. If it fails to do that, it falls back to old method.

 drivers/infiniband/core/cm.c              |    5 +-
 drivers/infiniband/core/cma.c             |  283 +++++++++++++++++++++++++++--
 drivers/infiniband/core/sa_query.c        |    5 +-
 drivers/infiniband/core/ucma.c            |   45 ++++-
 drivers/infiniband/ulp/ipoib/ipoib_main.c |    2 +-
 include/rdma/ib_addr.h                    |   98 ++++++++++-
 include/rdma/ib_sa.h                      |    3 +-
 7 files changed, 412 insertions(+), 29 deletions(-)
drivers/infiniband/core/cm.c +4 -1
index 5130fc5..6513b1c 100644
@@ -351,6 +351,7 @@ static int cm_init_av_by_path(struct ib_sa_path_rec *path, struct cm_av *av)
 	unsigned long flags;
 	int ret;
 	u8 p;
+	int force_grh;
 
 	read_lock_irqsave(&cm.device_lock, flags);
 	list_for_each_entry(cm_dev, &cm.device_list, list) {
@@ -371,8 +372,10 @@ static int cm_init_av_by_path(struct ib_sa_path_rec *path, struct cm_av *av)
 		return ret;
 
 	av->port = port;
+	force_grh = rdma_port_link_layer(cm_dev->ib_device, port->port_num) ==
+		IB_LINK_LAYER_ETHERNET ? 1 : 0;
 	ib_init_ah_from_path(cm_dev->ib_device, port->port_num, path,
-			     &av->ah_attr);
+			     &av->ah_attr, force_grh);
 	av->timeout = path->packet_life_time + 1;
 	return 0;
 }
drivers/infiniband/core/cma.c +266 -17

406 lines not on this page · raw message

drivers/infiniband/core/sa_query.c +3 -2
index 106284b..3e3821d 100644
@@ -492,7 +492,8 @@ static u8 get_src_path_mask(struct ib_device *device, u8 port_num)
 }
 
 int ib_init_ah_from_path(struct ib_device *device, u8 port_num,
-			 struct ib_sa_path_rec *rec, struct ib_ah_attr *ah_attr)
+			 struct ib_sa_path_rec *rec, struct ib_ah_attr *ah_attr,
+			 int force_grh)
 {
 	int ret;
 	u16 gid_index;
@@ -505,7 +506,7 @@ int ib_init_ah_from_path(struct ib_device *device, u8 port_num,
 	ah_attr->port_num = port_num;
 	ah_attr->static_rate = rec->rate;
 
-	if (rec->hop_limit > 1) {
+	if (rec->hop_limit > 1 || force_grh) {
 		ah_attr->ah_flags = IB_AH_GRH;
 		ah_attr->grh.dgid = rec->dgid;
 
drivers/infiniband/core/ucma.c +39 -6

57 lines not on this page · raw message

drivers/infiniband/ulp/ipoib/ipoib_main.c +1 -1
index df3eb8c..06014d2 100644
@@ -425,7 +425,7 @@ static void path_rec_completion(int status,
 	if (!status) {
 		struct ib_ah_attr av;
 
-		if (!ib_init_ah_from_path(priv->ca, priv->port, pathrec, &av))
+		if (!ib_init_ah_from_path(priv->ca, priv->port, pathrec, &av, 0))
 			ah = ipoib_create_ah(dev, priv->pd, &av);
 	}
 
include/rdma/ib_addr.h +97 -1

122 lines not on this page · raw message

include/rdma/ib_sa.h +2 -1
index 1082afa..3d1c3a7 100644
@@ -377,7 +377,8 @@ int ib_init_ah_from_mcmember(struct ib_device *device, u8 port_num,
  */
 int ib_init_ah_from_path(struct ib_device *device, u8 port_num,
 			 struct ib_sa_path_rec *rec,
-			 struct ib_ah_attr *ah_attr);
+			 struct ib_ah_attr *ah_attr,
+			 int force_grh);
 
 /**
  * ib_sa_unpack_path - Convert a path record from MAD format to struct
Signature · 6 lines
1.7.0

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
#1 lore raw

drivers/infiniband/core/sa_query.c static u8 get_src_path_mask(struct ib_device *device, u8 port_num) · line 496

Quoted 4 lines

int ib_init_ah_from_path(struct ib_device *device, u8 port_num,

drivers/infiniband/core/sa_query.c +2 -1
-			 struct ib_sa_path_rec *rec, struct ib_ah_attr *ah_attr)
+			 struct ib_sa_path_rec *rec, struct ib_ah_attr *ah_attr,
+			 int force_grh)

Rather than this change in API, could we just have this function look at the link layer, and if it's ethernet, then always set the GRH flag? Seems simpler than requiring the upper layers to do this and then pass the result in?

Signature · 6 lines
Roland Dreier <[email protected]> || For corporate legal information go to:
http://www.cisco.com/web/about/doing_business/legal/cri/index.html
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

A message that is not in the archive (sent off-list or not yet received): ada1vdgvqe9.fsf-BjVyx320WGW9gfZ95n9DRSW4+XlvGpQz@public.gmane.org

#3 lore raw
Quoted 9 lines · Roland Dreier

drivers/infiniband/core/sa_query.c static u8 get_src_path_mask(struct ib_device *device, u8 port_num) · line 496

Quoted 4 lines

int ib_init_ah_from_path(struct ib_device *device, u8 port_num,

drivers/infiniband/core/sa_query.c +2 -1
-			 struct ib_sa_path_rec *rec, struct ib_ah_attr *ah_attr)
+			 struct ib_sa_path_rec *rec, struct ib_ah_attr *ah_attr,
+			 int force_grh)

Rather than this change in API, could we just have this function look at the link layer, and if it's ethernet, then always set the GRH flag? Seems simpler than requiring the upper layers to do this and then pass the result in?

I guess that would keep the function more versatile but changing the implenetation to check the port's link layer seems OK to me. --
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

#4 lore raw
In reply to Eli Cohen

drivers/infiniband/core/cma.c static int cma_join_ib_multicast(struct rdma_id_private *id_priv, · line 2917

Quoted 11 lines
drivers/infiniband/core/cma.c +11 -0
+static void iboe_mcast_work_handler(struct work_struct *work)
+{
+	struct iboe_mcast_work *mw = container_of(work, struct iboe_mcast_work, work);
+	struct cma_multicast *mc = mw->mc;
+	struct ib_sa_multicast *m = mc->multicast.ib;
+
+	mc->multicast.ib->context = mc;
+	cma_ib_mc_handler(0, m);
+	kref_put(&mc->mcref, release_mc);
+	kfree(mw);
+}

I'm having a hard time working out why the iboe case needs to schedule to a work queue here since its already in process context, right? It seems it would be really preferable to avoid all the extra pointer munging and reference counting, and just call things directly.

- R.

Signature · 6 lines
Roland Dreier <[email protected]> || For corporate legal information go to:
http://www.cisco.com/web/about/doing_business/legal/cri/index.html
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

A message that is not in the archive (sent off-list or not yet received): adawrv8ubrh.fsf-BjVyx320WGW9gfZ95n9DRSW4+XlvGpQz@public.gmane.org

#6 lore raw
Quoted 8 lines · Roland Dreier
Quoted 8 lines
drivers/infiniband/core/cma.c +8 -0
+static void iboe_mcast_work_handler(struct work_struct *work)
+{
+	struct iboe_mcast_work *mw = container_of(work, struct iboe_mcast_work, work);
+	struct cma_multicast *mc = mw->mc;
+	struct ib_sa_multicast *m = mc->multicast.ib;
+
+	mc->multicast.ib->context = mc;
+	cma_ib_mc_handler(0, m);
Quoted 9 lines

drivers/infiniband/core/cma.c static int cma_join_ib_multicast(struct rdma_id_private *id_priv, · line 2917

Quoted 3 lines
drivers/infiniband/core/cma.c +3 -0
+	kref_put(&mc->mcref, release_mc);
+	kfree(mw);
+}

I'm having a hard time working out why the iboe case needs to schedule to a work queue here since its already in process context, right? It seems it would be really preferable to avoid all the extra pointer munging and reference counting, and just call things directly.

I assume that the caller might attempt to acquire the same lock when calling join and in the callback. Specifically, ucma_join_multicast() calls rdma_join_multicast() with file->mut acquired and ucma_event_handler() does the same.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

A message that is not in the archive (sent off-list or not yet received): 20100513082613.GC16073-8YAHvHwT2UEvbXDkjdHOrw/[email protected]

#8 lore raw
Quoted 9 lines
Quoted 4 lines

I'm having a hard time working out why the iboe case needs to schedule to a work queue here since its already in process context, right? It seems it would be really preferable to avoid all the extra pointer munging and reference counting, and just call things directly.

I assume that the caller might attempt to acquire the same lock when calling join and in the callback. Specifically, ucma_join_multicast() calls rdma_join_multicast() with file->mut acquired and ucma_event_handler() does the same.

I see... we can't call the consumer's callback directly since it might have locking assumptions.

It would be nice if we didn't have this reference counting sometimes used and sometimes not used. I'll have to think about whether this can be made cleaner.

- R.

Signature · 6 lines
Roland Dreier <[email protected]> || For corporate legal information go to:
http://www.cisco.com/web/about/doing_business/legal/cri/index.html
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
#9 lore raw
In reply to Eli Cohen
Quoted 5 lines

Multicast GIDs are always mapped to multicast MACs
as is done in IPv6. Some helper functions are added to ib_addr.h. IPv4 multicast is enabled by translating IPv4 multicast addresses to IPv6 multicast as described in
http://www.mail-archive.com/[email protected]/msg02134.html.

I guess it's a bit unfortunate that the RoCE annex completely ignored how to map multicast GIDs to ethernet addresses (I suppose as part of the larger decision to ignore address resolution entirely). Anyway, looking at the email message you reference, it seems to be someone asking what the right way to map IPv4 multicast addresses to IPv6 addresses is. Is there a more definitive document you can point to?

It seems that unfortunately the way the layering of addresses is done, there's no way to just use the standard mapping of IPv4 multicast addresses to Ethernet addresses (since IBoE is does addressing via the CMA mapping to GIDs followed by an unspecified mapping from GIDs to Ethernet addresses).

- R.

Signature · 6 lines
Roland Dreier <[email protected]> || For corporate legal information go to:
http://www.cisco.com/web/about/doing_business/legal/cri/index.html
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

A message that is not in the archive (sent off-list or not yet received): adaocgkubbq.fsf-BjVyx320WGW9gfZ95n9DRSW4+XlvGpQz@public.gmane.org

#11 lore raw
Quoted 6 lines · Roland Dreier
Quoted 5 lines

Multicast GIDs are always mapped to multicast MACs
as is done in IPv6. Some helper functions are added to ib_addr.h. IPv4 multicast is enabled by translating IPv4 multicast addresses to IPv6 multicast as described in
http://www.mail-archive.com/[email protected]/msg02134.html.

Quoted 6 lines

I guess it's a bit unfortunate that the RoCE annex completely ignored how to map multicast GIDs to ethernet addresses (I suppose as part of the larger decision to ignore address resolution entirely). Anyway, looking at the email message you reference, it seems to be someone asking what the right way to map IPv4 multicast addresses to IPv6 addresses is. Is there a more definitive document you can point to?

I am not aware of any definitive document that addresses this issue.

Quoted 7 lines

It seems that unfortunately the way the layering of addresses is done, there's no way to just use the standard mapping of IPv4 multicast addresses to Ethernet addresses (since IBoE is does addressing via the CMA mapping to GIDs followed by an unspecified mapping from GIDs to Ethernet addresses).

It is natural to treat gids as IPv6 addresses, so using the standard mapping from IPv6 multicast addresses to mac addresses seems reasonable.