[openib-general] Re: neigh->ops destructor patch

Michael S. Tsirkin mst at mellanox.co.il
Wed Mar 8 15:52:02 PST 2006


Quoting r. Roland Dreier <rdreier at cisco.com>:
> Subject: Re: neigh->ops destructor patch
> 
>     Michael> OK, can you do that? If not, someone from Mellanox will
>     Michael> look at it next week.
> 
> I'll put it on my list -- I have a few other things to take care of first.

OK, please take a look at this: and additional reason I want to do it this way
is to make it easier to solve the LID change problem, which as you might recall
will require tracking all address handles.

The following patch applies on top of the destructor patch already queued
for 2.6.17. so if its OK, just push the 2.6.17 in svn and then add this one on
top.

---

Add ipoib_neigh_alloc/ipoib_neigh_free to make it easier to track them.
Add both 2.6.16 and 2.6.17 code inside appropriate #if.

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

diff -rup linux-2.6.15/drivers/infiniband/ulp/ipoibg/ipoib.h linux-2.6.15/drivers/infiniband/ulp/ipoib/ipoib.h
--- linux-2.6.15/drivers/infiniband/ulp/ipoibg/ipoib.h	2006-03-09 03:43:24.000000000 +0200
+++ linux-2.6.15/drivers/infiniband/ulp/ipoib/ipoib.h	2006-03-09 04:23:47.000000000 +0200
@@ -236,6 +236,9 @@ static inline struct ipoib_neigh **to_ip
 				     INFINIBAND_ALEN, sizeof(void *));
 }
 
+struct ipoib_neigh *ipoib_neigh_alloc(struct neighbour *neigh);
+void ipoib_neigh_free(struct ipoib_neigh *neigh);
+
 extern struct workqueue_struct *ipoib_workqueue;
 
 /* functions */
diff -rup linux-2.6.15/drivers/infiniband/ulp/ipoibg/ipoib_main.c linux-2.6.15/drivers/infiniband/ulp/ipoib/ipoib_main.c
--- linux-2.6.15/drivers/infiniband/ulp/ipoibg/ipoib_main.c	2006-03-09 03:43:24.000000000 +0200
+++ linux-2.6.15/drivers/infiniband/ulp/ipoib/ipoib_main.c	2006-03-09 04:30:05.000000000 +0200
@@ -252,8 +252,8 @@ static void path_free(struct net_device 
 		 */
 		if (neigh->ah)
 			ipoib_put_ah(neigh->ah);
-		*to_ipoib_neigh(neigh->neighbour) = NULL;
-		kfree(neigh);
+
+		ipoib_neigh_free(neigh);
 	}
 
 	spin_unlock_irqrestore(&priv->lock, flags);
@@ -481,7 +481,7 @@ static void neigh_add_path(struct sk_buf
 	struct ipoib_path *path;
 	struct ipoib_neigh *neigh;
 
-	neigh = kmalloc(sizeof *neigh, GFP_ATOMIC);
+	neigh = ipoib_neigh_alloc(skb->dst->neighbour);
 	if (!neigh) {
 		++priv->stats.tx_dropped;
 		dev_kfree_skb_any(skb);
@@ -489,8 +489,6 @@ static void neigh_add_path(struct sk_buf
 	}
 
 	skb_queue_head_init(&neigh->queue);
-	neigh->neighbour = skb->dst->neighbour;
-	*to_ipoib_neigh(skb->dst->neighbour) = neigh;
 
 	/*
 	 * We can only be called from ipoib_start_xmit, so we're
@@ -503,7 +501,7 @@ static void neigh_add_path(struct sk_buf
 		path = path_rec_create(dev,
 				       (union ib_gid *) (skb->dst->neighbour->ha + 4));
 		if (!path)
-			goto err;
+			goto path_err;
 
 		__path_add(dev, path);
 	}
@@ -533,10 +531,9 @@ static void neigh_add_path(struct sk_buf
 	return;
 
 err:
-	*to_ipoib_neigh(skb->dst->neighbour) = NULL;
 	list_del(&neigh->list);
-	kfree(neigh);
-
+path_err:
+	ipoib_neigh_free(neigh);
 	++priv->stats.tx_dropped;
 	dev_kfree_skb_any(skb);
 
@@ -763,8 +760,7 @@ static void ipoib_neigh_destructor(struc
 		if (neigh->ah)
 			ah = neigh->ah;
 		list_del(&neigh->list);
-		*to_ipoib_neigh(n) = NULL;
-		kfree(neigh);
+		ipoib_neigh_free(neigh);
 	}
 
 	spin_unlock_irqrestore(&priv->lock, flags);
@@ -773,13 +769,46 @@ static void ipoib_neigh_destructor(struc
 		ipoib_put_ah(ah);
 }
 
+struct ipoib_neigh *ipoib_neigh_alloc(struct neighbour *neighbour)
+{
+	struct ipoib_neigh *neigh;
+
+	neigh = kmalloc(sizeof *neigh, GFP_ATOMIC);
+	if (!neigh)
+		return NULL;
+
+	neigh->neighbour = neighbour;
+	*to_ipoib_neigh(neighbour) = neigh;
+
+#if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,17)
+	/*
+	 * Is this kosher?  I can't find anybody in the kernel that
+	 * sets neigh->destructor, so we should be able to set it here
+	 * without trouble.
+	 */
+	neigh->neighbour->ops->destructor = ipoib_neigh_destructor;
+#endif
+	return neigh;
+}
+
 static int ipoib_neigh_setup_dev(struct net_device *dev, struct neigh_parms *parms)
 {
+#if LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,17)
 	parms->neigh_destructor = ipoib_neigh_destructor;
+#endif
 
 	return 0;
 }
 
+void ipoib_neigh_free(struct ipoib_neigh *neigh)
+{
+#if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,17)
+	neigh->neighbour->ops->destructor = NULL;
+#endif
+	*to_ipoib_neigh(neigh->neighbour) = NULL;
+	kfree(neigh);
+}
+
 int ipoib_dev_init(struct net_device *dev, struct ib_device *ca, int port)
 {
 	struct ipoib_dev_priv *priv = netdev_priv(dev);
diff -rup linux-2.6.15/drivers/infiniband/ulp/ipoibg/ipoib_multicast.c linux-2.6.15/drivers/infiniband/ulp/ipoib/ipoib_multicast.c
--- linux-2.6.15/drivers/infiniband/ulp/ipoibg/ipoib_multicast.c	2006-03-09 03:43:24.000000000 +0200
+++ linux-2.6.15/drivers/infiniband/ulp/ipoib/ipoib_multicast.c	2006-03-09 03:49:43.000000000 +0200
@@ -114,8 +114,7 @@ static void ipoib_mcast_free(struct ipoi
 		 */
 		if (neigh->ah)
 			ipoib_put_ah(neigh->ah);
-		*to_ipoib_neigh(neigh->neighbour) = NULL;
-		kfree(neigh);
+		ipoib_neigh_free(neigh);
 	}
 
 	spin_unlock_irqrestore(&priv->lock, flags);
@@ -772,13 +771,11 @@ out:
 		if (skb->dst            &&
 		    skb->dst->neighbour &&
 		    !*to_ipoib_neigh(skb->dst->neighbour)) {
-			struct ipoib_neigh *neigh = kmalloc(sizeof *neigh, GFP_ATOMIC);
+			struct ipoib_neigh *neigh = ipoib_neigh_alloc(skb->dst->neighbour);
 
 			if (neigh) {
 				kref_get(&mcast->ah->ref);
 				neigh->ah  	= mcast->ah;
-				neigh->neighbour = skb->dst->neighbour;
-				*to_ipoib_neigh(skb->dst->neighbour) = neigh;
 				list_add_tail(&neigh->list, &mcast->neigh_list);
 			}
 		}


-- 
Michael S. Tsirkin
Staff Engineer, Mellanox Technologies



More information about the general mailing list