Commit 988bd503 authored by Eli Cohen's avatar Eli Cohen Committed by Roland Dreier
Browse files

IPoIB: Fix memory leak of multicast group structures



The current handling of multicast groups in IPoIB ends up never
freeing send-only multicast groups.  It turns out the logic was much
more complicated than it needed to be; we can fix this bug and
completely kill ipoib_mcast_dev_down() at the same time.

Signed-off-by: default avatarEli Cohen <eli@mellanox.co.il>
Signed-off-by: default avatarMichael S. Tsirkin <mst@mellanox.co.il>
Signed-off-by: default avatarRoland Dreier <rolandd@cisco.com>
parent 78bfe0b5
Loading
Loading
Loading
Loading
+1 −12
Original line number Diff line number Diff line
@@ -453,17 +453,8 @@ int ipoib_ib_dev_down(struct net_device *dev)
	}

	ipoib_mcast_stop_thread(dev, 1);

	/*
	 * Flush the multicast groups first so we stop any multicast joins. The
	 * completion thread may have already died and we may deadlock waiting
	 * for the completion thread to finish some multicast joins.
	 */
	ipoib_mcast_dev_flush(dev);

	/* Delete broadcast and local addresses since they will be recreated */
	ipoib_mcast_dev_down(dev);

	ipoib_flush_paths(dev);

	return 0;
@@ -624,9 +615,7 @@ void ipoib_ib_dev_cleanup(struct net_device *dev)
	ipoib_dbg(priv, "cleaning up ib_dev\n");

	ipoib_mcast_stop_thread(dev, 1);

	/* Delete the broadcast address and the local address */
	ipoib_mcast_dev_down(dev);
	ipoib_mcast_dev_flush(dev);

	ipoib_transport_dev_cleanup(dev);
}
+8 −53
Original line number Diff line number Diff line
@@ -742,50 +742,23 @@ void ipoib_mcast_dev_flush(struct net_device *dev)
{
	struct ipoib_dev_priv *priv = netdev_priv(dev);
	LIST_HEAD(remove_list);
	struct ipoib_mcast *mcast, *tmcast, *nmcast;
	struct ipoib_mcast *mcast, *tmcast;
	unsigned long flags;

	ipoib_dbg_mcast(priv, "flushing multicast list\n");

	spin_lock_irqsave(&priv->lock, flags);
	list_for_each_entry_safe(mcast, tmcast, &priv->multicast_list, list) {
		nmcast = ipoib_mcast_alloc(dev, 0);
		if (nmcast) {
			nmcast->flags =
				mcast->flags & (1 << IPOIB_MCAST_FLAG_SENDONLY);

			nmcast->mcmember.mgid = mcast->mcmember.mgid;

			/* Add the new group in before the to-be-destroyed group */
			list_add_tail(&nmcast->list, &mcast->list);
			list_del_init(&mcast->list);

			rb_replace_node(&mcast->rb_node, &nmcast->rb_node,
					&priv->multicast_tree);

	list_for_each_entry_safe(mcast, tmcast, &priv->multicast_list, list) {
		list_del(&mcast->list);
		rb_erase(&mcast->rb_node, &priv->multicast_tree);
		list_add_tail(&mcast->list, &remove_list);
		} else {
			ipoib_warn(priv, "could not reallocate multicast group "
				   IPOIB_GID_FMT "\n",
				   IPOIB_GID_ARG(mcast->mcmember.mgid));
		}
	}

	if (priv->broadcast) {
		nmcast = ipoib_mcast_alloc(dev, 0);
		if (nmcast) {
			nmcast->mcmember.mgid = priv->broadcast->mcmember.mgid;

			rb_replace_node(&priv->broadcast->rb_node,
					&nmcast->rb_node,
					&priv->multicast_tree);

 		rb_erase(&priv->broadcast->rb_node, &priv->multicast_tree);
		list_add_tail(&priv->broadcast->list, &remove_list);
			priv->broadcast = nmcast;
		} else
			ipoib_warn(priv, "could not reallocate broadcast group "
                        	          IPOIB_GID_FMT "\n",
                                	  IPOIB_GID_ARG(priv->broadcast->mcmember.mgid));
		priv->broadcast = NULL;
	}

	spin_unlock_irqrestore(&priv->lock, flags);
@@ -796,24 +769,6 @@ void ipoib_mcast_dev_flush(struct net_device *dev)
	}
}

void ipoib_mcast_dev_down(struct net_device *dev)
{
	struct ipoib_dev_priv *priv = netdev_priv(dev);
	unsigned long flags;

	/* Delete broadcast since it will be recreated */
	if (priv->broadcast) {
		ipoib_dbg_mcast(priv, "deleting broadcast group\n");

		spin_lock_irqsave(&priv->lock, flags);
		rb_erase(&priv->broadcast->rb_node, &priv->multicast_tree);
		spin_unlock_irqrestore(&priv->lock, flags);
		ipoib_mcast_leave(dev, priv->broadcast);
		ipoib_mcast_free(priv->broadcast);
		priv->broadcast = NULL;
	}
}

void ipoib_mcast_restart_task(void *dev_ptr)
{
	struct net_device *dev = dev_ptr;