On some platforms including NetBSD,
GNU make is usually installed as "gmake".
Signed-off-by: YAMAMOTO Takashi <yamamoto@valinux.co.jp>
Acked-by: Ben Pfaff <blp@nicira.com>
OVS is slow when compiled with pthreads atomics. Add a generic note
in INSTALL, with a reference to lib/ovs-atomic.h, where a new comment
provides additional detail.
Signed-off-by: Jarno Rajahalme <jrajahalme@nicira.com>
In the case that an flow for an IP packet has an mpls_push action applied
the L3 and L4 portions of the flow will be cleared in flow_push_mpls().
Without this change commit_set_port_action() will set the tp_src and tp_dst
mask for the flow to all-ones because the base and flow port values no
longer match. Even though there will be no corresponding set action for the
ports; because the flow is no longer IP.
In this case where nw_proto is not part of the match this manifests
in a problem because the kernel datapath rejects flows whose masks
have non-zero values for tp_src or dp_dst if the nw_proto mask is
not all-ones.
This patch resolves this problem by having commit_set_port_action() return
without doing anything if flow->nw_proto is zero. The same logic is present
in commit_set_nw_action().
Also enhance one of the MPLS tests to exercise this logic. The enhanced
tests inputs a UDP packet with non-zero ports rather than an IP packet with
zeroed ports: zeroed ports cause commit_set_port_action() always return
without doing anything..
Commit 691d39b ("upcall: Remove redundant xlate_actions_for_side_effects().")
causes xlate_in_init() to be called for every packet that has an upcall.
This has the effect of indirectly calling commit_set_port_action() when
translating a controller action which may not have previously been the case
depending on the flow.
The result is that the behaviour described in the changelog above can be
exercised via a minor enhancement to one of the existing MPLS tests. This
illustrates that the problem exists for the user-space datapath whereas I
had previously incorrectly assumed it only manifested when using the kernel
datapath because I had only observed it there.
Signed-off-by: Simon Horman <horms@verge.net.au>
Acked-by: Jarno Rajahalme <jrajahame@nicira.com>
The only user of this function was removed in the previous patch, so
remove it.
Signed-off-by: Joe Stringer <joestringer@nicira.com>
Acked-by: Alex Wang <alexw@nicira.com>
Caching the results of xlate_learn was previously dependent on the state
of the 'may_learn' flag. This meant that if the caller did not specify
that this flow may learn, then a learn entry would not be cached.
However, the xlate_cache tends to be used on a recurring basis, so
failing to cache the learn entry can provide unexpected behaviour later
on, particularly in corner cases.
Such a corner case occurred previously:-
* Revalidation was requested.
* A flow with a learn action was dumped.
* The flow had no packets.
* The flow's corresponding xcache was cleared, and the flow revalidated.
* The flow went on to receive packets after the xcache is re-created.
In this case, the xcache would be re-created, but would not refresh the
timeouts on the learnt flow until the next time it was cleared, even if
it received more traffic. This would cause flows to time out sooner than
expected. Symptoms of this bug may include unexpected forwarding
behaviour or extraneous statistics being attributed to the wrong flow.
This patch fixes the issue by caching the entire flow_mod, including
actions, upon translating an xlate_learn action. This is used to perform
a flow_mod from scratch with the original flow, rather than simply
refreshing the rule that was created during the creation of the xcache.
Bug #1252997.
Reported-by: Scott Hendricks <shendricks@vmware.com>
Signed-off-by: Joe Stringer <joestringer@nicira.com>
Acked-by: Alex Wang <alexw@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
ovsrcu_init() was named after the atomic_init(), but the semantics are
different enough to warrant a different name. Basically C11
atomic_init is defined in a way that allows the implementation to
assign the value without any syncronization, so in theory stores via
atomic_init could be seen by others after stores via atomic_set, even
if the atomic_init appeared earlier in the code.
ovsrcu_set_hidden() can be used to set an RCU protected variable when
it is not yet accessible by any active reader, but will be made
visible later via an ovsrcu_set call on some other pointer.
This patch also adds a new ovsrcu_init() that can be used to initilize
RCU protected variables when the readers are not yet executing. The
new ovsrcu_init() is implemented with atomic_init(), so it does not
provide any kind of syncronization.
Signed-off-by: Jarno Rajahalme <jrajahalme@nicira.com>
Acked-by: YAMAMOTO Takashi <yamamoto@valinux.co.jp>
ovsrcu_set() and ovsrcu_init() look like functions, so users are
justified in expecting the arguments to be evaluated before any of
the body of ovsrcu_set or ovsrcu_init().
With ovs-atomic-pthreads, a fallback ovs-atomics implementation used
when no C11 atomics are available or with GCC older than 4.0, the
prior definitions led to nested mutex locking when the 'VALUE'
argument was an atomic_read(). This is resolved by ensuring function
argument semantics for the parameters. For non-GCC compilers this is
done with an inline function. GCC implementation of ovs-rcu does not
fix the RCU variable type, so a GCC macro needs to be used instead.
Reported-by: YAMAMOTO Takashi <yamamoto@valinux.co.jp>
Signed-off-by: Jarno Rajahalme <jrajahalme@nicira.com>
Acked-by: YAMAMOTO Takashi <yamamoto@valinux.co.jp>
'struct ovsrcu_pointer' is always used with the 'struct' keyword, so
remove the unneeded 'typedef'.
Signed-off-by: Jarno Rajahalme <jrajahalme@nicira.com>
Acked-by: YAMAMOTO Takashi <yamamoto@valinux.co.jp>
A series of bugs have been identified recently that are caused by a
combination of the awkward flow dump API, possibility of duplicate flows
in a flow dump, and premature optimisation of the revalidator logic.
This patch attempts to simplify the revalidator logic by combining
multiple critical sections into one, which should make the state more
consistent.
The new flow of logic is:
+ Lookup the ukey.
+ If the ukey doesn't exist, create it.
+ Insert the ukey into the udpif. If we can't insert it, skip this flow.
+ Lock the ukey. If we can't lock it, skip it.
+ Determine if the ukey was already handled. If it has, skip it.
+ Revalidate.
+ Update ukey's fields (mark, flow_exists).
+ Unlock the ukey.
Previously, we would attempt process a flow without creating a ukey if
it hadn't been dumped before and it was due to be deleted. This patch
changes this to always create a ukey, allowing the ukey's
mutex to be used as the basis for preventing a flow from being handled
twice. This improves code correctness and readability.
Signed-off-by: Joe Stringer <joestringer@nicira.com>
Acked-by: Ethan Jackson <ethan@nicira.com>
This patch addresses two bugs related to ICMPv6(NDP) packets:
- In miniflow_extract() push the words in the correct order
- In parse_icmpv6() use sizeof struct, not size of struct *
match_wc_init() has been modified, to include the nd_target field
when the transport layer protocol is ICMPv6
A testcase has been added to detect the bugs.
Signed-off-by: Daniele Di Proietto <ddiproietto@vmware.com>
Acked-by: Jarno Rajahalme <jrajahalme@nicira.com>
Add OFPERR_OFPBIC_DUP_INST (type = OFPET_BAD_INSTRUCTION, code = 9)
and use it for OpenFlow1.4+.
For OpenFlow1.1 - 1.3 map this error to ONFBIC_DUP_INSTRUCTION
(experimenter = ONF, type = 2600) which is proposed in
OpenFlow enhancement proposal EXT-260 "Add error
code for duplicate instruction.".
Previously ONFBIC_DUP_INSTRUCTION was used for OpenFlow1.3+.
Signed-off-by: Simon Horman <horms@verge.net.au>
Signed-off-by: Ben Pfaff <blp@nicira.com>
Older versions of Python do not have ctypes as a default installed
package. This patch puts the 'import ctypes' statement inside a try
statement.
This fixes a bug introduced by commit 8396f (timeval: Use monotonic
time in OVS Python timeval library).
Signed-off-by: Ryan Wilson <wryan@nicira.com>
Acked-by: Alex Wang <alexw@nicira.com>
Python's time.time() function uses the system wall clock. However,
if NTP resets the wall clock to be a time in the past, then this
causes any applications that poll block based on time, such as
ovs-xapi-sync, to poll block indefinitely since the time is
unexpectedly negative.
This patch fixes the issue by using time.monotonic() if Python's
version >= 3.3. Otherwise, the timeval module calls out to the
librt C shared library and uses the clock_gettime function with
CLOCK_MONOTONIC.
Note this is only enabled on Linux-based platforms. This has been
tested on Ubuntu 12.04 and Redhat 6.4.
Bug #1189434
Signed-off-by: Ryan Wilson <wryan@nicira.com>
Acked-by: Ethan Jackson <ethan@nicira.com>
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
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
