[openib-general] Re: ia64: kernel unaligned access in ipoib

Michael S. Tsirkin mst at mellanox.co.il
Wed May 31 01:16:44 PDT 2006


Roland, it seems you've taken the wrong patch (the one I posted
for review pre-testing) into your tree.
Please revert and take the one below. It's been running here without issues.

-----

Quoting r. Michael S. Tsirkin <mst at mellanox.co.il>:
Subject: Re: ia64: kernel unaligned access in ipoib

Quoting r. Roland Dreier <rdreier at cisco.com>:
> Subject: Re: ia64: kernel unaligned access in ipoib
> 
>     Michael> We've written up a patch with Jack - do you want us to
>     Michael> test it or prefer to re-write it yourself?
> 
> Go ahead and test it -- I replied before I saw your patch.

The following fixed the issue for us, pls review. Can this go into 2.6.17?
If yes, I think it's prudent to let it run for another night before pushing it
out, since we had to touch a lot of lines here.  We'll do that and let you know
tomorrow.

---

Fix misaligned access faults on ia64: never cast a misaligned ha + 4 pointer to
union ib_gid type, pass a void * pointer instead.

Signed-off-by: Jack Morgenstein <jackm at mellanox.co.il>
Signed-off-by: Michael S. Tsirkin <mst at mellanox.co.il>

Index: src/drivers/infiniband/ulp/ipoib/ipoib_main.c
===================================================================
--- src.orig/drivers/infiniband/ulp/ipoib/ipoib_main.c	2006-04-16 11:12:16.105871000 +0300
+++ src/drivers/infiniband/ulp/ipoib/ipoib_main.c	2006-05-30 12:09:43.113172000 +0300
@@ -190,8 +190,7 @@ static int ipoib_change_mtu(struct net_d
 	return 0;
 }
 
-static struct ipoib_path *__path_find(struct net_device *dev,
-				      union ib_gid *gid)
+static struct ipoib_path *__path_find(struct net_device *dev, void *gid)
 {
 	struct ipoib_dev_priv *priv = netdev_priv(dev);
 	struct rb_node *n = priv->path_tree.rb_node;
@@ -201,7 +200,7 @@ static struct ipoib_path *__path_find(st
 	while (n) {
 		path = rb_entry(n, struct ipoib_path, rb_node);
 
-		ret = memcmp(gid->raw, path->pathrec.dgid.raw,
+		ret = memcmp(gid, path->pathrec.dgid.raw,
 			     sizeof (union ib_gid));
 
 		if (ret < 0)
@@ -430,8 +429,7 @@ static void path_rec_completion(int stat
 	}
 }
 
-static struct ipoib_path *path_rec_create(struct net_device *dev,
-					  union ib_gid *gid)
+static struct ipoib_path *path_rec_create(struct net_device *dev, void *gid)
 {
 	struct ipoib_dev_priv *priv = netdev_priv(dev);
 	struct ipoib_path *path;
@@ -446,7 +444,7 @@ static struct ipoib_path *path_rec_creat
 
 	INIT_LIST_HEAD(&path->neigh_list);
 
-	memcpy(path->pathrec.dgid.raw, gid->raw, sizeof (union ib_gid));
+	memcpy(path->pathrec.dgid.raw, gid, sizeof (union ib_gid));
 	path->pathrec.sgid      = priv->local_gid;
 	path->pathrec.pkey      = cpu_to_be16(priv->pkey);
 	path->pathrec.numb_path = 1;
@@ -504,10 +502,9 @@ static void neigh_add_path(struct sk_buf
 	 */
 	spin_lock(&priv->lock);
 
-	path = __path_find(dev, (union ib_gid *) (skb->dst->neighbour->ha + 4));
+	path = __path_find(dev, skb->dst->neighbour->ha + 4);
 	if (!path) {
-		path = path_rec_create(dev,
-				       (union ib_gid *) (skb->dst->neighbour->ha + 4));
+		path = path_rec_create(dev, skb->dst->neighbour->ha + 4);
 		if (!path)
 			goto err_path;
 
@@ -557,7 +554,7 @@ static void ipoib_path_lookup(struct sk_
 	/* Add in the P_Key for multicasts */
 	skb->dst->neighbour->ha[8] = (priv->pkey >> 8) & 0xff;
 	skb->dst->neighbour->ha[9] = priv->pkey & 0xff;
-	ipoib_mcast_send(dev, (union ib_gid *) (skb->dst->neighbour->ha + 4), skb);
+	ipoib_mcast_send(dev, skb->dst->neighbour->ha + 4, skb);
 }
 
 static void unicast_arp_send(struct sk_buff *skb, struct net_device *dev,
@@ -572,10 +569,9 @@ static void unicast_arp_send(struct sk_b
 	 */
 	spin_lock(&priv->lock);
 
-	path = __path_find(dev, (union ib_gid *) (phdr->hwaddr + 4));
+	path = __path_find(dev, phdr->hwaddr + 4);
 	if (!path) {
-		path = path_rec_create(dev,
-				       (union ib_gid *) (phdr->hwaddr + 4));
+		path = path_rec_create(dev, phdr->hwaddr + 4);
 		if (path) {
 			/* put pseudoheader back on for next time */
 			skb_push(skb, sizeof *phdr);
@@ -666,7 +662,7 @@ static int ipoib_start_xmit(struct sk_bu
 			phdr->hwaddr[8] = (priv->pkey >> 8) & 0xff;
 			phdr->hwaddr[9] = priv->pkey & 0xff;
 
-			ipoib_mcast_send(dev, (union ib_gid *) (phdr->hwaddr + 4), skb);
+			ipoib_mcast_send(dev, phdr->hwaddr + 4, skb);
 		} else {
 			/* unicast GID -- should be ARP or RARP reply */
 
@@ -677,7 +673,7 @@ static int ipoib_start_xmit(struct sk_bu
 					   skb->dst ? "neigh" : "dst",
 					   be16_to_cpup((__be16 *) skb->data),
 					   be32_to_cpup((__be32 *) phdr->hwaddr),
-					   IPOIB_GID_ARG(*(union ib_gid *) (phdr->hwaddr + 4)));
+					   IPOIB_GID_RAW_ARG(phdr->hwaddr + 4));
 				dev_kfree_skb_any(skb);
 				++priv->stats.tx_dropped;
 				goto out;
@@ -773,7 +769,7 @@ static void ipoib_neigh_destructor(struc
 	ipoib_dbg(priv,
 		  "neigh_destructor for %06x " IPOIB_GID_FMT "\n",
 		  be32_to_cpup((__be32 *) n->ha),
-		  IPOIB_GID_ARG(*((union ib_gid *) (n->ha + 4))));
+		  IPOIB_GID_RAW_ARG(n->ha + 4));
 
 	spin_lock_irqsave(&priv->lock, flags);
 
Index: src/drivers/infiniband/ulp/ipoib/ipoib_multicast.c
===================================================================
--- src.orig/drivers/infiniband/ulp/ipoib/ipoib_multicast.c	2006-05-25 11:35:23.334409000 +0300
+++ src/drivers/infiniband/ulp/ipoib/ipoib_multicast.c	2006-05-30 12:09:28.568595000 +0300
@@ -153,7 +153,7 @@ static struct ipoib_mcast *ipoib_mcast_a
 	return mcast;
 }
 
-static struct ipoib_mcast *__ipoib_mcast_find(struct net_device *dev, union ib_gid *mgid)
+static struct ipoib_mcast *__ipoib_mcast_find(struct net_device *dev, void *mgid)
 {
 	struct ipoib_dev_priv *priv = netdev_priv(dev);
 	struct rb_node *n = priv->multicast_tree.rb_node;
@@ -164,7 +164,7 @@ static struct ipoib_mcast *__ipoib_mcast
 
 		mcast = rb_entry(n, struct ipoib_mcast, rb_node);
 
-		ret = memcmp(mgid->raw, mcast->mcmember.mgid.raw,
+		ret = memcmp(mgid, mcast->mcmember.mgid.raw,
 			     sizeof (union ib_gid));
 		if (ret < 0)
 			n = n->rb_left;
@@ -639,8 +639,7 @@ static int ipoib_mcast_leave(struct net_
 	return 0;
 }
 
-void ipoib_mcast_send(struct net_device *dev, union ib_gid *mgid,
-		      struct sk_buff *skb)
+void ipoib_mcast_send(struct net_device *dev, void *mgid, struct sk_buff *skb)
 {
 	struct ipoib_dev_priv *priv = netdev_priv(dev);
 	struct ipoib_mcast *mcast;
@@ -663,7 +662,7 @@ void ipoib_mcast_send(struct net_device 
 	if (!mcast) {
 		/* Let's create a new send only group now */
 		ipoib_dbg_mcast(priv, "setting up send only multicast group for "
-				IPOIB_GID_FMT "\n", IPOIB_GID_ARG(*mgid));
+				IPOIB_GID_FMT "\n", IPOIB_GID_RAW_ARG(mgid));
 
 		mcast = ipoib_mcast_alloc(dev, 0);
 		if (!mcast) {
@@ -675,7 +674,7 @@ void ipoib_mcast_send(struct net_device 
 		}
 
 		set_bit(IPOIB_MCAST_FLAG_SENDONLY, &mcast->flags);
-		mcast->mcmember.mgid = *mgid;
+		memcpy(mcast->mcmember.mgid.raw, mgid, sizeof (union ib_gid));
 		__ipoib_mcast_add(dev, mcast);
 		list_add_tail(&mcast->list, &priv->multicast_list);
 	}
Index: src/drivers/infiniband/ulp/ipoib/ipoib.h
===================================================================
--- src.orig/drivers/infiniband/ulp/ipoib/ipoib.h	2006-04-06 10:03:29.420250000 +0300
+++ src/drivers/infiniband/ulp/ipoib/ipoib.h	2006-05-30 12:20:39.572837000 +0300
@@ -278,8 +278,7 @@ int ipoib_dev_init(struct net_device *de
 void ipoib_dev_cleanup(struct net_device *dev);
 
 void ipoib_mcast_join_task(void *dev_ptr);
-void ipoib_mcast_send(struct net_device *dev, union ib_gid *mgid,
-		      struct sk_buff *skb);
+void ipoib_mcast_send(struct net_device *dev, void *mgid, struct sk_buff *skb);
 
 void ipoib_mcast_restart_task(void *dev_ptr);
 int ipoib_mcast_start_thread(struct net_device *dev);
@@ -375,15 +374,26 @@ extern int ipoib_debug_level;
 #endif /* CONFIG_INFINIBAND_IPOIB_DEBUG_DATA */
 
 
-#define IPOIB_GID_FMT		"%x:%x:%x:%x:%x:%x:%x:%x"
+#define IPOIB_GID_FMT		"%2.2x%2.2x:%2.2x%2.2x:%2.2x%2.2x:%2.2x%2.2x:" \
+				"%2.2x%2.2x:%2.2x%2.2x:%2.2x%2.2x:%2.2x%2.2x"
 
-#define IPOIB_GID_ARG(gid)	be16_to_cpup((__be16 *) ((gid).raw +  0)), \
-				be16_to_cpup((__be16 *) ((gid).raw +  2)), \
-				be16_to_cpup((__be16 *) ((gid).raw +  4)), \
-				be16_to_cpup((__be16 *) ((gid).raw +  6)), \
-				be16_to_cpup((__be16 *) ((gid).raw +  8)), \
-				be16_to_cpup((__be16 *) ((gid).raw + 10)), \
-				be16_to_cpup((__be16 *) ((gid).raw + 12)), \
-				be16_to_cpup((__be16 *) ((gid).raw + 14))
+#define IPOIB_GID_RAW_ARG(gid)	((u8 *)(gid))[0], \
+				((u8 *)(gid))[1], \
+				((u8 *)(gid))[2], \
+				((u8 *)(gid))[3], \
+				((u8 *)(gid))[4], \
+				((u8 *)(gid))[5], \
+				((u8 *)(gid))[6], \
+				((u8 *)(gid))[7], \
+				((u8 *)(gid))[8], \
+				((u8 *)(gid))[9], \
+				((u8 *)(gid))[10],\
+				((u8 *)(gid))[11],\
+				((u8 *)(gid))[12],\
+				((u8 *)(gid))[13],\
+				((u8 *)(gid))[14],\
+				((u8 *)(gid))[15]
+
+#define IPOIB_GID_ARG(gid)	IPOIB_GID_RAW_ARG((gid).raw)
 
 #endif /* _IPOIB_H */


-- 
MST



More information about the general mailing list