RYAN D. MOATS [Mon, 21 Mar 2016 20:40:01 +0000 (15:40 -0500)]
Separating OVN NB and SB database processes
OVN NB & SB DB's should be run in separate ovsdb-server processes
and should run with ovn-ctl start_northd / stop_northd. This patch
includes changes to unit tests, tutorial and debian scripts to remain
self-consistent.
Signed-off-by: RYAN D. MOATS <rmoats@us.ibm.com> Signed-off-by: Michael Arnaldi <arnaldimichael@gmail.com> Co-authored-by: Michael Arnaldi <arnaldimichael@gmail.com> Signed-off-by: Russell Bryant <russell@ovn.org>
Lance Richardson [Wed, 23 Mar 2016 20:04:47 +0000 (16:04 -0400)]
travis: support OS X builds
Add support for travis-ci OS X builds:
- Add linux- prefix to existing build/prepare scripts
- Create new OS X flavored build/prepare scripts
- Update .travis.yml for OS X
At this time only one build job included in the matrix for OS X.
Signed-off-by: Lance Richardson <lrichard@redhat.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Lance Richardson [Wed, 23 Mar 2016 20:04:46 +0000 (16:04 -0400)]
osx: Fix OS X build issues
Newer versions of OS X define ntohll() and htonll() macros,
while older versions (including the version used for travis-ci
builds) do not. Adjust conditional compilation test to work
for both cases.
Eliminate an unused variable warning in lib/rtbsd.c.
Signed-off-by: Lance Richardson <lrichard@redhat.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Russell Bryant [Wed, 23 Mar 2016 19:56:32 +0000 (15:56 -0400)]
ovn: Document chassis->name for ovn-controller-vtep.
ovn-controller-vtep populates the Chassis name field with the name
column from the Physical_Switch table in the hardware_vtep database.
Add this to the documentation of the OVN_Southbound schema.
Han Zhou [Sun, 20 Mar 2016 07:08:48 +0000 (00:08 -0700)]
lib/util.c: Optimise bitwise_rscan.
bitwise_rscan() is found to be hot spot in ovn-controller during OVN
scalability tests. It is triggered by lflow_run() when processing
lflow updates from SB ovsdb. The perf result shows:
Russell Bryant [Tue, 22 Mar 2016 02:29:38 +0000 (22:29 -0400)]
ovn: Add hostname to Chassis.
We currently use the system-id from the Open_vSwitch schema on each host
to populate the unique name field of a Chassis in OVN_Southbound. On
most systems, this is a UUID. It would be very convenient to also have
the hostname available as that will allow people to more quickly
identify which host a Chassis record is associated with in most cases.
This is also useful for correlating an OVN Chassis with the knowledge of
hosts that exists in other systems (such as OpenStack).
Also add the hostname of each chassis to the output of "ovn-sbctl show".
Signed-off-by: Russell Bryant <russell@ovn.org> Acked-by: Justin Pettit <jpettit@ovn.org>
Alin Serdean [Tue, 15 Mar 2016 19:41:44 +0000 (19:41 +0000)]
datapath-windows: Change reported time for flows
Currently the datapath reports the tick counter to the userspace.
The userspace uses KeQueryPerformanceCounter as a monotonic clock.
This patch changes the flow stats to be reported in a monotonic format, while
also decaying the time between the flow actual usage and the flow report usage.
This patch also changes to report EEXIST if the userspace tries to add the same
flow twice.
After adding a flow, lookup the flow only if the extension is compiled in debug
mode.
Signed-off-by: Alin Gabriel Serdean <aserdean@cloudbasesolutions.com> Acked-by: Sorin Vinturis <svinturis@cloudbasesolutions.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
YAMAMOTO Takashi [Wed, 16 Mar 2016 07:48:48 +0000 (07:48 +0000)]
vlog.at: Avoid using GNU sed extension
BRE alternative (\|) is an GNU sed extension. [1]
It isn't available in NetBSD sed.
[1] http://www.gnu.org/software/sed/manual/sed.html#Regular-Expressions
regexp1\|regexp2
Matches either regexp1 or regexp2. Use parentheses to use
complex alternative regular expressions. The matching process
tries each alternative in turn, from left to right, and the
first one that succeeds is used. It is a GNU extension.
Signed-off-by: YAMAMOTO Takashi <yamamoto@midokura.com> Acked-by: Ben Pfaff <blp@ovn.org>
Lance Richardson [Tue, 15 Mar 2016 15:52:58 +0000 (11:52 -0400)]
osx: handle differences between OS X and other BSDs
Conditional compilation to account for:
- OS X does not implement RTM_IFANNOUNCE.
- OS X does not implement tap netdeivces.
- OS X does not implement RT_ROUNDUP().
Signed-off-by: Lance Richardson <lrichard@redhat.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Ilya Maximets [Tue, 22 Mar 2016 12:42:03 +0000 (15:42 +0300)]
netdev-dpdk: Fix crash when changing the vhost-user port.
According to netdev-provider API:
'The "destruct" function is not allowed to fail.'
netdev-dpdk breaks this restriction for vhost-user ports.
This leads to SIGABRT or SIGSEGV in dpdk_watchdog thread
because 'dealloc' will be called anyway indifferently
to result of 'destruct'.
For example, if we call
# ovs-vsctl set interface vhost1 ofport_request=5
while QEMU still attached, we'll get:
------------------[cut]------------------
|dpdk|ERR|Can not remove port, vhost device still attached
VHOST_CONFIG: socket created, fd:98
VHOST_CONFIG: fail to bind fd:98, remove file:/home/vhost1 and try again.
|dpdk|ERR|vhost-user socket device setup failure for socket /home/vhost1
|bridge|WARN|could not open network device vhost1 (Unknown error -1)
ovs-vswitchd(dpdk_watchdog1): lib/netdev-dpdk.c:532: ovs_mutex_lock_at()
passed uninitialized ovs_mutex
Program received signal SIGABRT, Aborted.
------------------[cut]------------------
Fix that by removing port anyway even when guest is still
attached. Guest becomes an orphan in that case but OVS
will not crash and will continue forwarding for other ports.
VM restart required to restore connectivity.
Fixes: 58397e6c1e6c ("netdev-dpdk: add dpdk vhost-cuse ports") Signed-off-by: Ilya Maximets <i.maximets@samsung.com> Acked-by: Kevin Traynor <kevin.traynor@intel.com> Acked-by: Daniele Di Proietto <diproiettod@vmware.com>
RYAN D. MOATS [Fri, 11 Mar 2016 21:06:17 +0000 (15:06 -0600)]
ovsdb-idl: Present tracked changes in increasing change number order.
Currently changes are added to the front of the track list, so
they are looped through in LIFO order. Incremental processing
is more efficient with a FIFO presentation, so
(1) add new changes to the back of the track list, and
(2) move updated changes to the back of the track list
Signed-off-by: RYAN D. MOATS <rmoats@us.ibm.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Ian Stokes [Fri, 18 Mar 2016 17:14:16 +0000 (17:14 +0000)]
bridge: Dump configurable QoS types.
This commit adds a new command 'qos/show-types' for use with appctl.
This allows a user to query the types of QoS which are configurable via
Open vSwitch on a given interface.
Signed-off-by: Ian Stokes <ian.stokes@intel.com>
[blp@ovn.org made style and message changes] Signed-off-by: Ben Pfaff <blp@ovn.org>
Ian Stokes [Tue, 8 Mar 2016 23:10:31 +0000 (23:10 +0000)]
bridge: Fix qos_unixctl_show bug.
netdev_get_qos returns a value to indicate if an error has occurred while
attempting to query the QoS configuration of an interface. If an error does
occur the pointer argument passed to it will be set to null before returning.
Currently the vswitch will segfault if this occurs as qos_unixctl_show will
attempt to access the pointer directly after it calls netdev_get_qos.
Avoid this by adding a check for the return value and flagging an appropriate
error message to appctl.
Signed-off-by: Ian Stokes <ian.stokes@intel.com>
[blp@ovn.org changed details of error report] Signed-off-by: Ben Pfaff <blp@ovn.org>
Russell Bryant [Mon, 21 Mar 2016 15:09:52 +0000 (11:09 -0400)]
ovn: Remove outdated debug messages.
ovn-northd used to only call ovnsb_db_run() and ovnnb_db_run() when
seqnos indicated that they had changed. It now always calls these
functions from the main loop. Remove these outdated log messages
that indicate that we called this function because contents changed.
Signed-off-by: Russell Bryant <russell@ovn.org> Acked-by: Justin Pettit <jpettit@ovn.org>
Jesse Gross [Mon, 21 Mar 2016 18:39:38 +0000 (11:39 -0700)]
datapath: Check for sock argument to v6ops->fragment.
Ubuntu 3.13.0-83-generic has backported a patch that adds an intermediate
version of the v6ops->fragment function that doesn't seem to ever been
part of a released upstream kernel. This version is missing the sock
argument to the fragment function.
Since we already have a backported version of the function from a newer
kernel, this simply ignores the version that Ubuntu is now making available
and continues to use the OVS version, similar to what it was doing before.
Reported-by: Zoltán Balogh <zoltan.balogh@ericsson.com> Reported-by: Aaron Rosen <aaronorosen@gmail.com> Reported-by: Russell Bryant <russell@ovn.org> Signed-off-by: Jesse Gross <jesse@kernel.org> Acked-by: Russell Bryant <russell@ovn.org>
Nithin Raju [Fri, 18 Mar 2016 20:17:54 +0000 (13:17 -0700)]
list.h: Define OVS_LIST_POISON statically
The previous definitions of these variables using designated
initializers caused a variety of issues when attempting to
compile with MSVC, particularly if including these headers from C++
code. By defining them like this, we can appease MSVC and keep the
definitions the same on all platforms.
Suggested-by: Yin Lin <linyi@vmware.com> Signed-off-by: Nithin Raju <nithin@vmware.com>
[blp@ovn.org changed large literal to avoid sparse warning] Signed-off-by: Ben Pfaff <blp@ovn.org>
tests: consistently use OVS_APP_EXIT_AND_WAIT() for daemon termination
Tools such as gcov rely on normal termination (via exit()), ensure
consistent graceful termination of daemons in tests by using
the OVS_APP_EXIT_AND_WAIT() macro.
Signed-off-by: Lance Richardson <lrichard@redhat.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
As documented, the as() function should only create a subshell
to execute the provided command when a command is present. Correct
the implementation to check for the presence of the command parameter
instead of the sandbox name (which is always present in current
usage).
Signed-off-by: Lance Richardson <lrichard@redhat.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
tests: Make OVS_APP_EXIT_AND_WAIT() wait for process termination
Currently OVS_APP_EXIT_AND_WAIT() only waits for the pid file
to be removed. When using gcov (and likely other utilities),
we need to ensure that the application has actually exited.
Also adding AT_CHECK() around the exit request to allow simplification
of tests using open-coded versions of this macro.
Signed-off-by: Lance Richardson <lrichard@redhat.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Numan Siddique [Wed, 2 Mar 2016 19:08:42 +0000 (00:38 +0530)]
ovn: Add l3 port security for IPv4 and IPv6
This patch extends the port security to support L3.
The ingress stage 'ls_in_port_sec' is renamed to 'ls_in_port_sec_l2'
and 2 new stages 'ls_in_port_sec_ip' (table 1) and 'ls_in_port_sec_nd'
(table 2) are added. 'ls_in_port_sec_ip' adds flows to restrict
the IPv4 and IPv6 traffic to valid IPv4 and IPv6 addresses of the port.
'ls_in_port_sec_nd' adds flows to restricts the ARP and IPv6 ND
packets.
For egress pipeline, 'ls_out_port_sec' is renamed to 'ls_out_port_sec_l2'
and a new stage 'ls_out_port_sec_ip' is added before 'ls_out_port_sec_l2'
to restrict the IPv4 and IPv6 traffic for valid IPs.
Signed-off-by: Numan Siddique <nusiddiq@redhat.com> Co-authored-by: Ben Pfaff <blp@ovn.org> Signed-off-by: Ben Pfaff <blp@ovn.org>
Quentin Monnet [Wed, 2 Mar 2016 14:56:21 +0000 (15:56 +0100)]
ovs-ofctl: Update manpage for --color option.
Document the `--color` option for ovs-ofctl in its manpage. The option
documentation has been added into a new file (included into the
manpage) so that it can easily be added to other manpages as well when
support for colorized output is extended to other utilities.
The use of the environment variable OVS_COLORS (used for user-defined
colors) has been documented as well.
Signed-off-by: Quentin Monnet <quentin.monnet@6wind.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Quentin Monnet [Wed, 2 Mar 2016 14:56:20 +0000 (15:56 +0100)]
ofp-actions: Color output of flow actions for ovs-ofctl dump-flows.
Add color output for flow actions for ovs-ofctl dump-flows command
utility, by inserting color markers in the functions responsible for
printing those actions.
Signed-off-by: Quentin Monnet <quentin.monnet@6wind.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Quentin Monnet [Wed, 2 Mar 2016 14:56:19 +0000 (15:56 +0100)]
match: Color output of match conditions for ovs-ofctl dump-flows.
Add color output for flow match conditions for ovs-ofctl dump-flows
command utility, by inserting color markers in the functions responsible
for printing those match condictions.
Signed-off-by: Quentin Monnet <quentin.monnet@6wind.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Quentin Monnet [Wed, 2 Mar 2016 14:56:17 +0000 (15:56 +0100)]
ovs-ofctl: Declare / set up colors for command output.
OVS_COLORS environment variable is parsed to extract user-defined
preferences regarding colors (this is used to set up a color theme, not
to replace the `--color` option for activating color output).
The string should be of a format similar to LS_COLORS or GREP_COLORS,
with available colors being as follows:
* ac: action field
* dr: drop keyword
* le: learn keyword
* pm: parameters receiving attributes
* pr: keyword having parenthesis
* sp: some special keywords
* vl: lone values with no parameter name
For color whose idendifier does not appear in the string, the default
hardcoded value is used instead.
As an example, setting OVS_COLORS to the following string is equivalent
to using the default values:
Quentin Monnet [Wed, 2 Mar 2016 14:56:16 +0000 (15:56 +0100)]
ovs-ofctl: Add option for color output to dump-flows command.
Add an option to ovs-ofctl utility so as to obtain colorized output in
tty, for easier reading. Currently, only the dump-flows command supports
colors.
A new `--color` option has been added to ovs-ofctl so as to indicate
whether color markers should be used or not. It can be set to `always`
(force colors), `never` (no colors) or `auto` (use colors only if output
is a tty). If provided without any value, it is the same as `auto`. If
the option is not provided at all, colors are disabled by default.
Examples:
This first call will output colorized flows:
ovs-ofctl dump-flows br0 --color=always
These two calls will produce colorized output on a tty, but they will
not use color markers if the output is redirected to a file or piped
into another command:
The result of this option is stored into a variable which is to be
forwarded (in next commits) as a function argument until it reaches the
functions that print the elements of the flows.
Signed-off-by: Quentin Monnet <quentin.monnet@6wind.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
ovs-vtep: Delete flows with untagged vlan carefully.
A (physical port + vlan) or (physical port + no tag) can be bound to a
logical switch. When one unbinds (physical port + no tag) from a logical
switch, the emulator inadvertantly deletes flows for (physical port + vlan)
too. This commit fixes it.
VMware-BZ: #1609938 Reported-by: Mike Qing <mqing@vmware.com> Signed-off-by: Gurucharan Shetty <guru@ovn.org> Acked-by: Ryan Moats <rmoats@us.ibm.com>
Ben Pfaff [Wed, 16 Mar 2016 23:45:29 +0000 (16:45 -0700)]
ovs-ctl: Remove code for upgrading from Open vSwitch 1.6 and earlier.
OVS 1.7 was released in July 2012, so by the time current OVS is released,
it will be almost four years old. I think that this is long enough to
remove special upgrade code.
It's almost impossible to do this upgrade in any case since there is
no overlap in the base kernel versions supported by the out of tree
modules in the two versions (the only possibility is running the new
version of OVS on the upstream module from Linux 3.3).
Signed-off-by: Ben Pfaff <blp@ovn.org> Acked-by: Jesse Gross <jesse@kernel.org>
Ben Pfaff [Thu, 17 Mar 2016 03:10:40 +0000 (20:10 -0700)]
ovs-ctl: Remove code for upgrading from Open vSwitch 1.9 and earlier.
OVS 1.10 was released in May 2013, so by the time current OVS is released,
it will be about three years old. I think that this is long enough to
remove special upgrade code from the startup scripts.
Signed-off-by: Ben Pfaff <blp@ovn.org> Acked-by: Jesse Gross <jesse@kernel.org>
Sairam Venugopal [Mon, 29 Feb 2016 19:42:12 +0000 (11:42 -0800)]
datapath-windows: Support for IPv6 in TCP segmentation
When a packet which needs segmentation is received, the header for
each segment is being calculated, i.e. IP length, checksum, TCP seq,
TCP checksum.
The problem with the current code is that it wrongly assumes that
the Ethernet frame payload is always an IPv4 packet.
This patch checks the EtherType field of the Ethernet frame to see
which protocol is encapsulated in its payload, IPv4 or IPv6, and
calculates the segment's header accordingly.
Pravin B Shelar [Mon, 14 Mar 2016 19:42:48 +0000 (12:42 -0700)]
datapath: Fix build failure related to missing vlan.h
---8<---
make[1]: Entering directory `/home/travis/build/openvswitch/ovs/datapath'
make[1]: *** No rule to make target `vlan.h', needed by `distdir'. Stop.
make[1]: Leaving directory `/home/travis/build/openvswitch/ovs/datapath'
make: *** [distdir] Error 1
cat: */_build/tests/testsuite.log: No such file or directory
Signed-off-by: Pravin B Shelar <pshelar@ovn.org> Acked-by: Jesse Gross <jesse@kernel.org>
Lance Richardson [Wed, 24 Feb 2016 15:48:34 +0000 (10:48 -0500)]
ovsdb-idl: improve error handling when schema is not available
A common error scenario with OVN is to attempt to use ovn-nbctl when
the OVN databases have not been created in ovsdb-server:
1. ovn-nbctl sends a "get_schema" request for the OVN db to ovsdb-server.
2. ovsdb-server fails to find requested db, sends error response
to ovn-nbctl.
3. ovn-nbctl receives the error response in ovsdb_idl_run(), but
takes no specific action.
4. ovn-nbctl hangs forever in IDL_S_SCHEMA_REQUESTED state (assuming
a timeout wasn't requested on the command line).
This commit adds a new IDL state, IDL_S_NO_SCHEMA, which is entered
when a negative response to a schema request is received. When in
this state, ovsdb_idl_is_alive() now returns 'false', allowing clients
(currently ovn-nbctl, ovn-sbctl, vtep-ctl, and ovs-vsctl) to detect this
condition and exit with an appropriate error message.
Signed-off-by: Lance Richardson <lrichard@redhat.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Pravin B Shelar [Mon, 29 Feb 2016 17:54:15 +0000 (09:54 -0800)]
datapath: Drop support for kernel older than 3.10
Currently OVS out of tree datapath supports a large number of kernel
versions. From 2.6.32 to 4.3 and various distribution-specific
kernels. But at this point major features are only available on more
recent kernels. For example, stateful services are only available
starting in kernel 3.10 and STT is available on starting with 3.5.
Since these features are becoming essential to many OVS deployments,
and the effort of maintaining the backports is high. We have decided
to drop support for older kernel. Following patch drops supports
for kernel older than 3.10.
Signed-off-by: Pravin B Shelar <pshelar@ovn.org> Acked-by: Jesse Gross <jesse@kernel.org>
Ben Pfaff [Sat, 20 Feb 2016 00:34:19 +0000 (16:34 -0800)]
ovn: Implement basic ARP support for L3 logical routers.
This is sufficient support that an L3 logical router can now transmit
packets to VMs (and other destinations) without having to know the
IP-to-MAC binding in advance. The details are carefully documented in all
of the appropriate places.
There are several important caveats that need to be fixed before this can
be taken seriously in production. These are documented in ovn/TODO. The
most important of these are renewal, expiration, and limiting the size of
the ARP table.
Signed-off-by: Ben Pfaff <blp@ovn.org> Acked-by: Justin Pettit <jpettit@ovn.org>
Ben Pfaff [Thu, 28 Jan 2016 05:17:11 +0000 (21:17 -0800)]
ovn-controller: Add data structure for indexing lports, multicast groups.
This was more or less implemented inside lflow.c until now, but some
upcoming code that shouldn't be in that file needs to use it too.
This also adds a second index on lports, so that lports can be looked up
based on the logical datapath tunnel key and the logical port tunnel key.
An upcoming commit will add a user for this new index.
Signed-off-by: Ben Pfaff <blp@ovn.org> Acked-by: Justin Pettit <jpettit@ovn.org>
Ben Pfaff [Thu, 28 Jan 2016 04:50:59 +0000 (20:50 -0800)]
ovn: Use callback function instead of simap for logical port number map.
An simap is convenient but it isn't very flexible. If the client wants to
keep extra data with each node then it has to build a second parallel data
structure. A callback function is kind of a pain for the clients from the
point of view of having to write it and deal with auxiliary data, etc., but
it allows the storage to be more flexible.
An upcoming commit will make further use of this capability.
Signed-off-by: Ben Pfaff <blp@ovn.org> Acked-by: Justin Pettit <jpettit@ovn.org>
lib/ovs-thread: make use of the pthread_attr object
The pthread_attr object needs to be passed to the pthread_create()
call in order to make use of it.
Fixes: 8147cec9ee (lib/ovs-thread: Ensure that thread stacks are
always at least 512 kB.) Signed-off-by: Alexandru Ardelean <ardeleanalex@gmail.com> Acked-by: Andy Zhou <azhou@ovn.org>
Pravin B Shelar [Thu, 10 Mar 2016 00:40:40 +0000 (16:40 -0800)]
smap: bsd: Fix compilation error.
I saw following error while testing this series.
---8<---
In file included from lib/smap.c:16:
./lib/smap.h:75:56: warning: declaration of 'struct in6_addr' will not be visible outside of this function [-Wvisibility]
void smap_add_ipv6(struct smap *, const char *, struct in6_addr *);
^
lib/smap.c:102:1: error: conflicting types for 'smap_add_ipv6'
smap_add_ipv6(struct smap *smap, const char *key, struct in6_addr *addr)
^
./lib/smap.h:75:6: note: previous declaration is here
void smap_add_ipv6(struct smap *, const char *, struct in6_addr *);
Signed-off-by: Pravin B Shelar <pshelar@ovn.org> Acked-by: Ben Pfaff <blp@ovn.org>
INSTALL.DPDK: remove issue with QEMU v2.4.0 and dpdkvhostuser
The patch mentioned in the commit e73b7508fb58 ("INSTALL.DPDK: Mention
issue with QEMU v2.4.0 & dpdkvhostuser") is present in DPDK v2.2.0,
then this issue is not valid anymore.
Signed-off-by: Mauricio Vasquez B <mauricio.vasquezbernal@studenti.polito.it> Acked-by: Daniele Di Proietto <diproiettod@vmware.com>
Yuanhan Liu [Tue, 8 Mar 2016 01:50:48 +0000 (09:50 +0800)]
netdev-dpdk: fix mbuf leaks
mbufs could be chained (by the "next" field of rte_mbuf struct), when
an mbuf is not big enough to hold a big packet, say when TSO is enabled.
rte_pktmbuf_free_seg() frees the head mbuf only, leading mbuf leaks.
This patch fix it by invoking the right API rte_pktmbuf_free(), to
free all mbufs in the chain.
Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com> Signed-off-by: Daniele Di Proietto <diproiettod@vmware.com>
andy zhou [Mon, 7 Mar 2016 23:44:34 +0000 (15:44 -0800)]
ovsdb-server: Fix a reference count leak bug
When destroying an ovsdb_jsonrpc_monitor, the jsonrpc monitor still
holds a reference count to the monitors 'changes' indexed with
'unflushed' transaction id. The bug is that the reference count was
not decremented as it should in the code path.
The bug caused 'changes' that have been flushed to all jsonrpc
clients to linger around unnecessarily, occupying increasingly
large amount of memory. See "Reported-at" URL for more details.
This bug is tricky to find since the memory is not leaked; they will
eventually be freed when monitors are destroyed.
Reported-by: Lei Huang <huang.f.lei@gmail.com>
Reported-at: http://openvswitch.org/pipermail/dev/2016-March/067274.html Signed-off-by: Andy Zhou <azhou@ovn.org> Tested-by: Han Zhou <zhouhan@gmail.com> Acked-by: Han Zhou <zhouhan@gmail.com> Acked-by: Liran Schour <lirans@il.ibm.com>
Ben Pfaff [Mon, 7 Mar 2016 22:58:25 +0000 (14:58 -0800)]
Revert "ovn-controller: race between binding-run and patch-run for localnet ports"
This reverts commit 3a83007a76bbf05144cee1fda7ad81c1c717dca7. It's really
nonobvious from the code why the condition added by that commit makes sense.
The new condition should not be necessary now that binding_run() always keeps
track of the local datapaths, since commit 7c040135cf351 (binding: Track local
datapaths even when no transaction is possible).
CC: Ramu Ramamurthy <ramu.ramamurthy@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org> Acked-by: Russell Bryant <russell@ovn.org>
Ben Pfaff [Mon, 7 Mar 2016 23:13:15 +0000 (15:13 -0800)]
unixctl: Log commands received and their replies (at debug level).
These commands are also visible through the "jsonrpc" module, but turning
up the log level there also exposes a lot of OVSDB traffic that usually
isn't interesting.
Also, enable this logging for the tests.
Signed-off-by: Ben Pfaff <blp@ovn.org> Acked-by: Justin Pettit <jpettit@ovn.org>
Ben Pfaff [Mon, 7 Mar 2016 20:53:15 +0000 (12:53 -0800)]
binding: Track local datapaths even when no transaction is possible.
Plenty of other code depends on the set of local datapaths. Most notably,
the lflow code will drop logical flows when their logical datapaths aren't
present locally.
Signed-off-by: Ben Pfaff <blp@ovn.org> Acked-by: Russell Bryant <russell@ovn.org>
Russell Bryant [Mon, 7 Mar 2016 15:47:21 +0000 (10:47 -0500)]
ovs-sandbox: Add note about OVN to initial output.
When you run ovs-sandbox, it finishes with a note describing the dummy
environment it has set up. Add some additional text that indicates that
OVN is also enabled when that is the case.
Signed-off-by: Russell Bryant <russell@ovn.org> Acked-by: Ben Pfaff <blp@ovn.org> Acked-by: Ryan Moats <rmoats@us.ibm.com>
William Tu [Fri, 4 Mar 2016 23:18:54 +0000 (15:18 -0800)]
ofp-util: Fix use-after-free in group append.
It is possible for ofpbuf_put() to realloc a newly allocated address,
casuing the previously referenced pointer, ogds, points to old/free'd
address. The issue is generated by forcing ofpbuf_put() to use newly
allocated buffer and valgrind reports invalid write. The similiar syndrome
is reported at: https://patchwork.ozlabs.org/patch/591330/
William Tu [Sat, 5 Mar 2016 02:00:46 +0000 (18:00 -0800)]
ofpbuf: Fix use-after-free in bundle parse.
Address pointed by bundle could be obsolete/free'd when
realloc, called from ofpbuf_put_zero(), returns new address.
Reported by Valgrind 367: ovs-ofctl parse-flows (NXM)
Jarno Rajahalme [Mon, 7 Mar 2016 19:00:44 +0000 (11:00 -0800)]
ofpbuf: Fix setting of 'msg' in ofpbuf_clone_with_headroom()
Commit 38876d31 fixed setting 'msg' when resizing an ofpbuf, but
failed to fix the same issue in ofpbuf_clone_with_headroom(). Without
this fix the newly cloned ofpbuf's 'msg', if non-NULL, will point to
the buffer of the original ofpbuf.
Signed-off-by: Jarno Rajahalme <jarno@ovn.org> Acked-by: Joe Stringer <joe@ovn.org>
Ramu Ramamurthy [Sat, 5 Mar 2016 01:40:26 +0000 (20:40 -0500)]
ovn-controller: race between binding-run and patch-run for localnet ports
when ctx->ovnsb_idl_txn is null, binding_run exits early
and does not add any local_datapaths, but patch_run
doesnt check this, and ends up deleting localnet ports,
because there are no local datapaths for them,
They get readded in a subsequent run causing unnecessary
deletion and readdition.
Signed-off-by: Ramu Ramamurthy <ramu.ramamurthy@us.ibm.com> Signed-off-by: Russell Bryant <russell@ovn.org>
Use 'RUNDIR' from make for rhel/ovn-controller.service
Perviously it was using the platform's runtime directory which can be
different from the runtime directory of ovsdb-server started by the
openvswitch service
Pravin B Shelar [Fri, 4 Mar 2016 00:15:40 +0000 (16:15 -0800)]
datapath: STT: Fix checksum handling.
On packet receive STT verifies the checksum if not done in
hardware. But IP and TCP were pulled before the verification
step. The verification expect to see packet with TCP header.
This causes STT to drop packet in certain cases.
Signed-off-by: Pravin B Shelar <pshelar@ovn.org> Acked-by: Joe Stringer <joe@ovn.org>
Joe Stringer [Thu, 3 Mar 2016 08:22:50 +0000 (21:22 +1300)]
ofp-actions: Prevent integer overflow in decode.
When decoding a variable-length action, if the length of the action
exceeds the length storable in a uint16_t then something has gone
terribly wrong. Assert that this is not the case.
Signed-off-by: Joe Stringer <joe@ovn.org> Acked-by: Jarno Rajahalme <jarno@ovn.org>
Joe Stringer [Thu, 3 Mar 2016 08:22:49 +0000 (21:22 +1300)]
ofp-actions: Fix use-after-free in bundle action.
If the actions list in an incoming flow mod is long enough, and there is
a bundle() action with 3 or more slaves, then it is possible for a
reallocation to occur after placing the ofpact_bundle into the ofpacts
buffer, while slave ports into the buffer. If the memory freed by this
reallocation is then passed to another thread, then that thread may
modify the value that bundle->n_slaves points to. If this occurs quickly
enough before the main thread finishes copying all of the slaves, then
the iteration may continue beyond the originally intended number of
slaves, copying (and swapping) an undetermined number of 2-byte chunks
from the openflow message. Finally, the length of the ofpact will be
updated based on how much data was written to the buffer, which may be
significantly longer than intended.
In many cases, the freed memory may not be allocated to another thread
and be left untouched. In some milder bug cases, this will lead to
'bundle' actions using more memory than required. In more serious cases,
this length may then exceed the maximum length of an OpenFlow action,
which is then stored (truncated) into the 16-bit length field in the
ofpact header. Later execution of ofpacts_verify() would then use this
length to iterate through the ofpacts, and may dereference memory in
unintended ways, causing crashes or infinite loops by attempting to
parse/validate arbitrary data as ofpact objects.
Fix the issue by updating 'bundle' within the iteration, immediately
after (potentially) expanding the bundle.
Thanks to Jarno Rajahalme for his keen pair of eyes on finding this
issue.
VMWare-BZ: #1614715 Fixes: f25d0cf3c366 ("Introduce ofpacts, an abstraction of OpenFlow actions.") Signed-off-by: Joe Stringer <joe@ovn.org> Acked-by: Jarno Rajahalme <jarno@ovn.org>
Joe Stringer [Thu, 3 Mar 2016 08:22:48 +0000 (21:22 +1300)]
tests: Add bundle action test with buffer realloc.
Add a test which causes internal reallocation of the ofpacts buffer,
followed by a large bundle action which should cause a subsequent
reallocation while decoding slave ports. Running this test under
valgrind reveals the issue below, which is fixed in the following
commit.
Invalid read of size 4
at 0x4CED87: decode_bundle (ofp-actions.c:1253)
by 0x4CEDFC: decode_NXAST_RAW_BUNDLE (ofp-actions.c:1272)
by 0x4DBDE6: ofpact_decode (ofp-actions.inc2:3765)
by 0x4D6914: ofpacts_decode (ofp-actions.c:5735)
by 0x4D6A3D: ofpacts_pull_openflow_actions__ (ofp-actions.c:5772)
by 0x4D74F3: ofpacts_pull_openflow_instructions (ofp-actions.c:6352)
by 0x4F59FA: ofputil_decode_flow_mod (ofp-util.c:1704)
by 0x4EAD18: ofp_print_flow_mod (ofp-print.c:786)
by 0x4F0711: ofp_to_string__ (ofp-print.c:3220)
by 0x4F0D98: ofp_to_string (ofp-print.c:3453)
by 0x5486B3: do_recv (vconn.c:644)
by 0x548498: vconn_recv (vconn.c:598)
by 0x524582: rconn_recv (rconn.c:703)
by 0x45DA61: ofconn_run (connmgr.c:1370)
by 0x45B3B4: connmgr_run (connmgr.c:323)
by 0x41D1E8: ofproto_run (ofproto.c:1762)
by 0x40CEE0: bridge_run__ (bridge.c:2885)
by 0x40D093: bridge_run (bridge.c:2940)
by 0x412F7E: main (ovs-vswitchd.c:120)
Address 0x66aa460 is 1,152 bytes inside a block of size 1,184 free'd
at 0x4C2AF2E: realloc (vg_replace_malloc.c:692)
by 0x543D27: xrealloc (util.c:123)
by 0x5089EF: ofpbuf_resize__ (ofpbuf.c:243)
by 0x508B81: ofpbuf_prealloc_tailroom (ofpbuf.c:290)
by 0x508D5C: ofpbuf_put_uninit (ofpbuf.c:364)
by 0x508DEF: ofpbuf_put (ofpbuf.c:387)
by 0x4CED7D: decode_bundle (ofp-actions.c:1255)
by 0x4CEDFC: decode_NXAST_RAW_BUNDLE (ofp-actions.c:1272)
by 0x4DBDE6: ofpact_decode (ofp-actions.inc2:3765)
by 0x4D6914: ofpacts_decode (ofp-actions.c:5735)
by 0x4D6A3D: ofpacts_pull_openflow_actions__ (ofp-actions.c:5772)
by 0x4D74F3: ofpacts_pull_openflow_instructions (ofp-actions.c:6352)
by 0x4F59FA: ofputil_decode_flow_mod (ofp-util.c:1704)
by 0x4EAD18: ofp_print_flow_mod (ofp-print.c:786)
by 0x4F0711: ofp_to_string__ (ofp-print.c:3220)
by 0x4F0D98: ofp_to_string (ofp-print.c:3453)
by 0x5486B3: do_recv (vconn.c:644)
by 0x548498: vconn_recv (vconn.c:598)
by 0x524582: rconn_recv (rconn.c:703)
by 0x45DA61: ofconn_run (connmgr.c:1370)
Signed-off-by: Joe Stringer <joe@ovn.org> Acked-by: Jarno Rajahalme <jarno@ovn.org>