[PATCH rdma-next] IB/IPoIB: Avoid restoring OPER_UP after multicast flush

[PATCH rdma-next] IB/IPoIB: Avoid restoring OPER_UP after multicast flush

[Edward Srouji]

From: Carolina Jubran <cjubran@nvidia.com>

ipoib_ib_dev_flush_light() temporarily clears IPOIB_FLAG_OPER_UP to
prevent multicast joins while ipoib_mcast_dev_flush() is running, and
restores the flag afterwards if it was previously set.

This restore races with ipoib_ib_dev_down(). If the interface is brought
down while the flush is in progress, ipoib_ib_dev_down() clears
IPOIB_FLAG_OPER_UP, but the flush path may set it again after the device
has already gone down.

Since commit 894021a75291 ("IB/ipoib: Make the carrier_on_task race
aware"), ipoib_mcast_carrier_on_task() relies on IPOIB_FLAG_OPER_UP
being cleared to terminate its rtnl_trylock() retry loop. If the flag is
left set after shutdown, the workqueue retries forever, causing teardown
to deadlock when ipoib_ndo_uninit() waits in destroy_workqueue() while
holding RTNL.

Instead of overloading IPOIB_FLAG_OPER_UP to block multicast joins
during a light flush, introduce a dedicated IPOIB_FLAG_MCAST_FLUSH flag.
Use it together with IPOIB_FLAG_OPER_UP to determine whether multicast
joins are allowed, avoiding the race with device shutdown.

Fixes: 344bacca8cd8 ("IB/ipoib: Don't allow MC joins during light MC flush")
Reported-by: Ben Davies <ben.davies@gresearch.co.uk>
Signed-off-by: Carolina Jubran <cjubran@nvidia.com>
Reviewed-by: Cosmin Ratiu <cratiu@nvidia.com>
Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
Signed-off-by: Edward Srouji <edwards@nvidia.com>
---
 drivers/infiniband/ulp/ipoib/ipoib.h           |  7 +++++++
 drivers/infiniband/ulp/ipoib/ipoib_ib.c        | 12 +++++++-----
 drivers/infiniband/ulp/ipoib/ipoib_multicast.c | 16 ++++++++--------
 3 files changed, 22 insertions(+), 13 deletions(-)

diff --git a/drivers/infiniband/ulp/ipoib/ipoib.h b/drivers/infiniband/ulp/ipoib/ipoib.h
index 91f866e3fb8bd2aa3b7f248c709e8782c1ad0e30..143e03b6490210b708da08c63ce23e9ffce16aee 100644
--- a/drivers/infiniband/ulp/ipoib/ipoib.h
+++ b/drivers/infiniband/ulp/ipoib/ipoib.h
@@ -87,6 +87,7 @@ enum {
 	IPOIB_FLAG_INITIALIZED	  = 1,
 	IPOIB_FLAG_ADMIN_UP	  = 2,
 	IPOIB_PKEY_ASSIGNED	  = 3,
+	IPOIB_FLAG_MCAST_FLUSH	  = 4,
 	IPOIB_FLAG_SUBINTERFACE	  = 5,
 	IPOIB_STOP_REAPER	  = 7,
 	IPOIB_FLAG_ADMIN_CM	  = 9,
@@ -414,6 +415,12 @@ struct ipoib_dev_priv {
 	const struct net_device_ops	*rn_ops;
 };
 
+static inline bool ipoib_mcast_allowed(struct ipoib_dev_priv *priv)
+{
+	return test_bit(IPOIB_FLAG_OPER_UP, &priv->flags) &&
+	       !test_bit(IPOIB_FLAG_MCAST_FLUSH, &priv->flags);
+}
+
 struct ipoib_ah {
 	struct net_device *dev;
 	struct ib_ah	  *ah;
diff --git a/drivers/infiniband/ulp/ipoib/ipoib_ib.c b/drivers/infiniband/ulp/ipoib/ipoib_ib.c
index 5061d52a7b12c7d5cfed60746a694d1438e2a166..81bbb3f7c11321cc845ce15c333ce7feeb8d5e52 100644
--- a/drivers/infiniband/ulp/ipoib/ipoib_ib.c
+++ b/drivers/infiniband/ulp/ipoib/ipoib_ib.c
@@ -1227,17 +1227,19 @@ static void __ipoib_ib_dev_flush(struct ipoib_dev_priv *priv,
 	}
 
 	if (level == IPOIB_FLUSH_LIGHT) {
-		int oper_up;
 		ipoib_mark_paths_invalid(dev);
-		/* Set IPoIB operation as down to prevent races between:
+		/* Set MCAST_FLUSH to prevent races between:
 		 * the flush flow which leaves MCG and on the fly joins
 		 * which can happen during that time. mcast restart task
 		 * should deal with join requests we missed.
+		 *
+		 * Do not clear OPER_UP for this; restoring it races with
+		 * ipoib_ib_dev_down() and can leave OPER_UP set after the
+		 * device is down.
 		 */
-		oper_up = test_and_clear_bit(IPOIB_FLAG_OPER_UP, &priv->flags);
+		set_bit(IPOIB_FLAG_MCAST_FLUSH, &priv->flags);
 		ipoib_mcast_dev_flush(dev);
-		if (oper_up)
-			set_bit(IPOIB_FLAG_OPER_UP, &priv->flags);
+		clear_bit(IPOIB_FLAG_MCAST_FLUSH, &priv->flags);
 		ipoib_reap_dead_ahs(priv);
 	}
 
diff --git a/drivers/infiniband/ulp/ipoib/ipoib_multicast.c b/drivers/infiniband/ulp/ipoib/ipoib_multicast.c
index 6401af2fd548f1a37e25b6fdaee968b4fc2ff1bd..379b78374e210c3a3f373bb121f038c3c043c212 100644
--- a/drivers/infiniband/ulp/ipoib/ipoib_multicast.c
+++ b/drivers/infiniband/ulp/ipoib/ipoib_multicast.c
@@ -74,7 +74,7 @@ static void __ipoib_mcast_schedule_join_thread(struct ipoib_dev_priv *priv,
 					       struct ipoib_mcast *mcast,
 					       bool delay)
 {
-	if (!test_bit(IPOIB_FLAG_OPER_UP, &priv->flags))
+	if (!ipoib_mcast_allowed(priv))
 		return;
 
 	/*
@@ -469,7 +469,7 @@ static int ipoib_mcast_join(struct net_device *dev, struct ipoib_mcast *mcast)
 	int ret = 0;
 
 	if (!priv->broadcast ||
-	    !test_bit(IPOIB_FLAG_OPER_UP, &priv->flags))
+	    !ipoib_mcast_allowed(priv))
 		return -EINVAL;
 
 	init_completion(&mcast->done);
@@ -555,7 +555,7 @@ void ipoib_mcast_join_task(struct work_struct *work)
 	unsigned long delay_until = 0;
 	struct ipoib_mcast *mcast = NULL;
 
-	if (!test_bit(IPOIB_FLAG_OPER_UP, &priv->flags))
+	if (!ipoib_mcast_allowed(priv))
 		return;
 
 	if (ib_query_port(priv->ca, priv->port, &port_attr)) {
@@ -577,7 +577,7 @@ void ipoib_mcast_join_task(struct work_struct *work)
 	netif_addr_unlock_bh(dev);
 
 	spin_lock_irq(&priv->lock);
-	if (!test_bit(IPOIB_FLAG_OPER_UP, &priv->flags))
+	if (!ipoib_mcast_allowed(priv))
 		goto out;
 
 	if (!priv->broadcast) {
@@ -749,7 +749,7 @@ void ipoib_mcast_send(struct net_device *dev, u8 *daddr, struct sk_buff *skb)
 
 	spin_lock_irqsave(&priv->lock, flags);
 
-	if (!test_bit(IPOIB_FLAG_OPER_UP, &priv->flags)		||
+	if (!ipoib_mcast_allowed(priv)				||
 	    !priv->broadcast					||
 	    !test_bit(IPOIB_MCAST_FLAG_ATTACHED, &priv->broadcast->flags)) {
 		++dev->stats.tx_dropped;
@@ -871,7 +871,7 @@ void ipoib_mcast_restart_task(struct work_struct *work)
 	LIST_HEAD(remove_list);
 	struct ib_sa_mcmember_rec rec;
 
-	if (!test_bit(IPOIB_FLAG_OPER_UP, &priv->flags))
+	if (!ipoib_mcast_allowed(priv))
 		/*
 		 * shortcut...on shutdown flush is called next, just
 		 * let it do all the work
@@ -965,9 +965,9 @@ void ipoib_mcast_restart_task(struct work_struct *work)
 	ipoib_mcast_remove_list(&remove_list);
 
 	/*
-	 * Double check that we are still up
+	 * Double check that we are still up and not flushing
 	 */
-	if (test_bit(IPOIB_FLAG_OPER_UP, &priv->flags)) {
+	if (ipoib_mcast_allowed(priv)) {
 		spin_lock_irq(&priv->lock);
 		__ipoib_mcast_schedule_join_thread(priv, NULL, 0);
 		spin_unlock_irq(&priv->lock);

---
base-commit: cee9395acd8043be0644b25c34bfa86623f2b935
change-id: 20260902-avoid-rest-oper-up-55675774def8

Best regards,
-- 
Edward Srouji <edwards@nvidia.com>