netdev
[Top] [All Lists]

modular net drivers

To: "netdev@xxxxxxxxxxx" <netdev@xxxxxxxxxxx>
Subject: modular net drivers
From: Andrew Morton <andrewm@xxxxxxxxxx>
Date: Mon, 19 Jun 2000 23:54:30 +1000
Sender: owner-netdev@xxxxxxxxxxx
As you may have noticed, Al Viro is running around the kernel getting
rid of MOD_INC_USE_COUNT and MOD_DEC_USE_COUNT.  His long-term plan
is to remove MOD_INC_USE_COUNT and MOD_DEC_USE_COUNT completely.
I'm looking into the changes required for the net drivers.

I have attached here a patch which implements this.  It works with
3c59x.c, tested as a module and statically linked.  Also compiles
and works with CONFIG_MODULES turned off.

The implications of this are:

- All 2.4-only netdrivers can have all their MOD_DEC and MOD_INC calls
  removed.  All that twisty logic to keep track of the counts can be
  tossed.  A single

        SET_NETDEVICE_OWNER(dev);

  will be needed when the netdevice fields are being filled in.

- Drivers which support 2.2 will need to retain their MOD_INC/MOD_DEC
  macros and they will need to be given a SET_NETDEVICE_OWNER(). They
  will need to do this:

#if LINUX_VERSION_CODE < KERNEL_VERSION(2,4,0)
#define SET_NETDEVICE_OWNER(d)
#else
#define MOD_INC_USE_COUNT
#define MOD_DEC_USE_COUNT
#endif


A couple of sticky drivers which Al has identified are com20020_cs.c
and hysdn_net.c.  These both call dev->stop() in bizarre ways and
should be repaired anyway - they're either trying to bypass
notification and dev_clear_fastroute() or they're trying hard to
leak module refcounts and crash the machine.  They can continue
to use the legacy MOD_DEC and MOD_INC calls, but only until
these are tossed out altogether.

Al suggested that the SET_NETDEVICE_OWNER(dev) functionality
could be embedded within ether_setup() but I think it's better
open-coded in this manner - a few drivers (eg, shaper.c) don't
use ether_setup.

If/when this patch goes in it will NOT require that all netdevice
drivers be edited at the same time.  We can migrate them gradually.

Comments?  Gnashing of teeth?
--- linux-2.4.0-test1-ac21/include/linux/netdevice.h    Mon Jun 19 16:47:53 2000
+++ linux-akpm/include/linux/netdevice.h        Mon Jun 19 22:10:09 2000
@@ -136,6 +136,11 @@
 struct neigh_parms;
 struct sk_buff;
 
+/* Centralised module refcounting for netdevices */
+struct module;
+#define SET_NETDEVICE_OWNER(dev)       \
+       do { dev->owner = THIS_MODULE; } while (0)
+
 struct netif_rx_stats
 {
        unsigned total;
@@ -372,6 +377,9 @@
                                                     unsigned char *haddr);
        int                     (*neigh_setup)(struct net_device *dev, struct 
neigh_parms *);
        int                     (*accept_fastpath)(struct net_device *, struct 
dst_entry*);
+
+       /* open/release and usage marking */
+       struct module *owner;
 
        /* bridge stuff */
        struct net_bridge_port  *br_port;
--- linux-2.4.0-test1-ac21/net/core/dev.c       Mon Jun 19 16:47:54 2000
+++ linux-akpm/net/core/dev.c   Mon Jun 19 21:28:14 2000
@@ -89,6 +89,7 @@
 #include <net/profile.h>
 #include <linux/init.h>
 #include <linux/kmod.h>
+#include <linux/module.h>
 #if defined(CONFIG_NET_RADIO) || defined(CONFIG_NET_PCMCIA_RADIO)
 #include <linux/wireless.h>            /* Note : will define WIRELESS_EXT */
 #endif /* CONFIG_NET_RADIO || CONFIG_NET_PCMCIA_RADIO */
@@ -664,8 +665,13 @@
         *      Call device private open method
         */
         
-       if (dev->open) 
+       if (dev->open) {
+               if (dev->owner)
+                       __MOD_INC_USE_COUNT(dev->owner);
                ret = dev->open(dev);
+               if (ret != 0 && dev->owner)
+                       __MOD_DEC_USE_COUNT(dev->owner);
+       }
 
        /*
         *      If it went open OK then:
@@ -780,6 +786,13 @@
         *      Tell people we are down
         */
        notifier_call_chain(&netdev_chain, NETDEV_DOWN, dev);
+
+       /*
+        * Drop the module refcount
+        */
+       if (dev->owner) {
+               __MOD_DEC_USE_COUNT(dev->owner);
+       }
 
        return(0);
 }
<Prev in Thread] Current Thread [Next in Thread>