IPoIB uses neighbour ops->destructor to clean up struct ipoib_neigh,
but ignores the fact that multiple neighbour objects can share
the same ops structure, so setting it to NULL affects multiple neighbours.

Fix this, by tracking all ipoib_neigh objects, and only cleaning 
destructor after no neighbour is going to use it.
Note that ops structure isnt per device, so we track them in a global list.

Signed-off-by: Michael S. Tsirkin <[EMAIL PROTECTED]>

Index: linux-2.6.14/drivers/infiniband/ulp/ipoib/ipoib_main.c
===================================================================
--- linux-2.6.14.orig/drivers/infiniband/ulp/ipoib/ipoib_main.c 2005-12-16 
01:48:54.000000000 +0200
+++ linux-2.6.14/drivers/infiniband/ulp/ipoib/ipoib_main.c      2005-12-16 
01:52:26.000000000 +0200
@@ -71,6 +71,9 @@ static const u8 ipv4_bcast_addr[] = {
 
 struct workqueue_struct *ipoib_workqueue;
 
+static spinlock_t ipoib_neigh_ops_list_lock;
+static LIST_HEAD(ipoib_neigh_ops_list);
+
 static void ipoib_add_one(struct ib_device *device);
 static void ipoib_remove_one(struct ib_device *device);
 
@@ -244,9 +247,8 @@ static void path_free(struct net_device 
                 */
                if (neigh->ah)
                        ipoib_put_ah(neigh->ah);
-               *to_ipoib_neigh(neigh->neighbour) = NULL;
-               neigh->neighbour->ops->destructor = NULL;
-               kfree(neigh);
+
+               ipoib_neigh_free(neigh);
        }
 
        spin_unlock_irqrestore(&priv->lock, flags);
@@ -474,7 +476,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);
@@ -482,8 +484,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
@@ -526,11 +526,8 @@ static void neigh_add_path(struct sk_buf
        return;
 
 err:
-       *to_ipoib_neigh(skb->dst->neighbour) = NULL;
        list_del(&neigh->list);
-       neigh->neighbour->ops->destructor = NULL;
-       kfree(neigh);
-
+       ipoib_neigh_free(neigh);
        ++priv->stats.tx_dropped;
        dev_kfree_skb_any(skb);
 
@@ -757,8 +754,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);
@@ -767,23 +763,45 @@ static void ipoib_neigh_destructor(struc
                ipoib_put_ah(ah);
 }
 
-static int ipoib_neigh_setup(struct neighbour *neigh)
+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;
+
        /*
         * 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->ops->destructor = ipoib_neigh_destructor;
-
-       return 0;
+       spin_lock(&ipoib_neigh_ops_list_lock);
+       list_add_tail(&neigh->ops_list, &ipoib_neigh_ops_list);
+       neigh->neighbour->ops->destructor = ipoib_neigh_destructor;
+       spin_unlock(&ipoib_neigh_ops_list_lock);
+       return neigh;
 }
 
-static int ipoib_neigh_setup_dev(struct net_device *dev, struct neigh_parms 
*parms)
+void ipoib_neigh_free(struct ipoib_neigh *neigh)
 {
-       parms->neigh_setup = ipoib_neigh_setup;
+       struct ipoib_neigh *n;
 
-       return 0;
+       spin_lock(&ipoib_neigh_ops_list_lock);
+       list_del(&neigh->ops_list);
+
+       list_for_each_entry(n, &ipoib_neigh_ops_list, ops_list)
+               if (n->neighbour->ops == neigh->neighbour->ops)
+                       goto found;
+
+       neigh->neighbour->ops->destructor = NULL;
+found:
+       spin_unlock(&ipoib_neigh_ops_list_lock);
+       *to_ipoib_neigh(neigh->neighbour) = NULL;
+       kfree(neigh);
 }
 
 int ipoib_dev_init(struct net_device *dev, struct ib_device *ca, int port)
@@ -859,7 +877,6 @@ static void ipoib_setup(struct net_devic
        dev->tx_timeout          = ipoib_timeout;
        dev->hard_header         = ipoib_hard_header;
        dev->set_multicast_list  = ipoib_set_mcast_list;
-       dev->neigh_setup         = ipoib_neigh_setup_dev;
 
        dev->watchdog_timeo      = HZ;
 
@@ -1146,6 +1163,8 @@ static int __init ipoib_init_module(void
        if (ret)
                goto err_wq;
 
+       spin_lock_init(&ipoib_neigh_ops_list_lock);
+
        return 0;
 
 err_wq:
Index: linux-2.6.14/drivers/infiniband/ulp/ipoib/ipoib_multicast.c
===================================================================
--- linux-2.6.14.orig/drivers/infiniband/ulp/ipoib/ipoib_multicast.c    
2005-12-16 01:48:54.000000000 +0200
+++ linux-2.6.14/drivers/infiniband/ulp/ipoib/ipoib_multicast.c 2005-12-16 
01:49:08.000000000 +0200
@@ -107,9 +107,7 @@ static void ipoib_mcast_free(struct ipoi
        list_for_each_entry_safe(neigh, tmp, &mcast->neigh_list, list) {
                if (neigh->ah)
                        list_add_tail(&neigh->ah->list, &ah_list);
-               *to_ipoib_neigh(neigh->neighbour) = NULL;
-               neigh->neighbour->ops->destructor = NULL;
-               kfree(neigh);
+               ipoib_neigh_free(neigh);
        }
 
        spin_unlock_irqrestore(&priv->lock, flags);
@@ -733,13 +731,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);
                        }
                }
Index: linux-2.6.14/drivers/infiniband/ulp/ipoib/ipoib.h
===================================================================
--- linux-2.6.14.orig/drivers/infiniband/ulp/ipoib/ipoib.h      2005-12-16 
01:48:54.000000000 +0200
+++ linux-2.6.14/drivers/infiniband/ulp/ipoib/ipoib.h   2005-12-16 
01:50:46.000000000 +0200
@@ -215,6 +215,7 @@ struct ipoib_neigh {
        struct neighbour   *neighbour;
 
        struct list_head    list;
+       struct list_head    ops_list;
 };
 
 static inline struct ipoib_neigh **to_ipoib_neigh(struct neighbour *neigh)
@@ -223,6 +224,9 @@ static inline struct ipoib_neigh **to_ip
                                        (offsetof(struct neighbour, ha) & 4));
 }
 
+struct ipoib_neigh *ipoib_neigh_alloc(struct neighbour *neigh);
+void ipoib_neigh_free(struct ipoib_neigh *neigh);
+
 extern struct workqueue_struct *ipoib_workqueue;
 
 /* functions */

-- 
MST
_______________________________________________
openib-general mailing list
openib-general@openib.org
http://openib.org/mailman/listinfo/openib-general

To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general

Reply via email to