datapath: Move sysfs support from brcompat_mod into openvswitch_mod.
authorBen Pfaff <blp@nicira.com>
Wed, 5 Aug 2009 19:51:30 +0000 (12:51 -0700)
committerBen Pfaff <blp@nicira.com>
Thu, 6 Aug 2009 23:57:06 +0000 (16:57 -0700)
In the past problems have arisen due to the different ways that datapaths
are created and destroyed in the three different cases:

1. sysfs supported, brcompat_mod loaded.
2. sysfs supported, brcompat_mod not loaded.
3. sysfs not supported.

The brcompat_mod loaded versus not loaded distinction is the stickiest
because we have to do all the calls into brcompat_mod through hook
functions, which in turn causes pressure to keep the number of hook
functions small and well-defined, which makes it really difficult to put
the hook call points at exactly the right place.  Witness, for example,
this piece of code in datapath.c:

        int dp_del_port(struct net_bridge_port *p)
        {
                ASSERT_RTNL();

        #ifdef SUPPORT_SYSFS
                if (p->port_no != ODPP_LOCAL && dp_del_if_hook)
                        sysfs_remove_link(&p->dp->ifobj, p->dev->name);
        #endif

The code inside the #ifdef is logically part of the brcompat_mod sysfs
support, but the author of this code (quite reasonably) didn't want to
add a hook function call there.  After all, what would you call the
hook function?  There's no obvious name from the dp_del_port() caller's
perspective.

All this argues that sysfs support should be in openvswitch_mod itself,
since it has to be tightly integrated, not bolted on.  So this commit
moves it there.

Now, this is not to say that openvswitch_mod should actually be
implementing bridge-compatible sysfs.  In the future, it probably should
not be; rather, it should implement something appropriate for Open vSwitch
datapaths instead.  But right now we have bridge-compatible sysfs, and so
that's what this commit moves.

datapath/Modules.mk
datapath/brcompat.c
datapath/datapath.c
datapath/datapath.h
datapath/linux-2.6/Modules.mk

index 1b5de4aba837a67461087bac5c9d6565d0dab556..b7d54d1a060769c369ae02fd0bb46893475321ab 100644 (file)
@@ -11,6 +11,8 @@ dist_modules = $(both_modules)        # Modules to distribute
 
 openvswitch_sources = \
        actions.c \
+       brc_sysfs_dp.c \
+       brc_sysfs_if.c \
        datapath.c \
        dp_dev.c \
        dp_notify.c \
@@ -19,6 +21,7 @@ openvswitch_sources = \
 
 openvswitch_headers = \
        actions.h \
+       brc_sysfs.h \
        compat.h \
        datapath.h \
        dp_dev.h \
index db0a70f53cc3b9712b0beb3d3fc5c43d663e3f0c..46e7f2b0866170450232dcc1128fdd07632b5fb6 100644 (file)
@@ -523,27 +523,6 @@ error:
        return ERR_PTR(error);
 }
 
-int brc_add_dp(struct datapath *dp)
-{
-       if (!try_module_get(THIS_MODULE))
-               return -ENODEV;
-#ifdef SUPPORT_SYSFS
-       brc_sysfs_add_dp(dp);
-#endif
-
-       return 0;
-}
-
-int brc_del_dp(struct datapath *dp) 
-{
-#ifdef SUPPORT_SYSFS
-       brc_sysfs_del_dp(dp);
-#endif
-       module_put(THIS_MODULE);
-
-       return 0;
-}
-
 static int 
 __init brc_init(void)
 {
@@ -568,16 +547,6 @@ __init brc_init(void)
        /* Set the openvswitch_mod device ioctl handler */
        dp_ioctl_hook = brc_dev_ioctl;
 
-       /* Register hooks for datapath adds and deletes */
-       dp_add_dp_hook = brc_add_dp;
-       dp_del_dp_hook = brc_del_dp;
-
-       /* Register hooks for interface adds and deletes */
-#ifdef SUPPORT_SYSFS
-       dp_add_if_hook = brc_sysfs_add_if;
-       dp_del_if_hook = brc_sysfs_del_if;
-#endif
-
        /* Randomize the initial sequence number.  This is not a security
         * feature; it only helps avoid crossed wires between userspace and
         * the kernel when the module is unloaded and reloaded. */
@@ -618,14 +587,6 @@ error:
 static void 
 brc_cleanup(void)
 {
-       /* Unregister hooks for datapath adds and deletes */
-       dp_add_dp_hook = NULL;
-       dp_del_dp_hook = NULL;
-       
-       /* Unregister hooks for interface adds and deletes */
-       dp_add_if_hook = NULL;
-       dp_del_if_hook = NULL;
-
        /* Unregister ioctl hooks */
        dp_ioctl_hook = NULL;
        brioctl_set(NULL);
index 43d96fbdecb58eb85f85092cb68c015cf3f31594..cfe660ff494af8a9fb59abbd4599fc24e953e835 100644 (file)
 int (*dp_ioctl_hook)(struct net_device *dev, struct ifreq *rq, int cmd);
 EXPORT_SYMBOL(dp_ioctl_hook);
 
-int (*dp_add_dp_hook)(struct datapath *dp);
-EXPORT_SYMBOL(dp_add_dp_hook);
-
-int (*dp_del_dp_hook)(struct datapath *dp);
-EXPORT_SYMBOL(dp_del_dp_hook);
-
-int (*dp_add_if_hook)(struct net_bridge_port *p);
-EXPORT_SYMBOL(dp_add_if_hook);
-
-int (*dp_del_if_hook)(struct net_bridge_port *p);
-EXPORT_SYMBOL(dp_del_if_hook);
-
 /* Datapaths.  Protected on the read side by rcu_read_lock, on the write side
  * by dp_mutex.  dp_mutex is almost completely redundant with genl_mutex
  * maintained by the Generic Netlink code, but the timeout path needs mutual
@@ -251,8 +239,9 @@ static int create_dp(int dp_idx, const char __user *devnamep)
        mutex_unlock(&dp_mutex);
        rtnl_unlock();
 
-       if (dp_add_dp_hook)
-               dp_add_dp_hook(dp);
+#ifdef SUPPORT_SYSFS
+       brc_sysfs_add_dp(dp);
+#endif
 
        return 0;
 
@@ -280,8 +269,9 @@ static void do_destroy_dp(struct datapath *dp)
                if (p->port_no != ODPP_LOCAL)
                        dp_del_port(p);
 
-       if (dp_del_dp_hook)
-               dp_del_dp_hook(dp);
+#ifdef SUPPORT_SYSFS
+       brc_sysfs_del_dp(dp);
+#endif
 
        rcu_assign_pointer(dps[dp->dp_idx], NULL);
 
@@ -403,8 +393,9 @@ static int add_port(int dp_idx, struct odp_port __user *portp)
        if (err)
                goto out_put;
 
-       if (dp_add_if_hook)
-               dp_add_if_hook(dp->ports[port_no]);
+#ifdef SUPPORT_SYSFS
+       brc_sysfs_add_if(dp->ports[port_no]);
+#endif
 
 out_put:
        dev_put(dev);
@@ -421,7 +412,7 @@ int dp_del_port(struct net_bridge_port *p)
        ASSERT_RTNL();
 
 #ifdef SUPPORT_SYSFS
-       if (p->port_no != ODPP_LOCAL && dp_del_if_hook)
+       if (p->port_no != ODPP_LOCAL)
                sysfs_remove_link(&p->dp->ifobj, p->dev->name);
 #endif
        dp_ifinfo_notify(RTM_DELLINK, p);
@@ -447,8 +438,8 @@ int dp_del_port(struct net_bridge_port *p)
        if (is_dp_dev(p->dev)) {
                dp_dev_destroy(p->dev);
        }
-       if (p->port_no != ODPP_LOCAL && dp_del_if_hook) {
-               dp_del_if_hook(p);
+       if (p->port_no != ODPP_LOCAL) {
+               brc_sysfs_del_if(p);
        } else {
                dev_put(p->dev);
                kfree(p);
index a03597d8ca309e1ecae4c9ed2fb18753dd7b5387..6ff8d2102878a5672a572ca87d1e657a4f446e7b 100644 (file)
@@ -102,10 +102,6 @@ struct net_bridge_port {
 
 extern struct notifier_block dp_device_notifier;
 extern int (*dp_ioctl_hook)(struct net_device *dev, struct ifreq *rq, int cmd);
-extern int (*dp_add_dp_hook)(struct datapath *dp);
-extern int (*dp_del_dp_hook)(struct datapath *dp);
-extern int (*dp_add_if_hook)(struct net_bridge_port *p);
-extern int (*dp_del_if_hook)(struct net_bridge_port *p);
 
 /* Flow table. */
 struct dp_table *dp_table_create(unsigned int n_buckets);
index bbc4c72f48aed0c79b7a097dab47ce99a6a2fab7..5c3540df72693087fe53c759328f6ce6d2bd922b 100644 (file)
@@ -37,12 +37,9 @@ both_modules += brcompat
 brcompat_sources = \
        linux-2.6/compat-2.6/genetlink-brcompat.c \
        brcompat.c \
-       brc_procfs.c \
-       brc_sysfs_dp.c \
-       brc_sysfs_if.c 
+       brc_procfs.c
 brcompat_headers = \
-       brc_procfs.h \
-       brc_sysfs.h 
+       brc_procfs.h
 
 dist_modules += veth
 build_modules += $(if $(BUILD_VETH),veth)