From 2e7dd8eca88d131112a76301da24709b0472e381 Mon Sep 17 00:00:00 2001 From: Ben Pfaff Date: Wed, 5 Aug 2009 12:51:30 -0700 Subject: [PATCH] datapath: Move sysfs support from brcompat_mod into openvswitch_mod. 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 1b5de4ab..b7d54d1a 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 db0a70f5..46e7f2b0 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 43d96fbd..cfe660ff 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 a03597d8..6ff8d210 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 bbc4c72f..5c3540df 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) -- 2.30.2