cascardo/ovs.git
9 years agoprocess: block signals while spawning child processes
Ansis Atteka [Fri, 23 May 2014 21:15:28 +0000 (14:15 -0700)]
process: block signals while spawning child processes

Between fork() and execvp() calls in the process_start()
function both child and parent processes share the same
file descriptors.  This means that, if a child process
received a signal during this time interval, then it could
potentially write data to a shared file descriptor.

One such example is fatal signal handler, where, if
child process received SIGTERM signal, then it would
write data into pipe.  Then a read event would occur
on the other end of the pipe where parent process is
listening and this would make parent process to incorrectly
believe that it was the one who received SIGTERM.
Also, since parent process never reads data from this
pipe, then this bug would make parent process to consume
100% CPU by immediately waking up from the event loop.

This patch will help to avoid this problem by blocking
signals until child closes all its file descriptors.

Signed-off-by: Ansis Atteka <aatteka@nicira.com>
Reported-by: Suganya Ramachandran <suganyar@vmware.com>
Issue: 1255110

9 years agoovsdb-idl: Add coverage counters for ovsdb commit return statuses.
Ryan Wilson [Fri, 30 May 2014 06:44:37 +0000 (23:44 -0700)]
ovsdb-idl: Add coverage counters for ovsdb commit return statuses.

Signed-off-by: Ryan Wilson <wryan@nicira.com>
Acked-by: Alex Wang <alexw@nicira.com>
Signed-off-by: Alex Wang <alexw@nicira.com>
9 years agortbsd: Make rtbsd module thread-safe.
Ryan Wilson [Fri, 30 May 2014 06:35:30 +0000 (23:35 -0700)]
rtbsd: Make rtbsd module thread-safe.

Due to patch fe83f8 (netdev: Remove netdev from global shash when
the user is changing interface configuration), netdevs can be
destructed and deallocated by revalidator and RCU threads. When
netdevs with class bsd are destroyed, the rtbsd notifier is
unregistered, possibly causing memory to be freed. This causes a
race condition with the main thread which calls rtbsd_notifier_run
periodically to check for any netdev change events.

This patch makes the rtbsd module thread-safe via a mutex.

Note this patch removes rtbsd_notifier_run() in
rtbsd_notifier_register() due to locking requirements. Since the
rtbsd_notifier_run() is always run by the main thread often,
receiving stale notifications from the notifier is unlikely.

Bug #1258532
Signed-off-by: Ryan Wilson <wryan@nicira.com>
Acked-by: YAMAMOTO Takashi <yamamoto@valinux.co.jp>
Acked-by: Alex Wang <alexw@nicira.com>
Signed-off-by: Alex Wang <alexw@nicira.com>
9 years agoroute-table: Make route-table module thread-safe.
Ryan Wilson [Thu, 29 May 2014 21:56:09 +0000 (14:56 -0700)]
route-table: Make route-table module thread-safe.

Due to patch fe83f8 (netdev: Remove netdev from global shash when
the user is changing interface configuration), netdevs can be
destructed and deallocated by revalidator and RCU threads. When
netdevs with class vport are destroyed, the routing table is
unregistered, possibly causing memory to be freed. This causes a
race condition with the main thread which calls route_table_run
periodically to check for routing table updates.

This patch makes the route-table module thread-safe via a mutex.

Bug #1258532
Signed-off-by: Ryan Wilson <wryan@nicira.com>
Acked-by: Ethan Jackson <ethan@nicira.com>
Signed-off-by: Alex Wang <alexw@nicira.com>
9 years agoofproto: Fix comments.
Ben Pfaff [Wed, 28 May 2014 23:27:02 +0000 (16:27 -0700)]
ofproto: Fix comments.

The comments on the "group" functions had been shamelessly copied without
significant update from the corresponding flow table functions.  This
commit fixes the errors.

This commit also removes an obsolete comment in ofopgroup_complete().

Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Jarno Rajahalme <jrajahalme@nicira.com>
9 years agoofproto-provider: Fix typo in comment.
Ben Pfaff [Thu, 29 May 2014 20:33:29 +0000 (13:33 -0700)]
ofproto-provider: Fix typo in comment.

Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Jarno Rajahalme <jrajahalme@nicira.com>
9 years agoovs-dev.py: add option to run with dpdk
Daniele Di Proietto [Tue, 13 May 2014 03:47:48 +0000 (03:47 +0000)]
ovs-dev.py: add option to run with dpdk

Signed-off-by: Daniele Di Proietto <ddiproietto@vmware.com>
Acked-by: Ethan Jackson <ethan@nicira.com>
9 years agoconfigure.ac: Check C99 compiler
YAMAMOTO Takashi [Thu, 29 May 2014 16:12:24 +0000 (16:12 +0000)]
configure.ac: Check C99 compiler

This ends up to add -std=gnu99 and fixes the following
compilation problem introduced by commit 08feeb75.
("lib/flow: Use C99 declaration in for statement.")

libtool: compile:  gcc -DHAVE_CONFIG_H -I. -I.. -I ../include -I ../lib -I ./lib -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith -Wno-format-zero-length -Wswitch-enum -Wunused-parameter -Wstrict-aliasing -Wbad-function-cast -Wcast-align -Wmissing-prototypes -Wmissing-field-initializers -g -O2 -MT lib/classifier.lo -MD -MP -MF lib/.deps/classifier.Tpo -c ../lib/classifier.c -o lib/classifier.o
../lib/classifier.c: In function 'miniflow_and_mask_matches_flow':
../lib/classifier.c:1722:5: error: 'for' loop initial declarations are only allowed in C99 mode
../lib/classifier.c:1722:5: note: use option -std=c99 or -std=gnu99 to compile your code
Makefile:3013: recipe for target 'lib/classifier.lo' failed

% gcc --version
gcc (NetBSD nb1 20120916) 4.5.4
Copyright (C) 2010 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

Signed-off-by: YAMAMOTO Takashi <yamamoto@valinux.co.jp>
Acked-by: Jarno Rajahalme <jrajahalme@nicira.com>
9 years agolib/cmap: Use atomics for all bucket data.
Jarno Rajahalme [Wed, 28 May 2014 23:56:29 +0000 (16:56 -0700)]
lib/cmap: Use atomics for all bucket data.

The documentation of the memory order argument of atomic operations
states that memory loads after an atomic load with a
memory_order_acquire cannot be moved before the atomic operation.
This still leaves open the possibility that non-atomic loads before
the atomic load could be moved after it, hence breaking down the
synchronization used for cmap_find().

This patch fixes this my using atomics (with appropriate memory_order)
also for the struct cmap_node pointer and hash values.

struct cmap_node itself is used for the pointer, since it already
contains an RCU pointer (which is atomic) for a struct cmap_node.
This also helps simplify some of the code previously dealing
separately with the cmap_node pointer in the bucket v.s. a cmap_node.

Hash values are also accessed using atomics.  Otherwise it might be
legal for a compiler to read the hash values once, and keep using the
same values through all the retries.

These should have no effect on performance.

Signed-off-by: Jarno Rajahalme <jrajahalme@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
9 years agolib/cmap: Add more hmap-like functionality.
Jarno Rajahalme [Wed, 28 May 2014 23:56:29 +0000 (16:56 -0700)]
lib/cmap: Add more hmap-like functionality.

Add cmap_replace() and cmap_first(), as well as CMAP_FOR_EACH_SAFE and
CMAP_FOR_EACH_CONTINUE to make porting existing hmap using code a bit
easier.

CMAP_FOR_EACH_SAFE is useful in RCU postponed destructors, when it is
known that additional postponing is not needed.

Signed-off-by: Jarno Rajahalme <jrajahalme@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
9 years agolib/ovs-rcu: Fix documentation, add ovsrcu_init().
Jarno Rajahalme [Wed, 28 May 2014 23:56:29 +0000 (16:56 -0700)]
lib/ovs-rcu: Fix documentation, add ovsrcu_init().

lib/ovs-rcu.h had some of the comments duplicated.

Add ovsrcu_init() that can be used like ovsrcu_set() when the RCU
protected pointer is not yet visible any readers.

ovs-rcu internal initialization function is renamed as ovsrcu_init_module().

Signed-off-by: Jarno Rajahalme <jrajahalme@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
9 years agolib/flow: Use C99 declaration in for statement.
Jarno Rajahalme [Wed, 28 May 2014 23:56:29 +0000 (16:56 -0700)]
lib/flow: Use C99 declaration in for statement.

C99 declarations within code are allowed now.  Change the
FLOW_FOR_EACH_IN_MAP to use loop variable within the for statement.
This makes this macro more generally useful.

The loop variable name is suffixed with two underscores with the
intention that there would be a low likelihood of collision with any
of the macro parameters.

Also fix the return type of flow_get_next_in_map().

Signed-off-by: Jarno Rajahalme <jrajahalme@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
9 years agoCodingStyle: Allow C99 mixing of declarations and code.
Jarno Rajahalme [Wed, 28 May 2014 23:56:29 +0000 (16:56 -0700)]
CodingStyle: Allow C99 mixing of declarations and code.

As even the MSVC 2013 now supports the C99 mixing of declarations and
code, we can now allow them in OVS code.

Signed-off-by: Jarno Rajahalme <jrajahalme@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
9 years agovtep: Don't try to both distribute and distclean the manpage.
Ben Pfaff [Wed, 28 May 2014 17:36:23 +0000 (10:36 -0700)]
vtep: Don't try to both distribute and distclean the manpage.

Since the initial checkin of the vtep code, the manpage has been both
distributed and distcleaned.  That's contradictory.  This commit fixes it.

This commit also moves the vtep manpage from the source directory to the
build directory.  There's no real reason for the manpage to be in the
source directory, and it can't be if it's not distributed (because "make"
is not supposed to update the source directory in a freshly untarred
source distribution, and doing so breaks "make distcheck").

Found by "make distcheck".

Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Joe Stringer <joestringer@nicira.com>
9 years agoofproto-dpif-xlate: Mark xcfgp and new_xcfg as static.
Ben Pfaff [Wed, 28 May 2014 22:21:21 +0000 (15:21 -0700)]
ofproto-dpif-xlate: Mark xcfgp and new_xcfg as static.

Found by sparse.

Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Ryan Wilson <wryan@nicira.com>
9 years agoofproto: Fix comment on struct meter.
Ben Pfaff [Wed, 28 May 2014 18:36:05 +0000 (11:36 -0700)]
ofproto: Fix comment on struct meter.

This comment seems misleading, because it implies that the struct ends in
a flexible array member when it doesn't.

Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Jarno Rajahalme <jrajahalme@nicira.com>
9 years agoofproto-dpif: Remove unused flows.txt from fin_timeout test
Simon Horman [Tue, 20 May 2014 07:42:37 +0000 (16:42 +0900)]
ofproto-dpif: Remove unused flows.txt from fin_timeout test

Signed-off-by: Simon Horman <horms@verge.net.au>
Signed-off-by: Ben Pfaff <blp@nicira.com>
9 years agosocket-util: Log the kernel assigned port number when asked.
Gurucharan Shetty [Mon, 19 May 2014 18:58:14 +0000 (11:58 -0700)]
socket-util: Log the kernel assigned port number when asked.

So far, we log the kernel assigned port number when the port number is
not specified. On Windows, this happens multiple times because "unix"
sockets are implemented internally via TCP ports. This means that many tests,
specially the ovs-ofctl monitor tests, need to filter out the
additional messages. Doing that is not a big deal, but I think it will
keep manifesting in future tests added by Linux developers.

With this commit, we simply don't print the kernel assigned TCP ports
on Windows when done for "unix" sockets.

Signed-off-by: Gurucharan Shetty <gshetty@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
9 years agoovs-ofctl: Enable the ability to 'detach' on Windows.
Gurucharan Shetty [Fri, 16 May 2014 19:10:50 +0000 (12:10 -0700)]
ovs-ofctl: Enable the ability to 'detach' on Windows.

This is mostly for unit tests that use ovs-ofctl monitor.
service_start() creates a new process and waits till
daemonize_complete() is called by the child.

Signed-off-by: Gurucharan Shetty <gshetty@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
9 years agoovs-ofctl: Flush monitored data.
Gurucharan Shetty [Fri, 16 May 2014 19:08:53 +0000 (12:08 -0700)]
ovs-ofctl: Flush monitored data.

Otherwise, on Windows unit tests, data sometimes is
not seen in output files.

Signed-off-by: Gurucharan Shetty <gshetty@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
9 years agobridge: Initialize dscp for mgmt connections.
Gurucharan Shetty [Fri, 16 May 2014 19:04:00 +0000 (12:04 -0700)]
bridge: Initialize dscp for mgmt connections.

Without it, garbage values make it to set_dscp function
in Windows.

Signed-off-by: Gurucharan Shetty <gshetty@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
9 years agoovsdb-monitor.at: Changes for Windows.
Gurucharan Shetty [Wed, 14 May 2014 19:48:17 +0000 (12:48 -0700)]
ovsdb-monitor.at: Changes for Windows.

ovsdb-client's 'monitor' command works with --detach such
that the parent detaches after printing initial transactions in
the database. This is a little tricky to implement
in windows. So for windows, send the process to background with
'&' and then sleep for a second to let the intial transactions
printed. (We can do the same for Linux, but it slows down the
test run)

Also let the perl script that looks at the o/p be aware of
CR LF in windows.

Signed-off-by: Gurucharan Shetty <gshetty@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
9 years agoofproto-dpif-xlate: Implement RCU locking in ofproto-dpif-xlate.
Ryan Wilson [Wed, 28 May 2014 00:34:14 +0000 (17:34 -0700)]
ofproto-dpif-xlate: Implement RCU locking in ofproto-dpif-xlate.

Before, a global read-write lock protected the ofproto-dpif
/ ofproto-dpif-xlate interface.  Handler and revalidator threads
had to wait while configuration was being changed.  This patch
implements RCU locking which allows handlers and revalidators
to operate while configuration is being updated.

Signed-off-by: Ryan Wilson <wryan@nicira.com>
Acked-by: Alex Wang <alexw@nicira.com>
Acked-by: Ethan Jackson <ethan@nicira.com>
Signed-off-by: Alex Wang <alexw@nicira.com>
9 years agolib/flow: call memcmp in miniflow_equal()
Daniele Di Proietto [Tue, 27 May 2014 22:20:08 +0000 (15:20 -0700)]
lib/flow: call memcmp in miniflow_equal()

This commit replace a while loop in miniflow_equal() with a call to
memcmp() for performace reasons.

Signed-off-by: Daniele Di Proietto <ddiproietto@vmware.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
10 years agosocket-util: Refactor unix specific code to a new file.
Gurucharan Shetty [Fri, 23 May 2014 16:35:47 +0000 (09:35 -0700)]
socket-util: Refactor unix specific code to a new file.

Signed-off-by: Gurucharan Shetty <gshetty@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
10 years agoofp-print: Merge ofp_print_stats_{reply, request}()
Simon Horman [Tue, 27 May 2014 09:05:37 +0000 (18:05 +0900)]
ofp-print: Merge ofp_print_stats_{reply, request}()

Merge ofp_print_stats_reply() and ofp_print_stats_request()
into a single new function, ofp_print_stats().

For stats replies there should be no run-time change in behaviour.

For pre-OpenFlow1.3 stats requests there should also be
no run-time change in behaviour.

For OpenFlow1.3+ stats requests the more flag is now printed
as present. Previously ***unknown flags 0x0001*** was printed.

Signed-off-by: Simon Horman <horms@verge.net.au>
Signed-off-by: Ben Pfaff <blp@nicira.com>
10 years agoAUTHORS: Add Lars Kellogg-Stedman.
Ben Pfaff [Tue, 27 May 2014 15:50:09 +0000 (08:50 -0700)]
AUTHORS: Add Lars Kellogg-Stedman.

Signed-off-by: Ben Pfaff <blp@nicira.com>
10 years agorhel: support persistent mac addresses on OVS bridges
Lars Kellogg-Stedman [Fri, 23 May 2014 21:14:35 +0000 (17:14 -0400)]
rhel: support persistent mac addresses on OVS bridges

This patch adds support for RHEL-derived systems (RHEL/CentOS/Fedora)
for setting the persistent MAC address of an OVS bridge via the MACADDR
setting in the interface configuration file.

Without this change, when an administrator provides MACADDR in the
interface configuration file that address will be set in ifup-eth using
the "ip link set" command.  While this appears to work, any operation
that updates the OVS configuration will cause the MAC address to revert.

Persistent MAC addresses must be set using ovs-vsctl.

(Resubmitted with whitespace and grammar corrections)

Signed-off-by: Lars Kellogg-Stedman <lars@redhat.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Flavio Leitner <fbl@redhat.com>
10 years agoxcache: Remove unused field xc_entry.u.learn.ofproto.
Joe Stringer [Fri, 23 May 2014 04:33:50 +0000 (16:33 +1200)]
xcache: Remove unused field xc_entry.u.learn.ofproto.

Signed-off-by: Joe Stringer <joestringer@nicira.com>
Acked-by: Alex Wang <alexw@nicira.com>
10 years agonetdev-dpdk: use defined values for queues length
Daniele Di Proietto [Fri, 23 May 2014 23:12:44 +0000 (16:12 -0700)]
netdev-dpdk: use defined values for queues length

Signed-off-by: Daniele Di Proietto <ddiproietto@vmware.com>
Acked-by: Pravin B Shelar <pshelar@nicira.com>
10 years agoupcall: Remove redundant xlate_actions_for_side_effects().
Ethan Jackson [Thu, 22 May 2014 17:53:27 +0000 (10:53 -0700)]
upcall: Remove redundant xlate_actions_for_side_effects().

As a result of commit a0bab87 (ofproto: Remove per-flow miss hash
table from upcall handler.) we're guaranteed that every packet has had
xlate_actions() called on it at least once.  Therefore, there's no
need to re-xlate slow path flows just to shove their packets through
the system.

This also may fix a bug discussed here:
http://openvswitch.org/pipermail/discuss/2014-April/013670.html

Signed-off-by: Ethan Jackson <ethan@nicira.com>
Reported-by: Murphy McCauley <murphy.mccauley@gmail.com>
Acked-by: Ben Pfaff <blp@nicira.com>
10 years agomakefile.am: Apply printf check only to OVS userspace.
Jesse Gross [Fri, 23 May 2014 02:15:26 +0000 (19:15 -0700)]
makefile.am: Apply printf check only to OVS userspace.

We shouldn't restrict printf specifiers in kernel or third party
code.

Signed-off-by: Jesse Gross <jesse@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
10 years agoofproto-dpif-xlate: Coding style fix for bucket_is_alive()
Andy Zhou [Thu, 22 May 2014 16:35:36 +0000 (09:35 -0700)]
ofproto-dpif-xlate: Coding style fix for bucket_is_alive()

Signed-off-by: Andy Zhou <azhou@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
10 years agoofproto-dpif-xlate: Simplify group_is_alive
Andy Zhou [Thu, 22 May 2014 16:24:23 +0000 (09:24 -0700)]
ofproto-dpif-xlate: Simplify group_is_alive

Signed-off-by: Andy Zhou <azhou@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
10 years agoofproto-dpif: remove a redundant assignment
Andy Zhou [Thu, 22 May 2014 16:04:09 +0000 (09:04 -0700)]
ofproto-dpif: remove a redundant assignment

Signed-off-by: Andy Zhou <azhou@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
10 years agoofproto: Remove ofproto_group_write_lookup()
Andy Zhou [Thu, 22 May 2014 06:57:21 +0000 (23:57 -0700)]
ofproto: Remove ofproto_group_write_lookup()

ofproto_group_write_lookup() slightly different from
ofproto_group_lookup() in handling reference counting.
Currently, it has only one caller: modify_group().
It seems the abstraction is not adding value here.

Remove the function, along with some refactoring, makes modify_group()
easier to understand.

Signed-off-by: Andy Zhou <azhou@nicira.com>
Acked-by: Ryan Wilson <wryan@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
10 years agobridge: Add test that ports that disappear get added back to the datapath.
Ben Pfaff [Thu, 22 May 2014 16:36:00 +0000 (09:36 -0700)]
bridge: Add test that ports that disappear get added back to the datapath.

The test added in this commit would have caught the bug fixed by commit
96be8de595150 (bridge: When ports disappear from a datapath, add them
back.).  With that commit reverted, the new test fails.

Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Gurucharan Shetty <gshetty@nicira.com>
10 years agoofproto: per-table statistics
Simon Horman [Thu, 22 May 2014 05:44:36 +0000 (14:44 +0900)]
ofproto: per-table statistics

Add per-table counters. This resolves some short-comings
in the data provided in a table stats reply message.

* Lookups and matches are calculated based on table
  accesses rather than datapath flow hits and misses.

* Lookups and matches are credited to the table where they
  occurred rather than all being credited to table 0.

These problems were observed when running make check-ryu
and this patch allows many of its tester.py match checks
to pass.

Reviewed-by: YAMAMOTO Takashi <yamamoto@valinux.co.jp>
Signed-off-by: Simon Horman <horms@verge.net.au>
Signed-off-by: Ben Pfaff <blp@nicira.com>
10 years agoseq: Attribute wakeups to seq_wait()'s caller, not to seq_wait() itself.
Ben Pfaff [Wed, 30 Apr 2014 21:34:27 +0000 (14:34 -0700)]
seq: Attribute wakeups to seq_wait()'s caller, not to seq_wait() itself.

The poll_loop code has a feature that, when turned on manually or
automatically (due to high CPU use), logs the source file and line number
of the code that caused a thread to wake up from poll().  Until now, when
a function calls seq_wait(), the source file and line number logged was
the code inside seq_wait().  seq_wait() has many callers, so that
information is not as useful as it could be.  This commit changes the
source file and line number used to be that of seq_wait()'s caller.

I found this useful for debugging.

Signed-off-by: Ben Pfaff <blp@nicira.com>
10 years agoofproto: Add const to immutable members of ofgroup.
Andy Zhou [Thu, 22 May 2014 04:08:01 +0000 (21:08 -0700)]
ofproto: Add const to immutable members of ofgroup.

Signed-off-by: Andy Zhou <azhou@nicira.com>
Acked-by: Ryan Wilson <wryan@nicira.com>
10 years agoFAQ: Add a list of upstream kernel version supporting tunneling.
Jesse Gross [Thu, 22 May 2014 02:04:44 +0000 (19:04 -0700)]
FAQ: Add a list of upstream kernel version supporting tunneling.

Reported-by: Ben Pfaff <blp@nicira.com>
Signed-off-by: Jesse Gross <jesse@nicira.com>
10 years agoTODO: Remove "OpenFlow Group Bucket Stats" from TODO list
Ryan Wilson [Thu, 22 May 2014 03:11:09 +0000 (20:11 -0700)]
TODO: Remove "OpenFlow Group Bucket Stats" from TODO list

Signed-off-by: Ryan Wilson <wryan@nicira.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
10 years agoofproto-dpif-xlate: Fix a bug.
Alex Wang [Thu, 22 May 2014 03:45:24 +0000 (20:45 -0700)]
ofproto-dpif-xlate: Fix a bug.

Commit b256dc525c8 (ofproto-dpif-xlate: Cache xlate_actions() effects.)
caches the variables needed for refreshing mac-learning table in
xlate_normal().  Wherein, the cache entry always records reference to
the original 'ofproto'.

When patch port is used to connect two 'ofproto's, packet goes through the
patch port will have two mac-learning cache entries created for each
'ofproto'.  So, each entry should reference to the corresponding 'ofproto'.
However, due to the bug mentioned above, all cache entries will refer to the
same 'ofproto'.  Subsequently, the mac-learning tables can be corrupted, which
causes connection loss.

This commit fixes the bug by making each cache entry refer to the correct
'ofproto'.

Signed-off-by: Alex Wang <alexw@nicira.com>
Acked-by: Joe Stringer <joestringer@nicira.com>
10 years agotests: Add tests for Openflow group stats
Andy Zhou [Mon, 19 May 2014 21:20:40 +0000 (21:20 +0000)]
tests: Add tests for Openflow group stats

Signed-off-by: Andy Zhou <azhou@nicira.com>
Signed-off-by: Ryan Wilson <wryan@nicira.com>
Acked-by: Alex Wang <alexw@nicira.com>
Signed-off-by: Andy Zhou <azhou@nicira.com>
10 years agoofproto: Add support for Openflow group and bucket stats.
Ryan Wilson [Thu, 22 May 2014 10:47:13 +0000 (10:47 +0000)]
ofproto: Add support for Openflow group and bucket stats.

Signed-off-by: Ryan Wilson <wryan@nicira.com>
Acked-by: Andy Zhou <azhou@nicira.com>
Signed-off-by: Andy Zhou <azhou@nicira.com>
10 years agoofproto: Add reference count for Openflow groups.
Ryan Wilson [Thu, 22 May 2014 08:12:02 +0000 (08:12 +0000)]
ofproto: Add reference count for Openflow groups.

When adding support for OpenFlow group and bucket stats, a group entry is added
to the xlate_cache. If the main thread removes the group from an ofproto, we
need to guarantee that the group remains accessible to users of
xlate_group_actions and the xlate_cache, as the xlate_cache will not be cleaned
up until the corresponding datapath flows are revalidated.

Before, modify_group could change the bucket list for a group. With group
entries in the xlate_cache, this could leave already-freed bucket pointers in
the cache. Modify_group now recreates the group and replaces the old group in
ofproto's ofgroup hash map. Thus, any subsequent group lookup will find the new
group while the old group and buckets will still exist until the xlate_cache
entries unref the group.

Since ofgroup's properties were only written in-place by modify_group and this
is no longer done, this eliminates the need for ofgroup's lock.

Signed-off-by: Ryan Wilson <wryan@nicira.com>
Signed-off-by: Andy Zhou <azhou@nicira.com>
10 years agodpif-netdev: Use cmap for ports.
Ben Pfaff [Tue, 20 May 2014 20:21:09 +0000 (13:21 -0700)]
dpif-netdev: Use cmap for ports.

Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Jarno Rajahalme <jrajahalme@nicira.com>
10 years agodpif-netdev: Fix memory leak.
Ben Pfaff [Wed, 21 May 2014 00:07:56 +0000 (17:07 -0700)]
dpif-netdev: Fix memory leak.

Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Jarno Rajahalme <jrajahalme@nicira.com>
10 years agodpif-netdev: Fix use-after-free in port_unref().
Ben Pfaff [Wed, 21 May 2014 00:09:59 +0000 (17:09 -0700)]
dpif-netdev: Fix use-after-free in port_unref().

When the last rxq is closed (which releases the rxq's internal reference
to its netdev) the next call to netdev_n_rxq() accesses freed memory.

Found by valgrind.

Signed-off-by: Ben Pfaff <blp@nicira.com>
Reported-by: Jarno Rajahalme <jrajahalme@nicira.com>
Acked-by: Jarno Rajahalme <jrajahalme@nicira.com>
10 years agocmap: Fix memory ordering for counter_changed().
Ben Pfaff [Wed, 21 May 2014 14:43:10 +0000 (07:43 -0700)]
cmap: Fix memory ordering for counter_changed().

Release memory ordering only affects visibility of stores, and is not
allowed on a memory read.  Some compilers enforce this, making this code
fail to compile.

Reported-by: Alex Wang <alexw@nicira.com>
Reported-by: Kmindg G <kmindg@gmail.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Jarno Rajahalme <jrajahalme@nicira.com>
10 years agotests: Fix an ofproto test.
Joe Stringer [Tue, 20 May 2014 00:44:52 +0000 (12:44 +1200)]
tests: Fix an ofproto test.

This was previously logging that there was no such function, but passing
the test anyway.

Signed-off-by: Joe Stringer <joestringer@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
10 years agorevalidator: Re-fix a flow duplication bug.
Joe Stringer [Wed, 21 May 2014 02:49:04 +0000 (14:49 +1200)]
revalidator: Re-fix a flow duplication bug.

Commit 73a3c4757e59 (revalidator: Prevent handling the same flow twice.)
fixed a bug where duplicated flows could be deleted twice. Commit
7d1700980b5d (ofproto-dpif-upcall: Remove the flow_dumper thread.)
partially re-introduced this bug.

The bug would cause the logs to show messages such as
"failed to flow_get (No such file or directory) skb_priority(0),..."
"failed to flow_del (No such file or directory) skb_priority(0),..."

This patch fixes the issue again.

Signed-off-by: Joe Stringer <joestringer@nicira.com>
Acked-by: Alex Wang <alexw@nicira.com>
10 years agoovs-vsctl: Add an example of how to limit the flows in a flow table.
Ben Pfaff [Wed, 30 Apr 2014 17:58:47 +0000 (10:58 -0700)]
ovs-vsctl: Add an example of how to limit the flows in a flow table.

Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Gurucharan Shetty <gshetty@nicira.com>
10 years agorhel: Remove creation of fake bond interface
Anoob Soman [Tue, 25 Mar 2014 12:32:03 +0000 (12:32 +0000)]
rhel: Remove creation of fake bond interface

fake-iface, option to add-bond, was added for compatibility
reasons and need not be used otherwise.

Signed-off-by: Anoob Soman <anoob.soman@citrix.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
10 years agoFAQ: Add an entry about datapath-id
YAMAMOTO Takashi [Tue, 20 May 2014 02:07:02 +0000 (11:07 +0900)]
FAQ: Add an entry about datapath-id

Acked-by: Ben Pfaff <blp@nicira.com>
Signed-off-by: YAMAMOTO Takashi <yamamoto@valinux.co.jp>
10 years agonetdev-bsd: Fix a whitespace
YAMAMOTO Takashi [Mon, 19 May 2014 08:06:45 +0000 (17:06 +0900)]
netdev-bsd: Fix a whitespace

Acked-by: Ben Pfaff <blp@nicira.com>
Signed-off-by: YAMAMOTO Takashi <yamamoto@valinux.co.jp>
10 years agoofproto: Fix comments
YAMAMOTO Takashi [Mon, 19 May 2014 07:57:49 +0000 (16:57 +0900)]
ofproto: Fix comments

Acked-by: Ben Pfaff <blp@nicira.com>
Signed-off-by: YAMAMOTO Takashi <yamamoto@valinux.co.jp>
10 years agoofproto: Remove per-flow miss hash table from upcall handler.
Ryan Wilson [Wed, 21 May 2014 04:50:19 +0000 (21:50 -0700)]
ofproto: Remove per-flow miss hash table from upcall handler.

The upcall handler keeps a hash table which hashes flow to a list
of corresponding packets.  This used to be necessary as packets with
the same flow had similar actions and calculating actions used to be
a performance bottleneck.  Now that userspace action calculation
performance has improved, there is no need for this hash map.

This patch removes this hash map and each packet has its own upcall.

Signed-off-by: Ryan Wilson <wryan@nicira.com>
Acked-by: Alex Wang <alexw@nicira.com>
10 years agodatapath: 16bit inner_network_header field in struct ovs_gso_cb
Simon Horman [Tue, 20 May 2014 23:31:47 +0000 (08:31 +0900)]
datapath: 16bit inner_network_header field in struct ovs_gso_cb

The motivation for this is to create a 16bit hole in struct ovs_gso_cb
which may be used for the inner_protocol field which is needed
for the proposed implementation of compatibility for MPLS GSO segmentation.

This should be safe as inner_network_header is now an offset to
the inner_mac_header rather than skb->head.

As pointed out by Thomas Graf simply making both inner offsets 16bis is not
safe as there have been cases of overflow with "with collapsed TCP frames
on IB when the headroom grew beyond 64K. See commit 50bceae9bd ``tcp:
Reallocate headroom if it would overflow csum_start'' for additional
details."

This patch is based on suggestions by Thomas Graf and Jesse Gross.

Cc: Thomas Graf <tgraf@suug.ch>
Cc: Jesse Gross <jesse@nicira.com>
Signed-off-by: Simon Horman <horms@verge.net.au>
Signed-off-by: Jesse Gross <jesse@nicira.com>
10 years agocmap: New module for cuckoo hash table.
Ben Pfaff [Tue, 20 May 2014 23:51:42 +0000 (16:51 -0700)]
cmap: New module for cuckoo hash table.

This implements an "optimistic concurrent cuckoo hash", a single-writer,
multiple-reader hash table data structure.  The point of this data
structure is performance, so this commit message focuses on performance.

I tested the performance of cmap with the test-cmap utility included in
this commit.  It takes three parameters for benchmarking:

    - n, the number of elements to insert.

    - n_threads, the number of threads to use for searching and
      mutating the hash table.

    - mutations, the percentage of operations that should modify the
      hash table, from 0% to 100%.

e.g. "test-cmap 1000000 16 1" inserts one million elements, uses 16
threads, and 1% of the operations modify the hash table.

Any given run does the following for both hmap and cmap
implementations:

    - Inserts n elements into a hash table.

    - Iterates over all of the elements.

    - Spawns n_threads threads, each of which searches for each of the
      elements in the hash table, once, and removes the specified
      percentage of them.

    - Removes each of the (remaining) elements and destroys the hash
      table.

and reports the time taken by each step,

The tables below report results for various parameters with a draft version
of this library.  The tests were not formally rerun for the final version,
but the intermediate changes should only have improved performance, and
this seemed to be the case in some informal testing.

n_threads=16 was used each time, on a 16-core x86-64 machine.  The compiler
used was Clang 3.5.  (GCC yields different numbers but similar relative
results.)

The results show:

    - Insertion is generally 3x to 5x faster in an hmap.

    - Iteration is generally about 3x faster in a cmap.

    - Search and mutation is 4x faster with .1% mutations and the
      advantage grows as the fraction of mutations grows.  This is
      because a cmap does not require locking for read operations,
      even in the presence of a writer.

      With no mutations, however, no locking is required in the hmap
      case, and the hmap is somewhat faster.  This is because raw hmap
      search is somewhat simpler and faster than raw cmap search.

    - Destruction is faster, usually by less than 2x, in an hmap.

n=10,000,000:

        .1% mutations    1% mutations    10% mutations     no mutations
          cmap  hmap     cmap   hmap      cmap   hmap       cmap  hmap
insert:   6132  2182     6136   2178      6111   2174       6124  2180
iterate:   370  1203      378   1201       370   1200        371  1202
search:   1375  8692     2393  28197     18402  80379       1281  1097
destroy:  1382  1187     1197   1034       324    241       1405  1205

n=1,000,000:

        .1% mutations    1% mutations    10% mutations     no mutations
          cmap  hmap     cmap   hmap      cmap   hmap       cmap  hmap
insert:    311    25      310     60       311     59        310    60
iterate:    25    62       25     64        25     57         25    60
search:    115   637      197   2266      1803   7284        101    67
destroy:   103    64       90     59        25     13        104    66

n=100,000:

        .1% mutations    1% mutations    10% mutations     no mutations
          cmap  hmap     cmap   hmap      cmap   hmap       cmap  hmap
insert:     25     6       26      5        25      5         25     5
iterate:     1     3        1      3         1      3          2     3
search:     12    57       27    219       164    735         10     5
destroy:     5     3        6      3         2      1          6     4

Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Jarno Rajahalme <jrajahalme@nicira.com>
10 years agoovs-ctl: Raise the limit on the number of open file descriptors.
Alex Wang [Tue, 20 May 2014 21:16:54 +0000 (14:16 -0700)]
ovs-ctl: Raise the limit on the number of open file descriptors.

Since the removal of dispatcher thread, OVS creates 'n-handler-threads'
file descriptors for each bridge port.  To allow more bridge ports
be supported, this commit raises the limit on the number of open file
descriptors from 7500 to 65535.

Bug #1254038.

Signed-off-by: Alex Wang <alexw@nicira.com>
10 years agodpif: Refactor flow dumping interface to make better sense for batching.
Ben Pfaff [Tue, 20 May 2014 18:37:02 +0000 (11:37 -0700)]
dpif: Refactor flow dumping interface to make better sense for batching.

Commit a6ce4b9d251 (ofproto-dpif-upcall: Avoid use-after-free in
revalidate() corner case.) showed that it is somewhat tricky to correctly
use the existing dpif flow dumping interface to obtain batches of flows.
One has to be careful about calling dpif_flow_dump_next_may_destroy_keys()
before going on to the next flow.

A better interface is possible, one that is naturally oriented toward
retrieving batches when that is a useful optimization.  This commit
replaces the dpif interface by such a design, and updates both the
implementations and the callers to adopt it.

This is a fairly large change, but I think that the code in
ofproto-dpif-upcall is easier to understand after the change.

Signed-off-by: Ben Pfaff <blp@nicira.com>
10 years agoAUTHORS: Fix spelling of Anoob Soman's name.
Ben Pfaff [Tue, 20 May 2014 18:22:11 +0000 (11:22 -0700)]
AUTHORS: Fix spelling of Anoob Soman's name.

Signed-off-by: Ben Pfaff <blp@nicira.com>
10 years agoofproto-dpif-xlate: Add xlate_normal_flood()
Flavio Leitner [Fri, 11 Apr 2014 21:34:15 +0000 (18:34 -0300)]
ofproto-dpif-xlate: Add xlate_normal_flood()

This is also needed for multicast snooping.

Acked-by: Thomas Graf <tgraf@redhat.com>
Acked-by: Daniel Borkmann <dborkman@redhat.com>
Signed-off-by: Flavio Leitner <fbl@redhat.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
10 years agoofproto/xlate: Move is_admissible() up
Flavio Leitner [Fri, 11 Apr 2014 21:34:14 +0000 (18:34 -0300)]
ofproto/xlate: Move is_admissible() up

No functional changes.  This is just for better readability
when the multicast snooping learning and sending functions
are added to the code.

Acked-by: Thomas Graf <tgraf@redhat.com>
Acked-by: Daniel Borkmann <dborkman@redhat.com>
Signed-off-by: Flavio Leitner <fbl@redhat.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
10 years agodpif: Comestic indentation fixes
Flavio Leitner [Fri, 11 Apr 2014 21:34:13 +0000 (18:34 -0300)]
dpif: Comestic indentation fixes

No functionality is changed. This is needed on
a further commit adding REV_MCAST_SNOOPING.

Acked-by: Thomas Graf <tgraf@redhat.com>
Acked-by: Daniel Borkmann <dborkman@redhat.com>
Signed-off-by: Flavio Leitner <fbl@redhat.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
10 years agopacket: Add function ip_is_local_multicast()
Flavio Leitner [Fri, 11 Apr 2014 21:34:08 +0000 (18:34 -0300)]
packet: Add function ip_is_local_multicast()

RFC4541- IGMP and MLD Snooping Switches Considerations
recommends to have different actions for local and
non-local multicast traffic.

Acked-by: Daniel Borkmann <dborkman@redhat.com>
Signed-off-by: Thomas Graf <tgraf@redhat.com>
Signed-off-by: Flavio Leitner <fbl@redhat.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
10 years agopackets: Add igmp header definitions
Daniel Borkmann [Fri, 11 Apr 2014 21:34:07 +0000 (18:34 -0300)]
packets: Add igmp header definitions

Add basic header structure and definitions into packet.h.

Acked-by: Thomas Graf <tgraf@redhat.com>
Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
Signed-off-by: Flavio Leitner <fbl@redhat.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
10 years agonetinet: Add IPPROTO_IGMP definition
Daniel Borkmann [Fri, 11 Apr 2014 21:34:06 +0000 (18:34 -0300)]
netinet: Add IPPROTO_IGMP definition

Add the definition of Internet Group Management Protocol.

Signed-off-by: Cong Wang <amwang@redhat.com>
Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
Acked-by: Thomas Graf <tgraf@redhat.com>
Signed-off-by: Flavio Leitner <fbl@redhat.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
10 years agoovs-vsctl: Add error column to show command
Thomas Graf [Thu, 10 Apr 2014 10:50:11 +0000 (12:50 +0200)]
ovs-vsctl: Add error column to show command

a425a102-c317-4743-b0ba-79d59ff04a74
    Bridge "br0"
        [...]
        Port test
            Interface test
                type: vxlan
                options: {unknown="1"}
                error: "test: could not set configuration (Invalid argument)"
    ovs_version: "2.1.90"

Signed-off-by: Thomas Graf <tgraf@redhat.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
10 years agovswitchd: Add error column to Interface table to store error condition
Thomas Graf [Thu, 10 Apr 2014 10:50:10 +0000 (12:50 +0200)]
vswitchd: Add error column to Interface table to store error condition

Store the error condition of a failed port configuration in a new
column 'error' in the Interface table.

Example:
$ ovs-vsctl add-port br0 test -- \
     set Interface test type=vxlan options:unknown=1
ovs-vsctl: Error detected while setting up 'test'.  [...]

$ ovs-vsctl list Interface test | grep error
error         : "test: could not set configuration (Invalid argument)"

Fixing the error will clear the error column:
$ ovs-vsctl set Interface test options:remote_ip=1.1.1.1
$ ovs-vsctl list Interface test | grep error
error         : []
$

For now, the high level error messages when opening and configuring
the netdev are used. Further patches can extend passing the error
pointer into the individual netdev implementations to allow for more
fine grained error messages to be stored.

Signed-off-by: Thomas Graf <tgraf@redhat.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
10 years agovlog: Provide convenience macros to additionally store log messages in buffer
Thomas Graf [Thu, 10 Apr 2014 10:50:09 +0000 (12:50 +0200)]
vlog: Provide convenience macros to additionally store log messages in buffer

Signed-off-by: Thomas Graf <tgraf@redhat.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
10 years agolib/classifier: Rename 'cls_subtable_cache' as 'cls_subtables'.
Jarno Rajahalme [Mon, 19 May 2014 17:41:03 +0000 (10:41 -0700)]
lib/classifier: Rename 'cls_subtable_cache' as 'cls_subtables'.

'cache' gives an inexact connotation, as the list is always expected
to be in order and contain pointers to all the subtables.

The struct cls_subtables fields are are also renamed to be more readable.

struct cls_classifier fields 'subtables' is remamed to 'subtables_map' and
'subtables_priority' is renamed to 'subtables',

There are no functional changes in this patch.

Signed-off-by: Jarno Rajahalme <jrajahalme@nicira.com>
10 years agoovs-rcu: Fix typo in comment.
Ben Pfaff [Thu, 8 May 2014 23:47:35 +0000 (16:47 -0700)]
ovs-rcu: Fix typo in comment.

Signed-off-by: Ben Pfaff <blp@nicira.com>
10 years agoovs-rcu: Add OVSRCU_TYPE_INITIALIZER which initializes OVSRCU_TYPE variables to NULL.
Ryan Wilson [Mon, 19 May 2014 07:09:24 +0000 (00:09 -0700)]
ovs-rcu: Add OVSRCU_TYPE_INITIALIZER which initializes OVSRCU_TYPE variables to NULL.

Signed-off-by: Ryan Wilson <wryan@nicira.com>
Acked-by: Alex Wang <alexw@nicira.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
10 years agoacinclude.m4: Fix "sparse", via detection of GNU make "if" directive.
Ben Pfaff [Mon, 19 May 2014 14:52:21 +0000 (07:52 -0700)]
acinclude.m4: Fix "sparse", via detection of GNU make "if" directive.

Make treats tabs very differently from spaces at the beginning of a line,
so this test must use a tab instead of a space.  This partially reverts
commit a0903134d2d60 (acinclude.m4: Expand tabs).

Without this commit, the build system never enables checking with sparse
because it never detects that GNU make "if" works.

Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: YAMAMOTO Takashi <yamamoto@valinux.co.jp>
10 years agotests: Check dpif-netdev odp_actions consistency.
Joe Stringer [Thu, 8 May 2014 00:37:52 +0000 (12:37 +1200)]
tests: Check dpif-netdev odp_actions consistency.

Ensure that upcall key matches flow install and flow_dump for userspace
datapath. This was previously assumed, but not tested. This makes the
assumption more explicit.

Signed-off-by: Joe Stringer <joestringer@nicira.com>
Acked-by: Andy Zhou <azhou@nicira.com>
10 years agotests: Create dpif-netdev.at.
Joe Stringer [Tue, 13 May 2014 02:45:30 +0000 (14:45 +1200)]
tests: Create dpif-netdev.at.

Shift datapath flow test macros and "ofproto-dpif - dummy interface" out
into a separate file.

Signed-off-by: Joe Stringer <joestringer@nicira.com>
Acked-by: Andy Zhou <azhou@nicira.com>
10 years agoodp-util: Always serialise recirculation in upcall key.
Joe Stringer [Fri, 9 May 2014 01:58:32 +0000 (13:58 +1200)]
odp-util: Always serialise recirculation in upcall key.

The userspace and kernel datapaths previously differed on their
treatment of the recirc_id and dp_hash fields when sending upcalls.
While the kernel datapath would always serialise these fields, the
userspace would not. When using the userspace datapath, this would cause
a mismatch between the odp flow key in an upcall compared to the one
that is serialised upon flow_dump.

This patch brings the userspace datapath behaviour back in line with the
kernel datapath by always serialising recirc_id and dp_hash to odp.

Signed-off-by: Joe Stringer <joestringer@nicira.com>
Acked-by: Andy Zhou <azhou@nicira.com>
10 years agotests: Remove duplicate STRIP_USED definition.
Joe Stringer [Tue, 13 May 2014 03:08:58 +0000 (15:08 +1200)]
tests: Remove duplicate STRIP_USED definition.

Signed-off-by: Joe Stringer <joestringer@nicira.com>
Acked-by: Andy Zhou <azhou@nicira.com>
10 years agoodp-util: Improve formatting of ND export to odp.
Joe Stringer [Thu, 15 May 2014 03:56:30 +0000 (15:56 +1200)]
odp-util: Improve formatting of ND export to odp.

Signed-off-by: Joe Stringer <joestringer@nicira.com>
Acked-by: Andy Zhou <azhou@nicira.com>
10 years agoUse prefix trie lookup for IPv4 by default.
Jarno Rajahalme [Fri, 16 May 2014 19:51:11 +0000 (12:51 -0700)]
Use prefix trie lookup for IPv4 by default.

Unless otherwise configured, the prefix trie lookup is enabled for
IPv4 destination and source address fields.  A new keyword "none" is
accepted as the value of "prefixes" in the OVSDB Flow_Table column.

Signed-off-by: Jarno Rajahalme <jrajahalme@nicira.com>
Acked-by: Ethan Jackson <ethan@nicira.com>
10 years agonetdev: Remove netdev from global shash when the user is changing interface configura...
Ryan Wilson [Fri, 16 May 2014 09:17:58 +0000 (02:17 -0700)]
netdev: Remove netdev from global shash when the user is changing interface configuration.

When the user changes port type (i.e. changing p0 from type 'internal' to
'gre'), the netdev must first be deleted, then re-created with the new type.
Deleting the netdev requires there exist no more references to the netdev.
However, the xlate cache holds references to netdevs and the cache is only
invalidated by revalidator threads. Thus, if cache is not invalidated prior to
the netdev being re-created, the netdev will not be able to be re-created and
the configuration change will fail.

This patch always removes the netdev from the global netdev shash when the
user changes port type. This ensures that the new netdev can always be created
while handler and revalidator threads can retain references to the old netdev
until they are finished.

Signed-off-by: Ryan Wilson <wryan@nicira.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
10 years agoofproto-dpif: Install internal rule should not change the match content.
Andy Zhou [Sat, 10 May 2014 02:13:47 +0000 (19:13 -0700)]
ofproto-dpif: Install internal rule should not change the match content.

Without this patch, the match passed into to
ofproto_dpif_add_internal_flow() are modified. The mask of dl_type will
always be converted from wildcarded match into exact match due to
calling rule_dpif_lookup_in_table(). The fix makes sure
ofproto_dpif_add_internal_flow() does not change the original match,
and makes the match passed as const in the
ofproto_dpif_add_internal_flow() API.

This bug prevents bond module from properly tracking the post
recirculation rules installed in the internal table. The existing rule
is always deleted followed by reinstalling of the same rule.

The observable behavior of the bug is that bond module losses track
of the slave's stats, after the slave is rebalanced. Although traffic
flows through the slave just fine.

Bug #1229225

Signed-off-by: Andy Zhou <azhou@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
10 years agoovs-atomic: Remove atomic_uint64_t and atomic_int64_t.
Simon Horman [Wed, 14 May 2014 07:19:35 +0000 (16:19 +0900)]
ovs-atomic: Remove atomic_uint64_t and atomic_int64_t.

Some concern has been raised by Ben Pfaff that atomic_uint64_t may not
be portable. In particular on 32bit platforms that do not have atomic
64bit integers.

Now that there are no longer any users of atomic_uint64_t remove it
entirely. Also remove atomic_int64_t which has no users.

Cc: YAMAMOTO Takashi <yamamoto@valinux.co.jp>
Signed-off-by: Simon Horman <horms@verge.net.au>
Signed-off-by: Ben Pfaff <blp@nicira.com>
10 years agoofproto-dpif-upcall: Use atomic_long in struct udpif
Simon Horman [Wed, 14 May 2014 07:19:34 +0000 (16:19 +0900)]
ofproto-dpif-upcall: Use atomic_long in struct udpif

Some concern has been raised by Ben Pfaff that atomic_uint64_t may not
be portable. Accordingly, use atomic_ulong instead of atomic_uint64_t
in struct ofproto.

This is in preparation for removing atomic_uint64_t entirely.

Cc: YAMAMOTO Takashi <yamamoto@valinux.co.jp>
Signed-off-by: Simon Horman <horms@verge.net.au>
Signed-off-by: Ben Pfaff <blp@nicira.com>
10 years agoofproto-dpif: Break out MPLS controller tests into their own test
Simon Horman [Fri, 16 May 2014 02:30:25 +0000 (11:30 +0900)]
ofproto-dpif: Break out MPLS controller tests into their own test

This makes maintaining the controller test slightly easier
by splitting it in two.

Based on a similar patch by Joe Stringer.

Cc: Joe Stringer <joe@wand.net.nz>
Signed-off-by: Simon Horman <horms@verge.net.au>
Signed-off-by: Ben Pfaff <blp@nicira.com>
10 years agoofproto-dpif-upcall: Avoid use-after-free in revalidate() corner cases.
Ben Pfaff [Thu, 15 May 2014 22:52:17 +0000 (15:52 -0700)]
ofproto-dpif-upcall: Avoid use-after-free in revalidate() corner cases.

The loop in revalidate() needs to ensure that any data obtained from
dpif_flow_dump_next() is used before it is destroyed, as indicated by
dpif_flow_dump_next_may_destroy_keys().  In the common case, where
processing reaches the end of the main "while" loop, it does this, but
in two corner cases the code in the loop execute "continue;", which skipped
the check.  This commit fixes the problem.

Bug #1249988.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Joe Stringer <joestringer@nicira.com>
10 years agodatapath: sample action without side effects
Simon Horman [Thu, 15 May 2014 00:05:03 +0000 (09:05 +0900)]
datapath: sample action without side effects

The sample action is rather generic, allowing arbitrary actions to be
executed based on a probability. However its use, within the Open vSwitch
code-base is limited: only a single user-space action is ever nested.

A consequence of the current implementation of sample actions is that
depending on weather the sample action executed (due to its probability)
any side-effects of nested actions may or may not be present before
executing subsequent actions.  This has the potential to complicate
verification of valid actions by the (kernel) datapath. And indeed adding
support for push and pop MPLS actions inside sample actions is one case
where such case.

In order to allow all supported actions to be continue to be nested inside
sample actions without the potential need for complex verification code
this patch changes the implementation of the sample action in the kernel
datapath so that sample actions are more like a function call and any side
effects of nested actions are not present when executing subsequent
actions.

With the above in mind the motivation for this change is twofold:

* To contain side-effects the sample action in the hope of making it
  easier to deal with in the future and;
* To avoid some rather complex verification code introduced in the MPLS
  datapath patch.

Some notes about the implementation:

* This patch silently changes the behaviour of sample actions whose nested
  actions have side-effects. There are no known users of such sample
  actions.

* sample() does not clone the skb for the only known use-case of the sample
  action: a single nested userspace action. In such a case a clone is not
  needed as the userspace action has no side effects.

  Given that there are no known users of other nested actions and in order
  to avoid the complexity of predicting if other sequences of actions have
  side-effects in such cases the skb is cloned.

* As sample() provides a cloned skb in the unlikely case where there are
  nested actions other than a single userspace action it is no longer
  necessary to clone the skb in do_execute_actions() when executing a
  recirculation action just because the keep_skb parameter is set: this
  parameter was only set when processing the nested actions of a sample
  action.  Moreover it is possible to remove the keep_skb parameter of
  do_execute_actions entirely.

* As sample() provides either a cloned skb or one that has had a
  reference taken (using keep_skb) to do_execute_actions()
  the original skb passed to sample() is never consumed. Thus the
  caller of sample() (also do_execute_actions()) can use its generic
  error handling to free the skb on error.

Signed-off-by: Simon Horman <horms@verge.net.au>
Signed-off-by: Jesse Gross <jesse@nicira.com>
10 years agotests: Change to parse dynamically allocated ports on windows.
Gurucharan Shetty [Mon, 12 May 2014 22:30:41 +0000 (15:30 -0700)]
tests: Change to parse dynamically allocated ports on windows.

In Windows, we use kernel assigned TCP port for ssl/tcp and
unixctl. In tests, we parse the log files of ovsdb-server.log,
test-sflow.log and test-netflow.log to get this port. In all
the above cases, tcp port is allocated first and then the unixctl port.
So a 'head -1' on the result should be safe.

Signed-off-by: Gurucharan Shetty <gshetty@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
10 years agoovsdb-server.at: Skip transient transaction tests for Windows.
Gurucharan Shetty [Mon, 12 May 2014 16:48:36 +0000 (09:48 -0700)]
ovsdb-server.at: Skip transient transaction tests for Windows.

Windows port does not yet implement ovsdb-server's "--run" option.
Till it is implemented, skip the tests.

Signed-off-by: Gurucharan Shetty <gshetty@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
10 years agoPrepare for post-2.3.0 (2.3.90).
Justin Pettit [Thu, 15 May 2014 21:12:06 +0000 (14:12 -0700)]
Prepare for post-2.3.0 (2.3.90).

Signed-off-by: Justin Pettit <jpettit@nicira.com>
10 years agoPrepare for 2.3.0.
Justin Pettit [Thu, 15 May 2014 21:09:39 +0000 (14:09 -0700)]
Prepare for 2.3.0.

Signed-off-by: Justin Pettit <jpettit@nicira.com>
10 years agolib/classifier: Simpilify array ordering.
Jarno Rajahalme [Thu, 15 May 2014 02:53:51 +0000 (19:53 -0700)]
lib/classifier: Simpilify array ordering.

The terminology we used for subtable ordering ('splice', 'right
before') was inherited from an earlier use of a linked list, and
turned out to be confusing when applied to an array.  Also, we only
ever move one subtable earlier or later within the array, so we can
simplify the code as well.

Signed-off-by: Jarno Rajahalme <jrajahalme@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
10 years agoovs-pki: Workaround lack of /dev/stdin in Windows.
Gurucharan Shetty [Mon, 12 May 2014 20:08:35 +0000 (13:08 -0700)]
ovs-pki: Workaround lack of /dev/stdin in Windows.

This lets us generate certs for unit tests on Windows

Signed-off-by: Gurucharan Shetty <gshetty@nicira.com>
10 years agoodp-util: Fix a comment.
Joe Stringer [Mon, 12 May 2014 22:07:18 +0000 (10:07 +1200)]
odp-util: Fix a comment.

The comment was very specific to one user of the function, and had a
typo. This change reflects the wider effect of the case.

Signed-off-by: Joe Stringer <joestringer@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
10 years agotest-ovsdb: Fix setvbuf incompatibility on Windows.
Gurucharan Shetty [Mon, 12 May 2014 16:38:48 +0000 (09:38 -0700)]
test-ovsdb: Fix setvbuf incompatibility on Windows.

setvbuf() in Windows treats _IOLBF to be the same as _IOFBF. So
we cannot have size as zero (otherwise, there is a crash).

Workaround is to set _IONBF. I don't see unit test failures
because of the change.

Signed-off-by: Gurucharan Shetty <gshetty@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
10 years agoutil: set_program_name() can be called twice with different names.
Gurucharan Shetty [Mon, 12 May 2014 21:55:46 +0000 (14:55 -0700)]
util: set_program_name() can be called twice with different names.

Ex: ovstest.c

Signed-off-by: Gurucharan Shetty <gshetty@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
10 years agoutil: Disable popups while asserting in windows.
Gurucharan Shetty [Mon, 12 May 2014 21:49:58 +0000 (14:49 -0700)]
util: Disable popups while asserting in windows.

The default behavior for programs is to display a popup
after an assert/abort etc. This is not an ideal behavior because
this needs user intervention.

set_program_name, though not an ideal place to disable this, is
a useful place because it is called by all programs including
unit test binaries.

Signed-off-by: Gurucharan Shetty <gshetty@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
10 years agotests/library.at: Disable unix socket tests on Windows.
Gurucharan Shetty [Fri, 9 May 2014 19:46:54 +0000 (12:46 -0700)]
tests/library.at: Disable unix socket tests on Windows.

Signed-off-by: Gurucharan Shetty <gshetty@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>