Skip to contentSkip to conversation

iseries_veth: Cleanup skbs to prevent unregister_netdevice() hanging

PATCH In review Michael Ellerman 2 participants 2 messages May 12, 2005

Subsystems (2)
Networking Network drivers
lore
Share & tools
lore

Michael Ellerman

author
#0 lore raw

PATCH 4/4 iseries_veth: Cleanup skbs to prevent unregister_netdevice() hanging

Hi Andrew, Jeff,

The iseries_veth driver is badly behaved in that it will keep TX packets hanging around forever if they're not ACK'ed and the queue never fills up.

This causes the unregister_netdevice code to wait forever when we try to take the device down, because there's still skbs around with references to our struct net_device.

There's already code to cleanup any un-ACK'ed packets in veth_stop_connection() but it's being called after we unregister the net_device, which is too late.

The fix is to rearrange the module exit function so that we cleanup any outstanding skbs and then unregister the driver.

Signed-off-by Michael Ellerman <[email protected]>

--

 drivers/net/iseries_veth.c |   11 +++++++++--
 1 files changed, 9 insertions(+), 2 deletions(-)
drivers/net/iseries_veth.c +9 -2
Index: veth-fixes/drivers/net/iseries_veth.c
===================================================================
@@ -1388,18 +1388,25 @@
 {
 	int i;
 
-	vio_unregister_driver(&veth_driver);
+	/* Stop the queues first to stop any new packets being sent. */
+	for (i = 0; i < HVMAXARCHITECTEDVIRTUALLANS; i++)
+		if (veth_dev[i])
+			netif_stop_queue(veth_dev[i]);
 
+	/* Stop the connections before we unregister the driver. This
+	 * ensures there's no skbs lying around holding the device open. */
 	for (i = 0; i < HVMAXARCHITECTEDLPS; ++i)
 		veth_stop_connection(i);
 
 	HvLpEvent_unregisterHandler(HvLpEvent_Type_VirtualLan);
 
 	/* Hypervisor callbacks may have scheduled more work while we
-	 * were destroying connections. Now that we've disconnected from
+	 * were stoping connections. Now that we've disconnected from
 	 * the hypervisor make sure everything's finished. */
 	flush_scheduled_work();
 
+	vio_unregister_driver(&veth_driver);
+
 	for (i = 0; i < HVMAXARCHITECTEDLPS; ++i)
 		veth_destroy_connection(i);
Acked-by on v1 4/4
#1 lore raw
Quoted 9 lines · Michael Ellerman

Hi Andrew, Jeff,

The iseries_veth driver is badly behaved in that it will keep TX packets hanging around forever if they're not ACK'ed and the queue never fills up.

This causes the unregister_netdevice code to wait forever when we try to take the device down, because there's still skbs around with references to our struct net_device.

Quoted 7 lines

There's already code to cleanup any un-ACK'ed packets in veth_stop_connection() but it's being called after we unregister the net_device, which is too late.

The fix is to rearrange the module exit function so that we cleanup any outstanding skbs and then unregister the driver.

Signed-off-by Michael Ellerman <[email protected]>

Nice catch.

Acked-by David Gibson <[email protected]>
Signature · 4 lines
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/people/dgibson