From 6d7356763a882cc5dbd814cafd8dacffc6544af6 Mon Sep 17 00:00:00 2001 From: Justin Pettit Date: Sun, 29 Nov 2009 11:23:32 -0800 Subject: [PATCH 1/5] Updates for 0.90.6 release. --- ChangeLog | 5 +++++ configure.ac | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/ChangeLog b/ChangeLog index eff97aa5d..9cfa8b936 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,8 @@ +v0.90.7 - 29 Nov 2009 +--------------------- + - Add support for NetFlow active timeouts + - Bug fixes + v0.90.6 - 6 Oct 2009 -------------------- - Bug fixes diff --git a/configure.ac b/configure.ac index edfe0f75b..45d9decb9 100644 --- a/configure.ac +++ b/configure.ac @@ -13,7 +13,7 @@ # limitations under the License. AC_PREREQ(2.60) -AC_INIT(openvswitch, 0.90.6, ovs-bugs@openvswitch.org) +AC_INIT(openvswitch, 0.90.7, ovs-bugs@openvswitch.org) NX_BUILDNR AC_CONFIG_SRCDIR([datapath/datapath.c]) AC_CONFIG_MACRO_DIR([m4]) From 88aa338eed96ef3cf45bfe9e0048440c740db71b Mon Sep 17 00:00:00 2001 From: Ben Pfaff Date: Wed, 29 Jul 2009 16:48:45 -0700 Subject: [PATCH 2/5] Remove --disable-userspace "configure" option, since it breaks "make dist". I had thought that Automake was smart enough to ignore conditionals around EXTRA_DIST, so that all files always got distributed regardless of whether Automake conditionals were set. I was wrong. This commit removes the --disable-userspace option to "configure", which put a conditional around most of Makefile.am and thus unintentionally caused most of the distribution to be left out if --disable-userspace was specified. The alternative (fixing --disable-userspace) seems like too much work--it would require pushing "if ENABLE_USERSPACE" down into lots of subdirectory--and would be difficult to maintain. --- Makefile.am | 2 -- acinclude.m4 | 14 -------------- configure.ac | 49 +++++++++++++++++++++++-------------------------- 3 files changed, 23 insertions(+), 42 deletions(-) diff --git a/Makefile.am b/Makefile.am index 59ffc1939..e9789856d 100644 --- a/Makefile.am +++ b/Makefile.am @@ -9,7 +9,6 @@ AUTOMAKE_OPTIONS = foreign subdir-objects ACLOCAL_AMFLAGS = -I m4 SUBDIRS = datapath -if ENABLE_USERSPACE AM_CPPFLAGS = $(SSL_CFLAGS) AM_CPPFLAGS += $(NCURSES_CFLAGS) AM_CPPFLAGS += $(PCRE_CFLAGS) @@ -70,4 +69,3 @@ if HAVE_PCRE include extras/ezio/automake.mk endif endif -endif # ENABLE_USERSPACE diff --git a/acinclude.m4 b/acinclude.m4 index d5d7c0959..6ba647ab9 100644 --- a/acinclude.m4 +++ b/acinclude.m4 @@ -14,20 +14,6 @@ # See the License for the specific language governing permissions and # limitations under the License. -dnl Checks for --disable-userspace. -AC_DEFUN([OVS_CHECK_USERSPACE], - [AC_ARG_ENABLE( - [userspace], - [AC_HELP_STRING([--disable-userspace], - [Disable building userspace components.])], - [case "${enableval}" in - (yes) build_userspace=true ;; - (no) build_userspace=false ;; - (*) AC_MSG_ERROR([bad value ${enableval} for --enable-userspace]) ;; - esac], - [build_userspace=true]) - AM_CONDITIONAL([ENABLE_USERSPACE], [$build_userspace])]) - dnl OVS_CHECK_LINUX(OPTION, VERSION, VARIABLE, CONDITIONAL) dnl dnl Configure linux kernel source tree diff --git a/configure.ac b/configure.ac index 45d9decb9..86cbe1607 100644 --- a/configure.ac +++ b/configure.ac @@ -37,7 +37,6 @@ AC_USE_SYSTEM_EXTENSIONS AC_C_BIGENDIAN AC_SYS_LARGEFILE -OVS_CHECK_USERSPACE OVS_CHECK_NDEBUG OVS_CHECK_NETLINK OVS_CHECK_OPENSSL @@ -48,33 +47,31 @@ OVS_CHECK_PCRE OVS_CHECK_IF_PACKET OVS_CHECK_STRTOK_R -if $build_userspace; then - OVS_CHECK_PKIDIR - OVS_CHECK_RUNDIR - OVS_CHECK_MALLOC_HOOKS - OVS_CHECK_VALGRIND - OVS_CHECK_TTY_LOCK_DIR - OVS_CHECK_SOCKET_LIBS - OVS_CHECK_FAULT_LIBS +OVS_CHECK_PKIDIR +OVS_CHECK_RUNDIR +OVS_CHECK_MALLOC_HOOKS +OVS_CHECK_VALGRIND +OVS_CHECK_TTY_LOCK_DIR +OVS_CHECK_SOCKET_LIBS +OVS_CHECK_FAULT_LIBS - AC_CHECK_FUNCS([strsignal]) +AC_CHECK_FUNCS([strsignal]) - OVS_ENABLE_OPTION([-Wall]) - OVS_ENABLE_OPTION([-Wno-sign-compare]) - OVS_ENABLE_OPTION([-Wpointer-arith]) - OVS_ENABLE_OPTION([-Wdeclaration-after-statement]) - OVS_ENABLE_OPTION([-Wformat-security]) - OVS_ENABLE_OPTION([-Wswitch-enum]) - OVS_ENABLE_OPTION([-Wunused-parameter]) - OVS_ENABLE_OPTION([-Wstrict-aliasing]) - OVS_ENABLE_OPTION([-Wbad-function-cast]) - OVS_ENABLE_OPTION([-Wcast-align]) - 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([-Wno-override-init]) -fi +OVS_ENABLE_OPTION([-Wall]) +OVS_ENABLE_OPTION([-Wno-sign-compare]) +OVS_ENABLE_OPTION([-Wpointer-arith]) +OVS_ENABLE_OPTION([-Wdeclaration-after-statement]) +OVS_ENABLE_OPTION([-Wformat-security]) +OVS_ENABLE_OPTION([-Wswitch-enum]) +OVS_ENABLE_OPTION([-Wunused-parameter]) +OVS_ENABLE_OPTION([-Wstrict-aliasing]) +OVS_ENABLE_OPTION([-Wbad-function-cast]) +OVS_ENABLE_OPTION([-Wcast-align]) +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([-Wno-override-init]) AC_ARG_VAR(KARCH, [Kernel Architecture String]) AC_SUBST(KARCH) From 651880308c1fb26071f1f0788e3a9c842d927126 Mon Sep 17 00:00:00 2001 From: Ben Pfaff Date: Wed, 29 Jul 2009 17:04:10 -0700 Subject: [PATCH 3/5] Always distribute extras/ezio/ezio3.ti. I had thought that Automake was smart enough to ignore conditionals around EXTRA_DIST, so that all files always got distributed regardless of whether Automake conditionals were set. I was wrong. This pushes the conditionals for building the ezio binaries down into extras/ezio/automake.mk and thereby makes adding ezio3.ti to EXTRA_DIST unconditional, so that it always gets distributed. Otherwise, this file will not be distributed on systems that don't have curses or don't have PCRE, which is very surprising. --- Makefile.am | 4 ---- extras/ezio/automake.mk | 5 +++++ 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/Makefile.am b/Makefile.am index e9789856d..0b6f00e2d 100644 --- a/Makefile.am +++ b/Makefile.am @@ -64,8 +64,4 @@ include third-party/automake.mk include debian/automake.mk include vswitchd/automake.mk include xenserver/automake.mk -if HAVE_CURSES -if HAVE_PCRE include extras/ezio/automake.mk -endif -endif diff --git a/extras/ezio/automake.mk b/extras/ezio/automake.mk index 2aeaa6440..7c7db2412 100644 --- a/extras/ezio/automake.mk +++ b/extras/ezio/automake.mk @@ -6,6 +6,9 @@ # without warranty of any kind. EXTRA_DIST += extras/ezio/ezio3.ti + +if HAVE_CURSES +if HAVE_PCRE install-data-hook: @echo tic -x $(srcdir)/extras/ezio/ezio3.ti @if ! tic -x $(srcdir)/extras/ezio/ezio3.ti; then \ @@ -47,3 +50,5 @@ extras_ezio_ovs_switchui_LDADD = \ $(PCRE_LIBS) \ $(SSL_LIBS) \ -lm +endif # HAVE_PCRE +endif # HAVE_CURSES From 86a06318bdfbea056b04eb78bcdea5672d0b200e Mon Sep 17 00:00:00 2001 From: Ben Pfaff Date: Mon, 30 Nov 2009 13:17:34 -0800 Subject: [PATCH 4/5] ovs-vswitchd: Add --mlockall option and enable on XenServer. On XenServer 5.5 we found that running 4 simultaneous vm-import operations on iSCSI caused so much disk and cache activity that (we suspect) parts of ovs-vswitchd were paged out to disk and were not paged back in for over 10 seconds, causing the XenServer to fall off the network and the XenCenter connection to fail. Locking ovs-vswitchd into memory appears to avoid this problem. Henrik reports that, with memory locking, importing 11 VMs simultaneously completed successfully. Bug #2344. --- configure.ac | 1 + vswitchd/ovs-vswitchd.8.in | 11 +++++++++++ vswitchd/ovs-vswitchd.c | 15 +++++++++++++++ xenserver/etc_init.d_vswitch | 8 ++++++-- xenserver/root_vswitch_scripts_sysconfig.template | 7 +++++++ 5 files changed, 40 insertions(+), 2 deletions(-) diff --git a/configure.ac b/configure.ac index 86cbe1607..55df342b0 100644 --- a/configure.ac +++ b/configure.ac @@ -46,6 +46,7 @@ OVS_CHECK_LINUX_VT_H OVS_CHECK_PCRE OVS_CHECK_IF_PACKET OVS_CHECK_STRTOK_R +AC_CHECK_FUNCS([mlockall]) OVS_CHECK_PKIDIR OVS_CHECK_RUNDIR diff --git a/vswitchd/ovs-vswitchd.8.in b/vswitchd/ovs-vswitchd.8.in index 5c8d6c772..e9c11f419 100644 --- a/vswitchd/ovs-vswitchd.8.in +++ b/vswitchd/ovs-vswitchd.8.in @@ -67,6 +67,17 @@ Open vSwitch distribution for instructions on how to build and load the Open vSwitch kernel module. .PP .SH OPTIONS +.IP "\fB--mlockall\fR" +Causes \fBovs\-vswitchd\fR to call the \fBmlockall()\fR function, to +attempt to lock all of its process memory into physical RAM, +preventing the kernel from paging any of its memory to disk. This +helps to avoid networking interruptions due to system memory pressure. +.IP +Some systems do not support \fBmlockall()\fR at all, and other systems +only allow privileged users, such as the superuser, to use it. +\fBovs\-vswitchd\fR emits a log message if \fBmlockall()\fR is +unavailable or unsuccessful. +. .IP "\fB--fake-proc-net\fR" Causes \fBovs\-vswitchd\fR to simulate some files in \fB/proc/net/vlan\fR and \fB/proc/net/bonding\fR that some legacy software expects to diff --git a/vswitchd/ovs-vswitchd.c b/vswitchd/ovs-vswitchd.c index 01645adf9..3309c080d 100644 --- a/vswitchd/ovs-vswitchd.c +++ b/vswitchd/ovs-vswitchd.c @@ -22,6 +22,9 @@ #include #include #include +#ifdef HAVE_MLOCKALL +#include +#endif #include "bridge.h" #include "cfg.h" @@ -148,6 +151,7 @@ parse_options(int argc, char *argv[]) { enum { OPT_PEER_CA_CERT = UCHAR_MAX + 1, + OPT_MLOCKALL, OPT_FAKE_PROC_NET, VLOG_OPTION_ENUMS, LEAK_CHECKER_OPTION_ENUMS @@ -155,6 +159,7 @@ parse_options(int argc, char *argv[]) static struct option long_options[] = { {"help", no_argument, 0, 'h'}, {"version", no_argument, 0, 'V'}, + {"mlockall", no_argument, 0, OPT_MLOCKALL}, {"fake-proc-net", no_argument, 0, OPT_FAKE_PROC_NET}, DAEMON_LONG_OPTIONS, VLOG_LONG_OPTIONS, @@ -186,6 +191,16 @@ parse_options(int argc, char *argv[]) OVS_PRINT_VERSION(OFP_VERSION, OFP_VERSION); exit(EXIT_SUCCESS); + case OPT_MLOCKALL: +#ifdef HAVE_MLOCKALL + if (mlockall(MCL_CURRENT | MCL_FUTURE)) { + VLOG_ERR("mlockall failed: %s", strerror(errno)); + } +#else + VLOG_ERR("mlockall not supported on this system"); +#endif + break; + case OPT_FAKE_PROC_NET: error = proc_net_compat_init(); if (error) { diff --git a/xenserver/etc_init.d_vswitch b/xenserver/etc_init.d_vswitch index 4050d5ae0..e8e04ad23 100755 --- a/xenserver/etc_init.d_vswitch +++ b/xenserver/etc_init.d_vswitch @@ -34,6 +34,7 @@ VSWITCHD_CONF="${VSWITCHD_CONF:-/etc/ovs-vswitchd.conf}" VSWITCHD_PIDFILE="${VSWITCHD_PIDFILE:-/var/run/ovs-vswitchd.pid}" VSWITCHD_RUN_DIR="${VSWITCHD_RUN_DIR:-/var/xen/vswitch}" VSWITCHD_PRIORITY="${VSWITCHD_PRIORITY:--10}" +VSWITCHD_MLOCKALL="${VSWITCHD_MLOCKALL:-yes}" VSWITCHD_LOGFILE="${VSWITCHD_LOGFILE:-/var/log/ovs-vswitchd.log}" VSWITCHD_FILE_LOGLEVEL="${VSWITCHD_FILE_LOGLEVEL:-INFO}" VSWITCHD_SYSLOG_LOGLEVEL="${VSWITCHD_SYSLOG_LOGLEVEL:-ERR}" @@ -159,12 +160,15 @@ function start_vswitchd { if [ "$ENABLE_FAKE_PROC_NET" = "y" ]; then fake_proc_net_opt="--fake-proc-net" fi + if [ "$VSWITCHD_MLOCKALL" != "no" ]; then + mlockall_opt="--mlockall" + fi if [ "$daemonize" != "y" ]; then # Start in background and force a "success" message action "Starting ovs-vswitchd ($strace_opt$valgrind_opt)" true - (nice -n "$VSWITCHD_PRIORITY" $strace_opt $valgrind_opt "$vswitchd" --pidfile="$VSWITCHD_PIDFILE" --detach --no-chdir $fake_proc_net_opt -vANY:CONSOLE:EMER $syslog_opt $logfile_level_opt $logfile_file_opt $leak_opt "$VSWITCHD_CONF") & + (nice -n "$VSWITCHD_PRIORITY" $strace_opt $valgrind_opt "$vswitchd" --pidfile="$VSWITCHD_PIDFILE" --detach --no-chdir $fake_proc_net_opt -vANY:CONSOLE:EMER $syslog_opt $logfile_level_opt $logfile_file_opt $leak_opt $mlockall_opt "$VSWITCHD_CONF") & else - action "Starting ovs-vswitchd" nice -n "$VSWITCHD_PRIORITY" "$vswitchd" --pidfile="$VSWITCHD_PIDFILE" --detach --no-chdir $fake_proc_net_opt -vANY:CONSOLE:EMER $syslog_opt $logfile_level_opt $logfile_file_opt $leak_opt "$VSWITCHD_CONF" + action "Starting ovs-vswitchd" nice -n "$VSWITCHD_PRIORITY" "$vswitchd" --pidfile="$VSWITCHD_PIDFILE" --detach --no-chdir $fake_proc_net_opt -vANY:CONSOLE:EMER $syslog_opt $logfile_level_opt $logfile_file_opt $leak_opt $mlockall_opt "$VSWITCHD_CONF" fi } diff --git a/xenserver/root_vswitch_scripts_sysconfig.template b/xenserver/root_vswitch_scripts_sysconfig.template index f848e7b42..2578d11ce 100644 --- a/xenserver/root_vswitch_scripts_sysconfig.template +++ b/xenserver/root_vswitch_scripts_sysconfig.template @@ -43,6 +43,13 @@ # processes. # VSWITCHD_PRIORITY=-10 +# VSWITCHD_MLOCKALL: Whether to pass ovs-vswitchd the --mlockall option. +# This option should be set to "yes" or "no". The default is "yes". +# Enabling this option can avoid networking interruptions due to +# system memory pressure in extraordinary situations, such as multiple +# concurrent VM import operations. +# VSWITCHD_MLOCKALL=yes + # VSWITCHD_LOGFILE: File to send the FILE_LOGLEVEL log messages to. # VSWITCHD_LOGFILE=/var/log/ovs-vswitchd.log From 347401f756e6678fced43ecee27f5107c803fda2 Mon Sep 17 00:00:00 2001 From: Justin Pettit Date: Thu, 10 Dec 2009 21:15:16 -0800 Subject: [PATCH 5/5] vconn: Have check_action() perform all validation The function check_action() returned before it could finish its validation. The only checks that were missed were length checks, which were verified earlier. Thanks to Jean Tourrilhes for pointing out the issue. --- lib/vconn.c | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/lib/vconn.c b/lib/vconn.c index a3ce214eb..922198a3f 100644 --- a/lib/vconn.c +++ b/lib/vconn.c @@ -1269,6 +1269,17 @@ check_action(const union ofp_action *a, unsigned int len, int max_ports) { int error; + if (!len) { + VLOG_DBG_RL(&bad_ofmsg_rl, "action has invalid length 0"); + return ofp_mkerr(OFPET_BAD_ACTION, OFPBAC_BAD_LEN); + } + + if (len % ACTION_ALIGNMENT) { + VLOG_DBG_RL(&bad_ofmsg_rl, "action length %u is not a multiple of %d", + len, ACTION_ALIGNMENT); + return ofp_mkerr(OFPET_BAD_ACTION, OFPBAC_BAD_LEN); + } + switch (ntohs(a->type)) { case OFPAT_OUTPUT: error = check_action_port(ntohs(a->output.port), max_ports); @@ -1302,17 +1313,6 @@ check_action(const union ofp_action *a, unsigned int len, int max_ports) VLOG_WARN_RL(&bad_ofmsg_rl, "unknown action type %"PRIu16, a->type); return ofp_mkerr(OFPET_BAD_ACTION, OFPBAC_BAD_TYPE); } - - if (!len) { - VLOG_DBG_RL(&bad_ofmsg_rl, "action has invalid length 0"); - return ofp_mkerr(OFPET_BAD_ACTION, OFPBAC_BAD_LEN); - } - if (len % ACTION_ALIGNMENT) { - VLOG_DBG_RL(&bad_ofmsg_rl, "action length %u is not a multiple of %d", - len, ACTION_ALIGNMENT); - return ofp_mkerr(OFPET_BAD_ACTION, OFPBAC_BAD_LEN); - } - return 0; } int