util: Suppress a warning by adding CONST_CAST

Suppress a gcc warning which was introduced by
commit e0b48482c16b6eaa7f14d8c7e7c6275528881b9e.
("util: create a copy of program_name")
I guess MSVC doesn't have a corresponding warning.

Signed-off-by: YAMAMOTO Takashi <yamamoto@valinux.co.jp>
Acked-by: Lorand Jakab <lojakab@cisco.com>

ovs-thread: Implement OVS specific barrier.

Non-leader revalidator thread uses pthread_barrier_* functions in their
main loop to synchronize with leader thread.  However, since those threads
only call poll_block() intermittently, the poll interval check in
poll_block() can wrongly take the time since last call as poll interval
and issue the following warnings:

"Unreasonably long XXXXms poll interval".

To prevent it, this commit implements the barrier struct and operations
for OVS which allow thread to block on barrier via poll_block().

Signed-off-by: Alex Wang <alexw@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>

util: create a copy of program_name

Commit 8a9562 ("dpif-netdev: Add DPDK netdev.") reversed sequence
in which set_program_name() and proctitle_init() functions are
called.  This introduced a regression where program_name and argv_start
would point to exactly the same memory (previously both of these
pointers were pointing to different memory locations because
proctitle_init() would have beforehand created a copy of argv[0]
for the succeeding set_program_name() call).

This regression on my system caused ovs-vswitchd monitoring process to
show up without process name:

...  00:00:00 : monitoring pid 26308 (healthy)

Ps output was lacking process name because following code was
using overlapping memory for source and target buffer:.

proctitle_set(const char *format, ...)
{
    ...
    n = snprintf(argv_start, argv_size, "%s: ", program_name);

Overall C99 and POSIX standards state that behavior is undefined
if source and target buffers overlap.

Signed-Off-By: Ansis Atteka <aatteka@nicira.com>
Acked-By: Ben Pfaff <blp@nicira.com>

netlink-socket: Work around upstream kernel Netlink bug.

The upstream kernel net/netlink/af_netlink.c netlink_recvmsg() contains the
following code to refill the Netlink socket buffer with more dump skbs
while a dump is in progress:

if (nlk->cb && atomic_read(&sk->sk_rmem_alloc) <= sk->sk_rcvbuf / 2) {
ret = netlink_dump(sk);
if (ret) {
sk->sk_err = ret;
sk->sk_error_report(sk);
}
}

The netlink_dump() function that this calls returns a negative number on
error, the convention used throughout the kernel, and thus sk->sk_err
receives a negative value on error.

However, sk->sk_err is supposed to contain either 0 or a positive errno
value, as one can see from a quick "grep" through net for 'sk_err =', e.g.:

    ipv4/tcp.c:2067: sk->sk_err = ECONNRESET;
    ipv4/tcp.c:2069: sk->sk_err = ECONNRESET;
    ipv4/tcp_input.c:4106: sk->sk_err = ECONNREFUSED;
    ipv4/tcp_input.c:4109: sk->sk_err = EPIPE;
    ipv4/tcp_input.c:4114: sk->sk_err = ECONNRESET;
    netlink/af_netlink.c:741: sk->sk_err = ENOBUFS;
    netlink/af_netlink.c:1796: sk->sk_err = ENOBUFS;
    packet/af_packet.c:2476: sk->sk_err = ENETDOWN;
    unix/af_unix.c:341: other->sk_err = ECONNRESET;
    unix/af_unix.c:407: skpair->sk_err = ECONNRESET;

The result is that the next attempt to receive from the socket will return
the error to userspace with the wrong sign.

(The root of the error in this case is that multiple threads are attempting
to read a single flow dump from a shared fd.  That should work, but the
kernel has an internal race that can result in one or more of those threads
hitting the EINVAL case at the start of netlink_dump().  The EINVAL is
harmless in this case and userspace should be able to ignore it, but
reporting the EINVAL as if it were a 22-byte message received in userspace
throws a real wrench in the works.)

This bug makes me think that there are probably not many programs doing
multithreaded Netlink dumps.  Maybe it is good that we are considering
other approaches.

VMware-BZ: #1255704
Reported-by: Mihir Gangar <gangarm@vmware.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Alex Wang <alexw@nicira.com>

revalidator: Improve optimization to skip revalidation.

The should_revalidate() optimisation introduced with commit 698ffe3623
(revalidator: Only revalidate high-throughput flows.) was a little
aggressive, occasionally deleting flows even when OVS is quite capable
of performing full revalidation.

This commit modifies the logic to:
* Firstly, check if we are likely to handle full revalidation, and
  attempt that instead.
* Secondly, fall back to the existing flow throughput estimations to
  determine whether to revalidate the flow or just delete it.

VMware-BZ: #1271926

Signed-off-by: Joe Stringer <joestringer@nicira.com>
Acked-by: Alex Wang <alexw@nicira.com>

datapath: Use exact lookup for flow_get and flow_del.

Due to the race condition in userspace, there is chance that two
overlapping megaflows could be installed in datapath.  And this
causes userspace unable to delete the less inclusive megaflow flow
even after it timeout, since the flow_del logic will stop at the
first match of masked flow.

This commit fixes the bug by making the kernel flow_del and flow_get
logic check all masks in that case.

Signed-off-by: Alex Wang <alexw@nicira.com>
Acked-by: Andy Zhou <azhou@nicira.com>
Acked-by: Pravin B Shelar <pshelar@nicira.com>

datapath: fix sparse warning in function tbl_mask_array_delete_mask()

Sparse gives "incompatible types in comparison expression (different
address spaces)" warning messages. Fix this by add rcu_dereference()
wrappers.

Reported-by: Ben Pfaff <blp@nicira.com>
Signed-off-by: Andy Zhou <azhou@nicira.com>
Acked-by: Pravin B Shelar <pshelar@nicira.com>

datapath: keep mask array compact when deleting mask

When deleting a mask from the mask array, we always move the last entry
into its current location. Another approach can be NULL in its
current place, and periodically compact it.

The approach taken by this patch is more efficient during run
time.  During look up, fast path packet don't have to skip over NULL
pointers.

A more important advantage of this approach is that it tries to
keep the mask array index stable by avoiding periodic index
reshuffle.

This patch implements an optimization to further promote index
stability.  By leaving the last entry value intact when moving it
to a new location, the old cache index can 'fix' themselves, by noticing
the index in the cache is outside the valid mask array region. The
new index can be found by scanning the mask pointer within the valid
rtegion.

Signed-off-by: Andy Zhou <azhou@nicira.com>
Acked-by: Pravin B Shelar <pshelar@nicira.com>

datapath: simplify ovs_flow_tbl_lookup_stats()

Simplify flow mask cache replacement without using expensive atomic
memory access to the mask pointers.

Signed-off-by: Andy Zhou <azhou@nicira.com>
Acked-by: Pravin B Shelar <pshelar@nicira.com>

datapath: Change u64_stats_* to use _irq instead of _bh().

The upstream u64_stats API has been changed to remove the _bh()
versions and switch all consumers to use IRQ safe variants instead.
This was done to be safe for netpoll generated packets, which can
occur in hard IRQ context. From a safety perspective, this doesn't
directly affect OVS since it doesn't support netpoll. However, this
change has been backported to older kernels so OVS needs to use the
new API to compile.

Signed-off-by: Jesse Gross <jesse@nicira.com>
Acked-by: Pritesh Kothari <pritesh.kothari@cisco.com>
Acked-by: Pravin B Shelar <pshelar@nicira.com>

rhel, xenserver: Run unit tests while creating rpms.

For RHEL, Fedora and Xenserver, run unit tests while
building rpms.  This may catch some cross-platform bugs.

The commit also allows the users to optionally skip unit tests.
(On debian, the default is to run unit tests. For consistency,
do the same for rpms.)

VMware-BZ: 1267127

CC: Flavio Leitner <fbl@redhat.com>
CC: Ben Pfaff <blp@nicira.com>
Signed-off-by: Gurucharan Shetty <gshetty@nicira.com>
Acked-by: Flavio Leitner <fbl@redhat.com>
Tested-by: Flavio Leitner <fbl@redhat.com>

test-ovsdb: Workaround unicode bug in Python 2.4.x.

Run the following command on Xenserver:
PYTHONPATH=`pwd`/python/compat::`pwd`/python python ./tests/test-ovsdb.py \
parse-atoms '{"type": "string", "minLength": 2}'    \
'[""]'     '["a"]'     '["ab"]'     '["abc"]'     '["\ud834\udd1e"]'

And we get the following error:
UnicodeEncodeError: 'ascii' codec can't encode character u'\U0001d11e'
in position 23: ordinal not in range(128).

It looks like we are hitting the following bug:
http://bugs.python.org/issue2517

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

socket_util.py: Make set_dscp() python 2.4.3 compatible.

There is no 'errno' field in socket.error. Instead use the
get_exception_errno() function to get the error number.

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

lib: Fix FreeBSD build.

Various recent commits have introduced build failures on FreeBSD. This
patch fixes them.

Signed-off-by: Joe Stringer <joestringer@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>

dpif-netdev: delete lost packets in dp_execute_cb()

This commit fixes memory leaks in dp_execute_cb() in two cases:
    - when the output port cannot be found
    - when the recirculation depth is exceeded

Reported-by: Pravin Shelar <pshelar@nicira.com>
Signed-off-by: Daniele Di Proietto <ddiproietto@vmware.com>
Acked-by: Pravin B Shelar <pshelar@nicira.com>

dpif-netdev: Fix memory leak in dpif_netdev_flow_put()

miniflow_destroy() needs to be called after using miniflow_init().
Otherwise, if the miniflow mallocs data, then a memory leak may
occur.

Found by inspection.

Signed-off-by: Ryan Wilson <wryan@nicira.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>

json: Fix parsing of strings that end with a backslash.

json_string_unescape() flagged a backslash at the end of a string as an
error, but of course "\\" is a valid string.  This fixes the problem.

VMware-BZ: #1275208
Reported-by: Michael Hu <mhu@nicira.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Alex Wang <alexw@nicira.com>

datapath: Rehash 16-bit skbuff hashes into 32 bits.

Currently, if the network stack provides skb->rxhash then we use it,
otherwise we compute our own. However, on at least some versions of
RHEL/CentOS, the stack provides a hash that is 16 bits rather than
32 bits. In cases where we use the uppermost bits of the hash this
is particularly bad because we detect that a hash is present and we
use it rather than computing our own but the result is always zero.

This is particularly noticible with tunnel ports that use the hash
to generate a source port, such as VXLAN. On these kernels the tunnel
source port is always the minimum value. To solve this problem while
still taking advantage of the precomputed hash, this rehashes the
hash so that the entropy is spread throughout 32 bits.

Signed-off-by: Jesse Gross <jesse@nicira.com>
Acked-by: Thomas Graf <tgraf@noironetworks.com>

dpif: When executing actions needs help, use "set" action to set tunnel.

Open vSwitch userspace is able to implement some actions that the kernel
doesn't support, such as modifying ARP fields.  When it does this for a
tunneled packet, it needs to supply the tunnel information with a "set"
action, because the Linux kernel datapath throws away tunnel information
supplied in the OVS_PACKET_CMD_EXECUTE metadata argument.

VMware-BZ: #1270110
Reported-by: Srinivas Neginhal <sneginha@vmware.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Jesse Gross <jesse@nicira.com>

netdev-vport: Fix use-after-free error in netdev_vport_route_changed().

We can't unlock the netdev's mutex after close the netdev, because closing
the netdev might destroy the mutex.

VMware-BZ: #1275187
Signed-off-by: Ben Pfaff <blp@nicira.com>

stream-ssl: Enable TLSv1.1 and TLSv1.2.

The Open vSwitch SSL code was inadvertently enabling only TLSv1, not
later versions.  This commit should fix it.

See https://www.openssl.org/docs/ssl/SSL_CTX_new.html
and http://www.postgresql.org/message-id/20131203213049.GA8259@gmail.com
for more information.

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

lib/classifier: Fix use of uninitialized memory.

When reaching the end of a prefix trie, we checked one bit off the end
to the intended data.  However, since the trie node in that case has
NULLs for both edge links, this did not result in incorrect
functionality.

Found via check-valgrind.

Reported-by: Ben Pfaff <blp@nicira.com>
Signed-off-by: Jarno Rajahalme <jrajahalme@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>

lib/classifier: Clarify trie_lookup_value().

trie_lookup_value() is easier to read with the local variable 'plen'
renamed as 'ofs'.

Signed-off-by: Jarno Rajahalme <jrajahalme@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>

datapath:  avoid memory corruption in queue_userspace_packet()

In queue_userspace_packet(), the ovs_nla_put_flow return value is
not checked. This is fine as long as key_attr_size() returns the
correct value. In case it does not, the current code may corrupt buffer
memory. Add a run time assertion catch this case to avoid silent
failure.

Reported-by: Ben Pfaff <blp@nicira.com>
Signed-off-by: Andy Zhou <azhou@nicira.com>
Acked-by: Pravin B Shelar <pshelar@nicira.com>

datapath: fix key size computation in key_attr_size()

The key_attr_size() was not updated when RECIRC_ID and DP_HASH
key fields are added to support recircualtion. This patch fixes it.

Reported-by: Ben Pfaff <blp@nicira.com>
Signed-off-by: Andy Zhou <azhou@nicira.com>
Acked-by: Pravin B Shelar <pshelar@nicira.com>
VMware-BZ: 1266214

Fix log message weird suffixes.

I think these were leftovers from the removal of %z for MSVC that happened
some time ago.

VMware-BZ: 1265762
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Pritesh Kothari <pritesh.kothari@cisco.com>

netflow: Fold netflow_expire() into netflow_flow_clear().

netflow_flow_clear() asserted that no packets or bytes were included
in the statistics for the flow being cleared.  Before threading Open
vSwitch, this assertion was always true because netflow_expire() was
always called before calling netflow_flow_clear().  Since Open
vSwitch was threaded, however, it was possible that a packet arrived
after netflow_expire() but before netflow_flow_clear(), since each of
these function separately took the netflow mutex.

This commit fixes the problem by merging netflow_expire() into
netflow_flow_clear(), under a single acquisition of the netflow
mutex.

Signed-off-by: Anoob Soman <anoob.soman@citrix.com>
[blp@nicira.com modified the patch to remove netflow_expire() and
 rewrote the commit message]
Signed-off-by: Ben Pfaff <blp@nicira.com>

ofproto-dpif-rid: Fix memory leak in recirc_id_pool_destroy().

recirc_id_pool_create() allocates memory but recirc_id_pool_destroy() did
not destroy it.

Found by valgrind.

Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Thomas Graf <tgraf@suug.ch>

ofproto: Fix memory leak in ofproto_destroy().

Found by valgrind.

Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Thomas Graf <tgraf@suug.ch>

ofproto: Send monitor updates if a flow mod changes a rules actions

Without this change a monitor update will be sent when a flow mod changes
a rules cookie but not if only the actions are updated. This appears
to be a logic error.

I noticed this while working on implementing OpenFlow1.4 flow monitor
as an OpenFlow1.4 flow mod does not update a rules cookie.

Signed-off-by: Simon Horman <horms@verge.net.au>
Signed-off-by: Ben Pfaff <blp@nicira.com>

poll-loop: Ignore 'wevent' in poll_fd_wait_at() on non-Windows.

'wevent' isn't actually used on non-Windows systems, but poll_fd_wait_at()
and find_poll_node() treat events with different 'wevent' as different, so
it seems better to make sure that 'wevent' doesn't matter.

Alternatively, one could ovs_assert(!wevent).  I guess that would catch
a caller accidentally swapping the 'fd' and 'wevent' arguments.

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

stream-ssl: Always initialize wevent member, even on non-Windows.

Otherwise the indeterminate 'wevent' could frustrate poll_fd_wait_at()'s
attempt to merge "poll_node"s for the same fd.

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

flow: Fix use-after-free in flow_compose().

flow_compose_l4() can cause 'b' to be reallocated, thus the network header
pointer needs to be refreshed afterward.

Found by valgrind in the IPv6 case.  I updated the IPv4 case too just in
case, and for consistency.

Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Andy Zhou <azhou@nicira.com>

dpif-netdev: Fix another use-after-free in port_unref().

Commit 87400a3d4cc4a (dpif-netdev: Fix use-after-free in port_unref().)
fixed one use-after-free in the common case of port_unref().  However,
there was another, similar case: if port->netdev has no rxqs, then
the netdev_close() causes port->netdev to be destroyed and thus the
following call to netdev_n_rxq() accesses freed memory.  This commit fixes
the problem.

Found by valgrind.

Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Andy Zhou <azhou@nicira.com>

ofproto-dpif: Fix uninitialized data in ofproto_dpif_execute_actions().

The dp_hash and recirc_id fields weren't being initialized.

Found by valgrind.

Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Andy Zhou <azhou@nicira.com>

dpif: Fix slow action handling for DP_HASH and RECIRC

In case DP_HASH and RECIRC actions need to be executed in slow path,
current implementation simply don't handle them -- vswitchd simply
crashes. This patch fixes them by supply an implementation for them.

RECIRC will be handled by the datapath, same as the output action.

DP_HASH, on the other hand, is handled in the user space. Although the
resulting hash values may not match those computed by the datapath, it
is less expensive; current use case (bonding) does not require a strict
match to work properly.

Reported-by: YAMAMOTO Takashi <yamamoto@valinux.co.jp>
Signed-off-by: Andy Zhou <azhou@nicira.com>
Acked-by: YAMAMOTO Takashi <yamamoto@valinux.co.jp>

ofproto: Remove ofproto_refresh_rule().

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>

ofproto-dpif-xlate: Cache full flowmod for learning.

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>

lib/ovs-rcu: Evaluate parameters before ovsrcu_set.

ovsrcu_set() looks like a function, so users are justified in expecting
the arguments to be evaluated before any of the body of ovsrcu_set().

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>

ovs-rcu: Remove the extra 'typedef' keyword.

'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>

lib/flow: fix minimatch_extract() ICMPv6 parsing

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>
Signed-off-by: Jarno Rajahalme <jrajahalme@nicira.com>

bridge: Resend status changes to database if previous transaction
was not successful.

Bridge, port and interface status changes are not sent to the
database if the connectivity and netdev sequence numbers have not
changed. However, if the previous database transaction fails, then
status changes will not be updated in the database until the
connectivity and netdev sequence numbers change again. This could
leave the database in an incorrect state for a long period of time.

This patch always sends status changes to the database if the last
transaction was not successful.

Bug #1256577
Signed-off-by: Ryan Wilson <wryan@nicira.com>
Acked-by: Alex Wang <alexw@nicira.com>
Signed-off-by: Alex Wang <alexw@nicira.com>

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>

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>

revalidator: Eliminate duplicate flow handling.

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 is expected to cause some minor performance regression for
cases like TCP_CRR, in favour of correctness and readability.

Bug #1252997.

Signed-off-by: Joe Stringer <joestringer@nicira.com>
Acked-by: Alex Wang <alexw@nicira.com>

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>

dpif-linux: Fix revalidator core caused by flow dumps.

The dpif flow dump API guarantees that keys it returns are not deleted
unless dpif_flow_dump_next_may_destroy_keys() warns that they might
be.  When dpif_linux_flow_dump_next() needed extra memory for a
datapath flow's actions, it would use a special temporary buffer.
Unfortunately, it also used memory from this temporary buffer for the
keys it returned.  Thus, on the next loop this memory would be freed,
breaking our invariant.

The correct solution to this problem is probably to fix this rather
awkward API.  However, this patch's solution is small and simple, so
it's fine for now.

The following is a valgrind stack trace which lead to finding this
bug.

Invalid read of size 2
  at 0x4C2D050: memcpy@@GLIBC_2.14 (in ...)
  by 0x4D40B8: dpif_linux_flow_to_ofpbuf (dpif-linux.c:2373)
  by 0x4D4DE4: dpif_linux_operate__ (dpif-linux.c:1394)
  by 0x4D5055: dpif_linux_operate (dpif-linux.c:1480)
  by 0x450D14: dpif_operate (dpif.c:1225)
  by 0x42D706: push_dump_ops__.isra.6 (ofproto-dpif-upcall.c:1362)
  by 0x42E5A8: revalidate.isra.11 (ofproto-dpif-upcall.c:1531)
  by 0x42EB0E: udpif_revalidator (ofproto-dpif-upcall.c:592)
  by 0x495070: ovsthread_wrapper (ovs-thread.c:302)
  by 0x5472E99: start_thread (pthread_create.c:308)
  by 0x5C7C4BC: clone (clone.S:112)
Address 0x227e12d0 is 160 bytes inside a block of size 4,976 free'd
  at 0x4C2A82E: free (in ...)
  by 0x494227: ofpbuf_uninit (ofpbuf.c:136)
  by 0x4D5D34: dpif_linux_flow_dump_next (ofpbuf.h:182)
  by 0x4509BA: dpif_flow_dump_next (dpif.c:1036)
  by 0x42E137: revalidate.isra.11 (ofproto-dpif-upcall.c:1451)
  by 0x42EB0E: udpif_revalidator (ofproto-dpif-upcall.c:592)
  by 0x495070: ovsthread_wrapper (ovs-thread.c:302)
  by 0x5472E99: start_thread (pthread_create.c:308)
  by 0x5C7C4BC: clone (clone.S:112)

Signed-off-by: Ethan Jackson <ethan@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>

ofproto-dpif-upcall: Fix additional use-after-free in revalidate().

Commit 1340ce0c175 (ofproto-dpif-upcall: Avoid use-after-free in
revalidate() corner cases.) fixed one use-after-free error in revalidate(),
but missed one more subtle case, in which dpif_linux_flow_dump_next()
attempts to retrieve actions for a flow that didn't have them in the main
dump result.  If retrieving those actions fails,
dpif_linux_flow_dump_next() goes on to the next flow, and as part of that
overwrites the old dumped flows in its buffer.  This is a problem because
dpif_linux_flow_dump_next_may_destroy_keys() would have indicated that
the old dumped flows would not have been destroyed, which means that the
data the caller relied on has gone away.  In the worst case, this causes
a segfault and core dump.

The best way to fix this problem is the refactoring that has already
happened on master in commit ac64794acb85 (dpif: Refactor flow dumping
interface to make better sense for batching.)  That is a fairly large
change, and not yet well-tested, so it doesn't yet seem suitable for a
stable branch.  For now, this commit refines the conditions that
dpif_linux_flow_dump_next_may_destroy_keys() uses, so that if the next
flow does not include actions it indicates that keys may be destroyed.

Thanks to Joe Stringer for suggesting this particular solution.

Bug #1249988.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Joe Stringer <joestringer@nicira.com>

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>

dpif-netdev: Fix memory leak.

Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Jarno Rajahalme <jrajahalme@nicira.com>

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>

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>

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>

AUTHORS: Fix spelling of Anoob Soman's name.

Signed-off-by: Ben Pfaff <blp@nicira.com>

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>

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>

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>

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>

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>

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>

Prepare for 2.3.0.

Signed-off-by: Justin Pettit <jpettit@nicira.com>

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>

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>

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>

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>

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>

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>

tests/library.at: Disable unix socket tests on Windows.

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

ovs-pki: Pass correct argument to 'sign' command.

The first argument passed to 'sign' command is used as prefix
for both req.pem and cert.pem.

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

timeval: Workaround strftime bug in VS 2013.

Visual Studio 2013's behavior is to crash when 0 is passed as second
argument to strftime.

Signed-off-by: Gurucharan Shetty <gshetty@nicira.com>

ovsdb: Open database in binary mode.

Some ovsdb-tool related unit tests fail with bad checksum errors
while reading transactions from database. It is most likely because
of the CR at the end of line. Using binary mode solves the problem.

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

meta-flow: Use OXM-defined constant for TCP flags in OpenFlow 1.5.

This also adds the definitions of a few other OXM headers we didn't have
yet.

EXT-109.
Signed-off-by: Ben Pfaff <blp@nicira.com>

meta-flow: Encode OXM version information into struct mf_field.

Signed-off-by: Ben Pfaff <blp@nicira.com>

nx-match: Refactor nxm_put_ip() to handle all IPv4 and IPv6 fields.

Until now, some fields have been handled in the caller, and the caller has
been responsible for distinguishing ICMPv4 from ICMPv6.  This
implementation seems to make the code a little easier to understand.

Signed-off-by: Ben Pfaff <blp@nicira.com>

Implement OpenFlow 1.5 port desc stats request.

OpenFlow 1.4 and earlier always send the description of every port in
response to an OFPMP_PORT_DESC request.  OpenFlow 1.5 proposes allowing
the controller to request a description of a single port.  This commit
implements a prototype.

EXT-69.
Signed-off-by: Ben Pfaff <blp@nicira.com>

Implement OpenFlow 1.5 group desc stats request.

OpenFlow 1.4 and earlier always send the description of every group in
response to an OFPMP_GROUP_DESC request.  OpenFlow 1.5 proposes allowing
the controller to request a description of a single group.  This commit
implements a prototype.

EXT-69.
Signed-off-by: Ben Pfaff <blp@nicira.com>

Add skeleton for OF1.5 support.

This allows OF1.5 prototyping to take place in a natural way.

Signed-off-by: Ben Pfaff <blp@nicira.com>

Remove --enable-of14 option because the implementation is now safe.

Signed-off-by: Ben Pfaff <blp@nicira.com>

ofp-util: Remove ofputil_get_phy_port_size().

The size is not fixed for OpenFLow 1.4 and later, so it's a little
deceptive to return any particular value.  This function was only used in
one place, so move it inline there.

Signed-off-by: Ben Pfaff <blp@nicira.com>

ofp-util: Simplify ofputil_decode_switch_features().

It does not need to check the size, because the decoder in ofp-msgs.c
checks for a valid size.

Signed-off-by: Ben Pfaff <blp@nicira.com>

Implement OpenFlow 1.4 queue statistics.

Signed-off-by: Ben Pfaff <blp@nicira.com>

Implement OpenFlow 1.4 port statistics.

Signed-off-by: Ben Pfaff <blp@nicira.com>

Implement basic OpenFlow 1.4 table-mod message.

Vacancy events and eviction are not yet implemented--see OPENFLOW-1.1+.

Signed-off-by: Ben Pfaff <blp@nicira.com>

Implement OpenFlow 1.4 port_mod messages.

Signed-off-by: Ben Pfaff <blp@nicira.com>

ofp-util: Implement OpenFlow 1.4 port status and port desc reply messages.

Signed-off-by: Ben Pfaff <blp@nicira.com>

ofp-util: Reduce duplicate code.

ofputil_put_phy_port() and ofputil_append_port_desc_stats_reply() had a
lot of code duplication.  This reduces it: it deletes some specialized
code from ofputil_put_phy_port(), moving it into its caller
ofputil_put_switch_features_port() that actually needed it.  That change
then allows ofputil_append_port_desc_stats_reply() to become a simple
wrapper around ofputil_put_phy_port().

Signed-off-by: Ben Pfaff <blp@nicira.com>

ofp-util: Generalize functions for parsing OF1.3+ properties.

The main effect is to move these functions a little earlier in the file.

Also, OpenFlow 1.4 changed the table-features specific error codes to new
values that apply to all property sets, so this commit updates the error
code names and adds the appropriate OpenFlow 1.4+ codes.

Signed-off-by: Ben Pfaff <blp@nicira.com>

ofp-util: Remove ofputil_count_phy_ports().

It's harder to calculate the number of ports in a given amount of space in
OpenFlow 1.4 and later, because the ofp_port structure becomes variable
length in those versions.  This commit removes the one caller, replacing
it by a version that doesn't need to know the number of ports in advance.

Signed-off-by: Ben Pfaff <blp@nicira.com>

ofp-msgs: New functions ofpmp_version() and ofpmp_decode_raw().

Each of these allows code in ofp-util.c to be simplified.

Signed-off-by: Ben Pfaff <blp@nicira.com>

ofp-util: Fix definition of OFPUTIL_P_OF13_UP.

It should include OF1.4.

Signed-off-by: Ben Pfaff <blp@nicira.com>

ovs-ofctl: Fix port lookup and "ovs-ofctl" behavior for OpenFlow 1.3+.

ovs-ofctl supports using port names in commands that operate on ports.  It
does this by connecting to the switch, listing the ports, and picking out
the one with the specified name.  However, this didn't work properly for
OpenFlow 1.3+, because it always used an OFPT_FEATURES_REQUEST to list the
ports, and in OpenFlow 1.3+ the reply to this request does not include a
list of ports.  This commit fixes the problem (using code that previously
was just a fallback when there were too many ports to fit in an
OFPT_FEATURES_REPLY).

For similar reasons, "ovs-ofctl show" wasn't listing the switch's ports
when it connected to a switch over OpenFlow 1.3 or later.  This commit
fixes that bug also.

Signed-off-by: Ben Pfaff <blp@nicira.com>
Conflicts:
utilities/ovs-ofctl.c

AUTHORS: Add Ashwin Swaminathan.

Signed-off-by: Ben Pfaff <blp@nicira.com>

ofproto-dpif-xlate: Fix null pointer dereference

actions (in xlate_actions__) would be NULL when xlate_actions()
is called from packet_out()->ofproto_dpif_execute_actions().
This causes a NULL pointer to be dereferenced when
ctx.xbridge->netflow is set.

Signed-off-by: Anoob Soman <anoob.soman@citrix.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>

datapath: Free skb(s) on recirculation error

This patch attempts to ensure that skb(s) are always freed (once)
if if an error occurs in execute_recirc(). It does so in two ways:

1. Ensure that execute_recirc() always consimes skb passed to it.
   Specifically, free the skb if the call to ovs_flow_extract() fails.

2. Return from the recirc case in execute_recirc() whenever
   the skb has not been cloned immediately above.

   This is only the case if the action is both the last action and the
   keep_skb parameter of execute_recirc is not true.  As it is the last
   action and the skb is consumed one way or another by execute_recirc() it
   is correct to return here.  In particular this avoids the possibility of
   the skb, which has been consumed by execute_recirc() from being freed.

   Conversely if this is not the case then the skb has been cloned
   and the clone has been consumed by execute_recirc().
   This leads to three sub-cases:
   * If execute_recirc() returned an error code then the original skb
     will be freed by the error handling code below the case statement in
     do_execute_actions().
   * If this is not the last action then action processing will continue,
     using the original skb.
   * If this is the last action then it must also be the case that keep_skb
     is true (otherwise the skb would not have been cloned). Thus
     do_execute_actions() will return without freeing the original skb.

Signed-off-by: Simon Horman <horms@verge.net.au>
[jesse: use kfree_skb instead of consume_skb on error path]
Signed-off-by: Jesse Gross <jesse@nicira.com>

lib/classifier: Fix array splicing.

Array splicing was broken when multiple elements were being moved,
resulting in the priority order being mixed.  This came up when the
highest priority rule from a subtable was removed and the subtable
needed to be moved down the priority list by more than one position.

Signed-off-by: Jarno Rajahalme <jrajahalme@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>

INSTALL: Describe how to use a separate build directory.

CC: YAMAMOTO Takashi <yamamoto@valinux.co.jp>
CC: Andy Zhou <azhou@nicira.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Andy Zhou <azhou@nicira.com>
Acked-by: YAMAMOTO Takashi <yamamoto@valinux.co.jp>

ofproto-dpif.at: Fix a race in "ofproto-dpif - patch ports" test

Acked-by: Ben Pfaff <blp@nicira.com>
Signed-off-by: YAMAMOTO Takashi <yamamoto@valinux.co.jp>

ofproto-dpif.h: Fix a comment

Acked-by: Ben Pfaff <blp@nicira.com>
Signed-off-by: YAMAMOTO Takashi <yamamoto@valinux.co.jp>

dpif-netdev: Remove unused members

Simplify code and update comments after commit 61e7deb1.
("dpif-netdev: Use RCU to protect data.")

Acked-by: Ben Pfaff <blp@nicira.com>
Signed-off-by: YAMAMOTO Takashi <yamamoto@valinux.co.jp>

ofproto-dpif: Whitespace fixes

Acked-by: Ben Pfaff <blp@nicira.com>
Signed-off-by: YAMAMOTO Takashi <yamamoto@valinux.co.jp>