ofp-errors: Duplicate instruction error

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>
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>
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 improves code correctness and readability.

Signed-off-by: Joe Stringer <joestringer@nicira.com>
Acked-by: Ethan Jackson <ethan@nicira.com>
revalidator: Fix a build issue.

Commit acaa8dac490cc6c36026d45f3010ec75f38ee142 (revalidator: Eliminate
duplicate flow handling.) introduced a build error. This fixes the bug.

Reported-by: Jarno Rajahalme <jrajahalme@nicira.com>
Signed-off-by: Joe Stringer <joestringer@nicira.com>
Acked-by: Jarno Rajahalme <jrajahalme@nicira.com>
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/ovs-rcu: Evaluate parameters before ovsrcu_set and ovsrcu_init.

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>
lib/ovs-rcu: Rename ovsrcu_init() as ovsrcu_set_hidden().

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>
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>
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>
ovs-dev.py: Add option to specify which tests to run.

Signed-off-by: Joe Stringer <joestringer@nicira.com>
Acked-by: Ethan Jackson <ethan@nicira.com>
ovs-dev.py: Add option to run tests in parallel.

Signed-off-by: Joe Stringer <joestringer@nicira.com>
Acked-by: Ethan Jackson <ethan@nicira.com>
datapath: Clean up files on distclean.

This was causing failures in 'make distcleancheck'.

Signed-off-by: Joe Stringer <joestringer@nicira.com>
Acked-by: Pravin B Shelar <pshelar@nicira.com>
BUILD.Windows: Add a TODO section.

And mention the lack of atomics support on Windows as the
first item.

Suggested-by: Jarno Rajahalme <jrajahalme@nicira.com>
Signed-off-by: Gurucharan Shetty <gshetty@nicira.com>
Acked-by: YAMAMOTO Takashi <yamamoto@valinux.co.jp>
Acked-by: Ben Pfaff <blp@nicira.com>
Acked-by: Jarno Rajahalme <jrajahalme@nicira.com>
odp-util: Do not set port mask of non-IP packets

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>
INSTALL: Note about compiler atomics support.

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>
ovs-sandbox: Use the correct make

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>
timeval: Add monotonic time functionality for NetBSD and FreeBSD.

This patch also checks the system platform as clock_gettime
could exist on different platforms but with different values of
CLOCK_MONOTONIC and different definitions of 'struct timespec'.
In this case, the system call would be expected to catch the
error, which is dangerous.

This patch ensures Linux, NetBSD and FreeBSD platforms use
clock_gettime with their corresponding correct values and
definitions. All other platforms use time.time().

Signed-off-by: Ryan Wilson <wryan@nicira.com>
Acked-by: YAMAMOTO Takashi <yamamoto@valinux.co.jp>
Signed-off-by: YAMAMOTO Takashi <yamamoto@valinux.co.jp>
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>
netdev-dpdk: receive up to NETDEV_MAX_RX_BATCH

As per netdev-provider interface, netdev_dpdk_rxq_recv should receive at most
NETDEV_MAX_RX_BATCH.

Signed-off-by: Daniele Di Proietto <ddiproietto@vmware.com>
Acked-by: Pravin B Shelar <pshelar@nicira.com>
netdev-dpdk: create queues on configured NUMA node

This patch makes sure that the tx and rx queues are allocated on the NUMA socket
chosen at device initalization time, instead of the NUMA socket 0.

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