[Open-FCoE] [PATCH V5 15/16] fcoe: Fix module ref count bug by adding NETDEV UNREGISTER handling

Chris Leech christopher.leech at intel.com
Tue Jul 28 18:02:33 UTC 2009


On Mon, Jul 27, 2009 at 07:12:33AM -0700, Fastabend, John R wrote:

> Also there is a memory leak with dev_addr_delete. If dev_addr_delete is 
> called after the netdevice has flushed the dev_addr_list (which is done 
> after the NETDEV_UNREGISTER notifiers return) calling dev_addr_delete 
> does not free memory correctly. If the work scheduled by the netdev 
> unregister handling happens after the dev_addr_list is flushed this 
> could occur. Testing this gave the error "__dev_addr_discard: address 
> leakage!" which seems to indicate this is happening.

I think this should fix it, I'm not crazy about it but it works.
Basically I took the networking cleanup (addresses and packet handlers)
out of the normal destroy path.  Then I made sure they were called
synchronously before destroying the instance (or scheduling the destroy
for later.)  That should fix the warnings that happen when there are
multicast addresses left over after the notifier is called.

	Chris

diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c
index 276d42f..3d0bd1e 100644
--- a/drivers/scsi/fcoe/fcoe.c
+++ b/drivers/scsi/fcoe/fcoe.c
@@ -253,6 +253,8 @@ static struct fcoe_interface *fcoe_interface_create(struct net_device *netdev)
 /**
  * fcoe_interface_cleanup() - clean up netdev configurations
  * @fcoe:
+ *
+ * Caller must be holding the RTNL mutex
  */
 void fcoe_interface_cleanup(struct fcoe_interface *fcoe)
 {
@@ -270,11 +272,7 @@ void fcoe_interface_cleanup(struct fcoe_interface *fcoe)
 	__dev_remove_pack(&fcoe->fip_packet_type);
 	synchronize_net();
 
-	/* tear-down the FCoE controller */
-	fcoe_ctlr_destroy(&fcoe->ctlr);
-
 	/* Delete secondary MAC addresses */
-	rtnl_lock();
 	memcpy(flogi_maddr, (u8[6]) FC_FCOE_FLOGI_MAC, ETH_ALEN);
 	dev_unicast_delete(netdev, flogi_maddr);
 	if (!is_zero_ether_addr(fip->data_src_addr))
@@ -282,7 +280,6 @@ void fcoe_interface_cleanup(struct fcoe_interface *fcoe)
 	if (fip->spma)
 		dev_unicast_delete(netdev, fip->ctl_src_addr);
 	dev_mc_delete(netdev, FIP_ALL_ENODE_MACS, ETH_ALEN, 0);
-	rtnl_unlock();
 }
 
 /**
@@ -296,7 +293,8 @@ static void fcoe_interface_release(struct kref *kref)
 
 	fcoe = container_of(kref, struct fcoe_interface, kref);
 	netdev = fcoe->netdev;
-	fcoe_interface_cleanup(fcoe);
+	/* tear-down the FCoE controller */
+	fcoe_ctlr_destroy(&fcoe->ctlr);
 	kfree(fcoe);
 	dev_put(netdev);
 }
@@ -1574,6 +1572,7 @@ static int fcoe_device_notification(struct notifier_block *notifier,
 	case NETDEV_UNREGISTER:
 		list_del(&fcoe->list);
 		port = lport_priv(fcoe->ctlr.lp);
+		fcoe_interface_cleanup(fcoe);
 		schedule_work(&port->destroy_work);
 		goto out;
 		break;
@@ -1653,6 +1652,7 @@ static int fcoe_destroy(const char *buffer, struct kernel_param *kp)
 		goto out_putdev;
 	}
 	list_del(&fcoe->list);
+	fcoe_interface_cleanup(fcoe);
 	rtnl_unlock();
 	fcoe_if_destroy(fcoe->ctlr.lp);
 out_putdev:
@@ -1722,6 +1722,9 @@ static int fcoe_create(const char *buffer, struct kernel_param *kp)
 		printk(KERN_ERR "fcoe: Failed to create interface (%s)\n",
 		       netdev->name);
 		rc = -EIO;
+		rtnl_lock();
+		fcoe_interface_cleanup(fcoe);
+		rtnl_unlock();
 		goto out_free;
 	}
 
@@ -1986,6 +1989,7 @@ static void __exit fcoe_exit(void)
 	list_for_each_entry_safe(fcoe, tmp, &fcoe_hostlist, list) {
 		list_del(&fcoe->list);
 		port = lport_priv(fcoe->ctlr.lp);
+		fcoe_interface_cleanup(fcoe);
 		schedule_work(&port->destroy_work);
 	}
 	rtnl_unlock();



More information about the devel mailing list