[ovs-discuss] [NIC-20 03/11] datapath: Move sysfs support from brcompat_mod into openvswitch_mod.
Ben Pfaff
blp at nicira.com
Wed Aug 5 22:37:06 UTC 2009
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 | 3 +++
datapath/brcompat.c | 39 ---------------------------------------
datapath/datapath.c | 33 ++++++++++++---------------------
datapath/datapath.h | 4 ----
datapath/linux-2.6/Modules.mk | 7 ++-----
5 files changed, 17 insertions(+), 69 deletions(-)
diff --git a/datapath/Modules.mk b/datapath/Modules.mk
index 1b5de4a..b7d54d1 100644
--- a/datapath/Modules.mk
+++ b/datapath/Modules.mk
@@ -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 \
diff --git a/datapath/brcompat.c b/datapath/brcompat.c
index db0a70f..46e7f2b 100644
--- a/datapath/brcompat.c
+++ b/datapath/brcompat.c
@@ -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);
diff --git a/datapath/datapath.c b/datapath/datapath.c
index 43d96fb..cfe660f 100644
--- a/datapath/datapath.c
+++ b/datapath/datapath.c
@@ -55,18 +55,6 @@
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);
diff --git a/datapath/datapath.h b/datapath/datapath.h
index a03597d..6ff8d21 100644
--- a/datapath/datapath.h
+++ b/datapath/datapath.h
@@ -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);
diff --git a/datapath/linux-2.6/Modules.mk b/datapath/linux-2.6/Modules.mk
index bbc4c72..5c3540d 100644
--- a/datapath/linux-2.6/Modules.mk
+++ b/datapath/linux-2.6/Modules.mk
@@ -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)
--
1.6.3.3
More information about the discuss
mailing list