From: Giuseppe Lettieri Date: Wed, 31 Jul 2013 20:17:34 +0000 (+0200) Subject: Merge remote-tracking branch 'ovs-dev/master' X-Git-Tag: sliver-openvswitch-2.0.90-1~34 X-Git-Url: http://git.onelab.eu/?a=commitdiff_plain;h=d017eeb9f9ebcb46c24a67fd301b3e36cd26a04e;hp=cc36576070df622d0fc7a6e26ce01027e12b5b59;p=sliver-openvswitch.git Merge remote-tracking branch 'ovs-dev/master' Conflicts: .gitignore --- diff --git a/.gitignore b/.gitignore index cc8cb2325..46b638781 100644 --- a/.gitignore +++ b/.gitignore @@ -50,3 +50,4 @@ TAGS cscope.* tags myexp/ +_debian diff --git a/AUTHORS b/AUTHORS index c2ef06cb7..d11c7c51c 100644 --- a/AUTHORS +++ b/AUTHORS @@ -59,6 +59,7 @@ Leo Alterman lalterman@nicira.com Linda Sun lsun@vmware.com Lorand Jakab lojakab@cisco.com Luca Giraudo lgiraudo@nicira.com +Mark Hamilton mhamilton@nicira.com Martin Casado casado@nicira.com Mehak Mahajan mmahajan@nicira.com Murphy McCauley murphy.mccauley@gmail.com @@ -197,6 +198,7 @@ Ramana Reddy gtvrreddy@gmail.com Rob Sherwood rob.sherwood@bigswitch.com Roger Leigh rleigh@codelibre.net Rogério Vinhal Nunes +Roman Sokolkov rsokolkov@gmail.com Saul St. John sstjohn@cs.wisc.edu Scott Hendricks shendricks@nicira.com Sean Brady sbrady@gtfservices.com diff --git a/INSTALL b/INSTALL index 5589fe7dd..a0eb26689 100644 --- a/INSTALL +++ b/INSTALL @@ -28,6 +28,11 @@ you will need the following software: libssl is installed, then Open vSwitch will automatically build with support for it. + - clang, from LLVM, is optional. It provides useful static semantic + analyzer and thread-safety check. clang version must be 3.4 or + later. For Ubuntu, there are nightly built packages available on + clang's website. + To compile the kernel module on Linux, you must also install the following. If you cannot build or install the kernel module, you may use the userspace-only implementation, at a cost in performance. The @@ -107,6 +112,8 @@ installing the following to obtain better warnings: - GNU make. + - clang, version 3.4 or later + Also, you may find the ovs-dev script found in utilities/ovs-dev.py useful. Installation Requirements @@ -173,6 +180,10 @@ Prerequisites section, follow the procedure below to build. % ./configure CC=gcc-4.2 + To use 'clang' compiler: + + % ./configure CC=clang + To build the Linux kernel module, so that you can run the kernel-based switch, pass the location of the kernel build directory on --with-linux. For example, to build for a running diff --git a/NEWS b/NEWS index b46fc4361..3bf442182 100644 --- a/NEWS +++ b/NEWS @@ -1,11 +1,15 @@ -post-v1.11.0 +post-v1.12.0 +--------------------- + + +v1.12.0 - xx xxx xxxx --------------------- - OpenFlow: * Experimental support for OpenFlow 1.1 (in addition to 1.2 and 1.3, which had experimental support in 1.10). * New support for matching outer source and destination IP address of tunneled packets, for tunnel ports configured with the newly - added "remote_ip=flow" and "local_ip=flow" options. + added "remote_ip=flow" and "local_ip=flow" options. - The Interface table in the database has a new "ifindex" column to report the interface's OS-assigned ifindex. - New "check-oftest" Makefile target for running OFTest against Open diff --git a/OPENFLOW-1.1+ b/OPENFLOW-1.1+ index d08a46ee4..aea689ec8 100644 --- a/OPENFLOW-1.1+ +++ b/OPENFLOW-1.1+ @@ -143,9 +143,6 @@ didn't compare the specs carefully yet.) * Rework tag order. I'm not sure whether we need to do anything for this. - * Duration for queue stats. (Duration for port stats is already - implemented.) - * On-demand flow counters. I think this might be a real optimization in some cases for the software switch. diff --git a/configure.ac b/configure.ac index 170a85221..89541aaef 100644 --- a/configure.ac +++ b/configure.ac @@ -13,7 +13,7 @@ # limitations under the License. AC_PREREQ(2.64) -AC_INIT(openvswitch, 1.11.90, ovs-bugs@openvswitch.org) +AC_INIT(openvswitch, 1.12.90, ovs-bugs@openvswitch.org) AC_CONFIG_SRCDIR([datapath/datapath.c]) AC_CONFIG_MACRO_DIR([m4]) AC_CONFIG_AUX_DIR([build-aux]) @@ -64,7 +64,8 @@ AC_CHECK_DECLS([sys_siglist], [], [], [[#include ]]) AC_CHECK_MEMBERS([struct stat.st_mtim.tv_nsec, struct stat.st_mtimensec], [], [], [[#include ]]) AC_CHECK_MEMBERS([struct ifreq.ifr_flagshigh], [], [], [[#include ]]) -AC_CHECK_FUNCS([mlockall strnlen getloadavg statvfs getmntent_r]) +AC_CHECK_FUNCS([mlockall strnlen getloadavg statvfs getmntent_r \ + pthread_setname_np pthread_set_name_np]) AC_CHECK_HEADERS([mntent.h sys/statvfs.h linux/types.h linux/if_ether.h stdatomic.h]) AC_CHECK_HEADERS([net/if_mib.h], [], [], [[#include #include ]]) @@ -82,6 +83,7 @@ OVS_CHECK_GROFF OVS_CHECK_GNU_MAKE OVS_CHECK_CACHE_TIME OVS_CHECK_TLS +OVS_CHECK_GCC4_ATOMICS OVS_CHECK_ATOMIC_ALWAYS_LOCK_FREE(1) OVS_CHECK_ATOMIC_ALWAYS_LOCK_FREE(2) OVS_CHECK_ATOMIC_ALWAYS_LOCK_FREE(4) @@ -103,6 +105,7 @@ OVS_ENABLE_OPTION([-Wstrict-prototypes]) OVS_ENABLE_OPTION([-Wold-style-definition]) OVS_ENABLE_OPTION([-Wmissing-prototypes]) OVS_ENABLE_OPTION([-Wmissing-field-initializers]) +OVS_ENABLE_OPTION([-Wthread-safety]) OVS_CONDITIONAL_CC_OPTION([-Wno-unused], [HAVE_WNO_UNUSED]) OVS_CONDITIONAL_CC_OPTION([-Wno-unused-parameter], [HAVE_WNO_UNUSED_PARAMETER]) OVS_ENABLE_WERROR diff --git a/datapath/flow.c b/datapath/flow.c index c6a90b936..62fdf8588 100644 --- a/datapath/flow.c +++ b/datapath/flow.c @@ -82,8 +82,9 @@ static void update_range__(struct sw_flow_match *match, do { \ update_range__(match, offsetof(struct sw_flow_key, field), \ sizeof((match)->key->field), is_mask); \ - if (is_mask && match->mask != NULL) { \ - (match)->mask->key.field = value; \ + if (is_mask) { \ + if ((match)->mask) \ + (match)->mask->key.field = value; \ } else { \ (match)->key->field = value; \ } \ @@ -93,8 +94,9 @@ static void update_range__(struct sw_flow_match *match, do { \ update_range__(match, offsetof(struct sw_flow_key, field), \ len, is_mask); \ - if (is_mask && match->mask != NULL) { \ - memcpy(&(match)->mask->key.field, value_p, len); \ + if (is_mask) { \ + if ((match)->mask) \ + memcpy(&(match)->mask->key.field, value_p, len);\ } else { \ memcpy(&(match)->key->field, value_p, len); \ } \ @@ -429,7 +431,7 @@ static struct flex_array *alloc_buckets(unsigned int n_buckets) struct flex_array *buckets; int i, err; - buckets = flex_array_alloc(sizeof(struct hlist_head *), + buckets = flex_array_alloc(sizeof(struct hlist_head), n_buckets, GFP_KERNEL); if (!buckets) return NULL; @@ -489,7 +491,7 @@ static void __flow_tbl_destroy(struct flow_table *table) int ver = table->node_ver; hlist_for_each_entry_safe(flow, n, head, hash_node[ver]) { - hlist_del_rcu(&flow->hash_node[ver]); + hlist_del(&flow->hash_node[ver]); ovs_flow_free(flow, false); } } @@ -1363,15 +1365,19 @@ static int ovs_key_from_nlattrs(struct sw_flow_match *match, u64 attrs, __be16 tci; tci = nla_get_be16(a[OVS_KEY_ATTR_VLAN]); - if (!is_mask) - if (!(tci & htons(VLAN_TAG_PRESENT))) { + if (!(tci & htons(VLAN_TAG_PRESENT))) { + if (is_mask) + OVS_NLERR("VLAN TCI mask does not have exact match for VLAN_TAG_PRESENT bit.\n"); + else OVS_NLERR("VLAN TCI does not have VLAN_TAG_PRESENT bit set.\n"); - return -EINVAL; - } + + return -EINVAL; + } SW_FLOW_KEY_PUT(match, eth.tci, tci, is_mask); attrs &= ~(1ULL << OVS_KEY_ATTR_VLAN); - } + } else if (!is_mask) + SW_FLOW_KEY_PUT(match, eth.tci, htons(0xffff), true); if (attrs & (1ULL << OVS_KEY_ATTR_ETHERTYPE)) { __be16 eth_type; @@ -1688,8 +1694,7 @@ int ovs_flow_to_nlattrs(const struct sw_flow_key *swkey, struct ovs_key_ethernet *eth_key; struct nlattr *nla, *encap; - if (output->phy.priority && - nla_put_u32(skb, OVS_KEY_ATTR_PRIORITY, output->phy.priority)) + if (nla_put_u32(skb, OVS_KEY_ATTR_PRIORITY, output->phy.priority)) goto nla_put_failure; if (swkey->tun_key.ipv4_dst && @@ -1709,8 +1714,7 @@ int ovs_flow_to_nlattrs(const struct sw_flow_key *swkey, goto nla_put_failure; } - if (output->phy.skb_mark && - nla_put_u32(skb, OVS_KEY_ATTR_SKB_MARK, output->phy.skb_mark)) + if (nla_put_u32(skb, OVS_KEY_ATTR_SKB_MARK, output->phy.skb_mark)) goto nla_put_failure; nla = nla_reserve(skb, OVS_KEY_ATTR_ETHERNET, sizeof(*eth_key)); diff --git a/datapath/linux/.gitignore b/datapath/linux/.gitignore index e7ac6c14c..874861302 100644 --- a/datapath/linux/.gitignore +++ b/datapath/linux/.gitignore @@ -14,6 +14,7 @@ /exthdrs_core.c /flex_array.c /flow.c +/flow_dissector.c /genetlink-openvswitch.c /genl_exec.c /gre.c @@ -42,4 +43,5 @@ /vport-patch.c /vport-vxlan.c /vport.c +/vxlan.c /workqueue.c diff --git a/datapath/linux/compat/include/linux/list.h b/datapath/linux/compat/include/linux/list.h index 44467794b..18cce8a37 100644 --- a/datapath/linux/compat/include/linux/list.h +++ b/datapath/linux/compat/include/linux/list.h @@ -5,7 +5,9 @@ #ifndef hlist_entry_safe #define hlist_entry_safe(ptr, type, member) \ - (ptr) ? hlist_entry(ptr, type, member) : NULL + ({ typeof(ptr) ____ptr = (ptr); \ + ____ptr ? hlist_entry(____ptr, type, member) : NULL; \ + }) #undef hlist_for_each_entry #define hlist_for_each_entry(pos, head, member) \ diff --git a/datapath/linux/compat/include/linux/netdevice.h b/datapath/linux/compat/include/linux/netdevice.h index ba1fc593e..3f66d3a32 100644 --- a/datapath/linux/compat/include/linux/netdevice.h +++ b/datapath/linux/compat/include/linux/netdevice.h @@ -188,4 +188,19 @@ static inline struct sk_buff *__skb_gso_segment(struct sk_buff *skb, return skb_gso_segment(skb, features); } #endif + +#if LINUX_VERSION_CODE < KERNEL_VERSION(3,9,0) +static inline int netdev_master_upper_dev_link(struct net_device *dev, + struct net_device *upper_dev) +{ + return netdev_set_master(dev, upper_dev); +} + +static inline void netdev_upper_dev_unlink(struct net_device *dev, + struct net_device *upper_dev) +{ + netdev_set_master(dev, NULL); +} +#endif + #endif diff --git a/datapath/vport-netdev.c b/datapath/vport-netdev.c index 06598fa4e..4bc16175c 100644 --- a/datapath/vport-netdev.c +++ b/datapath/vport-netdev.c @@ -25,6 +25,7 @@ #include #include #include +#include #include @@ -136,6 +137,15 @@ static void netdev_exit(void) } #endif +static struct net_device *get_dpdev(struct datapath *dp) +{ + struct vport *local; + + local = ovs_vport_ovsl(dp, OVSP_LOCAL); + BUG_ON(!local); + return netdev_vport_priv(local)->dev; +} + static struct vport *netdev_create(const struct vport_parms *parms) { struct vport *vport; @@ -165,10 +175,15 @@ static struct vport *netdev_create(const struct vport_parms *parms) } rtnl_lock(); + err = netdev_master_upper_dev_link(netdev_vport->dev, + get_dpdev(vport->dp)); + if (err) + goto error_unlock; + err = netdev_rx_handler_register(netdev_vport->dev, netdev_frame_hook, vport); if (err) - goto error_unlock; + goto error_master_upper_dev_unlink; dev_set_promiscuity(netdev_vport->dev, 1); #if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,24) @@ -180,6 +195,8 @@ static struct vport *netdev_create(const struct vport_parms *parms) netdev_init(); return vport; +error_master_upper_dev_unlink: + netdev_upper_dev_unlink(netdev_vport->dev, get_dpdev(vport->dp)); error_unlock: rtnl_unlock(); error_put: @@ -207,6 +224,7 @@ static void netdev_destroy(struct vport *vport) rtnl_lock(); netdev_vport->dev->priv_flags &= ~IFF_OVS_DATAPATH; netdev_rx_handler_unregister(netdev_vport->dev); + netdev_upper_dev_unlink(netdev_vport->dev, get_dpdev(vport->dp)); dev_set_promiscuity(netdev_vport->dev, -1); rtnl_unlock(); diff --git a/debian/changelog b/debian/changelog index f6010202a..09b97f8fe 100644 --- a/debian/changelog +++ b/debian/changelog @@ -1,13 +1,38 @@ -openvswitch (1.11.90-1) unstable; urgency=low +openvswitch (1.12.90-1) unstable; urgency=low [ Open vSwitch team ] * New upstream version - - Nothing yet! Try NEWS... + - Nothing yet! Try NEWS... - -- Open vSwitch team Mon, 29 Apr 2013 14:30:34 -0700 + -- Open vSwitch team Tue, 03 Jul 2013 15:05:34 -0700 + +openvswitch (1.12.0-1) unstable; urgency=low + [ Open vSwitch team ] + * New upstream version + - OpenFlow: + * Experimental support for OpenFlow 1.1 (in addition to 1.2 and + 1.3, which had experimental support in 1.10). + * New support for matching outer source and destination IP address + of tunneled packets, for tunnel ports configured with the newly + added "remote_ip=flow" and "local_ip=flow" options. + - The Interface table in the database has a new "ifindex" column to + report the interface's OS-assigned ifindex. + - New "check-oftest" Makefile target for running OFTest against Open + vSwitch. See README-OFTest for details. + - The flow eviction threshold has been moved to the Open_vSwitch table. + - Database names are now mandatory when specifying ovsdb-server options + through database paths (e.g. Private key option with the database name + should look like "--private-key=db:Open_vSwitch,SSL,private_key"). + - Added ovs-dev.py, a utility script helpful for Open vSwitch developers. + + -- Open vSwitch team Tue, 03 Jul 2013 15:02:34 -0700 openvswitch (1.11.0-1) unstable; urgency=low [ Open vSwitch team ] * New upstream version + - Support for megaflows, which allows wildcarding in the kernel (and + any dpif implementation that supports wildcards). Depending on + the flow table and switch configuration, flow set up rates are + close to the Linux bridge. - The "tutorial" directory contains a new tutorial for some advanced Open vSwitch features. - Stable bond mode has been removed. @@ -20,13 +45,20 @@ openvswitch (1.11.0-1) unstable; urgency=low 1.1 and later are now implemented. * New "stack" extension for use in actions, to push and pop from NXM fields. + * The "load" and "set_field" actions can now modify the "in_port". (This + allows one to enable output to a flow's input port by setting the + in_port to some unused value, such as OFPP_NONE.) - ovs-dpctl: * New debugging commands "add-flow", "mod-flow", "del-flow". + - In dpif-based bridges, cache action translations, which can improve + flow set up performance by 80% with a complicated flow table. - New syslog format, prefixed with "ovs|", to be easier to filter. - RHEL: Removes the default firewall rule that allowed GRE traffic to pass through. Any users that relied on this automatic firewall hole will have to manually configure it. The ovs-ctl(8) manpage documents the "enable-protocol" command that can be used as an alternative. + - New CFM demand mode which uses data traffic to indicate interface + liveness. -- Open vSwitch team Mon, 29 Apr 2013 14:30:34 -0700 diff --git a/include/sparse/pthread.h b/include/sparse/pthread.h index 6cdf5c8a7..aa4652efc 100644 --- a/include/sparse/pthread.h +++ b/include/sparse/pthread.h @@ -26,12 +26,12 @@ int pthread_mutex_lock(pthread_mutex_t *mutex) OVS_ACQUIRES(mutex); int pthread_mutex_unlock(pthread_mutex_t *mutex) OVS_RELEASES(mutex); -int pthread_rwlock_rdlock(pthread_rwlock_t *rwlock) OVS_ACQUIRES(rwlock); -int pthread_rwlock_wrlock(pthread_rwlock_t *rwlock) OVS_ACQUIRES(rwlock); +int pthread_rwlock_rdlock(pthread_rwlock_t *rwlock) OVS_ACQ_RDLOCK(rwlock); +int pthread_rwlock_wrlock(pthread_rwlock_t *rwlock) OVS_ACQ_WRLOCK(rwlock); int pthread_rwlock_unlock(pthread_rwlock_t *rwlock) OVS_RELEASES(rwlock); int pthread_cond_wait(pthread_cond_t *, pthread_mutex_t *mutex) - OVS_MUST_HOLD(mutex); + OVS_REQUIRES(mutex); /* Sparse complains about the proper PTHREAD_*_INITIALIZER definitions. * Luckily, it's not a real compiler so we can overwrite it with something @@ -52,7 +52,7 @@ int pthread_cond_wait(pthread_cond_t *, pthread_mutex_t *mutex) ({ \ int retval = pthread_mutex_trylock(mutex); \ if (!retval) { \ - OVS_ACQUIRE(MUTEX); \ + OVS_MACRO_LOCK(MUTEX); \ } \ retval; \ }) @@ -61,7 +61,7 @@ int pthread_cond_wait(pthread_cond_t *, pthread_mutex_t *mutex) ({ \ int retval = pthread_rwlock_tryrdlock(rwlock); \ if (!retval) { \ - OVS_ACQUIRE(RWLOCK); \ + OVS_MACRO_LOCK(RWLOCK); \ } \ retval; \ }) @@ -69,7 +69,7 @@ int pthread_cond_wait(pthread_cond_t *, pthread_mutex_t *mutex) ({ \ int retval = pthread_rwlock_trywrlock(rwlock); \ if (!retval) { \ - OVS_ACQUIRE(RWLOCK); \ + OVS_MACRO_LOCK(RWLOCK); \ } \ retval; \ }) diff --git a/lib/command-line.c b/lib/command-line.c index 7800c0b6e..805e51b35 100644 --- a/lib/command-line.c +++ b/lib/command-line.c @@ -94,9 +94,16 @@ run_command(int argc, char *argv[], const struct command commands[]) /* Process title. */ #ifdef LINUX_DATAPATH -static char *argv_start; /* Start of command-line arguments in memory. */ -static size_t argv_size; /* Number of bytes of command-line arguments. */ -static char *saved_proctitle; /* Saved command-line arguments. */ +static struct ovs_mutex proctitle_mutex = OVS_MUTEX_INITIALIZER; + +/* Start of command-line arguments in memory. */ +static char *argv_start OVS_GUARDED_BY(proctitle_mutex); + +/* Number of bytes of command-line arguments. */ +static size_t argv_size OVS_GUARDED_BY(proctitle_mutex); + +/* Saved command-line arguments. */ +static char *saved_proctitle OVS_GUARDED_BY(proctitle_mutex); /* Prepares the process so that proctitle_set() can later succeed. * @@ -117,6 +124,7 @@ proctitle_init(int argc, char **argv) return; } + ovs_mutex_lock(&proctitle_mutex); /* Specialized version of first loop iteration below. */ argv_start = argv[0]; argv_size = strlen(argv[0]) + 1; @@ -140,6 +148,7 @@ proctitle_init(int argc, char **argv) /* Copy out the old argument so we can reuse the space. */ argv[i] = xstrdup(argv[i]); } + ovs_mutex_unlock(&proctitle_mutex); } /* Changes the name of the process, as shown by "ps", to the program name @@ -150,8 +159,9 @@ proctitle_set(const char *format, ...) va_list args; int n; + ovs_mutex_lock(&proctitle_mutex); if (!argv_start || argv_size < 8) { - return; + goto out; } if (!saved_proctitle) { @@ -172,17 +182,22 @@ proctitle_set(const char *format, ...) memset(&argv_start[n], '\0', argv_size - n); } va_end(args); + +out: + ovs_mutex_unlock(&proctitle_mutex); } /* Restores the process's original command line, as seen by "ps". */ void proctitle_restore(void) { + ovs_mutex_lock(&proctitle_mutex); if (saved_proctitle) { memcpy(argv_start, saved_proctitle, argv_size); free(saved_proctitle); saved_proctitle = NULL; } + ovs_mutex_unlock(&proctitle_mutex); } #else /* !LINUX_DATAPATH*/ /* Stubs that don't do anything on non-Linux systems. */ diff --git a/lib/compiler.h b/lib/compiler.h index 4b1834f0f..94a2218a1 100644 --- a/lib/compiler.h +++ b/lib/compiler.h @@ -17,6 +17,10 @@ #ifndef COMPILER_H #define COMPILER_H 1 +#ifndef __has_feature + #define __has_feature(x) 0 +#endif + #if __GNUC__ && !__CHECKER__ #define NO_RETURN __attribute__((__noreturn__)) #define OVS_UNUSED __attribute__((__unused__)) @@ -41,38 +45,116 @@ #define OVS_UNLIKELY(CONDITION) (!!(CONDITION)) #endif -#ifdef __CHECKER__ -/* "sparse" annotations for mutexes and mutex-like constructs. +#if __has_feature(c_thread_safety_attributes) +/* "clang" annotations for thread safety check. + * + * OVS_LOCKABLE indicates that the struct contains mutex element + * which can be locked by functions like ovs_mutex_lock(). + * + * Below, the word MUTEX stands for the name of an object with an OVS_LOCKABLE + * struct type. It can also be a comma-separated list of multiple structs, + * e.g. to require a function to hold multiple locks while invoked. + * + * + * On a variable: + * + * - OVS_GUARDED indicates that the variable may only be accessed some mutex + * is held. + * + * - OVS_GUARDED_BY(MUTEX) indicates that the variable may only be accessed + * while the specific MUTEX is held. * - * In a function prototype, OVS_ACQUIRES(MUTEX) indicates that the function - * must be called without MUTEX acquired and that it returns with MUTEX - * acquired. OVS_RELEASES(MUTEX) indicates the reverse. OVS_MUST_HOLD - * indicates that the function must be called with MUTEX acquired by the - * caller and that the function does not release MUTEX. * - * In practice, sparse ignores the MUTEX argument. It need not even be a - * valid expression. It is meant to indicate to human readers what mutex is - * being acquired. + * On a function, the following attributes apply to mutexes: + * + * - OVS_ACQUIRES(MUTEX) indicate that the function must be called without + * holding MUTEX and that it returns holding MUTEX. + * + * - OVS_RELEASES(MUTEX) indicates that the function may only be called with + * MUTEX held and that it returns with MUTEX released. It can be used for + * all types of MUTEX. + * + * - OVS_TRY_LOCK(RETVAL, MUTEX) indicate that the function will try to + * acquire MUTEX. RETVAL is an integer or boolean value specifying the + * return value of a successful lock acquisition. + * + * - OVS_REQUIRES(MUTEX) indicate that the function may only be called with + * MUTEX held and that the function does not release MUTEX. + * + * - OVS_LOCKS_EXCLUDED(MUTEX) indicates that the function may only be + * called when MUTEX is not held. + * + * The following variants, with the same syntax, apply to reader-writer locks: + * + * mutex rwlock, for reading rwlock, for writing + * ------------------- ------------------- ------------------- + * OVS_ACQUIRES OVS_ACQ_RDLOCK OVS_ACQ_WRLOCK + * OVS_RELEASES OVS_RELEASES OVS_RELEASES + * OVS_TRY_LOCK OVS_TRY_RDLOCK OVS_TRY_WRLOCK + * OVS_REQUIRES OVS_REQ_RDLOCK OVS_REQ_WRLOCK + * OVS_LOCKS_EXCLUDED OVS_LOCKS_EXCLUDED OVS_LOCKS_EXCLUDED + */ +#define OVS_LOCKABLE __attribute__((lockable)) +#define OVS_REQ_RDLOCK(...) __attribute__((shared_locks_required(__VA_ARGS__))) +#define OVS_ACQ_RDLOCK(...) __attribute__((shared_lock_function(__VA_ARGS__))) +#define OVS_REQ_WRLOCK(...) \ + __attribute__((exclusive_locks_required(__VA_ARGS__))) +#define OVS_ACQ_WRLOCK(...) \ + __attribute__((exclusive_lock_function(__VA_ARGS__))) +#define OVS_REQUIRES(...) \ + __attribute__((exclusive_locks_required(__VA_ARGS__))) +#define OVS_ACQUIRES(...) \ + __attribute__((exclusive_lock_function(__VA_ARGS__))) +#define OVS_TRY_WRLOCK(RETVAL, ...) \ + __attribute__((exclusive_trylock_function(RETVAL, __VA_ARGS__))) +#define OVS_TRY_RDLOCK(RETVAL, ...) \ + __attribute__((shared_trylock_function(RETVAL, __VA_ARGS__))) +#define OVS_TRY_LOCK(RETVAL, ...) \ + __attribute__((exclusive_trylock_function(RETVAL, __VA_ARGS__))) +#define OVS_GUARDED __attribute__((guarded_var)) +#define OVS_GUARDED_BY(...) __attribute__((guarded_by(__VA_ARGS__))) +#define OVS_RELEASES(...) __attribute__((unlock_function(__VA_ARGS__))) +#define OVS_EXCLUDED(...) __attribute__((locks_excluded(__VA_ARGS__))) +#elif __CHECKER__ +/* "sparse" annotations for mutexes and mutex-like constructs. * - * Since sparse ignores MUTEX, it need not be an actual mutex. It can be - * any construct on which paired acquire and release semantics make sense: - * read/write locks, temporary memory allocations, whatever. + * Change the thread-safety check annotations to use "context" attribute. * - * OVS_ACQUIRE, OVS_RELEASE, and OVS_HOLDS are suitable for use within macros, + * OVS_MACRO_LOCK and OVS_MACRO_RELEASE are suitable for use within macros, * where there is no function prototype to annotate. */ -#define OVS_ACQUIRES(MUTEX) __attribute__((context(MUTEX, 0, 1))) -#define OVS_RELEASES(MUTEX) __attribute__((context(MUTEX, 1, 0))) -#define OVS_MUST_HOLD(MUTEX) __attribute__((context(MUTEX, 1, 1))) -#define OVS_ACQUIRE(MUTEX) __context__(MUTEX, 0, 1) -#define OVS_RELEASE(MUTEX) __context__(MUTEX, 1, 0) -#define OVS_HOLDS(MUTEX) __context__(MUTEX, 1, 1) +#define OVS_LOCKABLE +#define OVS_REQ_RDLOCK(...) __attribute__((context(MUTEX, 1, 1))) +#define OVS_ACQ_RDLOCK(...) __attribute__((context(MUTEX, 0, 1))) +#define OVS_REQ_WRLOCK(...) __attribute__((context(MUTEX, 1, 1))) +#define OVS_ACQ_WRLOCK(...) __attribute__((context(MUTEX, 0, 1))) +#define OVS_REQUIRES(...) __attribute__((context(MUTEX, 1, 1))) +#define OVS_ACQUIRES(...) __attribute__((context(MUTEX, 0, 1))) +#define OVS_TRY_WRLOCK(RETVAL, ...) +#define OVS_TRY_RDLOCK(RETVAL, ...) +#define OVS_TRY_LOCK(REVAL, ...) +#define OVS_GUARDED +#define OVS_GUARDED_BY(...) +#define OVS_EXCLUDED(...) +#define OVS_RELEASES(...) __attribute__((context(MUTEX, 1, 0))) +#define OVS_MACRO_LOCK(...) __context__(MUTEX, 0, 1) +#define OVS_MACRO_RELEASE(...) __context__(MUTEX, 1, 0) #else -#define OVS_ACQUIRES(MUTEX) -#define OVS_RELEASES(MUTEX) -#define OVS_MUST_HOLD(MUTEX) -#define OVS_ACQUIRE(MUTEX) -#define OVS_RELEASE(MUTEX) -#define OVS_HOLDS(MUTEX) +#define OVS_LOCKABLE +#define OVS_REQ_RDLOCK(...) +#define OVS_ACQ_RDLOCK(...) +#define OVS_REQ_WRLOCK(...) +#define OVS_ACQ_WRLOCK(...) +#define OVS_REQUIRES(...) +#define OVS_ACQUIRES(...) +#define OVS_TRY_WRLOCK(...) +#define OVS_TRY_RDLOCK(...) +#define OVS_TRY_LOCK(...) +#define OVS_GUARDED +#define OVS_GUARDED_BY(...) +#define OVS_EXCLUDED(...) +#define OVS_RELEASES(...) +#define OVS_MACRO_LOCK(...) +#define OVS_MACRO_RELEASE(...) #endif /* ISO C says that a C implementation may choose any integer type for an enum diff --git a/lib/dpif-linux.c b/lib/dpif-linux.c index 831da3bbb..27c622aa2 100644 --- a/lib/dpif-linux.c +++ b/lib/dpif-linux.c @@ -140,6 +140,7 @@ struct dpif_linux { int dp_ifindex; /* Upcall messages. */ + struct ovs_mutex upcall_lock; int uc_array_size; /* Size of 'channels' and 'epoll_events'. */ struct dpif_channel *channels; struct epoll_event *epoll_events; @@ -148,26 +149,26 @@ struct dpif_linux { int event_offset; /* Offset into 'epoll_events'. */ /* Change notification. */ - struct sset changed_ports; /* Ports that have changed. */ - struct nln_notifier *port_notifier; - bool change_error; + struct nl_sock *port_notifier; /* vport multicast group subscriber. */ }; static struct vlog_rate_limit error_rl = VLOG_RATE_LIMIT_INIT(9999, 5); -/* Generic Netlink family numbers for OVS. */ +/* Generic Netlink family numbers for OVS. + * + * Initialized by dpif_linux_init(). */ static int ovs_datapath_family; static int ovs_vport_family; static int ovs_flow_family; static int ovs_packet_family; -/* Generic Netlink socket. */ -static struct nln *nln = NULL; +/* Generic Netlink multicast groups for OVS. + * + * Initialized by dpif_linux_init(). */ +static unsigned int ovs_vport_mcgroup; static int dpif_linux_init(void); -static void open_dpif(const struct dpif_linux_dp *, struct dpif **); -static bool dpif_linux_nln_parse(struct ofpbuf *, void *); -static void dpif_linux_port_changed(const void *vport, void *dpif); +static int open_dpif(const struct dpif_linux_dp *, struct dpif **); static uint32_t dpif_linux_port_get_pid(const struct dpif *, odp_port_t port_no); @@ -235,27 +236,28 @@ dpif_linux_open(const struct dpif_class *class OVS_UNUSED, const char *name, return error; } - open_dpif(&dp, dpifp); + error = open_dpif(&dp, dpifp); ofpbuf_delete(buf); - return 0; + return error; } -static void +static int open_dpif(const struct dpif_linux_dp *dp, struct dpif **dpifp) { struct dpif_linux *dpif; dpif = xzalloc(sizeof *dpif); - dpif->port_notifier = nln_notifier_create(nln, dpif_linux_port_changed, - dpif); + dpif->port_notifier = NULL; + ovs_mutex_init(&dpif->upcall_lock, PTHREAD_MUTEX_DEFAULT); dpif->epoll_fd = -1; dpif_init(&dpif->dpif, &dpif_linux_class, dp->name, dp->dp_ifindex, dp->dp_ifindex); dpif->dp_ifindex = dp->dp_ifindex; - sset_init(&dpif->changed_ports); *dpifp = &dpif->dpif; + + return 0; } static void @@ -276,6 +278,8 @@ destroy_channels(struct dpif_linux *dpif) continue; } + epoll_ctl(dpif->epoll_fd, EPOLL_CTL_DEL, nl_sock_fd(ch->sock), NULL); + /* Turn off upcalls. */ dpif_linux_vport_init(&vport_request); vport_request.cmd = OVS_VPORT_CMD_SET; @@ -295,8 +299,8 @@ destroy_channels(struct dpif_linux *dpif) dpif->epoll_events = NULL; dpif->n_events = dpif->event_offset = 0; - close(dpif->epoll_fd); - dpif->epoll_fd = -1; + /* Don't close dpif->epoll_fd since that would cause other threads that + * call dpif_recv_wait(dpif) to wait on an arbitrary fd or a closed fd. */ } static int @@ -374,9 +378,12 @@ dpif_linux_close(struct dpif *dpif_) { struct dpif_linux *dpif = dpif_linux_cast(dpif_); - nln_notifier_destroy(dpif->port_notifier); + nl_sock_destroy(dpif->port_notifier); destroy_channels(dpif); - sset_destroy(&dpif->changed_ports); + if (dpif->epoll_fd >= 0) { + close(dpif->epoll_fd); + } + ovs_mutex_destroy(&dpif->upcall_lock); free(dpif); } @@ -392,22 +399,6 @@ dpif_linux_destroy(struct dpif *dpif_) return dpif_linux_dp_transact(&dp, NULL, NULL); } -static void -dpif_linux_run(struct dpif *dpif_ OVS_UNUSED) -{ - if (nln) { - nln_run(nln); - } -} - -static void -dpif_linux_wait(struct dpif *dpif OVS_UNUSED) -{ - if (nln) { - nln_wait(nln); - } -} - static int dpif_linux_get_stats(const struct dpif *dpif_, struct dpif_dp_stats *stats) { @@ -483,8 +474,8 @@ netdev_to_ovs_vport_type(const struct netdev *netdev) } static int -dpif_linux_port_add(struct dpif *dpif_, struct netdev *netdev, - odp_port_t *port_nop) +dpif_linux_port_add__(struct dpif *dpif_, struct netdev *netdev, + odp_port_t *port_nop) { struct dpif_linux *dpif = dpif_linux_cast(dpif_); const struct netdev_tunnel_config *tnl_cfg; @@ -575,7 +566,21 @@ dpif_linux_port_add(struct dpif *dpif_, struct netdev *netdev, } static int -dpif_linux_port_del(struct dpif *dpif_, odp_port_t port_no) +dpif_linux_port_add(struct dpif *dpif_, struct netdev *netdev, + odp_port_t *port_nop) +{ + struct dpif_linux *dpif = dpif_linux_cast(dpif_); + int error; + + ovs_mutex_lock(&dpif->upcall_lock); + error = dpif_linux_port_add__(dpif_, netdev, port_nop); + ovs_mutex_unlock(&dpif->upcall_lock); + + return error; +} + +static int +dpif_linux_port_del__(struct dpif *dpif_, odp_port_t port_no) { struct dpif_linux *dpif = dpif_linux_cast(dpif_); struct dpif_linux_vport vport; @@ -592,6 +597,19 @@ dpif_linux_port_del(struct dpif *dpif_, odp_port_t port_no) return error; } +static int +dpif_linux_port_del(struct dpif *dpif_, odp_port_t port_no) +{ + struct dpif_linux *dpif = dpif_linux_cast(dpif_); + int error; + + ovs_mutex_lock(&dpif->upcall_lock); + error = dpif_linux_port_del__(dpif_, port_no); + ovs_mutex_unlock(&dpif->upcall_lock); + + return error; +} + static int dpif_linux_port_query__(const struct dpif *dpif, odp_port_t port_no, const char *port_name, struct dpif_port *dpif_port) @@ -648,21 +666,24 @@ dpif_linux_port_get_pid(const struct dpif *dpif_, odp_port_t port_no) { struct dpif_linux *dpif = dpif_linux_cast(dpif_); uint32_t port_idx = odp_to_u32(port_no); + uint32_t pid = 0; - if (dpif->epoll_fd < 0) { - return 0; - } else { + ovs_mutex_lock(&dpif->upcall_lock); + if (dpif->epoll_fd >= 0) { /* The ODPP_NONE "reserved" port number uses the "ovs-system"'s * channel, since it is not heavily loaded. */ uint32_t idx = port_idx >= dpif->uc_array_size ? 0 : port_idx; - return nl_sock_pid(dpif->channels[idx].sock); + pid = nl_sock_pid(dpif->channels[idx].sock); } + ovs_mutex_unlock(&dpif->upcall_lock); + + return pid; } static int dpif_linux_flow_flush(struct dpif *dpif_) { - struct dpif_linux *dpif = dpif_linux_cast(dpif_); + const struct dpif_linux *dpif = dpif_linux_cast(dpif_); struct dpif_linux_flow flow; dpif_linux_flow_init(&flow); @@ -678,7 +699,7 @@ struct dpif_linux_port_state { static int dpif_linux_port_dump_start(const struct dpif *dpif_, void **statep) { - struct dpif_linux *dpif = dpif_linux_cast(dpif_); + const struct dpif_linux *dpif = dpif_linux_cast(dpif_); struct dpif_linux_port_state *state; struct dpif_linux_vport request; struct ofpbuf *buf; @@ -736,23 +757,72 @@ dpif_linux_port_poll(const struct dpif *dpif_, char **devnamep) { struct dpif_linux *dpif = dpif_linux_cast(dpif_); - if (dpif->change_error) { - dpif->change_error = false; - sset_clear(&dpif->changed_ports); + /* Lazily create the Netlink socket to listen for notifications. */ + if (!dpif->port_notifier) { + struct nl_sock *sock; + int error; + + error = nl_sock_create(NETLINK_GENERIC, &sock); + if (error) { + return error; + } + + error = nl_sock_join_mcgroup(sock, ovs_vport_mcgroup); + if (error) { + nl_sock_destroy(sock); + return error; + } + dpif->port_notifier = sock; + + /* We have no idea of the current state so report that everything + * changed. */ + return ENOBUFS; + } + + for (;;) { + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5); + uint64_t buf_stub[4096 / 8]; + struct ofpbuf buf; + int error; + + ofpbuf_use_stub(&buf, buf_stub, sizeof buf_stub); + error = nl_sock_recv(dpif->port_notifier, &buf, false); + if (!error) { + struct dpif_linux_vport vport; + + error = dpif_linux_vport_from_ofpbuf(&vport, &buf); + if (!error) { + if (vport.dp_ifindex == dpif->dp_ifindex + && (vport.cmd == OVS_VPORT_CMD_NEW + || vport.cmd == OVS_VPORT_CMD_DEL + || vport.cmd == OVS_VPORT_CMD_SET)) { + VLOG_DBG("port_changed: dpif:%s vport:%s cmd:%"PRIu8, + dpif->dpif.full_name, vport.name, vport.cmd); + *devnamep = xstrdup(vport.name); + return 0; + } else { + continue; + } + } + } else if (error == EAGAIN) { + return EAGAIN; + } + + VLOG_WARN_RL(&rl, "error reading or parsing netlink (%s)", + ovs_strerror(error)); + nl_sock_drain(dpif->port_notifier); return ENOBUFS; - } else if (!sset_is_empty(&dpif->changed_ports)) { - *devnamep = sset_pop(&dpif->changed_ports); - return 0; - } else { - return EAGAIN; } } static void dpif_linux_port_poll_wait(const struct dpif *dpif_) { - struct dpif_linux *dpif = dpif_linux_cast(dpif_); - if (!sset_is_empty(&dpif->changed_ports) || dpif->change_error) { + const struct dpif_linux *dpif = dpif_linux_cast(dpif_); + + if (dpif->port_notifier) { + nl_sock_wait(dpif->port_notifier, POLLIN); + } else { poll_immediate_wake(); } } @@ -762,7 +832,7 @@ dpif_linux_flow_get__(const struct dpif *dpif_, const struct nlattr *key, size_t key_len, struct dpif_linux_flow *reply, struct ofpbuf **bufp) { - struct dpif_linux *dpif = dpif_linux_cast(dpif_); + const struct dpif_linux *dpif = dpif_linux_cast(dpif_); struct dpif_linux_flow request; dpif_linux_flow_init(&request); @@ -804,7 +874,7 @@ dpif_linux_init_flow_put(struct dpif *dpif_, const struct dpif_flow_put *put, { static const struct nlattr dummy_action; - struct dpif_linux *dpif = dpif_linux_cast(dpif_); + const struct dpif_linux *dpif = dpif_linux_cast(dpif_); dpif_linux_flow_init(request); request->cmd = (put->flags & DPIF_FP_CREATE @@ -847,7 +917,7 @@ static void dpif_linux_init_flow_del(struct dpif *dpif_, const struct dpif_flow_del *del, struct dpif_linux_flow *request) { - struct dpif_linux *dpif = dpif_linux_cast(dpif_); + const struct dpif_linux *dpif = dpif_linux_cast(dpif_); dpif_linux_flow_init(request); request->cmd = OVS_FLOW_CMD_DEL; @@ -884,7 +954,7 @@ struct dpif_linux_flow_state { static int dpif_linux_flow_dump_start(const struct dpif *dpif_, void **statep) { - struct dpif_linux *dpif = dpif_linux_cast(dpif_); + const struct dpif_linux *dpif = dpif_linux_cast(dpif_); struct dpif_linux_flow_state *state; struct dpif_linux_flow request; struct ofpbuf *buf; @@ -1013,7 +1083,7 @@ dpif_linux_execute__(int dp_ifindex, const struct dpif_execute *execute) static int dpif_linux_execute(struct dpif *dpif_, const struct dpif_execute *execute) { - struct dpif_linux *dpif = dpif_linux_cast(dpif_); + const struct dpif_linux *dpif = dpif_linux_cast(dpif_); return dpif_linux_execute__(dpif->dp_ifindex, execute); } @@ -1023,7 +1093,7 @@ dpif_linux_execute(struct dpif *dpif_, const struct dpif_execute *execute) static void dpif_linux_operate__(struct dpif *dpif_, struct dpif_op **ops, size_t n_ops) { - struct dpif_linux *dpif = dpif_linux_cast(dpif_); + const struct dpif_linux *dpif = dpif_linux_cast(dpif_); struct op_auxdata { struct nl_transaction txn; @@ -1163,7 +1233,7 @@ dpif_linux_operate(struct dpif *dpif, struct dpif_op **ops, size_t n_ops) } static int -dpif_linux_recv_set(struct dpif *dpif_, bool enable) +dpif_linux_recv_set__(struct dpif *dpif_, bool enable) { struct dpif_linux *dpif = dpif_linux_cast(dpif_); @@ -1177,9 +1247,11 @@ dpif_linux_recv_set(struct dpif *dpif_, bool enable) struct dpif_port_dump port_dump; struct dpif_port port; - dpif->epoll_fd = epoll_create(10); if (dpif->epoll_fd < 0) { - return errno; + dpif->epoll_fd = epoll_create(10); + if (dpif->epoll_fd < 0) { + return errno; + } } DPIF_PORT_FOR_EACH (&port, &port_dump, &dpif->dpif) { @@ -1232,6 +1304,19 @@ dpif_linux_recv_set(struct dpif *dpif_, bool enable) return 0; } +static int +dpif_linux_recv_set(struct dpif *dpif_, bool enable) +{ + struct dpif_linux *dpif = dpif_linux_cast(dpif_); + int error; + + ovs_mutex_lock(&dpif->upcall_lock); + error = dpif_linux_recv_set__(dpif_, enable); + ovs_mutex_unlock(&dpif->upcall_lock); + + return error; +} + static int dpif_linux_queue_to_priority(const struct dpif *dpif OVS_UNUSED, uint32_t queue_id, uint32_t *priority) @@ -1300,8 +1385,8 @@ parse_odp_packet(struct ofpbuf *buf, struct dpif_upcall *upcall, } static int -dpif_linux_recv(struct dpif *dpif_, struct dpif_upcall *upcall, - struct ofpbuf *buf) +dpif_linux_recv__(struct dpif *dpif_, struct dpif_upcall *upcall, + struct ofpbuf *buf) { struct dpif_linux *dpif = dpif_linux_cast(dpif_); int read_tries = 0; @@ -1371,33 +1456,49 @@ dpif_linux_recv(struct dpif *dpif_, struct dpif_upcall *upcall, return EAGAIN; } +static int +dpif_linux_recv(struct dpif *dpif_, struct dpif_upcall *upcall, + struct ofpbuf *buf) +{ + struct dpif_linux *dpif = dpif_linux_cast(dpif_); + int error; + + ovs_mutex_lock(&dpif->upcall_lock); + error = dpif_linux_recv__(dpif_, upcall, buf); + ovs_mutex_unlock(&dpif->upcall_lock); + + return error; +} + static void dpif_linux_recv_wait(struct dpif *dpif_) { struct dpif_linux *dpif = dpif_linux_cast(dpif_); - if (dpif->epoll_fd < 0) { - return; + ovs_mutex_lock(&dpif->upcall_lock); + if (dpif->epoll_fd >= 0) { + poll_fd_wait(dpif->epoll_fd, POLLIN); } - - poll_fd_wait(dpif->epoll_fd, POLLIN); + ovs_mutex_unlock(&dpif->upcall_lock); } static void dpif_linux_recv_purge(struct dpif *dpif_) { struct dpif_linux *dpif = dpif_linux_cast(dpif_); - struct dpif_channel *ch; - if (dpif->epoll_fd < 0) { - return; - } + ovs_mutex_lock(&dpif->upcall_lock); + if (dpif->epoll_fd >= 0) { + struct dpif_channel *ch; - for (ch = dpif->channels; ch < &dpif->channels[dpif->uc_array_size]; ch++) { - if (ch->sock) { - nl_sock_drain(ch->sock); + for (ch = dpif->channels; ch < &dpif->channels[dpif->uc_array_size]; + ch++) { + if (ch->sock) { + nl_sock_drain(ch->sock); + } } } + ovs_mutex_unlock(&dpif->upcall_lock); } const struct dpif_class dpif_linux_class = { @@ -1407,8 +1508,8 @@ const struct dpif_class dpif_linux_class = { dpif_linux_open, dpif_linux_close, dpif_linux_destroy, - dpif_linux_run, - dpif_linux_wait, + NULL, /* run */ + NULL, /* wait */ dpif_linux_get_stats, dpif_linux_port_add, dpif_linux_port_del, @@ -1444,8 +1545,6 @@ dpif_linux_init(void) static int error; if (ovsthread_once_start(&once)) { - unsigned int ovs_vport_mcgroup; - error = nl_lookup_genl_family(OVS_DATAPATH_FAMILY, &ovs_datapath_family); if (error) { @@ -1468,11 +1567,6 @@ dpif_linux_init(void) &ovs_vport_mcgroup, OVS_VPORT_MCGROUP_FALLBACK_ID); } - if (!error) { - static struct dpif_linux_vport vport; - nln = nln_create(NETLINK_GENERIC, ovs_vport_mcgroup, - dpif_linux_nln_parse, &vport); - } ovsthread_once_done(&once); } @@ -1497,33 +1591,6 @@ dpif_linux_is_internal_device(const char *name) return reply.type == OVS_VPORT_TYPE_INTERNAL; } - -static bool -dpif_linux_nln_parse(struct ofpbuf *buf, void *vport_) -{ - struct dpif_linux_vport *vport = vport_; - return dpif_linux_vport_from_ofpbuf(vport, buf) == 0; -} - -static void -dpif_linux_port_changed(const void *vport_, void *dpif_) -{ - const struct dpif_linux_vport *vport = vport_; - struct dpif_linux *dpif = dpif_; - - if (vport) { - if (vport->dp_ifindex == dpif->dp_ifindex - && (vport->cmd == OVS_VPORT_CMD_NEW - || vport->cmd == OVS_VPORT_CMD_DEL - || vport->cmd == OVS_VPORT_CMD_SET)) { - VLOG_DBG("port_changed: dpif:%s vport:%s cmd:%"PRIu8, - dpif->dpif.full_name, vport->name, vport->cmd); - sset_add(&dpif->changed_ports, vport->name); - } - } else { - dpif->change_error = true; - } -} /* Parses the contents of 'buf', which contains a "struct ovs_header" followed * by Netlink attributes, into 'vport'. Returns 0 if successful, otherwise a diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c index 9a29b7eb3..a8a54a1a6 100644 --- a/lib/dpif-netdev.c +++ b/lib/dpif-netdev.c @@ -52,6 +52,7 @@ #include "shash.h" #include "sset.h" #include "timeval.h" +#include "unixctl.h" #include "util.h" #include "vlog.h" @@ -139,6 +140,9 @@ struct dpif_netdev { /* All netdev-based datapaths. */ static struct shash dp_netdevs = SHASH_INITIALIZER(&dp_netdevs); +/* Global lock for all data. */ +static struct ovs_mutex dp_netdev_mutex = OVS_MUTEX_INITIALIZER; + static int get_port_by_number(struct dp_netdev *, odp_port_t port_no, struct dp_netdev_port **portp); static int get_port_by_name(struct dp_netdev *, const char *devname, @@ -180,9 +184,12 @@ dpif_netdev_enumerate(struct sset *all_dps) { struct shash_node *node; + ovs_mutex_lock(&dp_netdev_mutex); SHASH_FOR_EACH(node, &dp_netdevs) { sset_add(all_dps, node->name); } + ovs_mutex_unlock(&dp_netdev_mutex); + return 0; } @@ -301,28 +308,23 @@ dpif_netdev_open(const struct dpif_class *class, const char *name, bool create, struct dpif **dpifp) { struct dp_netdev *dp; + int error; + ovs_mutex_lock(&dp_netdev_mutex); dp = shash_find_data(&dp_netdevs, name); if (!dp) { - if (!create) { - return ENODEV; - } else { - int error = create_dp_netdev(name, class, &dp); - if (error) { - return error; - } - ovs_assert(dp != NULL); - } + error = create ? create_dp_netdev(name, class, &dp) : ENODEV; } else { - if (dp->class != class) { - return EINVAL; - } else if (create) { - return EEXIST; - } + error = (dp->class != class ? EINVAL + : create ? EEXIST + : 0); + } + if (!error) { + *dpifp = create_dpif_netdev(dp); } + ovs_mutex_unlock(&dp_netdev_mutex); - *dpifp = create_dpif_netdev(dp); - return 0; + return error; } static void @@ -359,19 +361,28 @@ static void dpif_netdev_close(struct dpif *dpif) { struct dp_netdev *dp = get_dp_netdev(dpif); + + ovs_mutex_lock(&dp_netdev_mutex); + ovs_assert(dp->open_cnt > 0); if (--dp->open_cnt == 0 && dp->destroyed) { shash_find_and_delete(&dp_netdevs, dp->name); dp_netdev_free(dp); } free(dpif); + + ovs_mutex_unlock(&dp_netdev_mutex); } static int dpif_netdev_destroy(struct dpif *dpif) { struct dp_netdev *dp = get_dp_netdev(dpif); + + ovs_mutex_lock(&dp_netdev_mutex); dp->destroyed = true; + ovs_mutex_unlock(&dp_netdev_mutex); + return 0; } @@ -379,10 +390,14 @@ static int dpif_netdev_get_stats(const struct dpif *dpif, struct dpif_dp_stats *stats) { struct dp_netdev *dp = get_dp_netdev(dpif); + + ovs_mutex_lock(&dp_netdev_mutex); stats->n_flows = hmap_count(&dp->flow_table); stats->n_hit = dp->n_hit; stats->n_missed = dp->n_missed; stats->n_lost = dp->n_lost; + ovs_mutex_unlock(&dp_netdev_mutex); + return 0; } @@ -452,32 +467,44 @@ dpif_netdev_port_add(struct dpif *dpif, struct netdev *netdev, char namebuf[NETDEV_VPORT_NAME_BUFSIZE]; const char *dpif_port; odp_port_t port_no; + int error; + ovs_mutex_lock(&dp_netdev_mutex); dpif_port = netdev_vport_get_dpif_port(netdev, namebuf, sizeof namebuf); if (*port_nop != ODPP_NONE) { uint32_t port_idx = odp_to_u32(*port_nop); if (port_idx >= MAX_PORTS) { - return EFBIG; + error = EFBIG; } else if (dp->ports[port_idx]) { - return EBUSY; + error = EBUSY; + } else { + error = 0; + port_no = *port_nop; } - port_no = *port_nop; } else { port_no = choose_port(dp, dpif_port); + error = port_no == ODPP_NONE ? EFBIG : 0; } - if (port_no != ODPP_NONE) { + if (!error) { *port_nop = port_no; - return do_add_port(dp, dpif_port, netdev_get_type(netdev), port_no); + error = do_add_port(dp, dpif_port, netdev_get_type(netdev), port_no); } - return EFBIG; + ovs_mutex_unlock(&dp_netdev_mutex); + + return error; } static int dpif_netdev_port_del(struct dpif *dpif, odp_port_t port_no) { struct dp_netdev *dp = get_dp_netdev(dpif); - return (port_no == ODPP_LOCAL ? - EINVAL : do_del_port(dp, port_no)); + int error; + + ovs_mutex_lock(&dp_netdev_mutex); + error = port_no == ODPP_LOCAL ? EINVAL : do_del_port(dp, port_no); + ovs_mutex_unlock(&dp_netdev_mutex); + + return error; } static bool @@ -555,10 +582,13 @@ dpif_netdev_port_query_by_number(const struct dpif *dpif, odp_port_t port_no, struct dp_netdev_port *port; int error; + ovs_mutex_lock(&dp_netdev_mutex); error = get_port_by_number(dp, port_no, &port); if (!error && dpif_port) { answer_port_query(port, dpif_port); } + ovs_mutex_unlock(&dp_netdev_mutex); + return error; } @@ -570,10 +600,13 @@ dpif_netdev_port_query_by_name(const struct dpif *dpif, const char *devname, struct dp_netdev_port *port; int error; + ovs_mutex_lock(&dp_netdev_mutex); error = get_port_by_name(dp, devname, &port); if (!error && dpif_port) { answer_port_query(port, dpif_port); } + ovs_mutex_unlock(&dp_netdev_mutex); + return error; } @@ -605,7 +638,11 @@ static int dpif_netdev_flow_flush(struct dpif *dpif) { struct dp_netdev *dp = get_dp_netdev(dpif); + + ovs_mutex_lock(&dp_netdev_mutex); dp_netdev_flow_flush(dp); + ovs_mutex_unlock(&dp_netdev_mutex); + return 0; } @@ -629,6 +666,7 @@ dpif_netdev_port_dump_next(const struct dpif *dpif, void *state_, struct dp_netdev *dp = get_dp_netdev(dpif); uint32_t port_idx; + ovs_mutex_lock(&dp_netdev_mutex); for (port_idx = odp_to_u32(state->port_no); port_idx < MAX_PORTS; port_idx++) { struct dp_netdev_port *port = dp->ports[port_idx]; @@ -639,9 +677,13 @@ dpif_netdev_port_dump_next(const struct dpif *dpif, void *state_, dpif_port->type = port->type; dpif_port->port_no = port->port_no; state->port_no = u32_to_odp(port_idx + 1); + ovs_mutex_unlock(&dp_netdev_mutex); + return 0; } } + ovs_mutex_unlock(&dp_netdev_mutex); + return EOF; } @@ -658,21 +700,34 @@ static int dpif_netdev_port_poll(const struct dpif *dpif_, char **devnamep OVS_UNUSED) { struct dpif_netdev *dpif = dpif_netdev_cast(dpif_); + int error; + + ovs_mutex_lock(&dp_netdev_mutex); if (dpif->dp_serial != dpif->dp->serial) { dpif->dp_serial = dpif->dp->serial; - return ENOBUFS; + error = ENOBUFS; } else { - return EAGAIN; + error = EAGAIN; } + ovs_mutex_unlock(&dp_netdev_mutex); + + return error; } static void dpif_netdev_port_poll_wait(const struct dpif *dpif_) { struct dpif_netdev *dpif = dpif_netdev_cast(dpif_); + + /* XXX In a multithreaded process, there is a race window between this + * function and the poll_block() in one thread and a change in + * dpif->dp->serial in another thread. */ + + ovs_mutex_lock(&dp_netdev_mutex); if (dpif->dp_serial != dpif->dp->serial) { poll_immediate_wake(); } + ovs_mutex_unlock(&dp_netdev_mutex); } static struct dp_netdev_flow * @@ -745,18 +800,21 @@ dpif_netdev_flow_get(const struct dpif *dpif, return error; } + ovs_mutex_lock(&dp_netdev_mutex); flow = dp_netdev_lookup_flow(dp, &key); - if (!flow) { - return ENOENT; + if (flow) { + if (stats) { + get_dpif_flow_stats(flow, stats); + } + if (actionsp) { + *actionsp = ofpbuf_clone_data(flow->actions, flow->actions_len); + } + } else { + error = ENOENT; } + ovs_mutex_unlock(&dp_netdev_mutex); - if (stats) { - get_dpif_flow_stats(flow, stats); - } - if (actionsp) { - *actionsp = ofpbuf_clone_data(flow->actions, flow->actions_len); - } - return 0; + return error; } static int @@ -811,6 +869,7 @@ dpif_netdev_flow_put(struct dpif *dpif, const struct dpif_flow_put *put) return error; } + ovs_mutex_lock(&dp_netdev_mutex); flow = dp_netdev_lookup_flow(dp, &key); if (!flow) { if (put->flags & DPIF_FP_CREATE) { @@ -818,17 +877,17 @@ dpif_netdev_flow_put(struct dpif *dpif, const struct dpif_flow_put *put) if (put->stats) { memset(put->stats, 0, sizeof *put->stats); } - return dp_netdev_flow_add(dp, &key, put->actions, - put->actions_len); + error = dp_netdev_flow_add(dp, &key, put->actions, + put->actions_len); } else { - return EFBIG; + error = EFBIG; } } else { - return ENOENT; + error = ENOENT; } } else { if (put->flags & DPIF_FP_MODIFY) { - int error = set_flow_actions(flow, put->actions, put->actions_len); + error = set_flow_actions(flow, put->actions, put->actions_len); if (!error) { if (put->stats) { get_dpif_flow_stats(flow, put->stats); @@ -837,11 +896,13 @@ dpif_netdev_flow_put(struct dpif *dpif, const struct dpif_flow_put *put) clear_stats(flow); } } - return error; } else { - return EEXIST; + error = EEXIST; } } + ovs_mutex_unlock(&dp_netdev_mutex); + + return error; } static int @@ -857,16 +918,19 @@ dpif_netdev_flow_del(struct dpif *dpif, const struct dpif_flow_del *del) return error; } + ovs_mutex_lock(&dp_netdev_mutex); flow = dp_netdev_lookup_flow(dp, &key); if (flow) { if (del->stats) { get_dpif_flow_stats(flow, del->stats); } dp_netdev_free_flow(dp, flow); - return 0; } else { - return ENOENT; + error = ENOENT; } + ovs_mutex_unlock(&dp_netdev_mutex); + + return error; } struct dp_netdev_flow_state { @@ -901,8 +965,10 @@ dpif_netdev_flow_dump_next(const struct dpif *dpif, void *state_, struct dp_netdev_flow *flow; struct hmap_node *node; + ovs_mutex_lock(&dp_netdev_mutex); node = hmap_at_position(&dp->flow_table, &state->bucket, &state->offset); if (!node) { + ovs_mutex_unlock(&dp_netdev_mutex); return EOF; } @@ -936,6 +1002,7 @@ dpif_netdev_flow_dump_next(const struct dpif *dpif, void *state_, *stats = &state->stats; } + ovs_mutex_unlock(&dp_netdev_mutex); return 0; } @@ -971,8 +1038,10 @@ dpif_netdev_execute(struct dpif *dpif, const struct dpif_execute *execute) error = dpif_netdev_flow_from_nlattrs(execute->key, execute->key_len, &key); if (!error) { + ovs_mutex_lock(&dp_netdev_mutex); dp_netdev_execute_actions(dp, ©, &key, execute->actions, execute->actions_len); + ovs_mutex_unlock(&dp_netdev_mutex); } ofpbuf_uninit(©); @@ -1012,7 +1081,11 @@ static int dpif_netdev_recv(struct dpif *dpif, struct dpif_upcall *upcall, struct ofpbuf *buf) { - struct dp_netdev_queue *q = find_nonempty_queue(dpif); + struct dp_netdev_queue *q; + int error; + + ovs_mutex_lock(&dp_netdev_mutex); + q = find_nonempty_queue(dpif); if (q) { struct dp_netdev_upcall *u = &q->upcalls[q->tail++ & QUEUE_MASK]; @@ -1022,28 +1095,36 @@ dpif_netdev_recv(struct dpif *dpif, struct dpif_upcall *upcall, ofpbuf_uninit(buf); *buf = u->buf; - return 0; + error = 0; } else { - return EAGAIN; + error = EAGAIN; } + ovs_mutex_unlock(&dp_netdev_mutex); + + return error; } static void dpif_netdev_recv_wait(struct dpif *dpif) { + /* XXX In a multithreaded process, there is a race window between this + * function and the poll_block() in one thread and a packet being queued in + * another thread. */ + + ovs_mutex_lock(&dp_netdev_mutex); if (find_nonempty_queue(dpif)) { poll_immediate_wake(); - } else { - /* No messages ready to be received, and dp_wait() will ensure that we - * wake up to queue new messages, so there is nothing to do. */ } + ovs_mutex_unlock(&dp_netdev_mutex); } static void dpif_netdev_recv_purge(struct dpif *dpif) { struct dpif_netdev *dpif_netdev = dpif_netdev_cast(dpif); + ovs_mutex_lock(&dp_netdev_mutex); dp_netdev_purge_queues(dpif_netdev->dp); + ovs_mutex_unlock(&dp_netdev_mutex); } static void @@ -1084,10 +1165,12 @@ dp_netdev_port_input(struct dp_netdev *dp, struct dp_netdev_port *port, static void dpif_netdev_run(struct dpif *dpif) { - struct dp_netdev *dp = get_dp_netdev(dpif); struct dp_netdev_port *port; + struct dp_netdev *dp; struct ofpbuf packet; + ovs_mutex_lock(&dp_netdev_mutex); + dp = get_dp_netdev(dpif); ofpbuf_init(&packet, DP_NETDEV_HEADROOM + VLAN_ETH_HEADER_LEN + dp->max_mtu); @@ -1109,19 +1192,34 @@ dpif_netdev_run(struct dpif *dpif) } } ofpbuf_uninit(&packet); + ovs_mutex_unlock(&dp_netdev_mutex); } static void dpif_netdev_wait(struct dpif *dpif) { - struct dp_netdev *dp = get_dp_netdev(dpif); struct dp_netdev_port *port; - LIST_FOR_EACH (port, node, &dp->port_list) { + /* There is a race here, if thread A calls dpif_netdev_wait(dpif) and + * thread B calls dpif_port_add(dpif) or dpif_port_remove(dpif) before + * A makes it to poll_block(). + * + * But I think it doesn't matter: + * + * - In the dpif_port_add() case, A will not wake up when a packet + * arrives on the new port, but this would also happen if the + * ordering were reversed. + * + * - In the dpif_port_remove() case, A might wake up spuriously, but + * that is harmless. */ + + ovs_mutex_lock(&dp_netdev_mutex); + LIST_FOR_EACH (port, node, &get_dp_netdev(dpif)->port_list) { if (port->rx) { netdev_rx_wait(port->rx); } } + ovs_mutex_unlock(&dp_netdev_mutex); } static void @@ -1246,6 +1344,41 @@ const struct dpif_class dpif_planetlab_class = { DPIF_NETDEV_CLASS_FUNCTIONS }; +static void +dpif_dummy_change_port_number(struct unixctl_conn *conn, int argc OVS_UNUSED, + const char *argv[], void *aux OVS_UNUSED) +{ + struct dp_netdev_port *port; + struct dp_netdev *dp; + int port_no; + + dp = shash_find_data(&dp_netdevs, argv[1]); + if (!dp || !dpif_netdev_class_is_dummy(dp->class)) { + unixctl_command_reply_error(conn, "unknown datapath or not a dummy"); + return; + } + + if (get_port_by_name(dp, argv[2], &port)) { + unixctl_command_reply_error(conn, "unknown port"); + return; + } + + port_no = atoi(argv[3]); + if (port_no <= 0 || port_no >= MAX_PORTS) { + unixctl_command_reply_error(conn, "bad port number"); + return; + } + if (dp->ports[port_no]) { + unixctl_command_reply_error(conn, "port number already in use"); + return; + } + dp->ports[odp_to_u32(port->port_no)] = NULL; + dp->ports[port_no] = port; + port->port_no = u32_to_odp(port_no); + dp->serial++; + unixctl_command_reply(conn, NULL); +} + static void dpif_dummy_register__(const char *type) { @@ -1275,5 +1408,9 @@ dpif_dummy_register(bool override) } dpif_dummy_register__("dummy"); + + unixctl_command_register("dpif-dummy/change-port-number", + "DP PORT NEW-NUMBER", + 3, 3, dpif_dummy_change_port_number, NULL); } diff --git a/lib/dpif.c b/lib/dpif.c index c4b6ff675..9c5cf3d07 100644 --- a/lib/dpif.c +++ b/lib/dpif.c @@ -71,6 +71,9 @@ struct registered_dpif_class { static struct shash dpif_classes = SHASH_INITIALIZER(&dpif_classes); static struct sset dpif_blacklist = SSET_INITIALIZER(&dpif_blacklist); +/* Protects 'dpif_classes', including the refcount, and 'dpif_blacklist'. */ +static struct ovs_mutex dpif_mutex = OVS_MUTEX_INITIALIZER; + /* Rate limit for individual messages going to or from the datapath, output at * DBG level. This is very high because, if these are enabled, it is because * we really need to see them. */ @@ -110,10 +113,8 @@ dp_initialize(void) } } -/* Registers a new datapath provider. After successful registration, new - * datapaths of that type can be opened using dpif_open(). */ -int -dp_register_provider(const struct dpif_class *new_class) +static int +dp_register_provider__(const struct dpif_class *new_class) { struct registered_dpif_class *registered_class; @@ -138,11 +139,25 @@ dp_register_provider(const struct dpif_class *new_class) return 0; } +/* Registers a new datapath provider. After successful registration, new + * datapaths of that type can be opened using dpif_open(). */ +int +dp_register_provider(const struct dpif_class *new_class) +{ + int error; + + ovs_mutex_lock(&dpif_mutex); + error = dp_register_provider__(new_class); + ovs_mutex_unlock(&dpif_mutex); + + return error; +} + /* Unregisters a datapath provider. 'type' must have been previously * registered and not currently be in use by any dpifs. After unregistration * new datapaths of that type cannot be opened using dpif_open(). */ -int -dp_unregister_provider(const char *type) +static int +dp_unregister_provider__(const char *type) { struct shash_node *node; struct registered_dpif_class *registered_class; @@ -166,12 +181,31 @@ dp_unregister_provider(const char *type) return 0; } +/* Unregisters a datapath provider. 'type' must have been previously + * registered and not currently be in use by any dpifs. After unregistration + * new datapaths of that type cannot be opened using dpif_open(). */ +int +dp_unregister_provider(const char *type) +{ + int error; + + dp_initialize(); + + ovs_mutex_lock(&dpif_mutex); + error = dp_unregister_provider__(type); + ovs_mutex_unlock(&dpif_mutex); + + return error; +} + /* Blacklists a provider. Causes future calls of dp_register_provider() with * a dpif_class which implements 'type' to fail. */ void dp_blacklist_provider(const char *type) { + ovs_mutex_lock(&dpif_mutex); sset_add(&dpif_blacklist, type); + ovs_mutex_unlock(&dpif_mutex); } /* Clears 'types' and enumerates the types of all currently registered datapath @@ -184,10 +218,36 @@ dp_enumerate_types(struct sset *types) dp_initialize(); sset_clear(types); + ovs_mutex_lock(&dpif_mutex); SHASH_FOR_EACH(node, &dpif_classes) { const struct registered_dpif_class *registered_class = node->data; sset_add(types, registered_class->dpif_class->type); } + ovs_mutex_unlock(&dpif_mutex); +} + +static void +dp_class_unref(struct registered_dpif_class *rc) +{ + ovs_mutex_lock(&dpif_mutex); + ovs_assert(rc->refcount); + rc->refcount--; + ovs_mutex_unlock(&dpif_mutex); +} + +static struct registered_dpif_class * +dp_class_lookup(const char *type) +{ + struct registered_dpif_class *rc; + + ovs_mutex_lock(&dpif_mutex); + rc = shash_find_data(&dpif_classes, type); + if (rc) { + rc->refcount++; + } + ovs_mutex_unlock(&dpif_mutex); + + return rc; } /* Clears 'names' and enumerates the names of all known created datapaths with @@ -199,14 +259,14 @@ dp_enumerate_types(struct sset *types) int dp_enumerate_names(const char *type, struct sset *names) { - const struct registered_dpif_class *registered_class; + struct registered_dpif_class *registered_class; const struct dpif_class *dpif_class; int error; dp_initialize(); sset_clear(names); - registered_class = shash_find_data(&dpif_classes, type); + registered_class = dp_class_lookup(type); if (!registered_class) { VLOG_WARN("could not enumerate unknown type: %s", type); return EAFNOSUPPORT; @@ -214,11 +274,11 @@ dp_enumerate_names(const char *type, struct sset *names) dpif_class = registered_class->dpif_class; error = dpif_class->enumerate ? dpif_class->enumerate(names) : 0; - if (error) { VLOG_WARN("failed to enumerate %s datapaths: %s", dpif_class->type, ovs_strerror(error)); } + dp_class_unref(registered_class); return error; } @@ -254,8 +314,7 @@ do_open(const char *name, const char *type, bool create, struct dpif **dpifp) dp_initialize(); type = dpif_normalize_type(type); - - registered_class = shash_find_data(&dpif_classes, type); + registered_class = dp_class_lookup(type); if (!registered_class) { VLOG_WARN("could not create datapath %s of unknown type %s", name, type); @@ -267,7 +326,8 @@ do_open(const char *name, const char *type, bool create, struct dpif **dpifp) name, create, &dpif); if (!error) { ovs_assert(dpif->dpif_class == registered_class->dpif_class); - registered_class->refcount++; + } else { + dp_class_unref(registered_class); } exit: @@ -327,15 +387,11 @@ void dpif_close(struct dpif *dpif) { if (dpif) { - struct registered_dpif_class *registered_class; - - registered_class = shash_find_data(&dpif_classes, - dpif->dpif_class->type); - ovs_assert(registered_class); - ovs_assert(registered_class->refcount); + struct registered_dpif_class *rc; - registered_class->refcount--; + rc = shash_find_data(&dpif_classes, dpif->dpif_class->type); dpif_uninit(dpif, true); + dp_class_unref(rc); } } @@ -422,18 +478,18 @@ dpif_get_dp_stats(const struct dpif *dpif, struct dpif_dp_stats *stats) const char * dpif_port_open_type(const char *datapath_type, const char *port_type) { - struct registered_dpif_class *registered_class; + struct registered_dpif_class *rc; datapath_type = dpif_normalize_type(datapath_type); - registered_class = shash_find_data(&dpif_classes, datapath_type); - if (!registered_class - || !registered_class->dpif_class->port_open_type) { - return port_type; + ovs_mutex_lock(&dpif_mutex); + rc = shash_find_data(&dpif_classes, datapath_type); + if (rc && rc->dpif_class->port_open_type) { + port_type = rc->dpif_class->port_open_type(rc->dpif_class, port_type); } + ovs_mutex_unlock(&dpif_mutex); - return registered_class->dpif_class->port_open_type( - registered_class->dpif_class, port_type); + return port_type; } /* Attempts to add 'netdev' as a port on 'dpif'. If 'port_nop' is diff --git a/lib/dpif.h b/lib/dpif.h index 7f1b6c942..d0280858d 100644 --- a/lib/dpif.h +++ b/lib/dpif.h @@ -317,6 +317,24 @@ * location. * * - Adding and removing ports to achieve a new configuration. + * + * + * Thread-safety + * ============= + * + * Most of the dpif functions are fully thread-safe: they may be called from + * any number of threads on the same or different dpif objects. The exceptions + * are: + * + * - dpif_port_poll() and dpif_port_poll_wait() are conditionally + * thread-safe: they may be called from different threads only on + * different dpif objects. + * + * - Functions that operate on struct dpif_port_dump or struct + * dpif_flow_dump are conditionally thread-safe with respect to those + * objects. That is, one may dump ports or flows from any number of + * threads at once, but each thread must use its own struct dpif_port_dump + * or dpif_flow_dump. */ #ifndef DPIF_H #define DPIF_H 1 diff --git a/lib/fatal-signal.c b/lib/fatal-signal.c index db8d98eb3..b4c5ee6ac 100644 --- a/lib/fatal-signal.c +++ b/lib/fatal-signal.c @@ -23,6 +23,7 @@ #include #include #include +#include "ovs-thread.h" #include "poll-loop.h" #include "shash.h" #include "sset.h" @@ -56,11 +57,16 @@ static size_t n_hooks; static int signal_fds[2]; static volatile sig_atomic_t stored_sig_nr = SIG_ATOMIC_MAX; -static void fatal_signal_init(void); +static struct ovs_mutex mutex; + static void atexit_handler(void); static void call_hooks(int sig_nr); -static void +/* Initializes the fatal signal handling module. Calling this function is + * optional, because calling any other function in the module will also + * initialize it. However, in a multithreaded program, the module must be + * initialized while the process is still single-threaded. */ +void fatal_signal_init(void) { static bool inited = false; @@ -68,8 +74,10 @@ fatal_signal_init(void) if (!inited) { size_t i; + assert_single_threaded(); inited = true; + ovs_mutex_init(&mutex, PTHREAD_MUTEX_RECURSIVE); xpipe_nonblocking(signal_fds); for (i = 0; i < ARRAY_SIZE(fatal_signals); i++) { @@ -86,13 +94,13 @@ fatal_signal_init(void) } } -/* Registers 'hook_cb' to be called when a process termination signal is - * raised. If 'run_at_exit' is true, 'hook_cb' is also called during normal - * process termination, e.g. when exit() is called or when main() returns. +/* Registers 'hook_cb' to be called from inside poll_block() following a fatal + * signal. 'hook_cb' does not need to be async-signal-safe. In a + * multithreaded program 'hook_cb' might be called from any thread, with + * threads other than the one running 'hook_cb' in unknown states. * - * 'hook_cb' is not called immediately from the signal handler but rather the - * next time the poll loop iterates, so it is freed from the usual restrictions - * on signal handler functions. + * If 'run_at_exit' is true, 'hook_cb' is also called during normal process + * termination, e.g. when exit() is called or when main() returns. * * If the current process forks, fatal_signal_fork() may be called to clear the * parent process's fatal signal hooks, so that 'hook_cb' is only called when @@ -106,12 +114,14 @@ fatal_signal_add_hook(void (*hook_cb)(void *aux), void (*cancel_cb)(void *aux), { fatal_signal_init(); + ovs_mutex_lock(&mutex); ovs_assert(n_hooks < MAX_HOOKS); hooks[n_hooks].hook_cb = hook_cb; hooks[n_hooks].cancel_cb = cancel_cb; hooks[n_hooks].aux = aux; hooks[n_hooks].run_at_exit = run_at_exit; n_hooks++; + ovs_mutex_unlock(&mutex); } /* Handles fatal signal number 'sig_nr'. @@ -152,6 +162,8 @@ fatal_signal_run(void) if (sig_nr != SIG_ATOMIC_MAX) { char namebuf[SIGNAL_NAME_BUFSIZE]; + ovs_mutex_lock(&mutex); + VLOG_WARN("terminating with signal %d (%s)", (int)sig_nr, signal_name(sig_nr, namebuf, sizeof namebuf)); call_hooks(sig_nr); @@ -160,6 +172,9 @@ fatal_signal_run(void) * termination status reflects that we were killed by this signal */ signal(sig_nr, SIG_DFL); raise(sig_nr); + + ovs_mutex_unlock(&mutex); + NOT_REACHED(); } } @@ -210,12 +225,16 @@ static void do_unlink_files(void); void fatal_signal_add_file_to_unlink(const char *file) { + fatal_signal_init(); + + ovs_mutex_lock(&mutex); if (!added_hook) { added_hook = true; fatal_signal_add_hook(unlink_files, cancel_files, NULL, true); } sset_add(&files, file); + ovs_mutex_unlock(&mutex); } /* Unregisters 'file' from being unlinked when the program terminates via @@ -223,7 +242,11 @@ fatal_signal_add_file_to_unlink(const char *file) void fatal_signal_remove_file_to_unlink(const char *file) { + fatal_signal_init(); + + ovs_mutex_lock(&mutex); sset_find_and_delete(&files, file); + ovs_mutex_unlock(&mutex); } /* Like fatal_signal_remove_file_to_unlink(), but also unlinks 'file'. @@ -231,13 +254,21 @@ fatal_signal_remove_file_to_unlink(const char *file) int fatal_signal_unlink_file_now(const char *file) { - int error = unlink(file) ? errno : 0; + int error; + + fatal_signal_init(); + + ovs_mutex_lock(&mutex); + + error = unlink(file) ? errno : 0; if (error) { VLOG_WARN("could not unlink \"%s\" (%s)", file, ovs_strerror(error)); } fatal_signal_remove_file_to_unlink(file); + ovs_mutex_unlock(&mutex); + return error; } @@ -277,6 +308,8 @@ fatal_signal_fork(void) { size_t i; + assert_single_threaded(); + for (i = 0; i < n_hooks; i++) { struct hook *h = &hooks[i]; if (h->cancel_cb) { diff --git a/lib/fatal-signal.h b/lib/fatal-signal.h index 8a1a84b8c..b458d3d61 100644 --- a/lib/fatal-signal.h +++ b/lib/fatal-signal.h @@ -1,5 +1,5 @@ /* - * Copyright (c) 2008, 2009, 2010 Nicira, Inc. + * Copyright (c) 2008, 2009, 2010, 2013 Nicira, Inc. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -20,6 +20,7 @@ #include /* Basic interface. */ +void fatal_signal_init(void); void fatal_signal_add_hook(void (*hook_cb)(void *aux), void (*cancel_cb)(void *aux), void *aux, bool run_at_exit); diff --git a/lib/hash.c b/lib/hash.c index e954d7869..8f34493b5 100644 --- a/lib/hash.c +++ b/lib/hash.c @@ -29,15 +29,15 @@ hash_3words(uint32_t a, uint32_t b, uint32_t c) uint32_t hash_bytes(const void *p_, size_t n, uint32_t basis) { - const uint8_t *p = p_; + const uint32_t *p = p_; size_t orig_n = n; uint32_t hash; hash = basis; while (n >= 4) { - hash = mhash_add(hash, get_unaligned_u32((const uint32_t *) p)); + hash = mhash_add(hash, get_unaligned_u32(p)); n -= 4; - p += 4; + p += 1; } if (n) { diff --git a/lib/jhash.c b/lib/jhash.c index 4ec2871d7..c08c36894 100644 --- a/lib/jhash.c +++ b/lib/jhash.c @@ -96,18 +96,18 @@ jhash_words(const uint32_t *p, size_t n, uint32_t basis) uint32_t jhash_bytes(const void *p_, size_t n, uint32_t basis) { - const uint8_t *p = p_; + const uint32_t *p = p_; uint32_t a, b, c; a = b = c = 0xdeadbeef + n + basis; while (n >= 12) { - a += get_unaligned_u32((uint32_t *) p); - b += get_unaligned_u32((uint32_t *) (p + 4)); - c += get_unaligned_u32((uint32_t *) (p + 8)); + a += get_unaligned_u32(p); + b += get_unaligned_u32(p + 1); + c += get_unaligned_u32(p + 2); jhash_mix(&a, &b, &c); n -= 12; - p += 12; + p += 3; } if (n) { diff --git a/lib/jsonrpc.c b/lib/jsonrpc.c index 6c482c2d1..e02f03579 100644 --- a/lib/jsonrpc.c +++ b/lib/jsonrpc.c @@ -352,7 +352,7 @@ void jsonrpc_recv_wait(struct jsonrpc *rpc) { if (rpc->status || rpc->received || !byteq_is_empty(&rpc->input)) { - (poll_immediate_wake)(rpc->name); + poll_immediate_wake_at(rpc->name); } else { stream_recv_wait(rpc->stream); } @@ -515,14 +515,10 @@ jsonrpc_create(enum jsonrpc_msg_type type, const char *method, static struct json * jsonrpc_create_id(void) { - static pthread_mutex_t mutex = PTHREAD_ADAPTIVE_MUTEX_INITIALIZER; - static unsigned int next_id; + static atomic_uint next_id = ATOMIC_VAR_INIT(0); unsigned int id; - xpthread_mutex_lock(&mutex); - id = next_id++; - xpthread_mutex_unlock(&mutex); - + atomic_add(&next_id, 1, &id); return json_integer_create(id); } diff --git a/lib/lacp.c b/lib/lacp.c index 5d9085023..de0b6631f 100644 --- a/lib/lacp.c +++ b/lib/lacp.c @@ -103,7 +103,7 @@ struct lacp { bool negotiated; /* True if LACP negotiations were successful. */ bool update; /* True if lacp_update() needs to be called. */ - int ref_cnt; + atomic_int ref_cnt; }; struct slave { @@ -124,18 +124,25 @@ struct slave { struct timer rx; /* Expected message receive timer. */ }; -static struct list all_lacps = LIST_INITIALIZER(&all_lacps); - -static void lacp_update_attached(struct lacp *); - -static void slave_destroy(struct slave *); -static void slave_set_defaulted(struct slave *); -static void slave_set_expired(struct slave *); -static void slave_get_actor(struct slave *, struct lacp_info *actor); -static void slave_get_priority(struct slave *, struct lacp_info *priority); -static bool slave_may_tx(const struct slave *); -static struct slave *slave_lookup(const struct lacp *, const void *slave); -static bool info_tx_equal(struct lacp_info *, struct lacp_info *); +static struct ovs_mutex mutex; +static struct list all_lacps__ = LIST_INITIALIZER(&all_lacps__); +static struct list *const all_lacps OVS_GUARDED_BY(mutex) = &all_lacps__; + +static void lacp_update_attached(struct lacp *) OVS_REQ_WRLOCK(mutex); + +static void slave_destroy(struct slave *) OVS_REQ_WRLOCK(mutex); +static void slave_set_defaulted(struct slave *) OVS_REQ_WRLOCK(mutex); +static void slave_set_expired(struct slave *) OVS_REQ_WRLOCK(mutex); +static void slave_get_actor(struct slave *, struct lacp_info *actor) + OVS_REQ_WRLOCK(mutex); +static void slave_get_priority(struct slave *, struct lacp_info *priority) + OVS_REQ_WRLOCK(mutex); +static bool slave_may_tx(const struct slave *) + OVS_REQ_WRLOCK(mutex); +static struct slave *slave_lookup(const struct lacp *, const void *slave) + OVS_REQ_WRLOCK(mutex); +static bool info_tx_equal(struct lacp_info *, struct lacp_info *) + OVS_REQ_WRLOCK(mutex); static unixctl_cb_func lacp_unixctl_show; @@ -194,14 +201,23 @@ lacp_init(void) /* Creates a LACP object. */ struct lacp * -lacp_create(void) +lacp_create(void) OVS_EXCLUDED(mutex) { + static struct ovsthread_once once = OVSTHREAD_ONCE_INITIALIZER; struct lacp *lacp; + if (ovsthread_once_start(&once)) { + ovs_mutex_init(&mutex, PTHREAD_MUTEX_RECURSIVE); + ovsthread_once_done(&once); + } + lacp = xzalloc(sizeof *lacp); hmap_init(&lacp->slaves); - list_push_back(&all_lacps, &lacp->node); - lacp->ref_cnt = 1; + atomic_init(&lacp->ref_cnt, 1); + + ovs_mutex_lock(&mutex); + list_push_back(all_lacps, &lacp->node); + ovs_mutex_unlock(&mutex); return lacp; } @@ -210,24 +226,29 @@ lacp_ref(const struct lacp *lacp_) { struct lacp *lacp = CONST_CAST(struct lacp *, lacp_); if (lacp) { - ovs_assert(lacp->ref_cnt > 0); - lacp->ref_cnt++; + int orig; + atomic_add(&lacp->ref_cnt, 1, &orig); + ovs_assert(orig > 0); } return lacp; } /* Destroys 'lacp' and its slaves. Does nothing if 'lacp' is NULL. */ void -lacp_unref(struct lacp *lacp) +lacp_unref(struct lacp *lacp) OVS_EXCLUDED(mutex) { + int orig; + if (!lacp) { return; } - ovs_assert(lacp->ref_cnt > 0); - if (!--lacp->ref_cnt) { + atomic_sub(&lacp->ref_cnt, 1, &orig); + ovs_assert(orig > 0); + if (orig == 1) { struct slave *slave, *next; + ovs_mutex_lock(&mutex); HMAP_FOR_EACH_SAFE (slave, next, node, &lacp->slaves) { slave_destroy(slave); } @@ -236,15 +257,18 @@ lacp_unref(struct lacp *lacp) list_remove(&lacp->node); free(lacp->name); free(lacp); + ovs_mutex_unlock(&mutex); } } /* Configures 'lacp' with settings from 's'. */ void lacp_configure(struct lacp *lacp, const struct lacp_settings *s) + OVS_EXCLUDED(mutex) { ovs_assert(!eth_addr_is_zero(s->id)); + ovs_mutex_lock(&mutex); if (!lacp->name || strcmp(s->name, lacp->name)) { free(lacp->name); lacp->name = xstrdup(s->name); @@ -259,14 +283,19 @@ lacp_configure(struct lacp *lacp, const struct lacp_settings *s) lacp->active = s->active; lacp->fast = s->fast; + ovs_mutex_unlock(&mutex); } /* Returns true if 'lacp' is configured in active mode, false if 'lacp' is * configured for passive mode. */ bool -lacp_is_active(const struct lacp *lacp) +lacp_is_active(const struct lacp *lacp) OVS_EXCLUDED(mutex) { - return lacp->active; + bool ret; + ovs_mutex_lock(&mutex); + ret = lacp->active; + ovs_mutex_unlock(&mutex); + return ret; } /* Processes 'packet' which was received on 'slave_'. This function should be @@ -275,20 +304,23 @@ lacp_is_active(const struct lacp *lacp) void lacp_process_packet(struct lacp *lacp, const void *slave_, const struct ofpbuf *packet) + OVS_EXCLUDED(mutex) { static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5); - struct slave *slave = slave_lookup(lacp, slave_); const struct lacp_pdu *pdu; long long int tx_rate; + struct slave *slave; + ovs_mutex_lock(&mutex); + slave = slave_lookup(lacp, slave_); if (!slave) { - return; + goto out; } pdu = parse_lacp_packet(packet); if (!pdu) { VLOG_WARN_RL(&rl, "%s: received an unparsable LACP PDU.", lacp->name); - return; + goto out; } slave->status = LACP_CURRENT; @@ -304,19 +336,27 @@ lacp_process_packet(struct lacp *lacp, const void *slave_, lacp->update = true; slave->partner = pdu->actor; } + +out: + ovs_mutex_unlock(&mutex); } /* Returns the lacp_status of the given 'lacp' object (which may be NULL). */ enum lacp_status -lacp_status(const struct lacp *lacp) +lacp_status(const struct lacp *lacp) OVS_EXCLUDED(mutex) { + enum lacp_status ret; + + ovs_mutex_lock(&mutex); if (!lacp) { - return LACP_DISABLED; + ret = LACP_DISABLED; } else if (lacp->negotiated) { - return LACP_NEGOTIATED; + ret = LACP_NEGOTIATED; } else { - return LACP_CONFIGURED; + ret = LACP_CONFIGURED; } + ovs_mutex_unlock(&mutex); + return ret; } /* Registers 'slave_' as subordinate to 'lacp'. This should be called at least @@ -325,9 +365,12 @@ lacp_status(const struct lacp *lacp) void lacp_slave_register(struct lacp *lacp, void *slave_, const struct lacp_slave_settings *s) + OVS_EXCLUDED(mutex) { - struct slave *slave = slave_lookup(lacp, slave_); + struct slave *slave; + ovs_mutex_lock(&mutex); + slave = slave_lookup(lacp, slave_); if (!slave) { slave = xzalloc(sizeof *slave); slave->lacp = lacp; @@ -358,40 +401,52 @@ lacp_slave_register(struct lacp *lacp, void *slave_, slave_set_expired(slave); } } + ovs_mutex_unlock(&mutex); } /* Unregisters 'slave_' with 'lacp'. */ void lacp_slave_unregister(struct lacp *lacp, const void *slave_) + OVS_EXCLUDED(mutex) { - struct slave *slave = slave_lookup(lacp, slave_); + struct slave *slave; + ovs_mutex_lock(&mutex); + slave = slave_lookup(lacp, slave_); if (slave) { slave_destroy(slave); lacp->update = true; } + ovs_mutex_unlock(&mutex); } /* This function should be called whenever the carrier status of 'slave_' has * changed. If 'lacp' is null, this function has no effect.*/ void lacp_slave_carrier_changed(const struct lacp *lacp, const void *slave_) + OVS_EXCLUDED(mutex) { - if (lacp) { - struct slave *slave = slave_lookup(lacp, slave_); + struct slave *slave; + if (!lacp) { + return; + } - if (!slave) { - return; - } + ovs_mutex_lock(&mutex); + slave = slave_lookup(lacp, slave_); + if (!slave) { + goto out; + } - if (slave->status == LACP_CURRENT || slave->lacp->active) { - slave_set_expired(slave); - } + if (slave->status == LACP_CURRENT || slave->lacp->active) { + slave_set_expired(slave); } + +out: + ovs_mutex_unlock(&mutex); } static bool -slave_may_enable__(struct slave *slave) +slave_may_enable__(struct slave *slave) OVS_REQ_WRLOCK(mutex) { /* The slave may be enabled if it's attached to an aggregator and its * partner is synchronized.*/ @@ -403,10 +458,17 @@ slave_may_enable__(struct slave *slave) * convenience, returns true if 'lacp' is NULL. */ bool lacp_slave_may_enable(const struct lacp *lacp, const void *slave_) + OVS_EXCLUDED(mutex) { if (lacp) { - struct slave *slave = slave_lookup(lacp, slave_); - return slave ? slave_may_enable__(slave) : false; + struct slave *slave; + bool ret; + + ovs_mutex_lock(&mutex); + slave = slave_lookup(lacp, slave_); + ret = slave ? slave_may_enable__(slave) : false; + ovs_mutex_unlock(&mutex); + return ret; } else { return true; } @@ -417,17 +479,25 @@ lacp_slave_may_enable(const struct lacp *lacp, const void *slave_) * misconfigured (or broken) partner. */ bool lacp_slave_is_current(const struct lacp *lacp, const void *slave_) + OVS_EXCLUDED(mutex) { - struct slave *slave = slave_lookup(lacp, slave_); - return slave ? slave->status != LACP_DEFAULTED : false; + struct slave *slave; + bool ret; + + ovs_mutex_lock(&mutex); + slave = slave_lookup(lacp, slave_); + ret = slave ? slave->status != LACP_DEFAULTED : false; + ovs_mutex_unlock(&mutex); + return ret; } /* This function should be called periodically to update 'lacp'. */ void -lacp_run(struct lacp *lacp, lacp_send_pdu *send_pdu) +lacp_run(struct lacp *lacp, lacp_send_pdu *send_pdu) OVS_EXCLUDED(mutex) { struct slave *slave; + ovs_mutex_lock(&mutex); HMAP_FOR_EACH (slave, node, &lacp->slaves) { if (timer_expired(&slave->rx)) { if (slave->status == LACP_CURRENT) { @@ -467,14 +537,16 @@ lacp_run(struct lacp *lacp, lacp_send_pdu *send_pdu) timer_set_duration(&slave->tx, duration); } } + ovs_mutex_unlock(&mutex); } /* Causes poll_block() to wake up when lacp_run() needs to be called again. */ void -lacp_wait(struct lacp *lacp) +lacp_wait(struct lacp *lacp) OVS_EXCLUDED(mutex) { struct slave *slave; + ovs_mutex_lock(&mutex); HMAP_FOR_EACH (slave, node, &lacp->slaves) { if (slave_may_tx(slave)) { timer_wait(&slave->tx); @@ -484,6 +556,7 @@ lacp_wait(struct lacp *lacp) timer_wait(&slave->rx); } } + ovs_mutex_unlock(&mutex); } /* Static Helpers. */ @@ -491,7 +564,7 @@ lacp_wait(struct lacp *lacp) /* Updates the attached status of all slaves controlled by 'lacp' and sets its * negotiated parameter to true if any slaves are attachable. */ static void -lacp_update_attached(struct lacp *lacp) +lacp_update_attached(struct lacp *lacp) OVS_REQ_WRLOCK(mutex) { struct slave *lead, *slave; struct lacp_info lead_pri; @@ -540,7 +613,7 @@ lacp_update_attached(struct lacp *lacp) } static void -slave_destroy(struct slave *slave) +slave_destroy(struct slave *slave) OVS_REQ_WRLOCK(mutex) { if (slave) { struct lacp *lacp = slave->lacp; @@ -564,7 +637,7 @@ slave_destroy(struct slave *slave) } static void -slave_set_defaulted(struct slave *slave) +slave_set_defaulted(struct slave *slave) OVS_REQ_WRLOCK(mutex) { memset(&slave->partner, 0, sizeof slave->partner); @@ -573,7 +646,7 @@ slave_set_defaulted(struct slave *slave) } static void -slave_set_expired(struct slave *slave) +slave_set_expired(struct slave *slave) OVS_REQ_WRLOCK(mutex) { slave->status = LACP_EXPIRED; slave->partner.state |= LACP_STATE_TIME; @@ -584,6 +657,7 @@ slave_set_expired(struct slave *slave) static void slave_get_actor(struct slave *slave, struct lacp_info *actor) + OVS_REQ_WRLOCK(mutex) { struct lacp *lacp = slave->lacp; uint16_t key; @@ -636,6 +710,7 @@ slave_get_actor(struct slave *slave, struct lacp_info *actor) * link. */ static void slave_get_priority(struct slave *slave, struct lacp_info *priority) + OVS_REQ_WRLOCK(mutex) { uint16_t partner_priority, actor_priority; @@ -660,13 +735,13 @@ slave_get_priority(struct slave *slave, struct lacp_info *priority) } static bool -slave_may_tx(const struct slave *slave) +slave_may_tx(const struct slave *slave) OVS_REQ_WRLOCK(mutex) { return slave->lacp->active || slave->status != LACP_DEFAULTED; } static struct slave * -slave_lookup(const struct lacp *lacp, const void *slave_) +slave_lookup(const struct lacp *lacp, const void *slave_) OVS_REQ_WRLOCK(mutex) { struct slave *slave; @@ -703,11 +778,11 @@ info_tx_equal(struct lacp_info *a, struct lacp_info *b) } static struct lacp * -lacp_find(const char *name) +lacp_find(const char *name) OVS_REQ_WRLOCK(&mutex) { struct lacp *lacp; - LIST_FOR_EACH (lacp, node, &all_lacps) { + LIST_FOR_EACH (lacp, node, all_lacps) { if (!strcmp(lacp->name, name)) { return lacp; } @@ -753,7 +828,7 @@ ds_put_lacp_state(struct ds *ds, uint8_t state) } static void -lacp_print_details(struct ds *ds, struct lacp *lacp) +lacp_print_details(struct ds *ds, struct lacp *lacp) OVS_REQ_WRLOCK(&mutex) { struct shash slave_shash = SHASH_INITIALIZER(&slave_shash); const struct shash_node **sorted_slaves = NULL; @@ -854,24 +929,28 @@ lacp_print_details(struct ds *ds, struct lacp *lacp) static void lacp_unixctl_show(struct unixctl_conn *conn, int argc, const char *argv[], - void *aux OVS_UNUSED) + void *aux OVS_UNUSED) OVS_EXCLUDED(mutex) { struct ds ds = DS_EMPTY_INITIALIZER; struct lacp *lacp; + ovs_mutex_lock(&mutex); if (argc > 1) { lacp = lacp_find(argv[1]); if (!lacp) { unixctl_command_reply_error(conn, "no such lacp object"); - return; + goto out; } lacp_print_details(&ds, lacp); } else { - LIST_FOR_EACH (lacp, node, &all_lacps) { + LIST_FOR_EACH (lacp, node, all_lacps) { lacp_print_details(&ds, lacp); } } unixctl_command_reply(conn, ds_cstr(&ds)); ds_destroy(&ds); + +out: + ovs_mutex_unlock(&mutex); } diff --git a/lib/latch.c b/lib/latch.c index 9b130062b..bf518b9ca 100644 --- a/lib/latch.c +++ b/lib/latch.c @@ -75,9 +75,13 @@ latch_is_set(const struct latch *latch) return pfd.revents & POLLIN; } -/* Causes the next poll_block() to wake up when 'latch' is set. */ +/* Causes the next poll_block() to wake up when 'latch' is set. + * + * ('where' is used in debug logging. Commonly one would use latch_wait() to + * automatically provide the caller's source file and line number for + * 'where'.) */ void -(latch_wait)(const struct latch *latch, const char *where) +latch_wait_at(const struct latch *latch, const char *where) { - (poll_fd_wait)(latch->fds[0], POLLIN, where); + poll_fd_wait_at(latch->fds[0], POLLIN, where); } diff --git a/lib/latch.h b/lib/latch.h index 08f45b117..0b6e8a3c9 100644 --- a/lib/latch.h +++ b/lib/latch.h @@ -36,7 +36,7 @@ bool latch_poll(struct latch *); void latch_set(struct latch *); bool latch_is_set(const struct latch *); -void latch_wait(const struct latch *, const char *where); -#define latch_wait(latch) latch_wait(latch, SOURCE_LOCATOR) +void latch_wait_at(const struct latch *, const char *where); +#define latch_wait(latch) latch_wait_at(latch, SOURCE_LOCATOR) #endif /* latch.h */ diff --git a/lib/lockfile.c b/lib/lockfile.c index 50a4e0c07..43e55927e 100644 --- a/lib/lockfile.c +++ b/lib/lockfile.c @@ -53,10 +53,10 @@ struct lockfile { * descriptor for a file on which a process holds a lock drops *all* locks on * that file. That means that we can't afford to open a lockfile more than * once. */ -static struct hmap lock_table = HMAP_INITIALIZER(&lock_table); - -/* Protects 'lock_table'. */ -static pthread_mutex_t lock_table_mutex = PTHREAD_MUTEX_INITIALIZER; +static struct ovs_mutex lock_table_mutex = OVS_MUTEX_INITIALIZER; +static struct hmap lock_table__ = HMAP_INITIALIZER(&lock_table__); +static struct hmap *const lock_table OVS_GUARDED_BY(lock_table_mutex) + = &lock_table__; static void lockfile_unhash(struct lockfile *); static int lockfile_try_lock(const char *name, pid_t *pidp, @@ -110,9 +110,9 @@ lockfile_lock(const char *file, struct lockfile **lockfilep) lock_name = lockfile_name(file); - xpthread_mutex_lock(&lock_table_mutex); + ovs_mutex_lock(&lock_table_mutex); error = lockfile_try_lock(lock_name, &pid, lockfilep); - xpthread_mutex_unlock(&lock_table_mutex); + ovs_mutex_unlock(&lock_table_mutex); if (error) { COVERAGE_INC(lockfile_error); @@ -138,9 +138,9 @@ void lockfile_unlock(struct lockfile *lockfile) { if (lockfile) { - xpthread_mutex_lock(&lock_table_mutex); + ovs_mutex_lock(&lock_table_mutex); lockfile_unhash(lockfile); - xpthread_mutex_unlock(&lock_table_mutex); + ovs_mutex_unlock(&lock_table_mutex); COVERAGE_INC(lockfile_unlock); free(lockfile->name); @@ -156,12 +156,14 @@ lockfile_postfork(void) { struct lockfile *lockfile; - HMAP_FOR_EACH (lockfile, hmap_node, &lock_table) { + ovs_mutex_lock(&lock_table_mutex); + HMAP_FOR_EACH (lockfile, hmap_node, lock_table) { if (lockfile->fd >= 0) { VLOG_WARN("%s: child does not inherit lock", lockfile->name); lockfile_unhash(lockfile); } } + ovs_mutex_unlock(&lock_table_mutex); } static uint32_t @@ -172,12 +174,12 @@ lockfile_hash(dev_t device, ino_t inode) } static struct lockfile * -lockfile_find(dev_t device, ino_t inode) +lockfile_find(dev_t device, ino_t inode) OVS_REQUIRES(&lock_table_mutex) { struct lockfile *lockfile; HMAP_FOR_EACH_WITH_HASH (lockfile, hmap_node, - lockfile_hash(device, inode), &lock_table) { + lockfile_hash(device, inode), lock_table) { if (lockfile->device == device && lockfile->inode == inode) { return lockfile; } @@ -186,17 +188,18 @@ lockfile_find(dev_t device, ino_t inode) } static void -lockfile_unhash(struct lockfile *lockfile) +lockfile_unhash(struct lockfile *lockfile) OVS_REQUIRES(&lock_table_mutex) { if (lockfile->fd >= 0) { close(lockfile->fd); lockfile->fd = -1; - hmap_remove(&lock_table, &lockfile->hmap_node); + hmap_remove(lock_table, &lockfile->hmap_node); } } static struct lockfile * lockfile_register(const char *name, dev_t device, ino_t inode, int fd) + OVS_REQUIRES(&lock_table_mutex) { struct lockfile *lockfile; @@ -211,13 +214,14 @@ lockfile_register(const char *name, dev_t device, ino_t inode, int fd) lockfile->device = device; lockfile->inode = inode; lockfile->fd = fd; - hmap_insert(&lock_table, &lockfile->hmap_node, + hmap_insert(lock_table, &lockfile->hmap_node, lockfile_hash(device, inode)); return lockfile; } static int lockfile_try_lock(const char *name, pid_t *pidp, struct lockfile **lockfilep) + OVS_REQUIRES(&lock_table_mutex) { struct flock l; struct stat s; diff --git a/lib/mac-learning.c b/lib/mac-learning.c index ca0ccb8e1..e2ca02b41 100644 --- a/lib/mac-learning.c +++ b/lib/mac-learning.c @@ -49,8 +49,8 @@ static uint32_t mac_table_hash(const struct mac_learning *ml, const uint8_t mac[ETH_ADDR_LEN], uint16_t vlan) { - unsigned int mac1 = get_unaligned_u32((uint32_t *) mac); - unsigned int mac2 = get_unaligned_u16((uint16_t *) (mac + 4)); + unsigned int mac1 = get_unaligned_u32(ALIGNED_CAST(uint32_t *, mac)); + unsigned int mac2 = get_unaligned_u16(ALIGNED_CAST(uint16_t *, mac + 4)); return hash_3words(mac1, mac2 | (vlan << 16), ml->secret); } diff --git a/lib/meta-flow.c b/lib/meta-flow.c index 8f67b94c6..11fdfaa2f 100644 --- a/lib/meta-flow.c +++ b/lib/meta-flow.c @@ -2117,20 +2117,25 @@ mf_from_ethernet_string(const struct mf_field *mf, const char *s, uint8_t mac[ETH_ADDR_LEN], uint8_t mask[ETH_ADDR_LEN]) { - ovs_assert(mf->n_bytes == ETH_ADDR_LEN); + int n; - switch (sscanf(s, ETH_ADDR_SCAN_FMT"/"ETH_ADDR_SCAN_FMT, - ETH_ADDR_SCAN_ARGS(mac), ETH_ADDR_SCAN_ARGS(mask))){ - case ETH_ADDR_SCAN_COUNT * 2: - return NULL; + ovs_assert(mf->n_bytes == ETH_ADDR_LEN); - case ETH_ADDR_SCAN_COUNT: + n = -1; + if (sscanf(s, ETH_ADDR_SCAN_FMT"%n", ETH_ADDR_SCAN_ARGS(mac), &n) > 0 + && n == strlen(s)) { memset(mask, 0xff, ETH_ADDR_LEN); return NULL; + } - default: - return xasprintf("%s: invalid Ethernet address", s); + n = -1; + if (sscanf(s, ETH_ADDR_SCAN_FMT"/"ETH_ADDR_SCAN_FMT"%n", + ETH_ADDR_SCAN_ARGS(mac), ETH_ADDR_SCAN_ARGS(mask), &n) > 0 + && n == strlen(s)) { + return NULL; } + + return xasprintf("%s: invalid Ethernet address", s); } static char * diff --git a/lib/netdev-dummy.c b/lib/netdev-dummy.c index a940df81c..c4f58b7a2 100644 --- a/lib/netdev-dummy.c +++ b/lib/netdev-dummy.c @@ -241,9 +241,11 @@ static int netdev_dummy_create(const struct netdev_class *class, const char *name, struct netdev **netdevp) { - static unsigned int n = 0xaa550000; + static atomic_uint next_n = ATOMIC_VAR_INIT(0xaa550000); struct netdev_dummy *netdev; + unsigned int n; + atomic_add(&next_n, 1, &n); netdev = xzalloc(sizeof *netdev); netdev_init(&netdev->up, name, class); netdev->hwaddr[0] = 0xaa; @@ -265,8 +267,6 @@ netdev_dummy_create(const struct netdev_class *class, const char *name, shash_add(&dummy_netdevs, name, netdev); - n++; - *netdevp = &netdev->up; return 0; diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c index 05877c1a4..301a7544d 100644 --- a/lib/netdev-linux.c +++ b/lib/netdev-linux.c @@ -148,6 +148,7 @@ struct tc { struct tc_queue { struct hmap_node hmap_node; /* In struct tc's "queues" hmap. */ unsigned int queue_id; /* OpenFlow queue ID. */ + long long int created; /* Time queue was created, in msecs. */ }; /* A particular kind of traffic control. Each implementation generally maps to @@ -1340,11 +1341,13 @@ static int netdev_linux_sys_get_stats(const struct netdev *netdev_, struct netdev_stats *stats) { - static int use_netlink_stats = -1; + static struct ovsthread_once once = OVSTHREAD_ONCE_INITIALIZER; + static int use_netlink_stats; int error; - if (use_netlink_stats < 0) { + if (ovsthread_once_start(&once)) { use_netlink_stats = check_for_working_netlink_stats(); + ovsthread_once_done(&once); } if (use_netlink_stats) { @@ -1996,9 +1999,11 @@ netdev_linux_get_queue_stats(const struct netdev *netdev_, return EOPNOTSUPP; } else { const struct tc_queue *queue = tc_find_queue(netdev_, queue_id); - return (queue - ? netdev->tc->ops->class_get_stats(netdev_, queue, stats) - : ENOENT); + if (!queue) { + return ENOENT; + } + stats->created = queue->created; + return netdev->tc->ops->class_get_stats(netdev_, queue, stats); } } @@ -2805,6 +2810,7 @@ htb_update_queue__(struct netdev *netdev, unsigned int queue_id, hcp = xmalloc(sizeof *hcp); queue = &hcp->tc_queue; queue->queue_id = queue_id; + queue->created = time_msec(); hmap_insert(&htb->tc.queues, &queue->hmap_node, hash); } @@ -3038,6 +3044,7 @@ hfsc_update_queue__(struct netdev *netdev, unsigned int queue_id, hcp = xmalloc(sizeof *hcp); queue = &hcp->tc_queue; queue->queue_id = queue_id; + queue->created = time_msec(); hmap_insert(&hfsc->tc.queues, &queue->hmap_node, hash); } @@ -3777,30 +3784,35 @@ read_psched(void) * [5] 2.6.32.21.22 (approx.) from Ubuntu 10.04 on VMware Fusion * [6] 2.6.34 from kernel.org on KVM */ + static struct ovsthread_once once = OVSTHREAD_ONCE_INITIALIZER; static const char fn[] = "/proc/net/psched"; unsigned int a, b, c, d; FILE *stream; + if (!ovsthread_once_start(&once)) { + return; + } + ticks_per_s = 1.0; buffer_hz = 100; stream = fopen(fn, "r"); if (!stream) { VLOG_WARN("%s: open failed: %s", fn, ovs_strerror(errno)); - return; + goto exit; } if (fscanf(stream, "%x %x %x %x", &a, &b, &c, &d) != 4) { VLOG_WARN("%s: read failed", fn); fclose(stream); - return; + goto exit; } VLOG_DBG("%s: psched parameters are: %u %u %u %u", fn, a, b, c, d); fclose(stream); if (!a || !c) { VLOG_WARN("%s: invalid scheduler parameters", fn); - return; + goto exit; } ticks_per_s = (double) a * c / b; @@ -3811,6 +3823,9 @@ read_psched(void) fn, a, b, c, d); } VLOG_DBG("%s: ticks_per_s=%f buffer_hz=%u", fn, ticks_per_s, buffer_hz); + +exit: + ovsthread_once_done(&once); } /* Returns the number of bytes that can be transmitted in 'ticks' ticks at a @@ -3818,9 +3833,7 @@ read_psched(void) static unsigned int tc_ticks_to_bytes(unsigned int rate, unsigned int ticks) { - if (!buffer_hz) { - read_psched(); - } + read_psched(); return (rate * ticks) / ticks_per_s; } @@ -3829,9 +3842,7 @@ tc_ticks_to_bytes(unsigned int rate, unsigned int ticks) static unsigned int tc_bytes_to_ticks(unsigned int rate, unsigned int size) { - if (!buffer_hz) { - read_psched(); - } + read_psched(); return rate ? ((unsigned long long int) ticks_per_s * size) / rate : 0; } @@ -3840,9 +3851,7 @@ tc_bytes_to_ticks(unsigned int rate, unsigned int size) static unsigned int tc_buffer_per_jiffy(unsigned int rate) { - if (!buffer_hz) { - read_psched(); - } + read_psched(); return rate / buffer_hz; } @@ -4546,7 +4555,8 @@ netdev_linux_get_ipv4(const struct netdev *netdev, struct in_addr *ip, ifr.ifr_addr.sa_family = AF_INET; error = netdev_linux_do_ioctl(netdev_get_name(netdev), &ifr, cmd, cmd_name); if (!error) { - const struct sockaddr_in *sin = (struct sockaddr_in *) &ifr.ifr_addr; + const struct sockaddr_in *sin = ALIGNED_CAST(struct sockaddr_in *, + &ifr.ifr_addr); *ip = sin->sin_addr; } return error; @@ -4556,9 +4566,10 @@ netdev_linux_get_ipv4(const struct netdev *netdev, struct in_addr *ip, static int af_packet_sock(void) { - static int sock = INT_MIN; + static struct ovsthread_once once = OVSTHREAD_ONCE_INITIALIZER; + static int sock; - if (sock == INT_MIN) { + if (ovsthread_once_start(&once)) { sock = socket(AF_PACKET, SOCK_RAW, 0); if (sock >= 0) { int error = set_nonblocking(sock); @@ -4571,6 +4582,7 @@ af_packet_sock(void) VLOG_ERR("failed to create packet socket: %s", ovs_strerror(errno)); } + ovsthread_once_done(&once); } return sock; diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c index 885bf5e3a..4214b383d 100644 --- a/lib/netdev-vport.c +++ b/lib/netdev-vport.c @@ -413,13 +413,17 @@ set_tunnel_config(struct netdev *dev_, const struct smap *args) } if (tnl_cfg.ipsec) { + static pthread_mutex_t mutex = PTHREAD_MUTEX_INITIALIZER; static pid_t pid = 0; + + pthread_mutex_lock(&mutex); if (pid <= 0) { char *file_name = xasprintf("%s/%s", ovs_rundir(), "ovs-monitor-ipsec.pid"); pid = read_pidfile(file_name); free(file_name); } + pthread_mutex_unlock(&mutex); if (pid < 0) { VLOG_ERR("%s: IPsec requires the ovs-monitor-ipsec daemon", diff --git a/lib/netdev.c b/lib/netdev.c index 76c87288a..a8bbedd71 100644 --- a/lib/netdev.c +++ b/lib/netdev.c @@ -1248,7 +1248,8 @@ netdev_delete_queue(struct netdev *netdev, unsigned int queue_id) /* Obtains statistics about 'queue_id' on 'netdev'. On success, returns 0 and * fills 'stats' with the queue's statistics; individual members of 'stats' may * be set to all-1-bits if the statistic is unavailable. On failure, returns a - * positive errno value and fills 'stats' with all-1-bits. */ + * positive errno value and fills 'stats' with values indicating unsupported + * statistics. */ int netdev_get_queue_stats(const struct netdev *netdev, unsigned int queue_id, struct netdev_queue_stats *stats) @@ -1260,7 +1261,10 @@ netdev_get_queue_stats(const struct netdev *netdev, unsigned int queue_id, ? class->get_queue_stats(netdev, queue_id, stats) : EOPNOTSUPP); if (retval) { - memset(stats, 0xff, sizeof *stats); + stats->tx_bytes = UINT64_MAX; + stats->tx_packets = UINT64_MAX; + stats->tx_errors = UINT64_MAX; + stats->created = LLONG_MIN; } return retval; } diff --git a/lib/netdev.h b/lib/netdev.h index b1cc31940..eb1870b4e 100644 --- a/lib/netdev.h +++ b/lib/netdev.h @@ -227,6 +227,9 @@ struct netdev_queue_stats { uint64_t tx_bytes; uint64_t tx_packets; uint64_t tx_errors; + + /* Time at which the queue was created, in msecs, LLONG_MIN if unknown. */ + long long int created; }; int netdev_set_policing(struct netdev *, uint32_t kbits_rate, diff --git a/lib/netlink-socket.c b/lib/netlink-socket.c index da32284b1..99bd4cc05 100644 --- a/lib/netlink-socket.c +++ b/lib/netlink-socket.c @@ -1012,8 +1012,8 @@ struct nl_pool { int n; }; -static struct nl_pool pools[MAX_LINKS]; -static pthread_mutex_t pool_mutex = PTHREAD_ADAPTIVE_MUTEX_INITIALIZER; +static struct ovs_mutex pool_mutex = OVS_ADAPTIVE_MUTEX_INITIALIZER; +static struct nl_pool pools[MAX_LINKS] OVS_GUARDED_BY(pool_mutex); static int nl_pool_alloc(int protocol, struct nl_sock **sockp) @@ -1023,12 +1023,12 @@ nl_pool_alloc(int protocol, struct nl_sock **sockp) ovs_assert(protocol >= 0 && protocol < ARRAY_SIZE(pools)); - xpthread_mutex_lock(&pool_mutex); + ovs_mutex_lock(&pool_mutex); pool = &pools[protocol]; if (pool->n > 0) { sock = pool->socks[--pool->n]; } - xpthread_mutex_unlock(&pool_mutex); + ovs_mutex_unlock(&pool_mutex); if (sock) { *sockp = sock; @@ -1044,12 +1044,12 @@ nl_pool_release(struct nl_sock *sock) if (sock) { struct nl_pool *pool = &pools[sock->protocol]; - xpthread_mutex_lock(&pool_mutex); + ovs_mutex_lock(&pool_mutex); if (pool->n < ARRAY_SIZE(pool->socks)) { pool->socks[pool->n++] = sock; sock = NULL; } - xpthread_mutex_unlock(&pool_mutex); + ovs_mutex_unlock(&pool_mutex); nl_sock_destroy(sock); } diff --git a/lib/netlink.c b/lib/netlink.c index a6867847b..50444abd9 100644 --- a/lib/netlink.c +++ b/lib/netlink.c @@ -711,8 +711,7 @@ nl_policy_parse(const struct ofpbuf *msg, size_t nla_offset, return false; } - NL_ATTR_FOR_EACH (nla, left, - (struct nlattr *) ((char *) msg->data + nla_offset), + NL_ATTR_FOR_EACH (nla, left, ofpbuf_at(msg, nla_offset, 0), msg->size - nla_offset) { uint16_t type = nl_attr_type(nla); @@ -777,8 +776,7 @@ nl_attr_find__(const struct nlattr *attrs, size_t size, uint16_t type) const struct nlattr * nl_attr_find(const struct ofpbuf *buf, size_t hdr_len, uint16_t type) { - const uint8_t *start = (const uint8_t *) buf->data + hdr_len; - return nl_attr_find__((const struct nlattr *) start, buf->size - hdr_len, + return nl_attr_find__(ofpbuf_at(buf, hdr_len, 0), buf->size - hdr_len, type); } diff --git a/lib/nx-match.c b/lib/nx-match.c index 3a6d7cc2d..bdb3a2ba7 100644 --- a/lib/nx-match.c +++ b/lib/nx-match.c @@ -744,7 +744,7 @@ oxm_put_match(struct ofpbuf *b, const struct match *match) match_len = nx_put_raw(b, true, match, cookie, cookie_mask) + sizeof *omh; ofpbuf_put_zeros(b, ROUND_UP(match_len, 8) - match_len); - omh = (struct ofp11_match_header *)((char *)b->data + start_len); + omh = ofpbuf_at(b, start_len, sizeof *omh); omh->type = htons(OFPMT_OXM); omh->length = htons(match_len); @@ -807,9 +807,9 @@ nx_match_to_string(const uint8_t *p, unsigned int match_len) } char * -oxm_match_to_string(const uint8_t *p, unsigned int match_len) +oxm_match_to_string(const struct ofpbuf *p, unsigned int match_len) { - const struct ofp11_match_header *omh = (struct ofp11_match_header *)p; + const struct ofp11_match_header *omh = p->data; uint16_t match_len_; struct ds s; @@ -837,7 +837,8 @@ oxm_match_to_string(const uint8_t *p, unsigned int match_len) goto err; } - return nx_match_to_string(p + sizeof *omh, match_len - sizeof *omh); + return nx_match_to_string(ofpbuf_at(p, sizeof *omh, 0), + match_len - sizeof *omh); err: return ds_steal_cstr(&s); @@ -997,7 +998,7 @@ oxm_match_from_string(const char *s, struct ofpbuf *b) match_len = nx_match_from_string_raw(s, b) + sizeof *omh; ofpbuf_put_zeros(b, ROUND_UP(match_len, 8) - match_len); - omh = (struct ofp11_match_header *)((char *)b->data + start_len); + omh = ofpbuf_at(b, start_len, sizeof *omh); omh->type = htons(OFPMT_OXM); omh->length = htons(match_len); diff --git a/lib/nx-match.h b/lib/nx-match.h index b03688b28..a6b7c5233 100644 --- a/lib/nx-match.h +++ b/lib/nx-match.h @@ -54,7 +54,7 @@ int nx_put_match(struct ofpbuf *, const struct match *, int oxm_put_match(struct ofpbuf *, const struct match *); char *nx_match_to_string(const uint8_t *, unsigned int match_len); -char *oxm_match_to_string(const uint8_t *, unsigned int match_len); +char *oxm_match_to_string(const struct ofpbuf *, unsigned int match_len); int nx_match_from_string(const char *, struct ofpbuf *); int oxm_match_from_string(const char *, struct ofpbuf *); diff --git a/lib/odp-util.c b/lib/odp-util.c index 5a322217c..3c3063df7 100644 --- a/lib/odp-util.c +++ b/lib/odp-util.c @@ -2512,7 +2512,8 @@ uint32_t odp_flow_key_hash(const struct nlattr *key, size_t key_len) { BUILD_ASSERT_DECL(!(NLA_ALIGNTO % sizeof(uint32_t))); - return hash_words((const uint32_t *) key, key_len / sizeof(uint32_t), 0); + return hash_words(ALIGNED_CAST(const uint32_t *, key), + key_len / sizeof(uint32_t), 0); } static void diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c index 899928a72..61e285429 100644 --- a/lib/ofp-actions.c +++ b/lib/ofp-actions.c @@ -336,7 +336,7 @@ ofpact_from_nxast(const union ofp_action *a, enum ofputil_action_code code, break; case OFPUTIL_NXAST_WRITE_METADATA: - nawm = (const struct nx_action_write_metadata *) a; + nawm = ALIGNED_CAST(const struct nx_action_write_metadata *, a); error = metadata_from_nxast(nawm, out); break; @@ -356,7 +356,7 @@ ofpact_from_nxast(const union ofp_action *a, enum ofputil_action_code code, case OFPUTIL_NXAST_REG_LOAD: error = nxm_reg_load_from_openflow( - (const struct nx_action_reg_load *) a, out); + ALIGNED_CAST(const struct nx_action_reg_load *, a), out); break; case OFPUTIL_NXAST_STACK_PUSH: @@ -375,7 +375,7 @@ ofpact_from_nxast(const union ofp_action *a, enum ofputil_action_code code, break; case OFPUTIL_NXAST_SET_TUNNEL64: - nast64 = (const struct nx_action_set_tunnel64 *) a; + nast64 = ALIGNED_CAST(const struct nx_action_set_tunnel64 *, a); tunnel = ofpact_put_SET_TUNNEL(out); tunnel->ofpact.compat = code; tunnel->tun_id = ntohll(nast64->tun_id); @@ -402,7 +402,8 @@ ofpact_from_nxast(const union ofp_action *a, enum ofputil_action_code code, break; case OFPUTIL_NXAST_LEARN: - error = learn_from_openflow((const struct nx_action_learn *) a, out); + error = learn_from_openflow( + ALIGNED_CAST(const struct nx_action_learn *, a), out); break; case OFPUTIL_NXAST_EXIT: @@ -881,7 +882,7 @@ ofpacts_from_openflow11(const union ofp_action *in, size_t n_in, instruction_get_##ENUM(const struct ofp11_instruction *inst)\ { \ ovs_assert(inst->type == htons(ENUM)); \ - return (struct STRUCT *)inst; \ + return ALIGNED_CAST(struct STRUCT *, inst); \ } \ \ static inline void \ @@ -1070,10 +1071,10 @@ decode_openflow11_instructions(const struct ofp11_instruction insts[], static void get_actions_from_instruction(const struct ofp11_instruction *inst, - const union ofp_action **actions, - size_t *n_actions) + const union ofp_action **actions, + size_t *n_actions) { - *actions = (const union ofp_action *) (inst + 1); + *actions = ALIGNED_CAST(const union ofp_action *, inst + 1); *n_actions = (ntohs(inst->len) - sizeof *inst) / OFP11_INSTRUCTION_ALIGN; } @@ -1140,8 +1141,8 @@ ofpacts_pull_openflow11_instructions(struct ofpbuf *openflow, const struct ofp13_instruction_meter *oim; struct ofpact_meter *om; - oim = (const struct ofp13_instruction_meter *) - insts[OVSINST_OFPIT13_METER]; + oim = ALIGNED_CAST(const struct ofp13_instruction_meter *, + insts[OVSINST_OFPIT13_METER]); om = ofpact_put_METER(ofpacts); om->meter_id = ntohl(oim->meter_id); @@ -1167,8 +1168,8 @@ ofpacts_pull_openflow11_instructions(struct ofpbuf *openflow, const struct ofp11_instruction_write_metadata *oiwm; struct ofpact_metadata *om; - oiwm = (const struct ofp11_instruction_write_metadata *) - insts[OVSINST_OFPIT11_WRITE_METADATA]; + oiwm = ALIGNED_CAST(const struct ofp11_instruction_write_metadata *, + insts[OVSINST_OFPIT11_WRITE_METADATA]); om = ofpact_put_WRITE_METADATA(ofpacts); om->metadata = oiwm->metadata; @@ -1436,7 +1437,7 @@ ofpact_note_to_nxast(const struct ofpact_note *note, struct ofpbuf *out) if (remainder) { ofpbuf_put_zeros(out, OFP_ACTION_ALIGN - remainder); } - nan = (struct nx_action_note *)((char *)out->data + start_ofs); + nan = ofpbuf_at(out, start_ofs, sizeof *nan); nan->len = htons(out->size - start_ofs); } diff --git a/lib/ofp-actions.h b/lib/ofp-actions.h index b97afd0b4..ca33ca897 100644 --- a/lib/ofp-actions.h +++ b/lib/ofp-actions.h @@ -582,7 +582,7 @@ void *ofpact_put(struct ofpbuf *, enum ofpact_type, size_t len); ofpact_get_##ENUM(const struct ofpact *ofpact) \ { \ ovs_assert(ofpact->type == OFPACT_##ENUM); \ - return (struct STRUCT *) ofpact; \ + return ALIGNED_CAST(struct STRUCT *, ofpact); \ } \ \ static inline struct STRUCT * \ diff --git a/lib/ofp-msgs.c b/lib/ofp-msgs.c index 5e043d245..d136f73d4 100644 --- a/lib/ofp-msgs.c +++ b/lib/ofp-msgs.c @@ -110,8 +110,11 @@ static enum ofperr ofpraw_from_ofphdrs(enum ofpraw *, const struct ofphdrs *); static ovs_be32 alloc_xid(void) { - static uint32_t next_xid = 1; - return htonl(next_xid++); + static atomic_uint32_t next_xid = ATOMIC_VAR_INIT(1); + uint32_t xid; + + atomic_add(&next_xid, 1, &xid); + return htonl(xid); } static uint32_t diff --git a/lib/ofp-parse.c b/lib/ofp-parse.c index 618290b7a..5cb39f5a6 100644 --- a/lib/ofp-parse.c +++ b/lib/ofp-parse.c @@ -34,6 +34,7 @@ #include "ofp-util.h" #include "ofpbuf.h" #include "openflow/openflow.h" +#include "ovs-thread.h" #include "packets.h" #include "socket-util.h" #include "vconn.h" @@ -1519,12 +1520,12 @@ static char * WARN_UNUSED_RESULT parse_flow_monitor_request__(struct ofputil_flow_monitor_request *fmr, const char *str_, char *string) { - static uint32_t id; - + static atomic_uint32_t id = ATOMIC_VAR_INIT(0); char *save_ptr = NULL; char *name; - fmr->id = id++; + atomic_add(&id, 1, &fmr->id); + fmr->flags = (NXFMF_INITIAL | NXFMF_ADD | NXFMF_DELETE | NXFMF_MODIFY | NXFMF_OWN | NXFMF_ACTIONS); fmr->out_port = OFPP_NONE; diff --git a/lib/ofp-print.c b/lib/ofp-print.c index 1d8b98cca..1a4dd9ce8 100644 --- a/lib/ofp-print.c +++ b/lib/ofp-print.c @@ -1737,9 +1737,17 @@ ofp_print_ofpst_queue_reply(struct ds *string, const struct ofp_header *oh, ofp_print_queue_name(string, qs.queue_id); ds_put_cstr(string, ": "); - print_port_stat(string, "bytes=", qs.stats.tx_bytes, 1); - print_port_stat(string, "pkts=", qs.stats.tx_packets, 1); - print_port_stat(string, "errors=", qs.stats.tx_errors, 0); + print_port_stat(string, "bytes=", qs.tx_bytes, 1); + print_port_stat(string, "pkts=", qs.tx_packets, 1); + print_port_stat(string, "errors=", qs.tx_errors, 1); + + ds_put_cstr(string, "duration="); + if (qs.duration_sec != UINT32_MAX) { + ofp_print_duration(string, qs.duration_sec, qs.duration_nsec); + } else { + ds_put_char(string, '?'); + } + ds_put_char(string, '\n'); } } diff --git a/lib/ofp-util.c b/lib/ofp-util.c index bc85797a5..d1bcf9c7d 100644 --- a/lib/ofp-util.c +++ b/lib/ofp-util.c @@ -1293,7 +1293,7 @@ ofputil_decode_hello_bitmap(const struct ofp_hello_elem_header *oheh, uint32_t *allowed_versionsp) { uint16_t bitmap_len = ntohs(oheh->length) - sizeof *oheh; - const ovs_be32 *bitmap = (const ovs_be32 *) (oheh + 1); + const ovs_be32 *bitmap = ALIGNED_CAST(const ovs_be32 *, oheh + 1); uint32_t allowed_versions; if (!bitmap_len || bitmap_len % sizeof *bitmap) { @@ -1397,7 +1397,7 @@ ofputil_encode_hello(uint32_t allowed_versions) oheh = ofpbuf_put_zeros(msg, ROUND_UP(map_len + sizeof *oheh, 8)); oheh->type = htons(OFPHET_VERSIONBITMAP); oheh->length = htons(map_len + sizeof *oheh); - *(ovs_be32 *)(oheh + 1) = htonl(allowed_versions); + *ALIGNED_CAST(ovs_be32 *, oheh + 1) = htonl(allowed_versions); ofpmsg_update_length(msg); } @@ -1750,7 +1750,8 @@ ofputil_pull_bands(struct ofpbuf *msg, size_t len, uint16_t *n_bands, ((struct ofp13_meter_band_dscp_remark *)ombh)->prec_level : 0; n++; len -= ombh_len; - ombh = (struct ofp13_meter_band_header *)(((char *)ombh) + ombh_len); + ombh = ALIGNED_CAST(struct ofp13_meter_band_header *, + (char *) ombh + ombh_len); } if (len) { return OFPERR_OFPBRC_BAD_LEN; @@ -5183,6 +5184,21 @@ ofputil_port_stats_from_ofp13(struct ofputil_port_stats *ops, return error; } +static size_t +ofputil_get_port_stats_size(enum ofp_version ofp_version) +{ + switch (ofp_version) { + case OFP10_VERSION: + return sizeof(struct ofp10_port_stats); + case OFP11_VERSION: + case OFP12_VERSION: + return sizeof(struct ofp11_port_stats); + case OFP13_VERSION: + return sizeof(struct ofp13_port_stats); + default: + NOT_REACHED(); + } +} /* Returns the number of port stats elements in OFPTYPE_PORT_STATS_REPLY * message 'oh'. */ @@ -5194,9 +5210,7 @@ ofputil_count_port_stats(const struct ofp_header *oh) ofpbuf_use_const(&b, oh, ntohs(oh->length)); ofpraw_pull_assert(&b); - BUILD_ASSERT(sizeof(struct ofp10_port_stats) == - sizeof(struct ofp11_port_stats)); - return b.size / sizeof(struct ofp10_port_stats); + return b.size / ofputil_get_port_stats_size(oh->version); } /* Converts an OFPST_PORT_STATS reply in 'msg' into an abstract @@ -5352,6 +5366,22 @@ ofputil_encode_queue_stats_request(enum ofp_version ofp_version, return request; } +static size_t +ofputil_get_queue_stats_size(enum ofp_version ofp_version) +{ + switch (ofp_version) { + case OFP10_VERSION: + return sizeof(struct ofp10_queue_stats); + case OFP11_VERSION: + case OFP12_VERSION: + return sizeof(struct ofp11_queue_stats); + case OFP13_VERSION: + return sizeof(struct ofp13_queue_stats); + default: + NOT_REACHED(); + } +} + /* Returns the number of queue stats elements in OFPTYPE_QUEUE_STATS_REPLY * message 'oh'. */ size_t @@ -5362,9 +5392,7 @@ ofputil_count_queue_stats(const struct ofp_header *oh) ofpbuf_use_const(&b, oh, ntohs(oh->length)); ofpraw_pull_assert(&b); - BUILD_ASSERT(sizeof(struct ofp10_queue_stats) == - sizeof(struct ofp11_queue_stats)); - return b.size / sizeof(struct ofp10_queue_stats); + return b.size / ofputil_get_queue_stats_size(oh->version); } static enum ofperr @@ -5373,9 +5401,10 @@ ofputil_queue_stats_from_ofp10(struct ofputil_queue_stats *oqs, { oqs->port_no = u16_to_ofp(ntohs(qs10->port_no)); oqs->queue_id = ntohl(qs10->queue_id); - oqs->stats.tx_bytes = ntohll(get_32aligned_be64(&qs10->tx_bytes)); - oqs->stats.tx_packets = ntohll(get_32aligned_be64(&qs10->tx_packets)); - oqs->stats.tx_errors = ntohll(get_32aligned_be64(&qs10->tx_errors)); + oqs->tx_bytes = ntohll(get_32aligned_be64(&qs10->tx_bytes)); + oqs->tx_packets = ntohll(get_32aligned_be64(&qs10->tx_packets)); + oqs->tx_errors = ntohll(get_32aligned_be64(&qs10->tx_errors)); + oqs->duration_sec = oqs->duration_nsec = UINT32_MAX; return 0; } @@ -5392,9 +5421,10 @@ ofputil_queue_stats_from_ofp11(struct ofputil_queue_stats *oqs, } oqs->queue_id = ntohl(qs11->queue_id); - oqs->stats.tx_bytes = ntohll(qs11->tx_bytes); - oqs->stats.tx_packets = ntohll(qs11->tx_packets); - oqs->stats.tx_errors = ntohll(qs11->tx_errors); + oqs->tx_bytes = ntohll(qs11->tx_bytes); + oqs->tx_packets = ntohll(qs11->tx_packets); + oqs->tx_errors = ntohll(qs11->tx_errors); + oqs->duration_sec = oqs->duration_nsec = UINT32_MAX; return 0; } @@ -5403,11 +5433,10 @@ static enum ofperr ofputil_queue_stats_from_ofp13(struct ofputil_queue_stats *oqs, const struct ofp13_queue_stats *qs13) { - enum ofperr error - = ofputil_queue_stats_from_ofp11(oqs, &qs13->qs); + enum ofperr error = ofputil_queue_stats_from_ofp11(oqs, &qs13->qs); if (!error) { - /* FIXME: Get qs13->duration_sec and qs13->duration_nsec, - * Add to netdev_queue_stats? */ + oqs->duration_sec = ntohl(qs13->duration_sec); + oqs->duration_nsec = ntohl(qs13->duration_nsec); } return error; @@ -5479,9 +5508,9 @@ ofputil_queue_stats_to_ofp10(const struct ofputil_queue_stats *oqs, qs10->port_no = htons(ofp_to_u16(oqs->port_no)); memset(qs10->pad, 0, sizeof qs10->pad); qs10->queue_id = htonl(oqs->queue_id); - put_32aligned_be64(&qs10->tx_bytes, htonll(oqs->stats.tx_bytes)); - put_32aligned_be64(&qs10->tx_packets, htonll(oqs->stats.tx_packets)); - put_32aligned_be64(&qs10->tx_errors, htonll(oqs->stats.tx_errors)); + put_32aligned_be64(&qs10->tx_bytes, htonll(oqs->tx_bytes)); + put_32aligned_be64(&qs10->tx_packets, htonll(oqs->tx_packets)); + put_32aligned_be64(&qs10->tx_errors, htonll(oqs->tx_errors)); } static void @@ -5490,9 +5519,9 @@ ofputil_queue_stats_to_ofp11(const struct ofputil_queue_stats *oqs, { qs11->port_no = ofputil_port_to_ofp11(oqs->port_no); qs11->queue_id = htonl(oqs->queue_id); - qs11->tx_bytes = htonll(oqs->stats.tx_bytes); - qs11->tx_packets = htonll(oqs->stats.tx_packets); - qs11->tx_errors = htonll(oqs->stats.tx_errors); + qs11->tx_bytes = htonll(oqs->tx_bytes); + qs11->tx_packets = htonll(oqs->tx_packets); + qs11->tx_errors = htonll(oqs->tx_errors); } static void @@ -5500,10 +5529,13 @@ ofputil_queue_stats_to_ofp13(const struct ofputil_queue_stats *oqs, struct ofp13_queue_stats *qs13) { ofputil_queue_stats_to_ofp11(oqs, &qs13->qs); - /* OF 1.3 adds duration fields */ - /* FIXME: Need to implement queue alive duration (sec + nsec) */ - qs13->duration_sec = htonl(~0); - qs13->duration_nsec = htonl(~0); + if (oqs->duration_sec != UINT32_MAX) { + qs13->duration_sec = htonl(oqs->duration_sec); + qs13->duration_nsec = htonl(oqs->duration_nsec); + } else { + qs13->duration_sec = htonl(UINT32_MAX); + qs13->duration_nsec = htonl(UINT32_MAX); + } } /* Encode a queue stat for 'oqs' and append it to 'replies'. */ diff --git a/lib/ofp-util.h b/lib/ofp-util.h index 0385a578f..f94982ddc 100644 --- a/lib/ofp-util.h +++ b/lib/ofp-util.h @@ -832,7 +832,15 @@ ofputil_encode_queue_stats_request(enum ofp_version ofp_version, struct ofputil_queue_stats { ofp_port_t port_no; uint32_t queue_id; - struct netdev_queue_stats stats; + + /* Values of unsupported statistics are set to all-1-bits (UINT64_MAX). */ + uint64_t tx_bytes; + uint64_t tx_packets; + uint64_t tx_errors; + + /* UINT32_MAX if unknown. */ + uint32_t duration_sec; + uint32_t duration_nsec; }; size_t ofputil_count_queue_stats(const struct ofp_header *); diff --git a/lib/ovs-atomic-gcc4+.c b/lib/ovs-atomic-gcc4+.c index aeff84579..169384873 100644 --- a/lib/ovs-atomic-gcc4+.c +++ b/lib/ovs-atomic-gcc4+.c @@ -20,7 +20,7 @@ #include "ovs-thread.h" #if OVS_ATOMIC_GCC4P_IMPL -static pthread_mutex_t mutex = PTHREAD_ADAPTIVE_MUTEX_INITIALIZER; +static struct ovs_mutex mutex = OVS_ADAPTIVE_MUTEX_INITIALIZER; #define DEFINE_LOCKED_OP(TYPE, NAME, OPERATOR) \ TYPE##_t \ @@ -28,10 +28,10 @@ static pthread_mutex_t mutex = PTHREAD_ADAPTIVE_MUTEX_INITIALIZER; { \ TYPE##_t old_value; \ \ - xpthread_mutex_lock(&mutex); \ + ovs_mutex_lock(&mutex); \ old_value = u->value; \ u->value OPERATOR arg; \ - xpthread_mutex_unlock(&mutex); \ + ovs_mutex_unlock(&mutex); \ \ return old_value; \ } @@ -42,9 +42,9 @@ static pthread_mutex_t mutex = PTHREAD_ADAPTIVE_MUTEX_INITIALIZER; { \ TYPE##_t value; \ \ - xpthread_mutex_lock(&mutex); \ + ovs_mutex_lock(&mutex); \ value = u->value; \ - xpthread_mutex_unlock(&mutex); \ + ovs_mutex_unlock(&mutex); \ \ return value; \ } \ @@ -52,9 +52,9 @@ static pthread_mutex_t mutex = PTHREAD_ADAPTIVE_MUTEX_INITIALIZER; void \ locked_##TYPE##_store(struct locked_##TYPE *u, TYPE##_t value) \ { \ - xpthread_mutex_lock(&mutex); \ + ovs_mutex_lock(&mutex); \ u->value = value; \ - xpthread_mutex_unlock(&mutex); \ + ovs_mutex_unlock(&mutex); \ } \ DEFINE_LOCKED_OP(TYPE, add, +=); \ DEFINE_LOCKED_OP(TYPE, sub, -=); \ diff --git a/lib/ovs-atomic-gcc4+.h b/lib/ovs-atomic-gcc4+.h index b8649ede9..4476162b3 100644 --- a/lib/ovs-atomic-gcc4+.h +++ b/lib/ovs-atomic-gcc4+.h @@ -117,7 +117,7 @@ typedef enum { __builtin_choose_expr( \ __builtin_types_compatible_p(typeof(OBJECT), struct locked_uint64), \ (THEN), (ELSE)) -#define AS_LOCKED_UINT64(OBJECT) ((struct locked_uint64 *) (OBJECT)) +#define AS_LOCKED_UINT64(OBJECT) ((struct locked_uint64 *) (void *) (OBJECT)) #define AS_UINT64(OBJECT) ((uint64_t *) (OBJECT)) struct locked_uint64 { uint64_t value; @@ -135,7 +135,7 @@ uint64_t locked_uint64_and(struct locked_uint64 *, uint64_t arg); __builtin_choose_expr( \ __builtin_types_compatible_p(typeof(OBJECT), struct locked_int64), \ (THEN), (ELSE)) -#define AS_LOCKED_INT64(OBJECT) ((struct locked_int64 *) (OBJECT)) +#define AS_LOCKED_INT64(OBJECT) ((struct locked_int64 *) (void *) (OBJECT)) #define AS_INT64(OBJECT) ((int64_t *) (OBJECT)) struct locked_int64 { int64_t value; diff --git a/lib/ovs-atomic-pthreads.c b/lib/ovs-atomic-pthreads.c index a501b8256..7e7ef0508 100644 --- a/lib/ovs-atomic-pthreads.c +++ b/lib/ovs-atomic-pthreads.c @@ -26,10 +26,10 @@ atomic_flag_test_and_set(volatile atomic_flag *flag_) atomic_flag *flag = CONST_CAST(atomic_flag *, flag_); bool old_value; - xpthread_mutex_lock(&flag->mutex); + ovs_mutex_lock(&flag->mutex); old_value = flag->b; flag->b = true; - xpthread_mutex_unlock(&flag->mutex); + ovs_mutex_unlock(&flag->mutex); return old_value; } @@ -46,9 +46,9 @@ atomic_flag_clear(volatile atomic_flag *flag_) { atomic_flag *flag = CONST_CAST(atomic_flag *, flag_); - xpthread_mutex_lock(&flag->mutex); + ovs_mutex_lock(&flag->mutex); flag->b = false; - xpthread_mutex_unlock(&flag->mutex); + ovs_mutex_unlock(&flag->mutex); } void diff --git a/lib/ovs-atomic-pthreads.h b/lib/ovs-atomic-pthreads.h index 2f47a9c97..61a9771ac 100644 --- a/lib/ovs-atomic-pthreads.h +++ b/lib/ovs-atomic-pthreads.h @@ -144,7 +144,7 @@ atomic_signal_fence(memory_order order OVS_UNUSED) typedef struct { bool b; - pthread_mutex_t mutex; + struct ovs_mutex mutex; } atomic_flag; #define ATOMIC_FLAG_INIT { false, PTHREAD_MUTEX_INITIALIZER } diff --git a/lib/ovs-atomic.h b/lib/ovs-atomic.h index a0a34f30f..3fc9dcb8f 100644 --- a/lib/ovs-atomic.h +++ b/lib/ovs-atomic.h @@ -240,7 +240,7 @@ #include "ovs-atomic-c11.h" #elif __GNUC__ >= 4 && __GNUC_MINOR__ >= 7 #include "ovs-atomic-gcc4.7+.h" - #elif __GNUC__ >= 4 + #elif HAVE_GCC4_ATOMICS #include "ovs-atomic-gcc4+.h" #else #include "ovs-atomic-pthreads.h" diff --git a/lib/ovs-thread.c b/lib/ovs-thread.c index d08751c02..c8b2c1599 100644 --- a/lib/ovs-thread.c +++ b/lib/ovs-thread.c @@ -43,6 +43,58 @@ static const char *must_not_fork; /* True if we created any threads beyond the main initial thread. */ static bool multithreaded; +#define LOCK_FUNCTION(TYPE, FUN) \ + void \ + ovs_##TYPE##_##FUN##_at(const struct ovs_##TYPE *l_, \ + const char *where) \ + { \ + struct ovs_##TYPE *l = CONST_CAST(struct ovs_##TYPE *, l_); \ + int error = pthread_##TYPE##_##FUN(&l->lock); \ + if (OVS_UNLIKELY(error)) { \ + ovs_abort(error, "pthread_%s_%s failed", #TYPE, #FUN); \ + } \ + l->where = where; \ + } +LOCK_FUNCTION(mutex, lock); +LOCK_FUNCTION(rwlock, rdlock); +LOCK_FUNCTION(rwlock, wrlock); + +#define TRY_LOCK_FUNCTION(TYPE, FUN) \ + int \ + ovs_##TYPE##_##FUN##_at(const struct ovs_##TYPE *l_, \ + const char *where) \ + { \ + struct ovs_##TYPE *l = CONST_CAST(struct ovs_##TYPE *, l_); \ + int error = pthread_##TYPE##_##FUN(&l->lock); \ + if (OVS_UNLIKELY(error) && error != EBUSY) { \ + ovs_abort(error, "pthread_%s_%s failed", #TYPE, #FUN); \ + } \ + if (!error) { \ + l->where = where; \ + } \ + return error; \ + } +TRY_LOCK_FUNCTION(mutex, trylock); +TRY_LOCK_FUNCTION(rwlock, tryrdlock); +TRY_LOCK_FUNCTION(rwlock, trywrlock); + +#define UNLOCK_FUNCTION(TYPE, FUN) \ + void \ + ovs_##TYPE##_##FUN(const struct ovs_##TYPE *l_) \ + { \ + struct ovs_##TYPE *l = CONST_CAST(struct ovs_##TYPE *, l_); \ + int error; \ + l->where = NULL; \ + error = pthread_##TYPE##_##FUN(&l->lock); \ + if (OVS_UNLIKELY(error)) { \ + ovs_abort(error, "pthread_%s_%sfailed", #TYPE, #FUN); \ + } \ + } +UNLOCK_FUNCTION(mutex, unlock); +UNLOCK_FUNCTION(mutex, destroy); +UNLOCK_FUNCTION(rwlock, unlock); +UNLOCK_FUNCTION(rwlock, destroy); + #define XPTHREAD_FUNC1(FUNCTION, PARAM1) \ void \ x##FUNCTION(PARAM1 arg1) \ @@ -52,16 +104,6 @@ static bool multithreaded; ovs_abort(error, "%s failed", #FUNCTION); \ } \ } -#define XPTHREAD_TRY_FUNC1(FUNCTION, PARAM1) \ - int \ - x##FUNCTION(PARAM1 arg1) \ - { \ - int error = FUNCTION(arg1); \ - if (OVS_UNLIKELY(error && error != EBUSY)) { \ - ovs_abort(error, "%s failed", #FUNCTION); \ - } \ - return error; \ - } #define XPTHREAD_FUNC2(FUNCTION, PARAM1, PARAM2) \ void \ x##FUNCTION(PARAM1 arg1, PARAM2 arg2) \ @@ -72,27 +114,59 @@ static bool multithreaded; } \ } -XPTHREAD_FUNC2(pthread_mutex_init, pthread_mutex_t *, pthread_mutexattr_t *); -XPTHREAD_FUNC1(pthread_mutex_lock, pthread_mutex_t *); -XPTHREAD_FUNC1(pthread_mutex_unlock, pthread_mutex_t *); -XPTHREAD_TRY_FUNC1(pthread_mutex_trylock, pthread_mutex_t *); - -XPTHREAD_FUNC2(pthread_rwlock_init, - pthread_rwlock_t *, pthread_rwlockattr_t *); -XPTHREAD_FUNC1(pthread_rwlock_rdlock, pthread_rwlock_t *); -XPTHREAD_FUNC1(pthread_rwlock_wrlock, pthread_rwlock_t *); -XPTHREAD_FUNC1(pthread_rwlock_unlock, pthread_rwlock_t *); -XPTHREAD_TRY_FUNC1(pthread_rwlock_tryrdlock, pthread_rwlock_t *); -XPTHREAD_TRY_FUNC1(pthread_rwlock_trywrlock, pthread_rwlock_t *); +XPTHREAD_FUNC1(pthread_mutexattr_init, pthread_mutexattr_t *); +XPTHREAD_FUNC1(pthread_mutexattr_destroy, pthread_mutexattr_t *); +XPTHREAD_FUNC2(pthread_mutexattr_settype, pthread_mutexattr_t *, int); +XPTHREAD_FUNC2(pthread_mutexattr_gettype, pthread_mutexattr_t *, int *); XPTHREAD_FUNC2(pthread_cond_init, pthread_cond_t *, pthread_condattr_t *); +XPTHREAD_FUNC1(pthread_cond_destroy, pthread_cond_t *); XPTHREAD_FUNC1(pthread_cond_signal, pthread_cond_t *); XPTHREAD_FUNC1(pthread_cond_broadcast, pthread_cond_t *); -XPTHREAD_FUNC2(pthread_cond_wait, pthread_cond_t *, pthread_mutex_t *); typedef void destructor_func(void *); XPTHREAD_FUNC2(pthread_key_create, pthread_key_t *, destructor_func *); +void +ovs_mutex_init(const struct ovs_mutex *l_, int type) +{ + struct ovs_mutex *l = CONST_CAST(struct ovs_mutex *, l_); + pthread_mutexattr_t attr; + int error; + + l->where = NULL; + xpthread_mutexattr_init(&attr); + xpthread_mutexattr_settype(&attr, type); + error = pthread_mutex_init(&l->lock, &attr); + if (OVS_UNLIKELY(error)) { + ovs_abort(error, "pthread_mutex_init failed"); + } + xpthread_mutexattr_destroy(&attr); +} + +void +ovs_rwlock_init(const struct ovs_rwlock *l_) +{ + struct ovs_rwlock *l = CONST_CAST(struct ovs_rwlock *, l_); + int error; + + l->where = NULL; + error = pthread_rwlock_init(&l->lock, NULL); + if (OVS_UNLIKELY(error)) { + ovs_abort(error, "pthread_rwlock_init failed"); + } +} + +void +ovs_mutex_cond_wait(pthread_cond_t *cond, const struct ovs_mutex *mutex_) +{ + struct ovs_mutex *mutex = CONST_CAST(struct ovs_mutex *, mutex_); + int error = pthread_cond_wait(cond, &mutex->lock); + if (OVS_UNLIKELY(error)) { + ovs_abort(error, "pthread_cond_wait failed"); + } +} + void xpthread_create(pthread_t *threadp, pthread_attr_t *attr, void *(*start)(void *), void *arg) @@ -112,25 +186,29 @@ xpthread_create(pthread_t *threadp, pthread_attr_t *attr, bool ovsthread_once_start__(struct ovsthread_once *once) { - xpthread_mutex_lock(&once->mutex); + ovs_mutex_lock(&once->mutex); if (!ovsthread_once_is_done__(once)) { return false; } - xpthread_mutex_unlock(&once->mutex); + ovs_mutex_unlock(&once->mutex); return true; } -void OVS_RELEASES(once) +void ovsthread_once_done(struct ovsthread_once *once) { atomic_store(&once->done, true); - xpthread_mutex_unlock(&once->mutex); + ovs_mutex_unlock(&once->mutex); } /* Asserts that the process has not yet created any threads (beyond the initial - * thread). */ + * thread). + * + * ('where' is used in logging. Commonly one would use + * assert_single_threaded() to automatically provide the caller's source file + * and line number for 'where'.) */ void -(assert_single_threaded)(const char *where) +assert_single_threaded_at(const char *where) { if (multithreaded) { VLOG_FATAL("%s: attempted operation not allowed when multithreaded", @@ -140,9 +218,13 @@ void /* Forks the current process (checking that this is allowed). Aborts with * VLOG_FATAL if fork() returns an error, and otherwise returns the value - * returned by fork(). */ + * returned by fork(). + * + * ('where' is used in logging. Commonly one would use xfork() to + * automatically provide the caller's source file and line number for + * 'where'.) */ pid_t -(xfork)(const char *where) +xfork_at(const char *where) { pid_t pid; @@ -153,7 +235,7 @@ pid_t pid = fork(); if (pid < 0) { - VLOG_FATAL("fork failed (%s)", ovs_strerror(errno)); + VLOG_FATAL("%s: fork failed (%s)", where, ovs_strerror(errno)); } return pid; } diff --git a/lib/ovs-thread.h b/lib/ovs-thread.h index 9c8023e56..f5e171ac6 100644 --- a/lib/ovs-thread.h +++ b/lib/ovs-thread.h @@ -23,56 +23,118 @@ #include "ovs-atomic.h" #include "util.h" -/* glibc has some non-portable mutex types and initializers: - * - * - PTHREAD_MUTEX_ADAPTIVE_NP is a mutex type that works as a spinlock that - * falls back to a mutex after spinning for some number of iterations. + +/* Mutex. */ +struct OVS_LOCKABLE ovs_mutex { + pthread_mutex_t lock; + const char *where; +}; + +/* "struct ovs_mutex" initializers: * - * - PTHREAD_ERRORCHECK_MUTEX_INITIALIZER_NP is a non-portable initializer - * for an error-checking mutex. + * - OVS_MUTEX_INITIALIZER: common case. * - * We use these definitions to fall back to PTHREAD_MUTEX_NORMAL instead in - * these cases. + * - OVS_ADAPTIVE_MUTEX_INITIALIZER for a mutex that spins briefly then goes + * to sleeps after some number of iterations. * - * (glibc has other non-portable initializers, but we can't reasonably - * substitute for them here.) */ + * - OVS_ERRORCHECK_MUTEX_INITIALIZER for a mutex that is used for + * error-checking. */ +#define OVS_MUTEX_INITIALIZER { PTHREAD_MUTEX_INITIALIZER, NULL } #ifdef PTHREAD_ADAPTIVE_MUTEX_INITIALIZER_NP -#define PTHREAD_MUTEX_ADAPTIVE PTHREAD_MUTEX_ADAPTIVE_NP -#define PTHREAD_ADAPTIVE_MUTEX_INITIALIZER \ - PTHREAD_ADAPTIVE_MUTEX_INITIALIZER_NP +#define OVS_ADAPTIVE_MUTEX_INITIALIZER \ + { PTHREAD_ADAPTIVE_MUTEX_INITIALIZER_NP, NULL } #else -#define PTHREAD_MUTEX_ADAPTIVE PTHREAD_MUTEX_NORMAL -#define PTHREAD_ADAPTIVE_MUTEX_INITIALIZER PTHREAD_MUTEX_INITIALIZER +#define OVS_ADAPTIVE_MUTEX_INITIALIZER OVS_MUTEX_INITIALIZER #endif - #ifdef PTHREAD_ERRORCHECK_MUTEX_INITIALIZER_NP -#define PTHREAD_ERRORCHECK_MUTEX_INITIALIZER \ - PTHREAD_ERRORCHECK_MUTEX_INITIALIZER_NP +#define OVS_ERRORCHECK_MUTEX_INITIALIZER \ + { PTHREAD_ERRORCHECK_MUTEX_INITIALIZER_NP, NULL } #else -#define PTHREAD_ERRORCHECK_MUTEX_INITIALIZER PTHREAD_MUTEX_INITIALIZER +#define OVS_ERRORCHECK_MUTEX_INITIALIZER OVS_MUTEX_INITIALIZER #endif -/* Simple wrappers for pthreads functions. Most of these functions abort the - * process with an error message on any error. The *_trylock() functions are - * exceptions: they pass through a 0 or EBUSY return value to the caller and - * abort on any other error. */ -void xpthread_mutex_init(pthread_mutex_t *, pthread_mutexattr_t *); -void xpthread_mutex_lock(pthread_mutex_t *mutex) OVS_ACQUIRES(mutex); -void xpthread_mutex_unlock(pthread_mutex_t *mutex) OVS_RELEASES(mutex); -int xpthread_mutex_trylock(pthread_mutex_t *); - -void xpthread_rwlock_init(pthread_rwlock_t *, pthread_rwlockattr_t *); -void xpthread_rwlock_rdlock(pthread_rwlock_t *rwlock) OVS_ACQUIRES(rwlock); -void xpthread_rwlock_wrlock(pthread_rwlock_t *rwlock) OVS_ACQUIRES(rwlock); -void xpthread_rwlock_unlock(pthread_rwlock_t *rwlock) OVS_RELEASES(rwlock); -int xpthread_rwlock_tryrdlock(pthread_rwlock_t *); -int xpthread_rwlock_trywrlock(pthread_rwlock_t *); +/* Mutex types, suitable for use with pthread_mutexattr_settype(). + * There is only one nonstandard type: + * + * - PTHREAD_MUTEX_ADAPTIVE_NP, the type used for + * OVS_ADAPTIVE_MUTEX_INITIALIZER. */ +#ifdef PTHREAD_ADAPTIVE_MUTEX_INITIALIZER_NP +#define OVS_MUTEX_ADAPTIVE PTHREAD_MUTEX_ADAPTIVE_NP +#else +#define OVS_MUTEX_ADAPTIVE PTHREAD_MUTEX_NORMAL +#endif + +/* ovs_mutex functions analogous to pthread_mutex_*() functions. + * + * Most of these functions abort the process with an error message on any + * error. ovs_mutex_trylock() is an exception: it passes through a 0 or EBUSY + * return value to the caller and aborts on any other error. */ +void ovs_mutex_init(const struct ovs_mutex *, int type); +void ovs_mutex_destroy(const struct ovs_mutex *); +void ovs_mutex_unlock(const struct ovs_mutex *mutex) OVS_RELEASES(mutex); +void ovs_mutex_lock_at(const struct ovs_mutex *mutex, const char *where) + OVS_ACQUIRES(mutex); +#define ovs_mutex_lock(mutex) \ + ovs_mutex_lock_at(mutex, SOURCE_LOCATOR) + +int ovs_mutex_trylock_at(const struct ovs_mutex *mutex, const char *where) + OVS_TRY_LOCK(0, mutex); +#define ovs_mutex_trylock(mutex) \ + ovs_mutex_trylock_at(mutex, SOURCE_LOCATOR) +void ovs_mutex_cond_wait(pthread_cond_t *, const struct ovs_mutex *); + +/* Wrappers for pthread_mutexattr_*() that abort the process on any error. */ +void xpthread_mutexattr_init(pthread_mutexattr_t *); +void xpthread_mutexattr_destroy(pthread_mutexattr_t *); +void xpthread_mutexattr_settype(pthread_mutexattr_t *, int type); +void xpthread_mutexattr_gettype(pthread_mutexattr_t *, int *typep); + +/* Read-write lock. */ +struct OVS_LOCKABLE ovs_rwlock { + pthread_rwlock_t lock; + const char *where; +}; + +/* Initializer. */ +#define OVS_RWLOCK_INITIALIZER { PTHREAD_RWLOCK_INITIALIZER, NULL } + +/* ovs_rwlock functions analogous to pthread_rwlock_*() functions. + * + * Most of these functions abort the process with an error message on any + * error. The "trylock" functions are exception: they pass through a 0 or + * EBUSY return value to the caller and abort on any other error. */ +void ovs_rwlock_init(const struct ovs_rwlock *); +void ovs_rwlock_destroy(const struct ovs_rwlock *); +void ovs_rwlock_unlock(const struct ovs_rwlock *rwlock) OVS_RELEASES(rwlock); + +void ovs_rwlock_wrlock_at(const struct ovs_rwlock *rwlock, const char *where) + OVS_ACQ_WRLOCK(rwlock); +#define ovs_rwlock_wrlock(rwlock) \ + ovs_rwlock_wrlock_at(rwlock, SOURCE_LOCATOR); + +int ovs_rwlock_trywrlock_at(const struct ovs_rwlock *rwlock, const char *where) + OVS_TRY_WRLOCK(0, rwlock); +#define ovs_rwlock_trywrlock(rwlock) \ + ovs_rwlock_trywrlock_at(rwlock, SOURCE_LOCATOR) + +void ovs_rwlock_rdlock_at(const struct ovs_rwlock *rwlock, const char *where) + OVS_ACQ_RDLOCK(rwlock); +#define ovs_rwlock_rdlock(rwlock) \ + ovs_rwlock_rdlock_at(rwlock, SOURCE_LOCATOR); + +int ovs_rwlock_tryrdlock_at(const struct ovs_rwlock *rwlock, const char *where) + OVS_TRY_RDLOCK(0, rwlock); +#define ovs_rwlock_tryrdlock(rwlock) \ + ovs_rwlock_tryrdlock_at(rwlock, SOURCE_LOCATOR) + +/* Wrappers for xpthread_cond_*() that abort the process on any error. + * + * Use ovs_mutex_cond_wait() to wait for a condition. */ void xpthread_cond_init(pthread_cond_t *, pthread_condattr_t *); +void xpthread_cond_destroy(pthread_cond_t *); void xpthread_cond_signal(pthread_cond_t *); void xpthread_cond_broadcast(pthread_cond_t *); -void xpthread_cond_wait(pthread_cond_t *, pthread_mutex_t *mutex) - OVS_MUST_HOLD(mutex); #ifdef __CHECKER__ /* Replace these functions by the macros already defined in the @@ -326,19 +388,22 @@ void xpthread_create(pthread_t *, pthread_attr_t *, void *(*)(void *), void *); struct ovsthread_once { atomic_bool done; - pthread_mutex_t mutex; + struct ovs_mutex mutex; }; #define OVSTHREAD_ONCE_INITIALIZER \ { \ ATOMIC_VAR_INIT(false), \ - PTHREAD_ADAPTIVE_MUTEX_INITIALIZER, \ + OVS_ADAPTIVE_MUTEX_INITIALIZER, \ } -static inline bool ovsthread_once_start(struct ovsthread_once *); -void ovsthread_once_done(struct ovsthread_once *once) OVS_RELEASES(once); +static inline bool ovsthread_once_start(struct ovsthread_once *once) + OVS_TRY_LOCK(true, &once->mutex); +void ovsthread_once_done(struct ovsthread_once *once) + OVS_RELEASES(&once->mutex); -bool ovsthread_once_start__(struct ovsthread_once *); +bool ovsthread_once_start__(struct ovsthread_once *once) + OVS_TRY_LOCK(false, &once->mutex); static inline bool ovsthread_once_is_done__(const struct ovsthread_once *once) @@ -365,14 +430,14 @@ ovsthread_once_start(struct ovsthread_once *once) #ifdef __CHECKER__ #define ovsthread_once_start(ONCE) \ - ((ONCE)->done ? false : ({ OVS_ACQUIRE(ONCE); true; })) + ((ONCE)->done ? false : ({ OVS_MACRO_LOCK((&ONCE->mutex)); true; })) #endif -void assert_single_threaded(const char *where); -#define assert_single_threaded() assert_single_threaded(SOURCE_LOCATOR) +void assert_single_threaded_at(const char *where); +#define assert_single_threaded() assert_single_threaded_at(SOURCE_LOCATOR) -pid_t xfork(const char *where); -#define xfork() xfork(SOURCE_LOCATOR) +pid_t xfork_at(const char *where); +#define xfork() xfork_at(SOURCE_LOCATOR) void forbid_forking(const char *reason); bool may_fork(void); diff --git a/lib/ovsdb-types.c b/lib/ovsdb-types.c index b47bfad39..83e6a5629 100644 --- a/lib/ovsdb-types.c +++ b/lib/ovsdb-types.c @@ -1,4 +1,4 @@ -/* Copyright (c) 2009, 2010, 2011 Nicira, Inc. +/* Copyright (c) 2009, 2010, 2011, 2013 Nicira, Inc. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -22,6 +22,7 @@ #include "dynamic-string.h" #include "json.h" +#include "ovs-thread.h" #include "ovsdb-data.h" #include "ovsdb-error.h" #include "ovsdb-parser.h" @@ -158,16 +159,23 @@ ovsdb_base_type_init(struct ovsdb_base_type *base, enum ovsdb_atomic_type type) const struct ovsdb_type * ovsdb_base_type_get_enum_type(enum ovsdb_atomic_type atomic_type) { + static struct ovsthread_once once = OVSTHREAD_ONCE_INITIALIZER; static struct ovsdb_type *types[OVSDB_N_TYPES]; - if (!types[atomic_type]) { - struct ovsdb_type *type; + if (ovsthread_once_start(&once)) { + enum ovsdb_atomic_type i; - types[atomic_type] = type = xmalloc(sizeof *type); - ovsdb_base_type_init(&type->key, atomic_type); - ovsdb_base_type_init(&type->value, OVSDB_TYPE_VOID); - type->n_min = 1; - type->n_max = UINT_MAX; + for (i = 0; i < OVSDB_N_TYPES; i++) { + struct ovsdb_type *type; + + types[i] = type = xmalloc(sizeof *type); + ovsdb_base_type_init(&type->key, i); + ovsdb_base_type_init(&type->value, OVSDB_TYPE_VOID); + type->n_min = 1; + type->n_max = UINT_MAX; + } + + ovsthread_once_done(&once); } return types[atomic_type]; } diff --git a/lib/packets.c b/lib/packets.c index 7fe651305..b95e1e010 100644 --- a/lib/packets.c +++ b/lib/packets.c @@ -27,6 +27,7 @@ #include "hmap.h" #include "dynamic-string.h" #include "ofpbuf.h" +#include "ovs-thread.h" const struct in6_addr in6addr_exact = IN6ADDR_EXACT_INIT; @@ -54,7 +55,7 @@ eth_addr_is_reserved(const uint8_t ea[ETH_ADDR_LEN]) { struct eth_addr_node { struct hmap_node hmap_node; - uint64_t ea64; + const uint64_t ea64; }; static struct eth_addr_node nodes[] = { @@ -100,15 +101,18 @@ eth_addr_is_reserved(const uint8_t ea[ETH_ADDR_LEN]) { HMAP_NODE_NULL_INITIALIZER, 0x01000cccccc7ULL }, }; - static struct hmap addrs = HMAP_INITIALIZER(&addrs); + static struct ovsthread_once once = OVSTHREAD_ONCE_INITIALIZER; struct eth_addr_node *node; + static struct hmap addrs; uint64_t ea64; - if (hmap_is_empty(&addrs)) { + if (ovsthread_once_start(&once)) { + hmap_init(&addrs); for (node = nodes; node < &nodes[ARRAY_SIZE(nodes)]; node++) { hmap_insert(&addrs, &node->hmap_node, hash_2words(node->ea64, node->ea64 >> 32)); } + ovsthread_once_done(&once); } ea64 = eth_addr_to_uint64(ea); @@ -246,7 +250,8 @@ set_ethertype(struct ofpbuf *packet, ovs_be16 eth_type) if (eh->eth_type == htons(ETH_TYPE_VLAN)) { ovs_be16 *p; - p = (ovs_be16 *)((char *)(packet->l2_5 ? packet->l2_5 : packet->l3) - 2); + p = ALIGNED_CAST(ovs_be16 *, + (char *)(packet->l2_5 ? packet->l2_5 : packet->l3) - 2); *p = eth_type; } else { eh->eth_type = eth_type; @@ -666,7 +671,7 @@ packet_rh_present(struct ofpbuf *packet) if (remaining < sizeof *nh) { return false; } - nh = (struct ip6_hdr *)data; + nh = ALIGNED_CAST(struct ip6_hdr *, data); data += sizeof *nh; remaining -= sizeof *nh; nexthdr = nh->ip6_nxt; @@ -702,7 +707,8 @@ packet_rh_present(struct ofpbuf *packet) nexthdr = ext_hdr->ip6e_nxt; len = (ext_hdr->ip6e_len + 2) * 4; } else if (nexthdr == IPPROTO_FRAGMENT) { - const struct ip6_frag *frag_hdr = (struct ip6_frag *)data; + const struct ip6_frag *frag_hdr = ALIGNED_CAST(struct ip6_frag *, + data); nexthdr = frag_hdr->ip6f_nxt; len = sizeof *frag_hdr; diff --git a/lib/poll-loop.c b/lib/poll-loop.c index 97fc8068c..0f45d9835 100644 --- a/lib/poll-loop.c +++ b/lib/poll-loop.c @@ -30,11 +30,6 @@ #include "timeval.h" #include "vlog.h" -#undef poll_fd_wait -#undef poll_timer_wait -#undef poll_timer_wait_until -#undef poll_immediate_wake - VLOG_DEFINE_THIS_MODULE(poll_loop); COVERAGE_DEFINE(poll_fd_wait); @@ -63,10 +58,11 @@ static struct poll_loop *poll_loop(void); * is affected. The event will need to be re-registered after poll_block() is * called if it is to persist. * - * Ordinarily the 'where' argument is supplied automatically; see poll-loop.h - * for more information. */ + * ('where' is used in debug logging. Commonly one would use poll_fd_wait() to + * automatically provide the caller's source file and line number for + * 'where'.) */ void -poll_fd_wait(int fd, short int events, const char *where) +poll_fd_wait_at(int fd, short int events, const char *where) { struct poll_loop *loop = poll_loop(); @@ -93,10 +89,11 @@ poll_fd_wait(int fd, short int events, const char *where) * is affected. The timer will need to be re-registered after poll_block() is * called if it is to persist. * - * Ordinarily the 'where' argument is supplied automatically; see poll-loop.h - * for more information. */ + * ('where' is used in debug logging. Commonly one would use poll_timer_wait() + * to automatically provide the caller's source file and line number for + * 'where'.) */ void -poll_timer_wait(long long int msec, const char *where) +poll_timer_wait_at(long long int msec, const char *where) { long long int now = time_msec(); long long int when; @@ -112,7 +109,7 @@ poll_timer_wait(long long int msec, const char *where) when = LLONG_MAX; } - poll_timer_wait_until(when, where); + poll_timer_wait_until_at(when, where); } /* Causes the following call to poll_block() to wake up when the current time, @@ -124,10 +121,11 @@ poll_timer_wait(long long int msec, const char *where) * is affected. The timer will need to be re-registered after poll_block() is * called if it is to persist. * - * Ordinarily the 'where' argument is supplied automatically; see poll-loop.h - * for more information. */ + * ('where' is used in debug logging. Commonly one would use + * poll_timer_wait_until() to automatically provide the caller's source file + * and line number for 'where'.) */ void -poll_timer_wait_until(long long int when, const char *where) +poll_timer_wait_until_at(long long int when, const char *where) { struct poll_loop *loop = poll_loop(); if (when < loop->timeout_when) { @@ -139,12 +137,13 @@ poll_timer_wait_until(long long int when, const char *where) /* Causes the following call to poll_block() to wake up immediately, without * blocking. * - * Ordinarily the 'where' argument is supplied automatically; see poll-loop.h - * for more information. */ + * ('where' is used in debug logging. Commonly one would use + * poll_immediate_wake() to automatically provide the caller's source file and + * line number for 'where'.) */ void -poll_immediate_wake(const char *where) +poll_immediate_wake_at(const char *where) { - poll_timer_wait(0, where); + poll_timer_wait_at(0, where); } /* Logs, if appropriate, that the poll loop was awakened by an event diff --git a/lib/poll-loop.h b/lib/poll-loop.h index 6614ebe58..03978530e 100644 --- a/lib/poll-loop.h +++ b/lib/poll-loop.h @@ -44,25 +44,24 @@ extern "C" { /* Schedule events to wake up the following poll_block(). * * The poll_loop logs the 'where' argument to each function at "debug" level - * when an event causes a wakeup. Ordinarily, it is automatically filled in - * with the location in the source of the call, and the caller should therefore - * omit it. But, if the function you are implementing is very generic, so that - * its location in the source would not be very helpful for debugging, you can - * avoid the macro expansion and pass a different argument, e.g.: - * (poll_fd_wait)(fd, events, where); - * See timer_wait() for an example. - */ -void poll_fd_wait(int fd, short int events, const char *where); -#define poll_fd_wait(fd, events) poll_fd_wait(fd, events, SOURCE_LOCATOR) + * when an event causes a wakeup. Each of these ways to schedule an event has + * a function and a macro wrapper. The macro version automatically supplies + * the source code location of the caller. The function version allows the + * caller to supply a location explicitly, which is useful if the caller's own + * caller would be more useful in log output. See timer_wait_at() for an + * example. */ +void poll_fd_wait_at(int fd, short int events, const char *where); +#define poll_fd_wait(fd, events) poll_fd_wait_at(fd, events, SOURCE_LOCATOR) -void poll_timer_wait(long long int msec, const char *where); -#define poll_timer_wait(msec) poll_timer_wait(msec, SOURCE_LOCATOR) +void poll_timer_wait_at(long long int msec, const char *where); +#define poll_timer_wait(msec) poll_timer_wait_at(msec, SOURCE_LOCATOR) -void poll_timer_wait_until(long long int msec, const char *where); -#define poll_timer_wait_until(msec) poll_timer_wait_until(msec, SOURCE_LOCATOR) +void poll_timer_wait_until_at(long long int msec, const char *where); +#define poll_timer_wait_until(msec) \ + poll_timer_wait_until_at(msec, SOURCE_LOCATOR) -void poll_immediate_wake(const char *where); -#define poll_immediate_wake() poll_immediate_wake(SOURCE_LOCATOR) +void poll_immediate_wake_at(const char *where); +#define poll_immediate_wake() poll_immediate_wake_at(SOURCE_LOCATOR) /* Wait until an event occurs. */ void poll_block(void); diff --git a/lib/route-table.c b/lib/route-table.c index d572e8cea..1afc01d06 100644 --- a/lib/route-table.c +++ b/lib/route-table.c @@ -270,7 +270,7 @@ route_table_parse(struct ofpbuf *buf, struct route_table_msg *change) const struct nlmsghdr *nlmsg; nlmsg = buf->data; - rtm = (const struct rtmsg *) ((const char *) buf->data + NLMSG_HDRLEN); + rtm = ofpbuf_at(buf, NLMSG_HDRLEN, sizeof *rtm); if (rtm->rtm_family != AF_INET) { VLOG_DBG_RL(&rl, "received non AF_INET rtnetlink route message"); diff --git a/lib/rtnetlink-link.c b/lib/rtnetlink-link.c index 459e48559..308338fca 100644 --- a/lib/rtnetlink-link.c +++ b/lib/rtnetlink-link.c @@ -59,8 +59,7 @@ rtnetlink_link_parse(struct ofpbuf *buf, const struct ifinfomsg *ifinfo; nlmsg = buf->data; - ifinfo = ((const struct ifinfomsg *) - ((const char *) buf->data + NLMSG_HDRLEN)); + ifinfo = ofpbuf_at(buf, NLMSG_HDRLEN, sizeof *ifinfo); change->nlmsg_type = nlmsg->nlmsg_type; change->ifi_index = ifinfo->ifi_index; diff --git a/lib/socket-util.c b/lib/socket-util.c index fa55480d3..1d0cede88 100644 --- a/lib/socket-util.c +++ b/lib/socket-util.c @@ -132,8 +132,10 @@ rlim_is_finite(rlim_t limit) int get_max_fds(void) { - static int max_fds = -1; - if (max_fds < 0) { + static struct ovsthread_once once = OVSTHREAD_ONCE_INITIALIZER; + static int max_fds; + + if (ovsthread_once_start(&once)) { struct rlimit r; if (!getrlimit(RLIMIT_NOFILE, &r) && rlim_is_finite(r.rlim_cur)) { max_fds = r.rlim_cur; @@ -141,7 +143,9 @@ get_max_fds(void) VLOG_WARN("failed to obtain fd limit, defaulting to 1024"); max_fds = 1024; } + ovsthread_once_done(&once); } + return max_fds; } @@ -197,7 +201,8 @@ lookup_hostname(const char *host_name, struct in_addr *addr) switch (getaddrinfo(host_name, NULL, &hints, &result)) { case 0: - *addr = ((struct sockaddr_in *) result->ai_addr)->sin_addr; + *addr = ALIGNED_CAST(struct sockaddr_in *, + result->ai_addr)->sin_addr; freeaddrinfo(result); return 0; @@ -802,15 +807,19 @@ error: int get_null_fd(void) { - static int null_fd = -1; - if (null_fd < 0) { + static struct ovsthread_once once = OVSTHREAD_ONCE_INITIALIZER; + static int null_fd; + + if (ovsthread_once_start(&once)) { null_fd = open("/dev/null", O_RDWR); if (null_fd < 0) { int error = errno; VLOG_ERR("could not open /dev/null: %s", ovs_strerror(error)); - return -error; + null_fd = -error; } + ovsthread_once_done(&once); } + return null_fd; } @@ -1318,7 +1327,7 @@ recv_data_and_fds(int sock, goto error; } else { size_t n_fds = (p->cmsg_len - CMSG_LEN(0)) / sizeof *fds; - const int *fds_data = (const int *) CMSG_DATA(p); + const int *fds_data = ALIGNED_CAST(const int *, CMSG_DATA(p)); ovs_assert(n_fds > 0); if (n_fds > SOUTIL_MAX_FDS) { diff --git a/lib/stream-tcp.c b/lib/stream-tcp.c index d507208e2..a4cdf45a4 100644 --- a/lib/stream-tcp.c +++ b/lib/stream-tcp.c @@ -130,7 +130,8 @@ static int ptcp_accept(int fd, const struct sockaddr *sa, size_t sa_len, struct stream **streamp) { - const struct sockaddr_in *sin = (const struct sockaddr_in *) sa; + const struct sockaddr_in *sin = ALIGNED_CAST(const struct sockaddr_in *, + sa); char name[128]; if (sa_len == sizeof(struct sockaddr_in) && sin->sin_family == AF_INET) { diff --git a/lib/timer.c b/lib/timer.c index e767db65f..e90355bdb 100644 --- a/lib/timer.c +++ b/lib/timer.c @@ -1,5 +1,5 @@ /* - * Copyright (c) 2011 Nicira, Inc. + * Copyright (c) 2011, 2013 Nicira, Inc. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -32,11 +32,15 @@ timer_msecs_until_expired(const struct timer *timer) } } -/* Causes poll_block() to wake when 'timer' expires. */ +/* Causes poll_block() to wake when 'timer' expires. + * + * ('where' is used in debug logging. Commonly one would use timer_wait() to + * automatically provide the caller's source file and line number for + * 'where'.) */ void -(timer_wait)(const struct timer *timer, const char *where) +timer_wait_at(const struct timer *timer, const char *where) { if (timer->t < LLONG_MAX) { - (poll_timer_wait_until)(timer->t, where); + poll_timer_wait_until_at(timer->t, where); } } diff --git a/lib/timer.h b/lib/timer.h index e9650ada9..9afe3b75e 100644 --- a/lib/timer.h +++ b/lib/timer.h @@ -1,5 +1,5 @@ /* - * Copyright (c) 2011 Nicira, Inc. + * Copyright (c) 2011, 2013 Nicira, Inc. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -27,8 +27,8 @@ struct timer { }; long long int timer_msecs_until_expired(const struct timer *); -void timer_wait(const struct timer *, const char *where); -#define timer_wait(timer) timer_wait(timer, SOURCE_LOCATOR) +void timer_wait_at(const struct timer *, const char *where); +#define timer_wait(timer) timer_wait_at(timer, SOURCE_LOCATOR) /* Causes 'timer' to expire when 'duration' milliseconds have passed. * diff --git a/lib/timeval.c b/lib/timeval.c index 4ed3ebff5..05da99e56 100644 --- a/lib/timeval.c +++ b/lib/timeval.c @@ -41,7 +41,7 @@ VLOG_DEFINE_THIS_MODULE(timeval); struct clock { clockid_t id; /* CLOCK_MONOTONIC or CLOCK_REALTIME. */ - pthread_rwlock_t rwlock; /* Mutual exclusion for 'cache'. */ + struct ovs_rwlock rwlock; /* Mutual exclusion for 'cache'. */ /* Features for use by unit tests. Protected by 'rwlock'. */ struct timespec warp; /* Offset added for unit tests. */ @@ -83,7 +83,7 @@ init_clock(struct clock *c, clockid_t id) { memset(c, 0, sizeof *c); c->id = id; - xpthread_rwlock_init(&c->rwlock, NULL); + ovs_rwlock_init(&c->rwlock); xclock_gettime(c->id, &c->cache); } @@ -178,21 +178,21 @@ time_timespec__(struct clock *c, struct timespec *ts) for (;;) { /* Use the cached time by preference, but fall through if there's been * a clock tick. */ - xpthread_rwlock_rdlock(&c->rwlock); + ovs_rwlock_rdlock(&c->rwlock); if (c->stopped || !c->tick) { timespec_add(ts, &c->cache, &c->warp); - xpthread_rwlock_unlock(&c->rwlock); + ovs_rwlock_unlock(&c->rwlock); return; } - xpthread_rwlock_unlock(&c->rwlock); + ovs_rwlock_unlock(&c->rwlock); /* Refresh the cache. */ - xpthread_rwlock_wrlock(&c->rwlock); + ovs_rwlock_wrlock(&c->rwlock); if (c->tick) { c->tick = false; xclock_gettime(c->id, &c->cache); } - xpthread_rwlock_unlock(&c->rwlock); + ovs_rwlock_unlock(&c->rwlock); } } @@ -568,9 +568,9 @@ timeval_stop_cb(struct unixctl_conn *conn, int argc OVS_UNUSED, const char *argv[] OVS_UNUSED, void *aux OVS_UNUSED) { - xpthread_rwlock_wrlock(&monotonic_clock.rwlock); + ovs_rwlock_wrlock(&monotonic_clock.rwlock); monotonic_clock.stopped = true; - xpthread_rwlock_unlock(&monotonic_clock.rwlock); + ovs_rwlock_unlock(&monotonic_clock.rwlock); unixctl_command_reply(conn, NULL); } @@ -596,9 +596,9 @@ timeval_warp_cb(struct unixctl_conn *conn, ts.tv_sec = msecs / 1000; ts.tv_nsec = (msecs % 1000) * 1000 * 1000; - xpthread_rwlock_wrlock(&monotonic_clock.rwlock); + ovs_rwlock_wrlock(&monotonic_clock.rwlock); timespec_add(&monotonic_clock.warp, &monotonic_clock.warp, &ts); - xpthread_rwlock_unlock(&monotonic_clock.rwlock); + ovs_rwlock_unlock(&monotonic_clock.rwlock); unixctl_command_reply(conn, "warped"); } diff --git a/lib/util.c b/lib/util.c index c69d7d150..6a721070a 100644 --- a/lib/util.c +++ b/lib/util.c @@ -395,11 +395,17 @@ get_subprogram_name(void) } /* Sets 'name' as the name of the currently running thread or process. (This - * appears in log messages.) */ + * appears in log messages and may also be visible in system process listings + * and debuggers.) */ void set_subprogram_name(const char *name) { free(subprogram_name_set(xstrdup(name))); +#if HAVE_PTHREAD_SETNAME_NP + pthread_setname_np(pthread_self(), name); +#elif HAVE_PTHREAD_SET_NAME_NP + pthread_set_name_np(pthread_self(), name); +#endif } /* Returns a pointer to a string describing the program version. The diff --git a/lib/util.h b/lib/util.h index 911ad3218..0db41be92 100644 --- a/lib/util.h +++ b/lib/util.h @@ -193,6 +193,10 @@ is_pow2(uintmax_t x) #define ASSIGN_CONTAINER(OBJECT, POINTER, MEMBER) \ ((OBJECT) = OBJECT_CONTAINING(POINTER, OBJECT, MEMBER), (void) 0) +/* Given ATTR, and TYPE, cast the ATTR to TYPE by first casting ATTR to + * (void *). This is to suppress the alignment warning issued by clang. */ +#define ALIGNED_CAST(TYPE, ATTR) ((TYPE) (void *) (ATTR)) + #ifdef __cplusplus extern "C" { #endif diff --git a/lib/uuid.c b/lib/uuid.c index cfaf0c582..18748ea95 100644 --- a/lib/uuid.c +++ b/lib/uuid.c @@ -1,4 +1,4 @@ -/* Copyright (c) 2008, 2009, 2010, 2011 Nicira, Inc. +/* Copyright (c) 2008, 2009, 2010, 2011, 2013 Nicira, Inc. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -26,6 +26,7 @@ #include "aes128.h" #include "entropy.h" +#include "ovs-thread.h" #include "sha1.h" #include "timeval.h" #include "util.h" @@ -49,11 +50,8 @@ static void do_init(void); void uuid_init(void) { - static bool inited; - if (!inited) { - do_init(); - inited = true; - } + static pthread_once_t once = PTHREAD_ONCE_INIT; + pthread_once(&once, do_init); } /* Generates a new random UUID in 'uuid'. @@ -83,15 +81,22 @@ uuid_init(void) void uuid_generate(struct uuid *uuid) { + static struct ovs_mutex mutex = OVS_ADAPTIVE_MUTEX_INITIALIZER; + uint64_t copy[2]; + uuid_init(); - /* Increment the counter. */ + /* Copy out the counter's current value, then increment it. */ + ovs_mutex_lock(&mutex); + copy[0] = counter[0]; + copy[1] = counter[1]; if (++counter[1] == 0) { counter[0]++; } + ovs_mutex_unlock(&mutex); /* AES output is exactly 16 bytes, so we encrypt directly into 'uuid'. */ - aes128_encrypt(&key, counter, uuid); + aes128_encrypt(&key, copy, uuid); /* Set bits to indicate a random UUID. See RFC 4122 section 4.4. */ uuid->parts[2] &= ~0xc0000000; diff --git a/lib/vlog.c b/lib/vlog.c index c2fc8e024..6f0a25692 100644 --- a/lib/vlog.c +++ b/lib/vlog.c @@ -82,10 +82,15 @@ struct vlog_module *vlog_modules[] = { #define n_vlog_modules ARRAY_SIZE(vlog_modules) #endif +/* Protects the 'pattern' in all "struct facility"s, so that a race between + * changing and reading the pattern does not cause an access to freed + * memory. */ +static struct ovs_rwlock pattern_rwlock = OVS_RWLOCK_INITIALIZER; + /* Information about each facility. */ struct facility { const char *name; /* Name. */ - char *pattern; /* Current pattern (see 'pattern_rwlock'). */ + char *pattern OVS_GUARDED_BY(pattern_rwlock); /* Current pattern. */ bool default_pattern; /* Whether current pattern is the default. */ }; static struct facility facilities[VLF_N_FACILITIES] = { @@ -94,11 +99,6 @@ static struct facility facilities[VLF_N_FACILITIES] = { #undef VLOG_FACILITY }; -/* Protects the 'pattern' in all "struct facility"s, so that a race between - * changing and reading the pattern does not cause an access to freed - * memory. */ -static pthread_rwlock_t pattern_rwlock = PTHREAD_RWLOCK_INITIALIZER; - /* Sequence number for the message currently being composed. */ DEFINE_PER_THREAD_DATA(unsigned int, msg_num, 0); @@ -106,15 +106,15 @@ DEFINE_PER_THREAD_DATA(unsigned int, msg_num, 0); * * All of the following is protected by 'log_file_mutex', which nests inside * pattern_rwlock. */ -static pthread_mutex_t log_file_mutex = PTHREAD_ADAPTIVE_MUTEX_INITIALIZER; -static char *log_file_name; -static int log_fd = -1; -static struct async_append *log_writer; +static struct ovs_mutex log_file_mutex = OVS_ADAPTIVE_MUTEX_INITIALIZER; +static char *log_file_name OVS_GUARDED_BY(log_file_mutex); +static int log_fd OVS_GUARDED_BY(log_file_mutex) = -1; +static struct async_append *log_writer OVS_GUARDED_BY(log_file_mutex); static void format_log_message(const struct vlog_module *, enum vlog_level, enum vlog_facility, const char *message, va_list, struct ds *) - PRINTF_FORMAT(4, 0); + PRINTF_FORMAT(4, 0) OVS_REQ_RDLOCK(&pattern_rwlock); /* Searches the 'n_names' in 'names'. Returns the index of a match for * 'target', or 'n_names' if no name matches. */ @@ -201,8 +201,8 @@ vlog_get_level(const struct vlog_module *module, enum vlog_facility facility) return module->levels[facility]; } -static void OVS_MUST_HOLD(log_file_mutex) -update_min_level(struct vlog_module *module) +static void +update_min_level(struct vlog_module *module) OVS_REQUIRES(&log_file_mutex) { enum vlog_facility facility; @@ -224,7 +224,7 @@ set_facility_level(enum vlog_facility facility, struct vlog_module *module, assert(facility >= 0 && facility < VLF_N_FACILITIES); assert(level < VLL_N_LEVELS); - xpthread_mutex_lock(&log_file_mutex); + ovs_mutex_lock(&log_file_mutex); if (!module) { struct vlog_module **mp; @@ -236,7 +236,7 @@ set_facility_level(enum vlog_facility facility, struct vlog_module *module, module->levels[facility] = level; update_min_level(module); } - xpthread_mutex_unlock(&log_file_mutex); + ovs_mutex_unlock(&log_file_mutex); } /* Sets the logging level for the given 'module' and 'facility' to 'level'. A @@ -261,14 +261,14 @@ do_set_pattern(enum vlog_facility facility, const char *pattern) { struct facility *f = &facilities[facility]; - xpthread_rwlock_wrlock(&pattern_rwlock); + ovs_rwlock_wrlock(&pattern_rwlock); if (!f->default_pattern) { free(f->pattern); } else { f->default_pattern = false; } f->pattern = xstrdup(pattern); - xpthread_rwlock_unlock(&pattern_rwlock); + ovs_rwlock_unlock(&pattern_rwlock); } /* Sets the pattern for the given 'facility' to 'pattern'. */ @@ -297,6 +297,7 @@ vlog_set_log_file(const char *file_name) struct stat new_stat; int new_log_fd; bool same_file; + bool log_close; /* Open new log file. */ new_log_file_name = (file_name @@ -311,14 +312,14 @@ vlog_set_log_file(const char *file_name) } /* If the new log file is the same one we already have open, bail out. */ - xpthread_mutex_lock(&log_file_mutex); + ovs_mutex_lock(&log_file_mutex); same_file = (log_fd >= 0 && new_log_fd >= 0 && !fstat(log_fd, &old_stat) && !fstat(new_log_fd, &new_stat) && old_stat.st_dev == new_stat.st_dev && old_stat.st_ino == new_stat.st_ino); - xpthread_mutex_unlock(&log_file_mutex); + ovs_mutex_unlock(&log_file_mutex); if (same_file) { close(new_log_fd); free(new_log_file_name); @@ -326,12 +327,15 @@ vlog_set_log_file(const char *file_name) } /* Log closing old log file (we can't log while holding log_file_mutex). */ - if (log_fd >= 0) { + ovs_mutex_lock(&log_file_mutex); + log_close = log_fd >= 0; + ovs_mutex_unlock(&log_file_mutex); + if (log_close) { VLOG_INFO("closing log file"); } /* Close old log file, if any, and install new one. */ - xpthread_mutex_lock(&log_file_mutex); + ovs_mutex_lock(&log_file_mutex); if (log_fd >= 0) { free(log_file_name); close(log_fd); @@ -345,7 +349,7 @@ vlog_set_log_file(const char *file_name) for (mp = vlog_modules; mp < &vlog_modules[n_vlog_modules]; mp++) { update_min_level(*mp); } - xpthread_mutex_unlock(&log_file_mutex); + ovs_mutex_unlock(&log_file_mutex); /* Log opening new log file (we can't log while holding log_file_mutex). */ VLOG_INFO("opened log file %s", new_log_file_name); @@ -362,9 +366,9 @@ vlog_reopen_log_file(void) { char *fn; - xpthread_mutex_lock(&log_file_mutex); + ovs_mutex_lock(&log_file_mutex); fn = log_file_name ? xstrdup(log_file_name) : NULL; - xpthread_mutex_unlock(&log_file_mutex); + ovs_mutex_unlock(&log_file_mutex); if (fn) { int error = vlog_set_log_file(fn); @@ -504,7 +508,13 @@ static void vlog_unixctl_reopen(struct unixctl_conn *conn, int argc OVS_UNUSED, const char *argv[] OVS_UNUSED, void *aux OVS_UNUSED) { - if (log_file_name) { + bool has_log_file; + + ovs_mutex_lock(&log_file_mutex); + has_log_file = log_file_name != NULL; + ovs_mutex_unlock(&log_file_mutex); + + if (has_log_file) { int error = vlog_reopen_log_file(); if (error) { unixctl_command_reply_error(conn, ovs_strerror(errno)); @@ -786,7 +796,11 @@ vlog_valist(const struct vlog_module *module, enum vlog_level level, { bool log_to_console = module->levels[VLF_CONSOLE] >= level; bool log_to_syslog = module->levels[VLF_SYSLOG] >= level; - bool log_to_file = module->levels[VLF_FILE] >= level && log_fd >= 0; + bool log_to_file; + + ovs_mutex_lock(&log_file_mutex); + log_to_file = module->levels[VLF_FILE] >= level && log_fd >= 0; + ovs_mutex_unlock(&log_file_mutex); if (log_to_console || log_to_syslog || log_to_file) { int save_errno = errno; struct ds s; @@ -797,7 +811,7 @@ vlog_valist(const struct vlog_module *module, enum vlog_level level, ds_reserve(&s, 1024); ++*msg_num_get(); - xpthread_rwlock_rdlock(&pattern_rwlock); + ovs_rwlock_rdlock(&pattern_rwlock); if (log_to_console) { format_log_message(module, level, VLF_CONSOLE, message, args, &s); ds_put_char(&s, '\n'); @@ -820,16 +834,16 @@ vlog_valist(const struct vlog_module *module, enum vlog_level level, format_log_message(module, level, VLF_FILE, message, args, &s); ds_put_char(&s, '\n'); - xpthread_mutex_lock(&log_file_mutex); + ovs_mutex_lock(&log_file_mutex); if (log_fd >= 0) { async_append_write(log_writer, s.string, s.length); if (level == VLL_EMER) { async_append_flush(log_writer); } } - xpthread_mutex_unlock(&log_file_mutex); + ovs_mutex_unlock(&log_file_mutex); } - xpthread_rwlock_unlock(&pattern_rwlock); + ovs_rwlock_unlock(&pattern_rwlock); ds_destroy(&s); errno = save_errno; @@ -929,7 +943,7 @@ vlog_should_drop(const struct vlog_module *module, enum vlog_level level, return true; } - xpthread_mutex_lock(&rl->mutex); + ovs_mutex_lock(&rl->mutex); if (!token_bucket_withdraw(&rl->token_bucket, VLOG_MSG_TOKENS)) { time_t now = time_now(); if (!rl->n_dropped) { @@ -937,19 +951,19 @@ vlog_should_drop(const struct vlog_module *module, enum vlog_level level, } rl->last_dropped = now; rl->n_dropped++; - xpthread_mutex_unlock(&rl->mutex); + ovs_mutex_unlock(&rl->mutex); return true; } if (!rl->n_dropped) { - xpthread_mutex_unlock(&rl->mutex); + ovs_mutex_unlock(&rl->mutex); } else { time_t now = time_now(); unsigned int n_dropped = rl->n_dropped; unsigned int first_dropped_elapsed = now - rl->first_dropped; unsigned int last_dropped_elapsed = now - rl->last_dropped; rl->n_dropped = 0; - xpthread_mutex_unlock(&rl->mutex); + ovs_mutex_unlock(&rl->mutex); vlog(module, level, "Dropped %u log messages in last %u seconds (most recently, " diff --git a/lib/vlog.h b/lib/vlog.h index 95766875d..901b3d38a 100644 --- a/lib/vlog.h +++ b/lib/vlog.h @@ -88,8 +88,8 @@ struct vlog_module { #if USE_LINKER_SECTIONS #define VLOG_DEFINE_MODULE(MODULE) \ VLOG_DEFINE_MODULE__(MODULE) \ - extern struct vlog_module *vlog_module_ptr_##MODULE; \ - struct vlog_module *vlog_module_ptr_##MODULE \ + extern struct vlog_module *const vlog_module_ptr_##MODULE; \ + struct vlog_module *const vlog_module_ptr_##MODULE \ __attribute__((section("vlog_modules"))) = &VLM_##MODULE #else #define VLOG_DEFINE_MODULE(MODULE) extern struct vlog_module VLM_##MODULE @@ -104,7 +104,7 @@ struct vlog_rate_limit { time_t first_dropped; /* Time first message was dropped. */ time_t last_dropped; /* Time of most recent message drop. */ unsigned int n_dropped; /* Number of messages dropped. */ - pthread_mutex_t mutex; /* Mutual exclusion for rate limit. */ + struct ovs_mutex mutex; /* Mutual exclusion for rate limit. */ }; /* Number of tokens to emit a message. We add 'rate' tokens per millisecond, @@ -119,7 +119,7 @@ struct vlog_rate_limit { 0, /* first_dropped */ \ 0, /* last_dropped */ \ 0, /* n_dropped */ \ - PTHREAD_ADAPTIVE_MUTEX_INITIALIZER /* mutex */ \ + OVS_ADAPTIVE_MUTEX_INITIALIZER /* mutex */ \ } /* Configuring how each module logs messages. */ diff --git a/m4/openvswitch.m4 b/m4/openvswitch.m4 index dbfc7c425..bcdb942b9 100644 --- a/m4/openvswitch.m4 +++ b/m4/openvswitch.m4 @@ -426,6 +426,78 @@ static thread_local int var;], [return var;])], fi fi]) +dnl OVS_CHECK_GCC4_ATOMICS +dnl +dnl Checks whether the compiler and linker support GCC 4.0+ atomic built-ins. +dnl A compile-time only check is not enough because the compiler defers +dnl unimplemented built-ins to libgcc, which sometimes also lacks +dnl implementations. +AC_DEFUN([OVS_CHECK_GCC4_ATOMICS], + [AC_CACHE_CHECK( + [whether $CC supports GCC 4.0+ atomic built-ins], + [ovs_cv_gcc4_atomics], + [AC_LINK_IFELSE( + [AC_LANG_PROGRAM([[#include + +#define ovs_assert(expr) if (!(expr)) abort(); +#define TEST_ATOMIC_TYPE(TYPE) \ + { \ + TYPE x = 1; \ + TYPE orig; \ + \ + __sync_synchronize(); \ + ovs_assert(x == 1); \ + \ + __sync_synchronize(); \ + x = 3; \ + __sync_synchronize(); \ + ovs_assert(x == 3); \ + \ + orig = __sync_fetch_and_add(&x, 1); \ + ovs_assert(orig == 3); \ + __sync_synchronize(); \ + ovs_assert(x == 4); \ + \ + orig = __sync_fetch_and_sub(&x, 2); \ + ovs_assert(orig == 4); \ + __sync_synchronize(); \ + ovs_assert(x == 2); \ + \ + orig = __sync_fetch_and_or(&x, 6); \ + ovs_assert(orig == 2); \ + __sync_synchronize(); \ + ovs_assert(x == 6); \ + \ + orig = __sync_fetch_and_and(&x, 10); \ + ovs_assert(orig == 6); \ + __sync_synchronize(); \ + ovs_assert(x == 2); \ + \ + orig = __sync_fetch_and_xor(&x, 10); \ + ovs_assert(orig == 2); \ + __sync_synchronize(); \ + ovs_assert(x == 8); \ + }]], [dnl +TEST_ATOMIC_TYPE(char); +TEST_ATOMIC_TYPE(unsigned char); +TEST_ATOMIC_TYPE(signed char); +TEST_ATOMIC_TYPE(short); +TEST_ATOMIC_TYPE(unsigned short); +TEST_ATOMIC_TYPE(int); +TEST_ATOMIC_TYPE(unsigned int); +TEST_ATOMIC_TYPE(long int); +TEST_ATOMIC_TYPE(unsigned long int); +TEST_ATOMIC_TYPE(long long int); +TEST_ATOMIC_TYPE(unsigned long long int); +])], + [ovs_cv_gcc4_atomics=yes], + [ovs_cv_gcc4_atomics=no])]) + if test $ovs_cv_gcc4_atomics = yes; then + AC_DEFINE([HAVE_GCC4_ATOMICS], [1], + [Define to 1 if the C compiler and linker supports the GCC 4.0+ + atomic built-ins.]) + fi]) + dnl OVS_CHECK_ATOMIC_ALWAYS_LOCK_FREE(SIZE) dnl dnl Checks __atomic_always_lock_free(SIZE, 0) diff --git a/ofproto/ofproto-dpif-ipfix.c b/ofproto/ofproto-dpif-ipfix.c index 8523afb0c..5f0132782 100644 --- a/ofproto/ofproto-dpif-ipfix.c +++ b/ofproto/ofproto-dpif-ipfix.c @@ -33,6 +33,7 @@ VLOG_DEFINE_THIS_MODULE(ipfix); static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5); +static struct ovs_mutex mutex = OVS_MUTEX_INITIALIZER; /* Cf. IETF RFC 5101 Section 10.3.4. */ #define IPFIX_DEFAULT_COLLECTOR_PORT 4739 @@ -62,7 +63,7 @@ struct dpif_ipfix_flow_exporter_map_node { struct dpif_ipfix { struct dpif_ipfix_bridge_exporter bridge_exporter; struct hmap flow_exporter_map; /* dpif_ipfix_flow_exporter_map_nodes. */ - int ref_cnt; + atomic_int ref_cnt; }; #define IPFIX_VERSION 0x000a @@ -411,13 +412,14 @@ dpif_ipfix_set_options( struct dpif_ipfix *di, const struct ofproto_ipfix_bridge_exporter_options *bridge_exporter_options, const struct ofproto_ipfix_flow_exporter_options *flow_exporters_options, - size_t n_flow_exporters_options) + size_t n_flow_exporters_options) OVS_EXCLUDED(mutex) { int i; struct ofproto_ipfix_flow_exporter_options *options; struct dpif_ipfix_flow_exporter_map_node *node, *next; size_t n_broken_flow_exporters_options = 0; + ovs_mutex_lock(&mutex); dpif_ipfix_bridge_exporter_set_options(&di->bridge_exporter, bridge_exporter_options); @@ -466,6 +468,7 @@ dpif_ipfix_set_options( ovs_assert(hmap_count(&di->flow_exporter_map) == (n_flow_exporters_options - n_broken_flow_exporters_options)); + ovs_mutex_unlock(&mutex); } struct dpif_ipfix * @@ -475,7 +478,7 @@ dpif_ipfix_create(void) di = xzalloc(sizeof *di); dpif_ipfix_exporter_clear(&di->bridge_exporter.exporter); hmap_init(&di->flow_exporter_map); - di->ref_cnt = 1; + atomic_init(&di->ref_cnt, 1); return di; } @@ -484,20 +487,26 @@ dpif_ipfix_ref(const struct dpif_ipfix *di_) { struct dpif_ipfix *di = CONST_CAST(struct dpif_ipfix *, di_); if (di) { - ovs_assert(di->ref_cnt > 0); - di->ref_cnt++; + int orig; + atomic_add(&di->ref_cnt, 1, &orig); + ovs_assert(orig > 0); } return di; } uint32_t dpif_ipfix_get_bridge_exporter_probability(const struct dpif_ipfix *di) + OVS_EXCLUDED(mutex) { - return di->bridge_exporter.probability; + uint32_t ret; + ovs_mutex_lock(&mutex); + ret = di->bridge_exporter.probability; + ovs_mutex_unlock(&mutex); + return ret; } static void -dpif_ipfix_clear(struct dpif_ipfix *di) +dpif_ipfix_clear(struct dpif_ipfix *di) OVS_REQ_WRLOCK(mutex) { struct dpif_ipfix_flow_exporter_map_node *node, *next; @@ -511,17 +520,22 @@ dpif_ipfix_clear(struct dpif_ipfix *di) } void -dpif_ipfix_unref(struct dpif_ipfix *di) +dpif_ipfix_unref(struct dpif_ipfix *di) OVS_EXCLUDED(mutex) { + int orig; + if (!di) { return; } - ovs_assert(di->ref_cnt > 0); - if (!--di->ref_cnt) { + atomic_sub(&di->ref_cnt, 1, &orig); + ovs_assert(orig > 0); + if (orig == 1) { + ovs_mutex_lock(&mutex); dpif_ipfix_clear(di); hmap_destroy(&di->flow_exporter_map); free(di); + ovs_mutex_unlock(&mutex); } } @@ -842,35 +856,37 @@ dpif_ipfix_sample(struct dpif_ipfix_exporter *exporter, void dpif_ipfix_bridge_sample(struct dpif_ipfix *di, struct ofpbuf *packet, - const struct flow *flow) + const struct flow *flow) OVS_EXCLUDED(mutex) { + uint64_t packet_delta_count; + + ovs_mutex_lock(&mutex); /* Use the sampling probability as an approximation of the number * of matched packets. */ - uint64_t packet_delta_count = UINT32_MAX / di->bridge_exporter.probability; - + packet_delta_count = UINT32_MAX / di->bridge_exporter.probability; dpif_ipfix_sample(&di->bridge_exporter.exporter, packet, flow, packet_delta_count, di->bridge_exporter.options->obs_domain_id, di->bridge_exporter.options->obs_point_id); + ovs_mutex_unlock(&mutex); } void dpif_ipfix_flow_sample(struct dpif_ipfix *di, struct ofpbuf *packet, const struct flow *flow, uint32_t collector_set_id, uint16_t probability, uint32_t obs_domain_id, - uint32_t obs_point_id) + uint32_t obs_point_id) OVS_EXCLUDED(mutex) { struct dpif_ipfix_flow_exporter_map_node *node; /* Use the sampling probability as an approximation of the number * of matched packets. */ uint64_t packet_delta_count = USHRT_MAX / probability; + ovs_mutex_lock(&mutex); node = dpif_ipfix_find_flow_exporter_map_node(di, collector_set_id); - - if (!node) { - return; + if (node) { + dpif_ipfix_sample(&node->exporter.exporter, packet, flow, + packet_delta_count, obs_domain_id, obs_point_id); } - - dpif_ipfix_sample(&node->exporter.exporter, packet, flow, - packet_delta_count, obs_domain_id, obs_point_id); + ovs_mutex_unlock(&mutex); } diff --git a/ofproto/ofproto-dpif-sflow.c b/ofproto/ofproto-dpif-sflow.c index 64e6c9663..ac80ff9f1 100644 --- a/ofproto/ofproto-dpif-sflow.c +++ b/ofproto/ofproto-dpif-sflow.c @@ -42,6 +42,8 @@ VLOG_DEFINE_THIS_MODULE(sflow); +static struct ovs_mutex mutex; + struct dpif_sflow_port { struct hmap_node hmap_node; /* In struct dpif_sflow's "ports" hmap. */ SFLDataSource_instance dsi; /* sFlow library's notion of port number. */ @@ -57,7 +59,7 @@ struct dpif_sflow { size_t n_flood, n_all; struct hmap ports; /* Contains "struct dpif_sflow_port"s. */ uint32_t probability; - int ref_cnt; + atomic_int ref_cnt; }; static void dpif_sflow_del_port__(struct dpif_sflow *, @@ -144,6 +146,7 @@ sflow_agent_send_packet_cb(void *ds_, SFLAgent *agent OVS_UNUSED, static struct dpif_sflow_port * dpif_sflow_find_port(const struct dpif_sflow *ds, odp_port_t odp_port) + OVS_REQ_WRLOCK(&mutex) { struct dpif_sflow_port *dsp; @@ -159,6 +162,7 @@ dpif_sflow_find_port(const struct dpif_sflow *ds, odp_port_t odp_port) static void sflow_agent_get_counters(void *ds_, SFLPoller *poller, SFL_COUNTERS_SAMPLE_TYPE *cs) + OVS_REQ_WRLOCK(&mutex) { struct dpif_sflow *ds = ds_; SFLCounters_sample_element elem; @@ -271,8 +275,8 @@ success: return true; } -void -dpif_sflow_clear(struct dpif_sflow *ds) +static void +dpif_sflow_clear__(struct dpif_sflow *ds) OVS_REQ_WRLOCK(mutex) { if (ds->sflow_agent) { sfl_agent_release(ds->sflow_agent); @@ -287,23 +291,42 @@ dpif_sflow_clear(struct dpif_sflow *ds) ds->probability = 0; } +void +dpif_sflow_clear(struct dpif_sflow *ds) OVS_EXCLUDED(mutex) +{ + ovs_mutex_lock(&mutex); + dpif_sflow_clear__(ds); + ovs_mutex_unlock(&mutex); +} + bool -dpif_sflow_is_enabled(const struct dpif_sflow *ds) +dpif_sflow_is_enabled(const struct dpif_sflow *ds) OVS_EXCLUDED(mutex) { - return ds->collectors != NULL; + bool enabled; + + ovs_mutex_lock(&mutex); + enabled = ds->collectors != NULL; + ovs_mutex_unlock(&mutex); + return enabled; } struct dpif_sflow * dpif_sflow_create(void) { + static struct ovsthread_once once = OVSTHREAD_ONCE_INITIALIZER; struct dpif_sflow *ds; + if (ovsthread_once_start(&once)) { + ovs_mutex_init(&mutex, PTHREAD_MUTEX_RECURSIVE); + ovsthread_once_done(&once); + } + ds = xcalloc(1, sizeof *ds); ds->next_tick = time_now() + 1; hmap_init(&ds->ports); ds->probability = 0; route_table_register(); - ds->ref_cnt = 1; + atomic_init(&ds->ref_cnt, 1); return ds; } @@ -313,8 +336,9 @@ dpif_sflow_ref(const struct dpif_sflow *ds_) { struct dpif_sflow *ds = CONST_CAST(struct dpif_sflow *, ds_); if (ds) { - ovs_assert(ds->ref_cnt > 0); - ds->ref_cnt++; + int orig; + atomic_add(&ds->ref_cnt, 1, &orig); + ovs_assert(orig > 0); } return ds; } @@ -323,20 +347,27 @@ dpif_sflow_ref(const struct dpif_sflow *ds_) * a value of %UINT32_MAX samples all packets and intermediate values sample * intermediate fractions of packets. */ uint32_t -dpif_sflow_get_probability(const struct dpif_sflow *ds) +dpif_sflow_get_probability(const struct dpif_sflow *ds) OVS_EXCLUDED(mutex) { - return ds->probability; + uint32_t probability; + ovs_mutex_lock(&mutex); + probability = ds->probability; + ovs_mutex_unlock(&mutex); + return probability; } void -dpif_sflow_unref(struct dpif_sflow *ds) +dpif_sflow_unref(struct dpif_sflow *ds) OVS_EXCLUDED(mutex) { + int orig; + if (!ds) { return; } - ovs_assert(ds->ref_cnt > 0); - if (!--ds->ref_cnt) { + atomic_sub(&ds->ref_cnt, 1, &orig); + ovs_assert(orig > 0); + if (orig == 1) { struct dpif_sflow_port *dsp, *next; route_table_unregister(); @@ -351,6 +382,7 @@ dpif_sflow_unref(struct dpif_sflow *ds) static void dpif_sflow_add_poller(struct dpif_sflow *ds, struct dpif_sflow_port *dsp) + OVS_REQ_WRLOCK(mutex) { SFLPoller *poller = sfl_agent_addPoller(ds->sflow_agent, &dsp->dsi, ds, sflow_agent_get_counters); @@ -361,18 +393,19 @@ dpif_sflow_add_poller(struct dpif_sflow *ds, struct dpif_sflow_port *dsp) void dpif_sflow_add_port(struct dpif_sflow *ds, struct ofport *ofport, - odp_port_t odp_port) + odp_port_t odp_port) OVS_EXCLUDED(mutex) { struct dpif_sflow_port *dsp; int ifindex; + ovs_mutex_lock(&mutex); dpif_sflow_del_port(ds, odp_port); ifindex = netdev_get_ifindex(ofport->netdev); if (ifindex <= 0) { /* Not an ifindex port, so do not add a cross-reference to it here */ - return; + goto out; } /* Add to table of ports. */ @@ -386,10 +419,14 @@ dpif_sflow_add_port(struct dpif_sflow *ds, struct ofport *ofport, if (ds->sflow_agent) { dpif_sflow_add_poller(ds, dsp); } + +out: + ovs_mutex_unlock(&mutex); } static void dpif_sflow_del_port__(struct dpif_sflow *ds, struct dpif_sflow_port *dsp) + OVS_REQ_WRLOCK(mutex) { if (ds->sflow_agent) { sfl_agent_removePoller(ds->sflow_agent, &dsp->dsi); @@ -401,16 +438,22 @@ dpif_sflow_del_port__(struct dpif_sflow *ds, struct dpif_sflow_port *dsp) void dpif_sflow_del_port(struct dpif_sflow *ds, odp_port_t odp_port) + OVS_EXCLUDED(mutex) { - struct dpif_sflow_port *dsp = dpif_sflow_find_port(ds, odp_port); + struct dpif_sflow_port *dsp; + + ovs_mutex_lock(&mutex); + dsp = dpif_sflow_find_port(ds, odp_port); if (dsp) { dpif_sflow_del_port__(ds, dsp); } + ovs_mutex_unlock(&mutex); } void dpif_sflow_set_options(struct dpif_sflow *ds, const struct ofproto_sflow_options *options) + OVS_EXCLUDED(mutex) { struct dpif_sflow_port *dsp; bool options_changed; @@ -421,11 +464,12 @@ dpif_sflow_set_options(struct dpif_sflow *ds, uint32_t dsIndex; SFLSampler *sampler; + ovs_mutex_lock(&mutex); if (sset_is_empty(&options->targets) || !options->sampling_rate) { /* No point in doing any work if there are no targets or nothing to * sample. */ - dpif_sflow_clear(ds); - return; + dpif_sflow_clear__(ds); + goto out; } options_changed = (!ds->options @@ -442,8 +486,8 @@ dpif_sflow_set_options(struct dpif_sflow *ds, if (ds->collectors == NULL) { VLOG_WARN_RL(&rl, "no collectors could be initialized, " "sFlow disabled"); - dpif_sflow_clear(ds); - return; + dpif_sflow_clear__(ds); + goto out; } } @@ -451,13 +495,13 @@ dpif_sflow_set_options(struct dpif_sflow *ds, if (!sflow_choose_agent_address(options->agent_device, &options->targets, options->control_ip, &agentIP)) { - dpif_sflow_clear(ds); - return; + dpif_sflow_clear__(ds); + goto out; } /* Avoid reconfiguring if options didn't change. */ if (!options_changed) { - return; + goto out; } ofproto_sflow_options_destroy(ds->options); ds->options = ofproto_sflow_options_clone(options); @@ -502,20 +546,31 @@ dpif_sflow_set_options(struct dpif_sflow *ds, HMAP_FOR_EACH (dsp, hmap_node, &ds->ports) { dpif_sflow_add_poller(ds, dsp); } + + +out: + ovs_mutex_unlock(&mutex); } int dpif_sflow_odp_port_to_ifindex(const struct dpif_sflow *ds, - odp_port_t odp_port) + odp_port_t odp_port) OVS_EXCLUDED(mutex) { - struct dpif_sflow_port *dsp = dpif_sflow_find_port(ds, odp_port); - return dsp ? SFL_DS_INDEX(dsp->dsi) : 0; + struct dpif_sflow_port *dsp; + int ret; + + ovs_mutex_lock(&mutex); + dsp = dpif_sflow_find_port(ds, odp_port); + ret = dsp ? SFL_DS_INDEX(dsp->dsi) : 0; + ovs_mutex_unlock(&mutex); + return ret; } void dpif_sflow_received(struct dpif_sflow *ds, struct ofpbuf *packet, const struct flow *flow, odp_port_t odp_in_port, const union user_action_cookie *cookie) + OVS_EXCLUDED(mutex) { SFL_FLOW_SAMPLE_TYPE fs; SFLFlow_sample_element hdrElem; @@ -525,9 +580,10 @@ dpif_sflow_received(struct dpif_sflow *ds, struct ofpbuf *packet, struct dpif_sflow_port *in_dsp; ovs_be16 vlan_tci; + ovs_mutex_lock(&mutex); sampler = ds->sflow_agent->samplers; if (!sampler) { - return; + goto out; } /* Build a flow sample. */ @@ -576,12 +632,16 @@ dpif_sflow_received(struct dpif_sflow *ds, struct ofpbuf *packet, SFLADD_ELEMENT(&fs, &hdrElem); SFLADD_ELEMENT(&fs, &switchElem); sfl_sampler_writeFlowSample(sampler, &fs); + +out: + ovs_mutex_unlock(&mutex); } void -dpif_sflow_run(struct dpif_sflow *ds) +dpif_sflow_run(struct dpif_sflow *ds) OVS_EXCLUDED(mutex) { - if (dpif_sflow_is_enabled(ds)) { + ovs_mutex_lock(&mutex); + if (ds->collectors != NULL) { time_t now = time_now(); route_table_run(); if (now >= ds->next_tick) { @@ -589,12 +649,15 @@ dpif_sflow_run(struct dpif_sflow *ds) ds->next_tick = now + 1; } } + ovs_mutex_unlock(&mutex); } void -dpif_sflow_wait(struct dpif_sflow *ds) +dpif_sflow_wait(struct dpif_sflow *ds) OVS_EXCLUDED(mutex) { - if (dpif_sflow_is_enabled(ds)) { + ovs_mutex_lock(&mutex); + if (ds->collectors != NULL) { poll_timer_wait_until(ds->next_tick * 1000LL); } + ovs_mutex_unlock(&mutex); } diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c index eb4ed6957..e5556036b 100644 --- a/ofproto/ofproto-dpif-xlate.c +++ b/ofproto/ofproto-dpif-xlate.c @@ -400,7 +400,10 @@ xlate_ofport_remove(struct ofport_dpif *ofport) xport->peer = NULL; } - list_remove(&xport->bundle_node); + if (xport->xbundle) { + list_remove(&xport->bundle_node); + } + hmap_remove(&xports, &xport->hmap_node); hmap_remove(&xport->xbridge->xports, &xport->ofp_node); diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c index 79e23a406..839de6955 100644 --- a/ofproto/ofproto-dpif.c +++ b/ofproto/ofproto-dpif.c @@ -638,6 +638,12 @@ port_open_type(const char *datapath_type, const char *port_type) /* Type functions. */ +static void process_dpif_port_changes(struct dpif_backer *); +static void process_dpif_all_ports_changed(struct dpif_backer *); +static void process_dpif_port_change(struct dpif_backer *, + const char *devname); +static void process_dpif_port_error(struct dpif_backer *, int error); + static struct ofproto_dpif * lookup_ofproto_dpif_by_port_name(const char *name) { @@ -657,8 +663,6 @@ type_run(const char *type) { static long long int push_timer = LLONG_MIN; struct dpif_backer *backer; - char *devname; - int error; backer = shash_find_data(&all_dpif_backers, type); if (!backer) { @@ -683,6 +687,8 @@ type_run(const char *type) * and the configuration has now changed to "false", enable receiving * packets from the datapath. */ if (!backer->recv_set_enable && !ofproto_get_flow_restore_wait()) { + int error; + backer->recv_set_enable = true; error = dpif_recv_set(backer->dpif, backer->recv_set_enable); @@ -830,58 +836,7 @@ type_run(const char *type) timer_set_duration(&backer->next_expiration, delay); } - /* Check for port changes in the dpif. */ - while ((error = dpif_port_poll(backer->dpif, &devname)) == 0) { - struct ofproto_dpif *ofproto; - struct dpif_port port; - - /* Don't report on the datapath's device. */ - if (!strcmp(devname, dpif_base_name(backer->dpif))) { - goto next; - } - - HMAP_FOR_EACH (ofproto, all_ofproto_dpifs_node, - &all_ofproto_dpifs) { - if (simap_contains(&ofproto->backer->tnl_backers, devname)) { - goto next; - } - } - - ofproto = lookup_ofproto_dpif_by_port_name(devname); - if (dpif_port_query_by_name(backer->dpif, devname, &port)) { - /* The port was removed. If we know the datapath, - * report it through poll_set(). If we don't, it may be - * notifying us of a removal we initiated, so ignore it. - * If there's a pending ENOBUFS, let it stand, since - * everything will be reevaluated. */ - if (ofproto && ofproto->port_poll_errno != ENOBUFS) { - sset_add(&ofproto->port_poll_set, devname); - ofproto->port_poll_errno = 0; - } - } else if (!ofproto) { - /* The port was added, but we don't know with which - * ofproto we should associate it. Delete it. */ - dpif_port_del(backer->dpif, port.port_no); - } - dpif_port_destroy(&port); - - next: - free(devname); - } - - if (error != EAGAIN) { - struct ofproto_dpif *ofproto; - - /* There was some sort of error, so propagate it to all - * ofprotos that use this backer. */ - HMAP_FOR_EACH (ofproto, all_ofproto_dpifs_node, - &all_ofproto_dpifs) { - if (ofproto->backer == backer) { - sset_clear(&ofproto->port_poll_set); - ofproto->port_poll_errno = error; - } - } - } + process_dpif_port_changes(backer); if (backer->governor) { size_t n_subfacets; @@ -904,6 +859,133 @@ type_run(const char *type) return 0; } +/* Check for and handle port changes in 'backer''s dpif. */ +static void +process_dpif_port_changes(struct dpif_backer *backer) +{ + for (;;) { + char *devname; + int error; + + error = dpif_port_poll(backer->dpif, &devname); + switch (error) { + case EAGAIN: + return; + + case ENOBUFS: + process_dpif_all_ports_changed(backer); + break; + + case 0: + process_dpif_port_change(backer, devname); + free(devname); + break; + + default: + process_dpif_port_error(backer, error); + break; + } + } +} + +static void +process_dpif_all_ports_changed(struct dpif_backer *backer) +{ + struct ofproto_dpif *ofproto; + struct dpif_port dpif_port; + struct dpif_port_dump dump; + struct sset devnames; + const char *devname; + + sset_init(&devnames); + HMAP_FOR_EACH (ofproto, all_ofproto_dpifs_node, &all_ofproto_dpifs) { + if (ofproto->backer == backer) { + struct ofport *ofport; + + HMAP_FOR_EACH (ofport, hmap_node, &ofproto->up.ports) { + sset_add(&devnames, netdev_get_name(ofport->netdev)); + } + } + } + DPIF_PORT_FOR_EACH (&dpif_port, &dump, backer->dpif) { + sset_add(&devnames, dpif_port.name); + } + + SSET_FOR_EACH (devname, &devnames) { + process_dpif_port_change(backer, devname); + } + sset_destroy(&devnames); +} + +static void +process_dpif_port_change(struct dpif_backer *backer, const char *devname) +{ + struct ofproto_dpif *ofproto; + struct dpif_port port; + + /* Don't report on the datapath's device. */ + if (!strcmp(devname, dpif_base_name(backer->dpif))) { + return; + } + + HMAP_FOR_EACH (ofproto, all_ofproto_dpifs_node, + &all_ofproto_dpifs) { + if (simap_contains(&ofproto->backer->tnl_backers, devname)) { + return; + } + } + + ofproto = lookup_ofproto_dpif_by_port_name(devname); + if (dpif_port_query_by_name(backer->dpif, devname, &port)) { + /* The port was removed. If we know the datapath, + * report it through poll_set(). If we don't, it may be + * notifying us of a removal we initiated, so ignore it. + * If there's a pending ENOBUFS, let it stand, since + * everything will be reevaluated. */ + if (ofproto && ofproto->port_poll_errno != ENOBUFS) { + sset_add(&ofproto->port_poll_set, devname); + ofproto->port_poll_errno = 0; + } + } else if (!ofproto) { + /* The port was added, but we don't know with which + * ofproto we should associate it. Delete it. */ + dpif_port_del(backer->dpif, port.port_no); + } else { + struct ofport_dpif *ofport; + + ofport = ofport_dpif_cast(shash_find_data( + &ofproto->up.port_by_name, devname)); + if (ofport + && ofport->odp_port != port.port_no + && !odp_port_to_ofport(backer, port.port_no)) + { + /* 'ofport''s datapath port number has changed from + * 'ofport->odp_port' to 'port.port_no'. Update our internal data + * structures to match. */ + hmap_remove(&backer->odp_to_ofport_map, &ofport->odp_port_node); + ofport->odp_port = port.port_no; + hmap_insert(&backer->odp_to_ofport_map, &ofport->odp_port_node, + hash_odp_port(port.port_no)); + backer->need_revalidate = REV_RECONFIGURE; + } + } + dpif_port_destroy(&port); +} + +/* Propagate 'error' to all ofprotos based on 'backer'. */ +static void +process_dpif_port_error(struct dpif_backer *backer, int error) +{ + struct ofproto_dpif *ofproto; + + HMAP_FOR_EACH (ofproto, all_ofproto_dpifs_node, &all_ofproto_dpifs) { + if (ofproto->backer == backer) { + sset_clear(&ofproto->port_poll_set); + ofproto->port_poll_errno = error; + } + } +} + static int dpif_backer_run_fast(struct dpif_backer *backer, int max_batch) { @@ -3458,7 +3540,7 @@ handle_flow_miss_with_facet(struct flow_miss *miss, struct facet *facet, op->xout_garbage = false; op->dpif_op.type = DPIF_OP_FLOW_PUT; op->subfacet = subfacet; - put->flags = DPIF_FP_CREATE | DPIF_FP_MODIFY; + put->flags = DPIF_FP_CREATE; put->key = miss->key; put->key_len = miss->key_len; put->mask = op->mask.data; @@ -3717,15 +3799,20 @@ handle_miss_upcalls(struct dpif_backer *backer, struct dpif_upcall *upcalls, drop_key = drop_key_lookup(backer, upcall->key, upcall->key_len); if (!drop_key) { - drop_key = xmalloc(sizeof *drop_key); - drop_key->key = xmemdup(upcall->key, upcall->key_len); - drop_key->key_len = upcall->key_len; - - hmap_insert(&backer->drop_keys, &drop_key->hmap_node, - hash_bytes(drop_key->key, drop_key->key_len, 0)); - dpif_flow_put(backer->dpif, DPIF_FP_CREATE | DPIF_FP_MODIFY, - drop_key->key, drop_key->key_len, - NULL, 0, NULL, 0, NULL); + int ret; + ret = dpif_flow_put(backer->dpif, + DPIF_FP_CREATE | DPIF_FP_MODIFY, + upcall->key, upcall->key_len, + NULL, 0, NULL, 0, NULL); + + if (!ret) { + drop_key = xmalloc(sizeof *drop_key); + drop_key->key = xmemdup(upcall->key, upcall->key_len); + drop_key->key_len = upcall->key_len; + + hmap_insert(&backer->drop_keys, &drop_key->hmap_node, + hash_bytes(drop_key->key, drop_key->key_len, 0)); + } } continue; } @@ -5006,7 +5093,8 @@ subfacet_install(struct subfacet *subfacet, const struct ofpbuf *odp_actions, enum dpif_flow_put_flags flags; int ret; - flags = DPIF_FP_CREATE | DPIF_FP_MODIFY; + flags = subfacet->path == SF_NOT_INSTALLED ? DPIF_FP_CREATE + : DPIF_FP_MODIFY; if (stats) { flags |= DPIF_FP_ZERO_STATS; } diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c index 9119235e9..432aef371 100644 --- a/ofproto/ofproto.c +++ b/ofproto/ofproto.c @@ -3170,18 +3170,26 @@ handle_aggregate_stats_request(struct ofconn *ofconn, struct queue_stats_cbdata { struct ofport *ofport; struct list replies; + long long int now; }; static void put_queue_stats(struct queue_stats_cbdata *cbdata, uint32_t queue_id, const struct netdev_queue_stats *stats) { + struct ofputil_queue_stats oqs; - struct ofputil_queue_stats oqs = { - .port_no = cbdata->ofport->pp.port_no, - .queue_id = queue_id, - .stats = *stats, - }; + oqs.port_no = cbdata->ofport->pp.port_no; + oqs.queue_id = queue_id; + oqs.tx_bytes = stats->tx_bytes; + oqs.tx_packets = stats->tx_packets; + oqs.tx_errors = stats->tx_errors; + if (stats->created != LLONG_MIN) { + calc_duration(stats->created, cbdata->now, + &oqs.duration_sec, &oqs.duration_nsec); + } else { + oqs.duration_sec = oqs.duration_nsec = UINT32_MAX; + } ofputil_append_queue_stat(&cbdata->replies, &oqs); } @@ -3228,6 +3236,7 @@ handle_queue_stats_request(struct ofconn *ofconn, COVERAGE_INC(ofproto_queue_req); ofpmp_init(&cbdata.replies, rq); + cbdata.now = time_msec(); error = ofputil_decode_queue_stats_request(rq, &oqsr); if (error) { diff --git a/ofproto/tunnel.c b/ofproto/tunnel.c index 4b7f3046c..ce325c006 100644 --- a/ofproto/tunnel.c +++ b/ofproto/tunnel.c @@ -53,25 +53,36 @@ struct tnl_port { struct tnl_match match; }; -static struct hmap tnl_match_map = HMAP_INITIALIZER(&tnl_match_map); -static struct hmap ofport_map = HMAP_INITIALIZER(&ofport_map); +static struct ovs_rwlock rwlock = OVS_RWLOCK_INITIALIZER; + +static struct hmap tnl_match_map__ = HMAP_INITIALIZER(&tnl_match_map__); +static struct hmap *tnl_match_map OVS_GUARDED_BY(rwlock) = &tnl_match_map__; + +static struct hmap ofport_map__ = HMAP_INITIALIZER(&ofport_map__); +static struct hmap *ofport_map OVS_GUARDED_BY(rwlock) = &ofport_map__; static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5); static struct vlog_rate_limit dbg_rl = VLOG_RATE_LIMIT_INIT(60, 60); -static struct tnl_port *tnl_find(struct tnl_match *); -static struct tnl_port *tnl_find_exact(struct tnl_match *); -static struct tnl_port *tnl_find_ofport(const struct ofport_dpif *); +static struct tnl_port *tnl_find(struct tnl_match *) OVS_REQ_RDLOCK(&rwlock); +static struct tnl_port *tnl_find_exact(struct tnl_match *) + OVS_REQ_RDLOCK(&rwlock); +static struct tnl_port *tnl_find_ofport(const struct ofport_dpif *) + OVS_REQ_RDLOCK(&rwlock); static uint32_t tnl_hash(struct tnl_match *); static void tnl_match_fmt(const struct tnl_match *, struct ds *); -static char *tnl_port_fmt(const struct tnl_port *); -static void tnl_port_mod_log(const struct tnl_port *, const char *action); -static const char *tnl_port_get_name(const struct tnl_port *); +static char *tnl_port_fmt(const struct tnl_port *) OVS_REQ_RDLOCK(&rwlock); +static void tnl_port_mod_log(const struct tnl_port *, const char *action) + OVS_REQ_RDLOCK(&rwlock); +static const char *tnl_port_get_name(const struct tnl_port *) + OVS_REQ_RDLOCK(&rwlock); +static void tnl_port_del__(const struct ofport_dpif *) OVS_REQ_WRLOCK(&rwlock); static bool tnl_port_add__(const struct ofport_dpif *ofport, const struct netdev *netdev, odp_port_t odp_port, bool warn) + OVS_REQ_WRLOCK(&rwlock) { const struct netdev_tunnel_config *cfg; struct tnl_port *existing_port; @@ -108,8 +119,8 @@ tnl_port_add__(const struct ofport_dpif *ofport, const struct netdev *netdev, return false; } - hmap_insert(&ofport_map, &tnl_port->ofport_node, hash_pointer(ofport, 0)); - hmap_insert(&tnl_match_map, &tnl_port->match_node, + hmap_insert(ofport_map, &tnl_port->ofport_node, hash_pointer(ofport, 0)); + hmap_insert(tnl_match_map, &tnl_port->match_node, tnl_hash(&tnl_port->match)); tnl_port_mod_log(tnl_port, "adding"); return true; @@ -120,9 +131,11 @@ tnl_port_add__(const struct ofport_dpif *ofport, const struct netdev *netdev, * tunnel. */ void tnl_port_add(const struct ofport_dpif *ofport, const struct netdev *netdev, - odp_port_t odp_port) + odp_port_t odp_port) OVS_EXCLUDED(rwlock) { + ovs_rwlock_wrlock(&rwlock); tnl_port_add__(ofport, netdev, odp_port, true); + ovs_rwlock_unlock(&rwlock); } /* Checks if the tunnel represented by 'ofport' reconfiguration due to changes @@ -132,37 +145,55 @@ tnl_port_add(const struct ofport_dpif *ofport, const struct netdev *netdev, bool tnl_port_reconfigure(const struct ofport_dpif *ofport, const struct netdev *netdev, odp_port_t odp_port) + OVS_EXCLUDED(rwlock) { - struct tnl_port *tnl_port = tnl_find_ofport(ofport); + struct tnl_port *tnl_port; + bool changed = false; + ovs_rwlock_wrlock(&rwlock); + tnl_port = tnl_find_ofport(ofport); if (!tnl_port) { - return tnl_port_add__(ofport, netdev, odp_port, false); + changed = tnl_port_add__(ofport, netdev, odp_port, false); } else if (tnl_port->netdev != netdev || tnl_port->match.odp_port != odp_port || tnl_port->netdev_seq != netdev_change_seq(netdev)) { VLOG_DBG("reconfiguring %s", tnl_port_get_name(tnl_port)); - tnl_port_del(ofport); - tnl_port_add(ofport, netdev, odp_port); - return true; + tnl_port_del__(ofport); + tnl_port_add__(ofport, netdev, odp_port, true); + changed = true; } - return false; + ovs_rwlock_unlock(&rwlock); + return changed; } -/* Removes 'ofport' from the module. */ -void -tnl_port_del(const struct ofport_dpif *ofport) +static void +tnl_port_del__(const struct ofport_dpif *ofport) OVS_REQ_WRLOCK(rwlock) { - struct tnl_port *tnl_port = ofport ? tnl_find_ofport(ofport) : NULL; + struct tnl_port *tnl_port; + + if (!ofport) { + return; + } + tnl_port = tnl_find_ofport(ofport); if (tnl_port) { tnl_port_mod_log(tnl_port, "removing"); - hmap_remove(&tnl_match_map, &tnl_port->match_node); - hmap_remove(&ofport_map, &tnl_port->ofport_node); + hmap_remove(tnl_match_map, &tnl_port->match_node); + hmap_remove(ofport_map, &tnl_port->ofport_node); netdev_close(tnl_port->netdev); free(tnl_port); } } +/* Removes 'ofport' from the module. */ +void +tnl_port_del(const struct ofport_dpif *ofport) OVS_EXCLUDED(rwlock) +{ + ovs_rwlock_wrlock(&rwlock); + tnl_port_del__(ofport); + ovs_rwlock_unlock(&rwlock); +} + /* Looks in the table of tunnels for a tunnel matching the metadata in 'flow'. * Returns the 'ofport' corresponding to the new in_port, or a null pointer if * none is found. @@ -170,9 +201,10 @@ tnl_port_del(const struct ofport_dpif *ofport) * Callers should verify that 'flow' needs to be received by calling * tnl_port_should_receive() before this function. */ const struct ofport_dpif * -tnl_port_receive(const struct flow *flow) +tnl_port_receive(const struct flow *flow) OVS_EXCLUDED(rwlock) { char *pre_flow_str = NULL; + const struct ofport_dpif *ofport; struct tnl_port *tnl_port; struct tnl_match match; @@ -183,14 +215,16 @@ tnl_port_receive(const struct flow *flow) match.in_key = flow->tunnel.tun_id; match.skb_mark = flow->skb_mark; + ovs_rwlock_rdlock(&rwlock); tnl_port = tnl_find(&match); + ofport = tnl_port ? tnl_port->ofport : NULL; if (!tnl_port) { struct ds ds = DS_EMPTY_INITIALIZER; tnl_match_fmt(&match, &ds); VLOG_WARN_RL(&rl, "receive tunnel port not found (%s)", ds_cstr(&ds)); ds_destroy(&ds); - return NULL; + goto out; } if (!VLOG_DROP_DBG(&dbg_rl)) { @@ -209,7 +243,10 @@ tnl_port_receive(const struct flow *flow) free(pre_flow_str); free(post_flow_str); } - return tnl_port->ofport; + +out: + ovs_rwlock_unlock(&rwlock); + return ofport; } /* Given that 'flow' should be output to the ofport corresponding to @@ -218,14 +255,18 @@ tnl_port_receive(const struct flow *flow) * shouldn't occur. */ odp_port_t tnl_port_send(const struct ofport_dpif *ofport, struct flow *flow, - struct flow_wildcards *wc) + struct flow_wildcards *wc) OVS_EXCLUDED(rwlock) { - struct tnl_port *tnl_port = tnl_find_ofport(ofport); const struct netdev_tunnel_config *cfg; + struct tnl_port *tnl_port; char *pre_flow_str = NULL; + odp_port_t out_port; + ovs_rwlock_rdlock(&rwlock); + tnl_port = tnl_find_ofport(ofport); + out_port = tnl_port ? tnl_port->match.odp_port : ODPP_NONE; if (!tnl_port) { - return ODPP_NONE; + goto out; } cfg = netdev_get_tunnel_config(tnl_port->netdev); @@ -289,7 +330,9 @@ tnl_port_send(const struct ofport_dpif *ofport, struct flow *flow, free(post_flow_str); } - return tnl_port->match.odp_port; +out: + ovs_rwlock_unlock(&rwlock); + return out_port; } static uint32_t @@ -300,12 +343,12 @@ tnl_hash(struct tnl_match *match) } static struct tnl_port * -tnl_find_ofport(const struct ofport_dpif *ofport) +tnl_find_ofport(const struct ofport_dpif *ofport) OVS_REQ_RDLOCK(rwlock) { struct tnl_port *tnl_port; HMAP_FOR_EACH_IN_BUCKET (tnl_port, ofport_node, hash_pointer(ofport, 0), - &ofport_map) { + ofport_map) { if (tnl_port->ofport == ofport) { return tnl_port; } @@ -314,12 +357,12 @@ tnl_find_ofport(const struct ofport_dpif *ofport) } static struct tnl_port * -tnl_find_exact(struct tnl_match *match) +tnl_find_exact(struct tnl_match *match) OVS_REQ_RDLOCK(rwlock) { struct tnl_port *tnl_port; HMAP_FOR_EACH_WITH_HASH (tnl_port, match_node, tnl_hash(match), - &tnl_match_map) { + tnl_match_map) { if (!memcmp(match, &tnl_port->match, sizeof *match)) { return tnl_port; } @@ -328,7 +371,7 @@ tnl_find_exact(struct tnl_match *match) } static struct tnl_port * -tnl_find(struct tnl_match *match_) +tnl_find(struct tnl_match *match_) OVS_REQ_RDLOCK(rwlock) { struct tnl_match match = *match_; struct tnl_port *tnl_port; @@ -383,6 +426,7 @@ tnl_find(struct tnl_match *match_) static void tnl_match_fmt(const struct tnl_match *match, struct ds *ds) + OVS_REQ_RDLOCK(rwlock) { if (!match->ip_dst_flow) { ds_put_format(ds, IP_FMT"->"IP_FMT, IP_ARGS(match->ip_src), @@ -405,6 +449,7 @@ tnl_match_fmt(const struct tnl_match *match, struct ds *ds) static void tnl_port_mod_log(const struct tnl_port *tnl_port, const char *action) + OVS_REQ_RDLOCK(rwlock) { if (VLOG_IS_DBG_ENABLED()) { struct ds ds = DS_EMPTY_INITIALIZER; @@ -417,7 +462,7 @@ tnl_port_mod_log(const struct tnl_port *tnl_port, const char *action) } static char * -tnl_port_fmt(const struct tnl_port *tnl_port) +tnl_port_fmt(const struct tnl_port *tnl_port) OVS_REQ_RDLOCK(rwlock) { const struct netdev_tunnel_config *cfg = netdev_get_tunnel_config(tnl_port->netdev); @@ -467,7 +512,7 @@ tnl_port_fmt(const struct tnl_port *tnl_port) } static const char * -tnl_port_get_name(const struct tnl_port *tnl_port) +tnl_port_get_name(const struct tnl_port *tnl_port) OVS_REQ_RDLOCK(rwlock) { return netdev_get_name(tnl_port->netdev); } diff --git a/ovsdb/automake.mk b/ovsdb/automake.mk index d2e3f9ad9..3884d3ffd 100644 --- a/ovsdb/automake.mk +++ b/ovsdb/automake.mk @@ -89,10 +89,8 @@ BUILT_SOURCES += $(OVSIDL_BUILT) $(OVSIDL_BUILT): ovsdb/ovsdb-idlc.in # ovsdb-doc -EXTRA_DIST += ovsdb/ovsdb-doc.in -noinst_SCRIPTS += ovsdb/ovsdb-doc -DISTCLEANFILES += ovsdb/ovsdb-doc -OVSDB_DOC = $(run_python) $(srcdir)/ovsdb/ovsdb-doc.in +EXTRA_DIST += ovsdb/ovsdb-doc +OVSDB_DOC = $(run_python) $(srcdir)/ovsdb/ovsdb-doc # ovsdb-dot EXTRA_DIST += ovsdb/ovsdb-dot.in ovsdb/dot2pic diff --git a/ovsdb/ovsdb-doc.in b/ovsdb/ovsdb-doc similarity index 95% rename from ovsdb/ovsdb-doc.in rename to ovsdb/ovsdb-doc index aa4fae2e6..662ed974a 100755 --- a/ovsdb/ovsdb-doc.in +++ b/ovsdb/ovsdb-doc @@ -1,4 +1,4 @@ -#! @PYTHON@ +#! /usr/bin/python from datetime import date import getopt @@ -251,7 +251,7 @@ def tableToNroff(schema, tableXml): s += body return s -def docsToNroff(schemaFile, xmlFile, erFile, title=None): +def docsToNroff(schemaFile, xmlFile, erFile, title=None, version=None): schema = ovs.db.schema.DbSchema.from_json(ovs.json.from_file(schemaFile)) doc = xml.dom.minidom.parse(xmlFile).documentElement @@ -262,10 +262,13 @@ def docsToNroff(schemaFile, xmlFile, erFile, title=None): if title == None: title = schema.name + if version == None: + version = "UNKNOWN" + # Putting '\" p as the first line tells "man" that the manpage # needs to be preprocessed by "pic". s = r''''\" p -.TH @VERSION@ 5 "%s" "Open vSwitch" "Open vSwitch Manual" +.TH "%s" 5 "%s" "Open vSwitch" "Open vSwitch Manual" .\" -*- nroff -*- .de TQ . br @@ -281,7 +284,7 @@ def docsToNroff(schemaFile, xmlFile, erFile, title=None): .SH NAME %s \- %s database schema .PP -''' % (title, textToNroff(title), schema.name) +''' % (title, version, textToNroff(schema.name), schema.name) tables = "" introNodes = [] @@ -357,8 +360,8 @@ where SCHEMA is an OVSDB schema in JSON format The following options are also available: --er-diagram=DIAGRAM.PIC include E-R diagram from DIAGRAM.PIC --title=TITLE use TITLE as title instead of schema name - -h, --help display this help message - -V, --version display version information\ + --version=VERSION use VERSION to display on document footer + -h, --help display this help message\ """ % {'argv0': argv0} sys.exit(0) @@ -367,22 +370,23 @@ if __name__ == "__main__": try: options, args = getopt.gnu_getopt(sys.argv[1:], 'hV', ['er-diagram=', 'title=', - 'help', 'version']) + 'version=', 'help']) except getopt.GetoptError, geo: sys.stderr.write("%s: %s\n" % (argv0, geo.msg)) sys.exit(1) er_diagram = None title = None + version = None for key, value in options: if key == '--er-diagram': er_diagram = value elif key == '--title': title = value + elif key == '--version': + version = value elif key in ['-h', '--help']: usage() - elif key in ['-V', '--version']: - print "ovsdb-doc (Open vSwitch) @VERSION@" else: sys.exit(0) @@ -392,7 +396,7 @@ if __name__ == "__main__": sys.exit(1) # XXX we should warn about undocumented tables or columns - s = docsToNroff(args[0], args[1], er_diagram) + s = docsToNroff(args[0], args[1], er_diagram, title, version) for line in s.split("\n"): line = line.strip() if len(line): diff --git a/ovsdb/ovsdb-idlc.in b/ovsdb/ovsdb-idlc.in index 6d49dd68d..ec1c65536 100755 --- a/ovsdb/ovsdb-idlc.in +++ b/ovsdb/ovsdb-idlc.in @@ -187,6 +187,7 @@ def printCIDLSource(schemaFile): #include #include %s #include +#include "ovs-thread.h" #include "ovsdb-data.h" #include "ovsdb-error.h" #include "util.h" @@ -643,6 +644,7 @@ void if (inited) { return; } + assert_single_threaded(); inited = true; """ % prefix for tableName, table in sorted(schema.tables.iteritems()): diff --git a/tests/automake.mk b/tests/automake.mk index 755d88ef9..f531bf9f3 100644 --- a/tests/automake.mk +++ b/tests/automake.mk @@ -21,6 +21,7 @@ TESTSUITE_AT = \ tests/ovs-ofctl.at \ tests/odp.at \ tests/multipath.at \ + tests/bfd.at \ tests/lacp.at \ tests/learn.at \ tests/vconn.at \ diff --git a/tests/bfd.at b/tests/bfd.at new file mode 100644 index 000000000..d95f8ab7a --- /dev/null +++ b/tests/bfd.at @@ -0,0 +1,217 @@ +AT_BANNER([bfd]) + +m4_define([BFD_CHECK], [ +AT_CHECK([ovs-appctl bfd/show $1 | sed -e '/Time:/d' | sed -e '/Discriminator/d' | sed -e '/Interval:/d'],[0], +[dnl + Forwarding: $2 + Detect Multiplier: 3 + Concatenated Path Down: $3 + + Local Flags: $4 + Local Session State: $5 + Local Diagnostic: $6 + + Remote Flags: $7 + Remote Session State: $8 + Remote Diagnostic: $9 +]) +]) + +m4_define([BFD_CHECK_TX], [ +AT_CHECK([ovs-appctl bfd/show $1 | sed -n '/TX Interval/p'],[0], +[dnl + TX Interval: Approx 1000ms + Local Minimum TX Interval: $2 + Remote Minimum TX Interval: $3 +]) +]) + +m4_define([BFD_CHECK_RX], [ +AT_CHECK([ovs-appctl bfd/show $1 | sed -n '/RX Interval/p'],[0], +[dnl + RX Interval: Approx $2 + Local Minimum RX Interval: $2 + Remote Minimum RX Interval: $3 +]) +]) +AT_SETUP([bfd - basic config on different bridges]) +ovs-appctl time/stop +#Create 2 bridges connected by patch ports and enable BFD +OVS_VSWITCHD_START( + [add-br br1 -- \ + set bridge br1 datapath-type=dummy \ + other-config:hwaddr=aa:55:aa:56:00:00 -- \ + add-port br1 p1 -- set Interface p1 type=patch \ + options:peer=p0 -- \ + add-port br0 p0 -- set Interface p0 type=patch \ + options:peer=p1 -- \ + set Interface p0 bfd:enable=true -- \ + set Interface p1 bfd:enable=true ]) +for i in `seq 0 40`; do ovs-appctl time/warp 100; done + +#Verify that BFD has been enabled on both interfaces. +BFD_CHECK([p1], [true], [false], [none], [up], [No Diagnostic], [none], [up], [No Diagnostic]) +BFD_CHECK([p0], [true], [false], [none], [up], [No Diagnostic], [none], [up], [No Diagnostic]) + +AT_CHECK([ ovs-vsctl set interface p0 bfd:enable=false]) +for i in `seq 0 40`; do ovs-appctl time/warp 100; done +BFD_CHECK([p1], [false], [false], [none], [down], [Control Detection Time Expired], [none], [down], [No Diagnostic]) + +AT_CHECK([ ovs-vsctl set interface p0 bfd:enable=true]) +for i in `seq 0 40`; do ovs-appctl time/warp 100; done +BFD_CHECK([p1], [true], [false], [none], [up], [Control Detection Time Expired], [none], [up], [No Diagnostic]) +BFD_CHECK([p0], [true], [false], [none], [up], [No Diagnostic], [none], [up], [Control Detection Time Expired]) + +ovs-vsctl del-br br0 +AT_CHECK([ovs-appctl bfd/show p0], [2],[ignore], [no such bfd object +ovs-appctl: ovs-vswitchd: server returned an error +]) +ovs-vsctl del-br br1 +#Check that the entries are gone. +AT_CHECK([ovs-appctl bfd/show p1], [2],[ignore], [no such bfd object +ovs-appctl: ovs-vswitchd: server returned an error +]) + +OVS_VSWITCHD_STOP +AT_CLEANUP + + +AT_SETUP([bfd - Verify tunnel down detection]) +#Create 3 bridges - br-bfd0, br-bfd1 and br-sw which is midway between the two. br2 is +#connected to br-bfd0 and br-bfd1 through patch ports p0-2 and p1-2. Enable BFD on +#interfaces in br-bfd0 and br-bfd1. When br-sw is dropping all packets, BFD should detect +# that the tunnel is down, and come back up when br-sw is working fine. + +ovs-appctl time/stop +OVS_VSWITCHD_START( + [add-br br-bfd0 -- \ + set bridge br-bfd0 datapath-type=dummy \ + other-config:hwaddr=aa:55:aa:56:00:00 -- \ + add-br br-bfd1 -- \ + set bridge br-bfd1 datapath-type=dummy \ + other-config:hwaddr=aa:55:aa:57:00:00 -- \ + add-br br-sw -- \ + set bridge br-sw datapath-type=dummy \ + other-config:hwaddr=aa:55:aa:58:00:00 -- \ + add-port br-sw p1-sw -- set Interface p1-sw type=patch \ + options:peer=p1 -- \ + add-port br-sw p0-sw -- set Interface p0-sw type=patch \ + options:peer=p0 -- \ + add-port br-bfd1 p1 -- set Interface p1 type=patch \ + options:peer=p1-sw bfd:enable=true -- \ + add-port br-bfd0 p0 -- set Interface p0 type=patch \ + options:peer=p0-sw bfd:enable=true --]) + + +#Create 2 bridges connected by patch ports and enable BFD + +AT_CHECK([ovs-ofctl add-flow br-sw 'priority=0,actions=NORMAL']) +#Verify that BFD is enabled. +for i in `seq 0 40`; do ovs-appctl time/warp 100; done +BFD_CHECK([p1], [true], [false], [none], [up], [No Diagnostic], [none], [up], [No Diagnostic]) +BFD_CHECK([p0], [true], [false], [none], [up], [No Diagnostic], [none], [up], [No Diagnostic]) + +#Drop all packets in the br-sw bridge so that the tunnel is down. +AT_CHECK([ ovs-ofctl add-flow br-sw 'priority=5,actions=drop' ]) +for i in `seq 0 40`; do ovs-appctl time/warp 100; done +BFD_CHECK([p1], [false], [false], [none], [down], [Control Detection Time Expired], [none], [down], [No Diagnostic]) +BFD_CHECK([p0], [false], [false], [none], [down], [Control Detection Time Expired], [none], [down], [No Diagnostic]) + +#Delete the added flow +AT_CHECK([ovs-ofctl del-flows br-sw], [0]) +AT_CHECK([ovs-ofctl add-flow br-sw 'priority=0,actions=NORMAL']) +#Verify that BFD is back up again. +for i in `seq 0 40`; do ovs-appctl time/warp 100; done + +BFD_CHECK([p1], [true], [false], [none], [up], [Control Detection Time Expired], [none], [up], [Control Detection Time Expired]) +BFD_CHECK([p0], [true], [false], [none], [up], [Control Detection Time Expired], [none], [up], [Control Detection Time Expired]) + +#Now, Verify one-side tunnel down detection +#When br-sw is dropping packets from one end, BFD should detect +# that the tunnel is down, and come back up when br-sw is working fine. + +#Bring down the br-bfd1 - br-sw link. So BFD packets will be sent from p0, +# but not received by p1. p0 will receive all BFD packets from p1. + +AT_CHECK([ ovs-ofctl add-flow br-sw 'in_port=1,priority=5,actions=drop']) +for i in `seq 0 40`; do ovs-appctl time/warp 100; done +# Make sure p1 BFD state is down since it received no BFD packets. +BFD_CHECK([p1], [false], [false], [none], [down], [Control Detection Time Expired], [none], [down], [No Diagnostic]) +for i in `seq 0 40`; do ovs-appctl time/warp 100; done +# p0 will be in init state once it receives "down" BFD message from p1. +BFD_CHECK([p0], [false], [false], [none], [init], [Neighbor Signaled Session Down], [none], [down], [Control Detection Time Expired]) + +AT_CHECK([ovs-ofctl del-flows br-sw]) +AT_CHECK([ovs-ofctl add-flow br-sw 'priority=0,actions=NORMAL']) +#Ensure that BFD is back up again. + +for i in `seq 0 10`; do ovs-appctl time/warp 100; done +#Bring down the br-bfd0 - br-sw link +AT_CHECK([ ovs-ofctl add-flow br-sw 'in_port=2,priority=5,actions=drop']) +for i in `seq 0 40`; do ovs-appctl time/warp 100; done +BFD_CHECK([p0], [false], [false], [none], [down], [Control Detection Time Expired], [none], [down], [No Diagnostic]) +for i in `seq 0 40`; do ovs-appctl time/warp 100; done +BFD_CHECK([p1], [false], [false], [none], [init], [Neighbor Signaled Session Down], [none], [down], [Control Detection Time Expired]) +OVS_VSWITCHD_STOP +AT_CLEANUP + + +AT_SETUP([bfd - concatenated path down]) +#Create 2 bridges connected by patch ports and enable BFD +ovs-appctl time/stop +OVS_VSWITCHD_START() +AT_CHECK([ ovs-vsctl -- add-br br1 -- \ + set bridge br1 datapath-type=dummy \ + other-config:hwaddr=aa:55:aa:56:00:00 ]) +ovs-appctl time/stop +AT_CHECK([ ovs-vsctl -- add-port br1 p1 -- set Interface p1 type=patch \ + options:peer=p0 ]) +AT_CHECK([ ovs-vsctl -- add-port br0 p0 -- set Interface p0 type=patch \ + options:peer=p1 ]) +AT_CHECK([ ovs-vsctl -- set interface p0 bfd:enable=true ]) +AT_CHECK([ ovs-vsctl -- set interface p1 bfd:enable=true ]) +for i in `seq 0 40`; do ovs-appctl time/warp 100; done + +#Verify that BFD has been enabled on both interfaces. +BFD_CHECK([p1], [true], [false], [none], [up], [No Diagnostic], [none], [up], [No Diagnostic]) +BFD_CHECK([p0], [true], [false], [none], [up], [No Diagnostic], [none], [up], [No Diagnostic]) + +#Set cpath_down to true on one interface, make sure the remote interface updates its values. +AT_CHECK([ovs-vsctl set interface p0 bfd:cpath_down=true]) +for i in `seq 0 40`; do ovs-appctl time/warp 100; done +BFD_CHECK([p1], [false], [false], [none], [up], [No Diagnostic], [none], [up], [Concatenated Path Down]) +OVS_VSWITCHD_STOP +AT_CLEANUP + + +AT_SETUP([bfd - Edit the Min Tx/Rx values]) +#Create 2 bridges connected by patch ports and enable BFD +ovs-appctl time/stop +ovs-appctl vlog/set bfd:dbg +OVS_VSWITCHD_START() +AT_CHECK([ ovs-vsctl -- add-br br1 -- \ + set bridge br1 datapath-type=dummy ]) +AT_CHECK([ ovs-vsctl -- add-port br1 p1 -- set Interface p1 type=patch \ + options:peer=p0 ]) +AT_CHECK([ ovs-vsctl -- add-port br0 p0 -- set Interface p0 type=patch \ + options:peer=p1 ]) +AT_CHECK([ ovs-vsctl -- set interface p0 bfd:enable=true ]) +AT_CHECK([ ovs-vsctl -- set interface p1 bfd:enable=true ]) +for i in `seq 0 20`; do ovs-appctl time/warp 100; done +#Verify that BFD has been enabled on both interfaces. +BFD_CHECK([p1], [true], [false], [none], [up], [No Diagnostic], [none], [up], [No Diagnostic]) +BFD_CHECK([p0], [true], [false], [none], [up], [No Diagnostic], [none], [up], [No Diagnostic]) +#Edit the min Tx value. +AT_CHECK([ovs-vsctl set interface p0 bfd:min_tx=200]) +for i in `seq 0 20`; do ovs-appctl time/warp 100; done +BFD_CHECK_TX([p0], [200ms], [100ms]) +BFD_CHECK_TX([p1], [100ms], [200ms]) + +#Edit the min Rx value. +AT_CHECK([ovs-vsctl set interface p1 bfd:min_rx=300]) +for i in `seq 0 20`; do ovs-appctl time/warp 100; done +BFD_CHECK_RX([p1], [300ms], [1000ms]) +BFD_CHECK_RX([p0], [1000ms], [300ms]) + +OVS_VSWITCHD_STOP +AT_CLEANUP diff --git a/tests/glibc.supp b/tests/glibc.supp index 52d17bc99..948ee013f 100644 --- a/tests/glibc.supp +++ b/tests/glibc.supp @@ -11,6 +11,7 @@ timer_create Memcheck:Param timer_create(evp) - fun:timer_create + ... + fun:set_up_timer } diff --git a/tests/ofp-print.at b/tests/ofp-print.at index 22886fc49..986b9312c 100644 --- a/tests/ofp-print.at +++ b/tests/ofp-print.at @@ -1512,12 +1512,12 @@ AT_CHECK([ovs-ofctl ofp-print "\ 00 00 00 00 00 00 00 00 00 00 00 00 \ "], [0], [dnl OFPST_QUEUE reply (xid=0x1): 6 queues - port 3 queue 1: bytes=302, pkts=1, errors=0 - port 3 queue 2: bytes=0, pkts=0, errors=0 - port 2 queue 1: bytes=2100, pkts=20, errors=0 - port 2 queue 2: bytes=0, pkts=0, errors=0 - port 1 queue 1: bytes=0, pkts=0, errors=0 - port 1 queue 2: bytes=0, pkts=0, errors=0 + port 3 queue 1: bytes=302, pkts=1, errors=0, duration=? + port 3 queue 2: bytes=0, pkts=0, errors=0, duration=? + port 2 queue 1: bytes=2100, pkts=20, errors=0, duration=? + port 2 queue 2: bytes=0, pkts=0, errors=0, duration=? + port 1 queue 1: bytes=0, pkts=0, errors=0, duration=? + port 1 queue 2: bytes=0, pkts=0, errors=0, duration=? ]) AT_CLEANUP @@ -1546,12 +1546,12 @@ AT_CHECK([ovs-ofctl ofp-print "\ 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 \ "], [0], [dnl OFPST_QUEUE reply (OF1.1) (xid=0x1): 6 queues - port 3 queue 1: bytes=302, pkts=1, errors=0 - port 3 queue 2: bytes=0, pkts=0, errors=0 - port 2 queue 1: bytes=2100, pkts=20, errors=0 - port 2 queue 2: bytes=0, pkts=0, errors=0 - port 1 queue 1: bytes=0, pkts=0, errors=0 - port 1 queue 2: bytes=0, pkts=0, errors=0 + port 3 queue 1: bytes=302, pkts=1, errors=0, duration=? + port 3 queue 2: bytes=0, pkts=0, errors=0, duration=? + port 2 queue 1: bytes=2100, pkts=20, errors=0, duration=? + port 2 queue 2: bytes=0, pkts=0, errors=0, duration=? + port 1 queue 1: bytes=0, pkts=0, errors=0, duration=? + port 1 queue 2: bytes=0, pkts=0, errors=0, duration=? ]) AT_CLEANUP @@ -1573,12 +1573,45 @@ AT_CHECK([ovs-ofctl ofp-print "\ 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 \ "], [0], [dnl OFPST_QUEUE reply (OF1.2) (xid=0x1): 6 queues - port 3 queue 1: bytes=302, pkts=1, errors=0 - port 3 queue 2: bytes=0, pkts=0, errors=0 - port 2 queue 1: bytes=2100, pkts=20, errors=0 - port 2 queue 2: bytes=0, pkts=0, errors=0 - port 1 queue 1: bytes=0, pkts=0, errors=0 - port 1 queue 2: bytes=0, pkts=0, errors=0 + port 3 queue 1: bytes=302, pkts=1, errors=0, duration=? + port 3 queue 2: bytes=0, pkts=0, errors=0, duration=? + port 2 queue 1: bytes=2100, pkts=20, errors=0, duration=? + port 2 queue 2: bytes=0, pkts=0, errors=0, duration=? + port 1 queue 1: bytes=0, pkts=0, errors=0, duration=? + port 1 queue 2: bytes=0, pkts=0, errors=0, duration=? +]) +AT_CLEANUP + +AT_SETUP([OFPST_QUEUE reply - OF1.3]) +AT_KEYWORDS([ofp-print OFPT_STATS_REPLY]) +AT_CHECK([ovs-ofctl ofp-print "\ +04 13 01 00 00 00 00 01 00 05 00 00 00 00 00 00 \ +00 00 00 03 00 00 00 01 00 00 00 00 00 00 01 2e \ +00 00 00 00 00 00 00 01 00 00 00 00 00 00 00 00 \ +00 00 00 64 1d cd 65 00 \ +00 00 00 03 00 00 00 02 00 00 00 00 00 00 00 00 \ +00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 \ +00 00 00 64 1d cd 65 00 \ +00 00 00 02 00 00 00 01 00 00 00 00 00 00 08 34 \ +00 00 00 00 00 00 00 14 00 00 00 00 00 00 00 00 \ +00 00 00 64 1d cd 65 00 \ +00 00 00 02 00 00 00 02 00 00 00 00 00 00 00 00 \ +00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 \ +00 00 00 64 1d cd 65 00 \ +00 00 00 01 00 00 00 01 00 00 00 00 00 00 00 00 \ +00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 \ +00 00 00 64 1d cd 65 00 \ +00 00 00 01 00 00 00 02 00 00 00 00 00 00 00 00 \ +00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 \ +ff ff ff ff ff ff ff ff \ +"], [0], [dnl +OFPST_QUEUE reply (OF1.3) (xid=0x1): 6 queues + port 3 queue 1: bytes=302, pkts=1, errors=0, duration=100.5s + port 3 queue 2: bytes=0, pkts=0, errors=0, duration=100.5s + port 2 queue 1: bytes=2100, pkts=20, errors=0, duration=100.5s + port 2 queue 2: bytes=0, pkts=0, errors=0, duration=100.5s + port 1 queue 1: bytes=0, pkts=0, errors=0, duration=100.5s + port 1 queue 2: bytes=0, pkts=0, errors=0, duration=? ]) AT_CLEANUP diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at index c3c1c64bb..2728a2812 100644 --- a/tests/ofproto-dpif.at +++ b/tests/ofproto-dpif.at @@ -2632,3 +2632,24 @@ skb_priority=0,icmp,in_port=1,nw_src=10.0.0.4,nw_dst=10.0.0.3,nw_tos=0,nw_ecn=0, ]) OVS_VSWITCHD_STOP AT_CLEANUP + +AT_SETUP([ofproto-dpif - datapath port number change]) +OVS_VSWITCHD_START([set Bridge br0 fail-mode=standalone]) +ADD_OF_PORTS([br0], 1) + +# Trace a flow that should output to p1. +AT_CHECK([ovs-appctl ofproto/trace br0 in_port=LOCAL,dl_src=10:20:30:40:50:60], + [0], [stdout]) +AT_CHECK([tail -1 stdout], [0], [Datapath actions: 1 +]) + +# Change p1's port number to 5. +AT_CHECK([ovs-appctl dpif-dummy/change-port-number ovs-dummy p1 5]) + +# Trace a flow that should output to p1 in its new location. +AT_CHECK([ovs-appctl ofproto/trace br0 in_port=LOCAL,dl_src=10:20:30:40:50:60], + [0], [stdout]) +AT_CHECK([tail -1 stdout], [0], [Datapath actions: 5 +]) +OVS_VSWITCHD_STOP +AT_CLEANUP diff --git a/tests/test-atomic.c b/tests/test-atomic.c index 21d8c9e11..27bf5520e 100644 --- a/tests/test-atomic.c +++ b/tests/test-atomic.c @@ -22,7 +22,7 @@ #define TEST_ATOMIC_TYPE(ATOMIC_TYPE, BASE_TYPE) \ { \ ATOMIC_TYPE x = ATOMIC_VAR_INIT(1); \ - BASE_TYPE value; \ + BASE_TYPE value, orig; \ \ atomic_read(&x, &value); \ ovs_assert(value == 1); \ @@ -34,6 +34,31 @@ atomic_init(&x, 3); \ atomic_read(&x, &value); \ ovs_assert(value == 3); \ + \ + atomic_add(&x, 1, &orig); \ + ovs_assert(orig == 3); \ + atomic_read(&x, &value); \ + ovs_assert(value == 4); \ + \ + atomic_sub(&x, 2, &orig); \ + ovs_assert(orig == 4); \ + atomic_read(&x, &value); \ + ovs_assert(value == 2); \ + \ + atomic_or(&x, 6, &orig); \ + ovs_assert(orig == 2); \ + atomic_read(&x, &value); \ + ovs_assert(value == 6); \ + \ + atomic_and(&x, 10, &orig); \ + ovs_assert(orig == 6); \ + atomic_read(&x, &value); \ + ovs_assert(value == 2); \ + \ + atomic_xor(&x, 10, &orig); \ + ovs_assert(orig == 2); \ + atomic_read(&x, &value); \ + ovs_assert(value == 8); \ } int diff --git a/tests/testsuite.at b/tests/testsuite.at index fbc701b35..c9bcad980 100644 --- a/tests/testsuite.at +++ b/tests/testsuite.at @@ -67,6 +67,7 @@ EOF m4_include([tests/ovsdb-macros.at]) m4_include([tests/ofproto-macros.at]) +m4_include([tests/bfd.at]) m4_include([tests/lacp.at]) m4_include([tests/library.at]) m4_include([tests/heap.at]) diff --git a/utilities/bugtool/ovs-bugtool.in b/utilities/bugtool/ovs-bugtool.in index ffaf7538c..7e76d0e99 100755 --- a/utilities/bugtool/ovs-bugtool.in +++ b/utilities/bugtool/ovs-bugtool.in @@ -149,7 +149,6 @@ NETSTAT = 'netstat' OVS_DPCTL = 'ovs-dpctl' OVS_OFCTL = 'ovs-ofctl' OVS_VSCTL = 'ovs-vsctl' -OVS_APPCTL = 'ovs-appctl' PS = 'ps' ROUTE = 'route' RPM = 'rpm' @@ -205,6 +204,7 @@ CAP_KERNEL_INFO = 'kernel-info' CAP_LOSETUP_A = 'loopback-devices' CAP_MULTIPATH = 'multipath' CAP_NETWORK_CONFIG = 'network-config' +CAP_NETWORK_INFO = 'network-info' CAP_NETWORK_STATUS = 'network-status' CAP_OPENVSWITCH_LOGS = 'ovs-system-logs' CAP_PROCESS_LIST = 'process-list' @@ -233,7 +233,7 @@ cap(CAP_BOOT_LOADER, PII_NO, max_size=3*KB, max_time=5) cap(CAP_DISK_INFO, PII_MAYBE, max_size=50*KB, max_time=20) -cap(CAP_HARDWARE_INFO, PII_MAYBE, max_size=30*KB, +cap(CAP_HARDWARE_INFO, PII_MAYBE, max_size=2*MB, max_time=20) cap(CAP_KERNEL_INFO, PII_MAYBE, max_size=120*KB, max_time=5) @@ -242,7 +242,9 @@ cap(CAP_MULTIPATH, PII_MAYBE, max_size=20*KB, max_time=10) cap(CAP_NETWORK_CONFIG, PII_IF_CUSTOMIZED, min_size=0, max_size=40*KB) -cap(CAP_NETWORK_STATUS, PII_YES, max_size=50*MB, +cap(CAP_NETWORK_INFO, PII_YES, max_size=50*MB, + max_time=30) +cap(CAP_NETWORK_STATUS, PII_YES, max_size=-1, max_time=30) cap(CAP_OPENVSWITCH_LOGS, PII_MAYBE, max_size=-1, max_time=5) @@ -543,14 +545,14 @@ exclude those logs from the archive. file_output(CAP_NETWORK_CONFIG, [NTP_CONF, IPTABLES_CONFIG, HOSTS_ALLOW, HOSTS_DENY]) file_output(CAP_NETWORK_CONFIG, [OPENVSWITCH_CONF_DB]) - cmd_output(CAP_NETWORK_STATUS, [IFCONFIG, '-a']) - cmd_output(CAP_NETWORK_STATUS, [ROUTE, '-n']) - cmd_output(CAP_NETWORK_STATUS, [ARP, '-n']) - cmd_output(CAP_NETWORK_STATUS, [NETSTAT, '-an']) + cmd_output(CAP_NETWORK_INFO, [IFCONFIG, '-a']) + cmd_output(CAP_NETWORK_INFO, [ROUTE, '-n']) + cmd_output(CAP_NETWORK_INFO, [ARP, '-n']) + cmd_output(CAP_NETWORK_INFO, [NETSTAT, '-an']) for dir in DHCP_LEASE_DIR: - tree_output(CAP_NETWORK_STATUS, dir) + tree_output(CAP_NETWORK_INFO, dir) for table in ['filter', 'nat', 'mangle', 'raw', 'security']: - cmd_output(CAP_NETWORK_STATUS, [IPTABLES, '-t', table, '-nL']) + cmd_output(CAP_NETWORK_INFO, [IPTABLES, '-t', table, '-nL']) for p in os.listdir('/sys/class/net/'): try: f = open('/sys/class/net/%s/type' % p, 'r') @@ -558,35 +560,25 @@ exclude those logs from the archive. f.close() if os.path.islink('/sys/class/net/%s/device' % p) and int(t) == 1: # ARPHRD_ETHER - cmd_output(CAP_NETWORK_STATUS, [ETHTOOL, '-S', p]) + cmd_output(CAP_NETWORK_INFO, [ETHTOOL, '-S', p]) if not p.startswith('vif') and not p.startswith('tap'): - cmd_output(CAP_NETWORK_STATUS, [ETHTOOL, p]) - cmd_output(CAP_NETWORK_STATUS, [ETHTOOL, '-k', p]) - cmd_output(CAP_NETWORK_STATUS, [ETHTOOL, '-i', p]) - cmd_output(CAP_NETWORK_STATUS, [ETHTOOL, '-c', p]) + cmd_output(CAP_NETWORK_INFO, [ETHTOOL, p]) + cmd_output(CAP_NETWORK_INFO, [ETHTOOL, '-k', p]) + cmd_output(CAP_NETWORK_INFO, [ETHTOOL, '-i', p]) + cmd_output(CAP_NETWORK_INFO, [ETHTOOL, '-c', p]) if int(t) == 1: - cmd_output(CAP_NETWORK_STATUS, + cmd_output(CAP_NETWORK_INFO, [TC, '-s', '-d', 'class', 'show', 'dev', p]) except: pass - tree_output(CAP_NETWORK_STATUS, PROC_NET_BONDING_DIR) - tree_output(CAP_NETWORK_STATUS, PROC_NET_VLAN_DIR) - cmd_output(CAP_NETWORK_STATUS, [TC, '-s', 'qdisc']) - file_output(CAP_NETWORK_STATUS, [PROC_NET_SOFTNET_STAT]) + tree_output(CAP_NETWORK_INFO, PROC_NET_BONDING_DIR) + tree_output(CAP_NETWORK_INFO, PROC_NET_VLAN_DIR) + cmd_output(CAP_NETWORK_INFO, [TC, '-s', 'qdisc']) + file_output(CAP_NETWORK_INFO, [PROC_NET_SOFTNET_STAT]) if os.path.exists(OPENVSWITCH_VSWITCHD_PID): cmd_output(CAP_NETWORK_STATUS, [OVS_DPCTL, 'show', '-s']) for d in dp_list(): cmd_output(CAP_NETWORK_STATUS, [OVS_DPCTL, 'dump-flows', d]) - try: - vspidfile = open(OPENVSWITCH_VSWITCHD_PID) - vspid = int(vspidfile.readline().strip()) - vspidfile.close() - for b in bond_list(vspid): - cmd_output(CAP_NETWORK_STATUS, - [OVS_APPCTL, '-t', '@RUNDIR@/ovs-vswitchd.%s.ctl' % vspid, '-e' 'bond/show %s' % b], - 'ovs-appctl-bond-show-%s.out' % b) - except e: - pass cmd_output(CAP_PROCESS_LIST, [PS, 'wwwaxf', '-eo', 'pid,tty,stat,time,nice,psr,pcpu,pmem,nwchan,wchan:25,args'], label='process-tree') func_output(CAP_PROCESS_LIST, 'fd_usage', fd_usage) @@ -747,17 +739,6 @@ def dp_list(): return output.getvalue().splitlines() return [] -def bond_list(pid): - output = StringIO.StringIO() - procs = [ProcOutput([OVS_APPCTL, '-t', '@RUNDIR@/ovs-vswitchd.%s.ctl' % pid, '-e' 'bond/list'], caps[CAP_NETWORK_STATUS][MAX_TIME], output)] - - run_procs([procs]) - - if not procs[0].timed_out: - bonds = output.getvalue().splitlines()[1:] - return [x.split('\t')[1] for x in bonds] - return [] - def fd_usage(cap): output = '' fd_dict = {} diff --git a/utilities/ovs-dev.py b/utilities/ovs-dev.py index 8c8f5bd11..21f0fc5db 100755 --- a/utilities/ovs-dev.py +++ b/utilities/ovs-dev.py @@ -26,7 +26,7 @@ OVS_SRC = HOME + "/ovs" ROOT = HOME + "/root" PATH = "%(ovs)s/utilities:%(ovs)s/ovsdb:%(ovs)s/vswitchd" % {"ovs": OVS_SRC} -ENV["CFLAGS"] = "-g -O0 -Wall -Wextra -Wno-deprecated-declarations" +ENV["CFLAGS"] = "-g -O0" ENV["PATH"] = PATH + ":" + ENV["PATH"] options = None @@ -52,6 +52,9 @@ def uname(): def conf(): tag() + if options.clang: + ENV["CC"] = "clang" + configure = ["./configure", "--prefix=" + ROOT, "--localstatedir=" + ROOT, "--with-logdir=%s/log" % ROOT, "--with-rundir=%s/run" % ROOT, "--with-linux=/lib/modules/%s/build" % uname(), @@ -75,7 +78,11 @@ def make(args=""): make = "make -s -j 8 " + args try: _sh("cgcc", "--version", capture=True) - make += " C=1" + # XXX: For some reason the clang build doesn't place nicely with + # sparse. At some point this needs to be figured out and this check + # removed. + if not options.clang: + make += " C=1" except OSError: pass _sh(make) @@ -166,7 +173,9 @@ def run(): if options.gdb: cmd = ["gdb", "--args"] + cmd elif options.valgrind: - cmd = ["valgrind", "--track-origins=yes"] + cmd + cmd = ["valgrind", "--track-origins=yes", + "--suppressions=%s/tests/glibc.supp" % OVS_SRC, + "--suppressions=%s/tests/openssl.supp" % OVS_SRC] + cmd else: cmd = ["sudo"] + cmd opts = opts + ["-vconsole:off", "--detach"] @@ -275,6 +284,8 @@ def main(): help="run ovs-vswitchd under gdb") group.add_option("--valgrind", dest="valgrind", action="store_true", help="run ovs-vswitchd under valgrind") + group.add_option("--clang", dest="clang", action="store_true", + help="build ovs-vswitchd with clang") parser.add_option_group(group) options, args = parser.parse_args() diff --git a/utilities/ovs-dpctl.c b/utilities/ovs-dpctl.c index e285ed509..fa78b53ce 100644 --- a/utilities/ovs-dpctl.c +++ b/utilities/ovs-dpctl.c @@ -997,7 +997,8 @@ sort_output_actions(struct nlattr *actions, size_t length) } if (first_output) { uint8_t *end = (uint8_t *) actions + length; - sort_output_actions__(first_output, (struct nlattr *) end); + sort_output_actions__(first_output, + ALIGNED_CAST(struct nlattr *, end)); } } diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c index 262225576..68b73bfaf 100644 --- a/utilities/ovs-ofctl.c +++ b/utilities/ovs-ofctl.c @@ -2335,7 +2335,7 @@ ofctl_parse_nxm__(bool oxm) ofpbuf_init(&nx_match, 0); if (oxm) { match_len = oxm_put_match(&nx_match, &match); - out = oxm_match_to_string(nx_match.data, match_len); + out = oxm_match_to_string(&nx_match, match_len); } else { match_len = nx_put_match(&nx_match, &match, cookie, cookie_mask); @@ -2775,7 +2775,7 @@ ofctl_check_vlan(int argc OVS_UNUSED, char *argv[]) /* Convert to and from OXM. */ ofpbuf_init(&nxm, 0); nxm_match_len = oxm_put_match(&nxm, &match); - nxm_s = oxm_match_to_string(nxm.data, nxm_match_len); + nxm_s = oxm_match_to_string(&nxm, nxm_match_len); error = oxm_pull_match(&nxm, &nxm_match); printf("OXM: %s -> ", nxm_s); if (error) { diff --git a/vswitchd/automake.mk b/vswitchd/automake.mk index 62ace691c..3a271c6a5 100644 --- a/vswitchd/automake.mk +++ b/vswitchd/automake.mk @@ -57,13 +57,15 @@ EXTRA_DIST += vswitchd/vswitch.gv vswitchd/vswitch.pic # vswitch schema documentation EXTRA_DIST += vswitchd/vswitch.xml +DISTCLEANFILES += $(srcdir)/vswitchd/ovs-vswitchd.conf.db.5 dist_man_MANS += vswitchd/ovs-vswitchd.conf.db.5 $(srcdir)/vswitchd/ovs-vswitchd.conf.db.5: \ - ovsdb/ovsdb-doc.in vswitchd/vswitch.xml vswitchd/vswitch.ovsschema \ + ovsdb/ovsdb-doc vswitchd/vswitch.xml vswitchd/vswitch.ovsschema \ $(srcdir)/vswitchd/vswitch.pic $(OVSDB_DOC) \ --title="ovs-vswitchd.conf.db" \ --er-diagram=$(srcdir)/vswitchd/vswitch.pic \ + --version=$(VERSION) \ $(srcdir)/vswitchd/vswitch.ovsschema \ $(srcdir)/vswitchd/vswitch.xml > $@.tmp mv $@.tmp $@ diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c index a73379ca5..1460ea2d6 100644 --- a/vswitchd/bridge.c +++ b/vswitchd/bridge.c @@ -4090,10 +4090,10 @@ collect_splinter_vlans(const struct ovsrec_open_vswitch *ovs_cfg) if (!netdev_open(vlan_dev->name, "system", &netdev)) { if (!netdev_get_in4(netdev, NULL, NULL) || !netdev_get_in6(netdev, NULL)) { - vlandev_del(vlan_dev->name); - } else { /* It has an IP address configured, so we don't own * it. Don't delete it. */ + } else { + vlandev_del(vlan_dev->name); } netdev_close(netdev); } diff --git a/vswitchd/system-stats.c b/vswitchd/system-stats.c index b1642284c..9e3d228ad 100644 --- a/vswitchd/system-stats.c +++ b/vswitchd/system-stats.c @@ -506,12 +506,12 @@ get_filesys_stats(struct smap *stats OVS_UNUSED) #define SYSTEM_STATS_INTERVAL (5 * 1000) /* In milliseconds. */ -static pthread_mutex_t mutex = PTHREAD_ADAPTIVE_MUTEX_INITIALIZER; +static struct ovs_mutex mutex = OVS_ADAPTIVE_MUTEX_INITIALIZER; static pthread_cond_t cond = PTHREAD_COND_INITIALIZER; -static struct latch latch; +static struct latch latch OVS_GUARDED_BY(mutex); static bool enabled; -static bool started; -static struct smap *system_stats; +static bool started OVS_GUARDED_BY(mutex); +static struct smap *system_stats OVS_GUARDED_BY(mutex); static void *system_stats_thread_func(void *); static void discard_stats(void); @@ -521,7 +521,7 @@ void system_stats_enable(bool enable) { if (enabled != enable) { - xpthread_mutex_lock(&mutex); + ovs_mutex_lock(&mutex); if (enable) { if (!started) { xpthread_create(NULL, NULL, system_stats_thread_func, NULL); @@ -532,7 +532,7 @@ system_stats_enable(bool enable) xpthread_cond_signal(&cond); } enabled = enable; - xpthread_mutex_unlock(&mutex); + ovs_mutex_unlock(&mutex); } } @@ -549,7 +549,7 @@ system_stats_run(void) { struct smap *stats = NULL; - xpthread_mutex_lock(&mutex); + ovs_mutex_lock(&mutex); if (system_stats) { latch_poll(&latch); @@ -560,7 +560,7 @@ system_stats_run(void) discard_stats(); } } - xpthread_mutex_unlock(&mutex); + ovs_mutex_unlock(&mutex); return stats; } @@ -576,7 +576,7 @@ system_stats_wait(void) } static void -discard_stats(void) +discard_stats(void) OVS_REQUIRES(&mutex) { if (system_stats) { smap_destroy(system_stats); @@ -594,11 +594,11 @@ system_stats_thread_func(void *arg OVS_UNUSED) long long int next_refresh; struct smap *stats; - xpthread_mutex_lock(&mutex); + ovs_mutex_lock(&mutex); while (!enabled) { - xpthread_cond_wait(&cond, &mutex); + ovs_mutex_cond_wait(&cond, &mutex); } - xpthread_mutex_unlock(&mutex); + ovs_mutex_unlock(&mutex); stats = xmalloc(sizeof *stats); smap_init(stats); @@ -608,11 +608,11 @@ system_stats_thread_func(void *arg OVS_UNUSED) get_process_stats(stats); get_filesys_stats(stats); - xpthread_mutex_lock(&mutex); + ovs_mutex_lock(&mutex); discard_stats(); system_stats = stats; latch_set(&latch); - xpthread_mutex_unlock(&mutex); + ovs_mutex_unlock(&mutex); next_refresh = time_msec() + SYSTEM_STATS_INTERVAL; do {