[ofa-general] Re: [PATCH] IB/ipoib: avoid WARN_ON on NULL path->ah

[Roland Dreier]

Would it be simpler to just not update path->ah if the query fails?  In
the case of the new optimization on path flushing, path->valid will be 0
so there's no reason to change path->ah to NULL if we're not going to
change path->valid anyway.

In fact this seems more in keeping with the idea behind ee1e2c82
("IPoIB: Refresh paths instead of flushing them on SM change events"),
because your path kills the ah in the neigh struct as soon as a path
record query fails, when the point of the original patch was to keep
using addresses that are probably still valid across an SM failure.

The patch I'm proposing is the following.  If this looks OK, I'll send
it to Linus tomorrow.

diff --git a/drivers/infiniband/ulp/ipoib/ipoib_main.c b/drivers/infiniband/ulp/ipoib/ipoib_main.c
index 1b1df5c..e9ca3cb 100644
--- a/drivers/infiniband/ulp/ipoib/ipoib_main.c
+++ b/drivers/infiniband/ulp/ipoib/ipoib_main.c
@@ -404,7 +404,7 @@ static void path_rec_completion(int status,
 	struct net_device *dev = path->dev;
 	struct ipoib_dev_priv *priv = netdev_priv(dev);
 	struct ipoib_ah *ah = NULL;
-	struct ipoib_ah *old_ah;
+	struct ipoib_ah *old_ah = NULL;
 	struct ipoib_neigh *neigh, *tn;
 	struct sk_buff_head skqueue;
 	struct sk_buff *skb;
@@ -428,12 +428,12 @@ static void path_rec_completion(int status,
 
 	spin_lock_irqsave(&priv->lock, flags);
 
-	old_ah   = path->ah;
-	path->ah = ah;
-
 	if (ah) {
 		path->pathrec = *pathrec;
 
+		old_ah   = path->ah;
+		path->ah = ah;
+
 		ipoib_dbg(priv, "created address handle %p for LID 0x%04x, SL %d\n",
 			  ah, be16_to_cpu(pathrec->dlid), pathrec->sl);