[ewg] Re: [RFC] [PATCH v4] IB/ipoib: Add bonding support to IPoIB

[Moni Shoua]

Michael S. Tsirkin wrote:
>> Quoting Moni Shoua <monisonlists at gmail.com>:
>> Subject: [RFC] [PATCH v4] IB/ipoib: Add bonding support to IPoIB
>>
>> Hi,
>>
>> The previous version of the patch changed ipoib_neigh_destructor to take 
>> the pointer to neigh outside the lock. This might be a risk so I wrote a 
>> new version of this patch so the change would affect only when bonding is used.
>> Please see below...
> 
> This looks pretty safe for OFED 1.2 insofar as this won't affect
> someone not using bonding.
> 
> Long term, we still need to find a proper solution for whenever
> this bonding related code goes GA/upstream.
> 
Thanks Michael.

Vlad,
Can you add this patch to kernel_patches/fixes please?
Please note that this patch breaks one of the backport patches: ipoib_8111_to_2_6_16.patch.
I adapted the broken patch so it can be applied after the bonding patch. 
I tested that it works for RH4UP3. You can take it from below.


Index: ofa_kernel-1.2/drivers/infiniband/ulp/ipoib/ipoib.h
===================================================================
--- ofa_kernel-1.2.orig/drivers/infiniband/ulp/ipoib/ipoib.h	2007-03-28 17:52:59.000000000 +0200
+++ ofa_kernel-1.2/drivers/infiniband/ulp/ipoib/ipoib.h	2007-03-28 17:53:41.000000000 +0200
@@ -218,6 +218,7 @@ struct ipoib_neigh {
 	struct neighbour   *neighbour;
 	struct net_device *dev;
 
+	struct list_head    all_neigh_list;
 	struct list_head    list;
 };
 
Index: ofa_kernel-1.2/drivers/infiniband/ulp/ipoib/ipoib_main.c
===================================================================
--- ofa_kernel-1.2.orig/drivers/infiniband/ulp/ipoib/ipoib_main.c	2007-03-28 17:52:59.000000000 +0200
+++ ofa_kernel-1.2/drivers/infiniband/ulp/ipoib/ipoib_main.c	2007-03-28 18:07:50.000000000 +0200
@@ -85,6 +85,9 @@ struct workqueue_struct *ipoib_workqueue
 
 struct ib_sa_client ipoib_sa_client;
 
+static DEFINE_SPINLOCK(ipoib_all_neigh_list_lock);
+static LIST_HEAD(ipoib_all_neigh_list);
+
 static void ipoib_add_one(struct ib_device *device);
 static void ipoib_remove_one(struct ib_device *device);
 
@@ -783,6 +786,18 @@ static void ipoib_neigh_destructor(struc
 		} else
 			return;
 	}
+
+	struct ipoib_neigh *tn, *nn = NULL;
+	spin_lock(&ipoib_all_neigh_list_lock);
+	list_for_each_entry(tn, &ipoib_all_neigh_list, all_neigh_list)
+		if (tn->neighbour == n) {
+			nn = tn;
+			break;
+		}
+	spin_unlock(&ipoib_all_neigh_list_lock);
+	if (!nn)
+		return;
+
 	ipoib_dbg(priv,
 		  "neigh_destructor for %06x " IPOIB_GID_FMT "\n",
 		  IPOIB_QPN(n->ha),
@@ -819,6 +834,11 @@ struct ipoib_neigh *ipoib_neigh_alloc(st
 	*to_ipoib_neigh(neighbour) = neigh;
 	skb_queue_head_init(&neigh->queue);
 
+	spin_lock(&ipoib_all_neigh_list_lock);
+	list_add_tail(&neigh->all_neigh_list, &ipoib_all_neigh_list);
+	neigh->neighbour->ops->destructor = ipoib_neigh_destructor;
+	spin_unlock(&ipoib_all_neigh_list_lock);
+
 	return neigh;
 }
 
@@ -826,6 +846,17 @@ void ipoib_neigh_free(struct net_device 
 {
 	struct ipoib_dev_priv *priv = netdev_priv(dev);
 	struct sk_buff *skb;
+	struct ipoib_neigh *nn;
+	spin_lock(&ipoib_all_neigh_list_lock);
+	list_del(&neigh->all_neigh_list);
+	list_for_each_entry(nn, &ipoib_all_neigh_list, all_neigh_list)
+		if (nn->neighbour->ops == neigh->neighbour->ops)
+			goto found;
+
+	neigh->neighbour->ops->destructor = NULL;
+found:
+	spin_unlock(&ipoib_all_neigh_list_lock);
+
 	*to_ipoib_neigh(neigh->neighbour) = NULL;
 	while ((skb = __skb_dequeue(&neigh->queue))) {
 		++priv->stats.tx_dropped;
@@ -836,8 +867,6 @@ void ipoib_neigh_free(struct net_device 
 
 static int ipoib_neigh_setup_dev(struct net_device *dev, struct neigh_parms *parms)
 {
-	parms->neigh_destructor = ipoib_neigh_destructor;
-
 	return 0;
 }