Re: [PATCH 2.5.67] Update VLAN to new module semantics

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



One problem with the last patch was it did not correctly cleanup the
group stuff if try_module_get failed.

Here is a third version. It checks register_netdev() which could fail
and moves the register and try_module up to correctly handle the unwind
case.

Seems to make sense to get device registered and ref count before putting
in the group structure.

diff -urN -X dontdiff linux-2.5/net/8021q/vlan.c linux-2.5-vlan/net/8021q/vlan.c
--- linux-2.5/net/8021q/vlan.c	2003-04-03 12:13:09.000000000 -0800
+++ linux-2.5-vlan/net/8021q/vlan.c	2003-04-09 12:09:24.000000000 -0700
@@ -29,7 +29,6 @@
 #include <net/p8022.h>
 #include <net/arp.h>
 #include <linux/rtnetlink.h>
-#include <linux/brlock.h>
 #include <linux/notifier.h>
 
 #include <linux/if_vlan.h>
@@ -68,7 +67,6 @@
 	.dev =NULL,
 	.func = vlan_skb_recv, /* VLAN receive method */
 	.data = (void *)(-1),  /* Set here '(void *)1' when this code can SHARE SKBs */
-	.next = NULL
 };
 
 /* End of global variables definitions. */
@@ -108,7 +106,7 @@
 }
 
 /*
- *     Module 'remove' entry point.
+ *     Module 'remove' enry point.
  *     o delete /proc/net/router directory and static entries.
  */ 
 static void __exit vlan_cleanup_module(void)
@@ -231,9 +229,8 @@
 				real_dev->vlan_rx_kill_vid(real_dev, vlan_id);
 			}
 
-			br_write_lock(BR_NETPROTO_LOCK);
 			grp->vlan_devices[vlan_id] = NULL;
-			br_write_unlock(BR_NETPROTO_LOCK);
+			synchronize_net();
 
 
 			/* Caller unregisters (and if necessary, puts)
@@ -266,7 +263,7 @@
 				ret = 1;
 			}
 
-			MOD_DEC_USE_COUNT;
+			module_put(THIS_MODULE);
 		}
 	}
 
@@ -433,6 +430,7 @@
 	/* set up method calls */
 	new_dev->init = vlan_dev_init;
 	new_dev->destructor = vlan_dev_destruct;
+	new_dev->owner = THIS_MODULE;
 	    
 	/* new_dev->ifindex = 0;  it will be set when added to
 	 * the global list.
@@ -503,6 +501,15 @@
 	       real_dev->ifindex);
 #endif
 	    
+	if (register_netdevice(new_dev))
+		goto out_free_newdev_priv;
+
+	/* NOTE:  We have a reference to the real device,
+	 * so hold on to the reference. May fail if we are being removed
+	 */
+	if (!try_module_get(THIS_MODULE))
+		goto out_free_unregister;
+
 	/* So, got the sucker initialized, now lets place
 	 * it into our local structure.
 	 */
@@ -516,7 +523,7 @@
 	if (!grp) { /* need to add a new group */
 		grp = kmalloc(sizeof(struct vlan_group), GFP_KERNEL);
 		if (!grp)
-			goto out_free_newdev_priv;
+			goto out_free_put;
 					
 		/* printk(KERN_ALERT "VLAN REGISTER:  Allocated new group.\n"); */
 		memset(grp, 0, sizeof(struct vlan_group));
@@ -537,18 +544,18 @@
 	if (real_dev->features & NETIF_F_HW_VLAN_FILTER)
 		real_dev->vlan_rx_add_vid(real_dev, VLAN_ID);
 
-	register_netdevice(new_dev);
-
 	rtnl_unlock();
-	    
-	/* NOTE:  We have a reference to the real device,
-	 * so hold on to the reference.
-	 */
-	MOD_INC_USE_COUNT; /* Add was a success!! */
+
+
 #ifdef VLAN_DEBUG
 	printk(VLAN_DBG "Allocated new device successfully, returning.\n");
 #endif
 	return new_dev;
+out_free_put:
+	module_put(THIS_MODULE);
+
+out_free_unregister:
+	unregister_netdev(new_dev);
 
 out_free_newdev_priv:
 	kfree(new_dev->priv);
diff -urN -X dontdiff linux-2.5/net/8021q/vlan_dev.c linux-2.5-vlan/net/8021q/vlan_dev.c
--- linux-2.5/net/8021q/vlan_dev.c	2003-04-03 12:13:09.000000000 -0800
+++ linux-2.5-vlan/net/8021q/vlan_dev.c	2003-04-08 15:50:58.000000000 -0700
@@ -31,7 +31,6 @@
 #include <net/datalink.h>
 #include <net/p8022.h>
 #include <net/arp.h>
-#include <linux/brlock.h>
 
 #include "vlan.h"
 #include "vlanproc.h"
-
: send the line "unsubscribe linux-net" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Netdev]     [Ethernet Bridging]     [Linux 802.1Q VLAN]     [Linux Wireless]     [Kernel Newbies]     [Security]     [Linux for Hams]     [Netfilter]     [Git]     [Bugtraq]     [Yosemite News and Information]     [MIPS Linux]     [ARM Linux]     [Linux RAID]     [Linux PCI]     [Linux Admin]     [Samba]

  Powered by Linux