[ofa-general] [PATCH] IB/ipoib: drain cq in dev_stop

[Michael S. Tsirkin]

Fix 2 bugs in RX draining code:
1. The logic in time_after is reversed, so it was timing out immediately
2. Since netpoll is disabled while ipoib_cm_dev_stop is running,
   ipoib_cm_dev_stop must poll the CQ in order to see the draining packet.

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

---

Pls review the above.

I'm still uncomfortable with the fact that ipoib_ib_dev_stop could cause
packets to be passed up without poll being called. Is this OK?

It is possible we never saw problems in practice because the race window is
small, but it seems that we should pass a flag to handle_rx_wc routines to have
it drop all packets.  Roland, what do you think?

diff --git a/drivers/infiniband/ulp/ipoib/ipoib.h b/drivers/infiniband/ulp/ipoib/ipoib.h
index a0b3782..158759e 100644
--- a/drivers/infiniband/ulp/ipoib/ipoib.h
+++ b/drivers/infiniband/ulp/ipoib/ipoib.h
@@ -429,6 +429,7 @@ int ipoib_vlan_delete(struct net_device *pdev, unsigned short pkey);
 
 void ipoib_pkey_poll(struct work_struct *work);
 int ipoib_pkey_dev_delay_open(struct net_device *dev);
+void ipoib_drain_cq(struct net_device *dev);
 
 #ifdef CONFIG_INFINIBAND_IPOIB_CM
 
diff --git a/drivers/infiniband/ulp/ipoib/ipoib_cm.c b/drivers/infiniband/ulp/ipoib/ipoib_cm.c
index ffec794..dc299db 100644
--- a/drivers/infiniband/ulp/ipoib/ipoib_cm.c
+++ b/drivers/infiniband/ulp/ipoib/ipoib_cm.c
@@ -713,7 +713,7 @@ void ipoib_cm_dev_stop(struct net_device *dev)
 	while (!list_empty(&priv->cm.rx_error_list) ||
 	       !list_empty(&priv->cm.rx_flush_list) ||
 	       !list_empty(&priv->cm.rx_drain_list)) {
-		if (!time_after(jiffies, begin + 5 * HZ)) {
+		if (time_after(jiffies, begin + 5 * HZ)) {
 			ipoib_warn(priv, "RX drain timing out\n");
 
 			/*
@@ -725,6 +725,7 @@ void ipoib_cm_dev_stop(struct net_device *dev)
 			break;
 		}
 		spin_unlock_irq(&priv->lock);
+		ipoib_drain_cq(dev);
 		msleep(1);
 		spin_lock_irq(&priv->lock);
 	}
diff --git a/drivers/infiniband/ulp/ipoib/ipoib_ib.c b/drivers/infiniband/ulp/ipoib/ipoib_ib.c
index c1aad06..8404f05 100644
--- a/drivers/infiniband/ulp/ipoib/ipoib_ib.c
+++ b/drivers/infiniband/ulp/ipoib/ipoib_ib.c
@@ -550,13 +550,30 @@ static int recvs_pending(struct net_device *dev)
 	return pending;
 }
 
+void ipoib_drain_cq(struct net_device *dev)
+{
+	struct ipoib_dev_priv *priv = netdev_priv(dev);
+	int i, n;
+	do {
+		n = ib_poll_cq(priv->cq, IPOIB_NUM_WC, priv->ibwc);
+		for (i = 0; i < n; ++i) {
+			if (priv->ibwc[i].wr_id & IPOIB_CM_OP_SRQ)
+				ipoib_cm_handle_rx_wc(dev, priv->ibwc + i);
+			else if (priv->ibwc[i].wr_id & IPOIB_OP_RECV)
+				ipoib_ib_handle_rx_wc(dev, priv->ibwc + i);
+			else
+				ipoib_ib_handle_tx_wc(dev, priv->ibwc + i);
+		}
+	} while (n == IPOIB_NUM_WC);
+}
+
 int ipoib_ib_dev_stop(struct net_device *dev, int flush)
 {
 	struct ipoib_dev_priv *priv = netdev_priv(dev);
 	struct ib_qp_attr qp_attr;
 	unsigned long begin;
 	struct ipoib_tx_buf *tx_req;
-	int i, n;
+	int i;
 
 	clear_bit(IPOIB_FLAG_INITIALIZED, &priv->flags);
 	netif_poll_disable(dev);
@@ -611,17 +628,7 @@ int ipoib_ib_dev_stop(struct net_device *dev, int flush)
 			goto timeout;
 		}
 
-		do {
-			n = ib_poll_cq(priv->cq, IPOIB_NUM_WC, priv->ibwc);
-			for (i = 0; i < n; ++i) {
-				if (priv->ibwc[i].wr_id & IPOIB_CM_OP_SRQ)
-					ipoib_cm_handle_rx_wc(dev, priv->ibwc + i);
-				else if (priv->ibwc[i].wr_id & IPOIB_OP_RECV)
-					ipoib_ib_handle_rx_wc(dev, priv->ibwc + i);
-				else
-					ipoib_ib_handle_tx_wc(dev, priv->ibwc + i);
-			}
-		} while (n == IPOIB_NUM_WC);
+		ipoib_drain_cq(dev);
 
 		msleep(1);
 	}

-- 
MST