ofp-msgs: Add OFPRAW_OFPST14_FLOW_MONITOR_*

Add OFPRAW_OFPST14_FLOW_MONITOR_REQUEST and
OFPRAW_OFPST14_FLOW_MONITOR_REPLY.

This is a step towards supporting OpenFlow1.4 flow monitors.

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

ofp-errors: Add OFPET_FLOW_MONITOR_FAILED

Add OFPET_FLOW_MONITOR_FAILED which is added in OpenFLow1.4.
Map the OFPERR_NXBRC_FM_* errors to the corresponding new OpenFlow errors.

This is a step towards supporting OpenFlow1.4 flow monitors.

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

ofproto: Add ofp14_flow_monitor_{request, command, flags}

This is in preparation for supporting OpenFlow1.4
flow monitor requests.

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

ofproto: Initialise return value of modify_flows__

dd51dae29bccca3 ("ofproto: Move logic for collecting read-only rules into
rule_criteria.") modifies modify_flows__ such that the variable error,
the return value, may be uninitialised if either of the following is true:

1. ofproto->ofproto_class->rule_premodify_actions is NULL
2. rules->n is zero

It appears for the "bfd - Verify tunnel down detection" test
in the testsuite the first condition is true and the test fails.

This commit fixes the problem.

Signed-off-by: Simon Horman <horms@verge.net.au>
[blp@nicira.com changed the style of the fix]
Signed-off-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

dpif-netdev: enumerate dpif belonging to the right class

Since dpif_netdev_enumerate() is used for "netdev" and "dummy" class, it
incorrectly lists dpif-netdevs as "dummy" and vice versa.
This patches address the issue by changing the dpif-provider interface: a
dpif_class parameter is passed to the 'enumerate' call to match the right class.

Signed-off-by: Daniele Di Proietto <ddiproietto@vmware.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>

tests: Use set_field instead of load_reg in OF1.2 test for metadata.

ONF-JIRA: EXT-46
Signed-off-by: Jean Tourrilhes <jt@hpl.hp.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>

Implement learned flow deletion.

When a flow with a "learn" action is deleted, one often wants the flows
that it created (the "learned flows") to be deleted as well.  This commit
makes that possible.

I am aware of a race condition that could lead to a learned flow not being
properly deleted.  Suppose thread A deletes a flow with a "learn" action.
Meanwhile, thread B obtains the actions for this flow and translates and
executes them.  Thread B could obtain the actions for the flow before it is
deleted, but execute them after the "learn" flow and its learned flows are
deleted.  The result is that the flow created by thread B persists despite
its "learn" flow having been deleted.  This race can and should be fixed,
but I think that this commit is worth reviewing without it.

VMware-BZ: #1254021
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Thomas Graf <tgraf@suug.ch>
Acked-by: Ethan Jackson <ethan@nicira.com>

ofproto: Reduce duplication in deletion logic.

The delete_flow__() function had two callers, each of which had to
correctly call ofmonitor_flush() when completely done.  An upcoming commit
will add another function that the callers need to call.  This is easier
if delete_flow__() only has one caller, so this commit refactors so that
it does, and inlines delete_flow__() into that caller for further
simplification.

Also remove a parameter from delete_flows__() that wasn't really needed
and change its return type to void since it can't ever fail.

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

ofproto: Shrink struct rule_actions slightly.

Nothing actually used 'provider_meter_id', but some code did want to know
whether there was a meter, so we can save a little space by using a bool
instead.  An upcoming commit will add a second bool member.  This change
allows that bool to be added without using extra space.

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

ofproto: Additional simplifications.

This commit finishes the removal of asynchronous flow table operations
begun in the previous commit, by removing ofoperation and ofopgroup
entirely and all of the code that depended on them.  Following this commit,
all the internal documentation and comments should again be consistent and
correct.

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

ofproto: Do straightforward removal of asynchronous flow operations.

Open vSwitch has supported datapaths that cannot update their flow tables
synchronously for many versions.  In that time, I have talked to many
hardware implementers.  None of them has ever mentioned the asynchronous
interface.  Furthermore, the only public hardware implementation of an Open
vSwitch datapath (from Centec), does not use the asynchronous interface.

At the same time, the asynchronous interface makes ofproto hard to read and
hard to understand.  It also makes it hard to maintain and extend.  An
extension in an upcoming commit would be very difficult to implement
asynchronously.

Therefore, this commit begins to remove the asynchronous interface.  This
initial commit does only the most straightforward parts of the removal, the
ones that do not significantly change the structure of the code.  For
example, this commit does not remove the ofoperation or ofopgroup data
structures at the core of the asynchronous interface, but instead reduces
them to a vestigial form: where previously an ofoperation might span
multiple trips through the main loop (if the operation were truly
asynchronous), now it always completes immediately.

The following commit will do more structural changes.  It will also update
all the comments, which are mostly left alone here.

The hope is that this structuring of the asynchronous removal into two
stages will make it easier to understand and review.  If not, the commits
could be squashed.

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

ofproto: Combine "struct ofconn *" and "const struct ofp_header *" args.

It's nicer to pass around a single argument than a pair of them.  This
will also make it easier to change the 'const struct ofp_header *request'
parameters to 'ovs_be32 xid' ones in an upcoming commit.

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

ofproto: Move logic for collecting read-only rules into rule_criteria.

ofproto supports OpenFlow tables that the controller can view but not
modify.  Until now, the logic for this was separately implemented in each
flow table operation, which meant that it was somewhat error-prone and in
fact seems missing entirely for "delete" operations.  This commit
implements the "modify" and "delete" side of that logic in the common code
for collecting a group of rules to work on, thereby slightly simplifying
the somewhat tricky modify_flows__() function and fixing the bug in
deletion.

There isn't a good way to reuse this logic for "add" operations.

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

ofproto: Make ofproto_rule_is_hidden() public, and rename.

I'd like to use this from connmgr.

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

ofproto: Only initiate flow table modifications if they will succeed.

In OpenFlow, a single "flow_mod" operation can change the actions (and
some other properties) of an arbitrary number of flows.  Until now,
Open vSwitch has assumed that any subset of these operations could
fail.  However, it has come out in discussion in the OpenFlow extensibility
working group that "partial failure" of a flow table operation is
undesirable, and furthermore that it should be possible to avoid it on
hardware implementations.  (The latter is the reason that Open vSwitch
permitted it to be with.)

This commit changes Open vSwitch to check whether all of a set of flow
table modifications will succeed before it initiates any of them.
This will not change visible behavior of the Open vSwitch software
switch, which never failed flow table modifications anyway.  It might
change behavior of some hardware implementation, but I don't actually know
of any.

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

ofproto: Add missing lock annotations on prototypes.

Clang only does lock annotation checks based on annotations that are
already visible, so for best results they need to be on prototypes as
well as definitions.

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

ofproto: Merge do_add_flow() and oftable_insert_rule() into add_flow().

Each of these functions had only a single caller, and breaking them out as
separate functions didn't seem to many the code clearer.

I added a new function meter_insert_rule(): oftable_insert_rule() had used
members of struct meter directly, which worked because it was after the
definition of struct meter.  So there was a choice to either move the
definition of struct meter earlier or add a helper function; I chose the
latter.

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

OPENFLOW-1.1+: EXT-187/OF1.4 Flow Monitors Are Under Development

I am unsure if it is appropriate to annotate OPENFLOW-1.1+ in this way
but I wish to throw my hat into the ring for work on EXT-187/OF1.4
flow monitors.

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

test-jsonrpc: Add the ability to detach on Windows.

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

connmgr: Do not use OFPRR_METER_DELETE before OF1.4

OFPRR_METER_DELETE was introduced in OF1.4 however meters were introduced
in OF1.3.

Regardless of the OF version when flows are deleted cause flows to be
deleted handle_delete_meter() calls delete_flows__() with
OFPRR_METER_DELETE as the reason.

In order to avoid sending OFPRR_METER_DELETE to controllers connected
using OF1.3 map OFPRR_METER_DELETE to OFPRR_DELETE which exists in that
version.

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

ofproto-dpif: Add idle_timeout parameter to ofproto_dpif_add_internal_flow()

This is in preparation for using the same helper as part of support
for using recirculation in conjunction series of actions including
with MPLS actions that are currently not able to be translated.

In that scenario the idle timeout will be used to expire internal
rules that are added to handle recirculation.

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

ofproto-dpif: Add recirc_id field to struct rule_dpif

This is to allow a recirculation id to be associated with a rule
in the case that its actions cause recirculation.

In such a case if the recirc_id field is non-zero then that value should be
used, otherwise a value should be obtained using
ofproto_dpif_alloc_recirc_id and saved in recirc_id field.

When destructing the rule if the recirc_id field is non-zero then
the associated internal flow should be deleted.

This is in preparation for using the same helper as part of support
for using recirculation in conjunction series of actions including
with MPLS actions that are currently not able to be translated.

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

ofproto-dpif: Move RECIRC_RULE_PRIORITY to common header

This is in preparation for using this value
in ofproto-dpif-xlate.c when composing recirculation
actions added as a result of processing (MPLS) actions.

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

socket-util: Disable dscp setting on Windows.

According to msdn documentation, one is discouraged from using
IP_TOS for ipv4 sockets (it apparently does not actually set
anything). Also, IPV6_TCLASS does not work in
Windows (it always returns an error and also is undocumented).
Looks like Microsoft recommends QoS2 APIs to achieve
the same. Till we add those API calls, simply return on Windows.

(Noticed while running unit tests for ipv6. set_dscp would fail.)

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

lib/cmap: Simplify iteration with C99 loop declaration.

This further eases porting existing hmap code to use cmap instead.

The iterator variants taking an explicit cursor are retained (renamed)
as they are needed when iteration is to be continued from the last
iterated node.

Signed-off-by: Jarno Rajahalme <jrajahalme@nicira.com>

lib/classifier: Clean up includes.

Remove unnecessary includes from lib/classifier.h and add them to
lib/classifier.c as needed.

Signed-off-by: Jarno Rajahalme <jrajahalme@nicira.com>

lib/classifier: Fix incorrect pointer type.

This bug did not manifest due to 'hmap_node' being in the same offset
in both struct cls_partition and struct cls_subtable.

Signed-off-by: Jarno Rajahalme <jrajahalme@nicira.com>

OPENFLOW-1.1+: OFPRR_METER_DELETE is used

My reading of handle_delete_meter() is that OFPRR_METER_DELETE is used.
Accordingly delete the entry relating to it from OPENFLOW-1.1+.

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

ofproto: Use OFPRR_GROUP_DELETE

Use OFPRR_GROUP_DELETE as the reason for deleting flows due
to the removal of a group that they use.

This implementation adds an delete_reason member to struct ofputil_flow_mod
as a convenient way to set the reason used by delete_flows__() when it is
called indirectly from delete_group__().

Signed-off-by: Simon Horman <horms@verge.net.au>
[blp@nicira.com initialized the new member in a few more places]
Signed-off-by: Ben Pfaff <blp@nicira.com>

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>

test: Remove explicit sleeps from ofproto-dpif bond tests

Ofproto-dpif bond tests relies on netdev-dummy ports to set up
socket connection before actual tests. The time required
for socket connection varies depends on system load, making the test
prone to failure with simple sleep calls. On the other hand,
conservative sleep value for the slowest machine may be excessive on
a faster machine.

This patch removes the sleep calls. Replace them with
WAIT_FOR_DUMMY_PORTS() introduced in the last patch, thus removing
the timing dependency.

CC: Jarno Rajahalme <jrajahalme@nicira.com>
Signed-off-by: Andy Zhou <azhou@nicira.com>
Acked-by: Jarno Rajahalme <jrajahalme@nicira.com>

test: add WAIT_FOR_DUMMY_PORTS helper macro for writing tests

Add a macro to waiting until all ports supplied are connected.

CC: Jarno Rajahalme <jrajahalme@nicira.com>
Signed-off-by: Andy Zhou <azhou@nicira.com>
Acked-by: Jarno Rajahalme <jrajahalme@nicira.com>

netdev-dummy: add appctl netdev-dummy/conn-state command

Using without any parameter, this command list the connection
state of all netdev-dummy devices that are configured to make
active connections.

Optionally, the name of the netdev-dummy device can be supplied
as a parameter.

The states will be displayed as:

"connected": The socket has been connected to the listener.
"disconnected": The socket is not connected.
"unknown":  It is not an active dummy device.

CC: Jarno Rajahalme <jrajahalme@nicira.com>
Signed-off-by: Andy Zhou <azhou@nicira.com>
Acked-by: Jarno Rajahalme <jrajahalme@nicira.com>

INSTALL.DPDK: User space dpdk setup documentation addition.

Added details of dpdk poll mode setup to make it easier
for someone, not familiar, to get it operating.

Signed-off-by: Mike A. Polehn <mike.a.polehn@intel.com>
Signed-off-by: Pravin B Shelar <pshelar@nicira.com>

.gitignore: Update with couple of Windows specific creations.

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

daemon.at: Make changes for Windows.

Skip some of the tests that uses '--monitor' as an option as
it is not implemented on Windows.

When a 'kill pid' is done on windows (through 'taskkill //F'),
pidfiles are not deleted (because it is force kill), so use
'ovs-appctl exit' instead.

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

cmap: Rename a enum constant.

The constant MAX_PATH is already defined in Windows.

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

testsuite.at: pids can have zero after first character.

Fix the bug which did not kill the processes with pids that had
a zero in them. Also, some tests do a AT_CHECK([kill `cat pid`]) which
on windows prints something on the stdout causing the tests to fail.
So supress it.

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

test-flows: Change the way flows are read.

With Visual Studio and Msys combination, something is going
wrong when we do a '3<pcap' followed by fdopen(3, 'rb'). fdopen
fails. I do not know the exact reason for the failure. But the
workaround is straightforward.

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

acinclude.m4: dpdk: link with -ldl if necessary

On some systems libintel_dpdk.a fails to link with libopenvswitch
unless -ldl is used. This should address the issue

Signed-off-by: Daniele Di Proietto <ddiproietto@vmware.com>
Signed-off-by: Ben Pfaff <blp@nicira.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: Destroy rule in ofproto_get_vlan_usage().

Found by inspection.

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

ofp-actions: Store cookie in network byte order in struct ofpact_learn.

Most other code in Open vSwitch that works with flow cookies keeps them
in network byte order.  Using network byte order in struct ofpact_learn,
also, reduces the number of byte order conversions needed across the
source tree.

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

ovs-ofctl: Support "send_flow_rem" in "learn" actions.

This flag was overlooked when support for the "learn" action was added.
(It was always supported in the OpenFlow code, just not in the ovs-ofctl
interface.)

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

ovs-ofctl: Document 'cookie' argument to learn action.

This has always been supported, but the documentation was missing.

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

ofproto-dpif: Remove unused struct dpif_completion.

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

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>

revalidator: Replace ukey->mark with dump_seq.

Rather than setting and resetting the 'mark' field in the ukey, this
patch introduces a seq to track whether a flow has been seen during the
most recent dump. This tidies the code and simplifies the logic for
detecting when flows are duplicated from the datapath.

Signed-off-by: Joe Stringer <joestringer@nicira.com>
Acked-by: Ethan Jackson <ethan@nicira.com>

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>

CONTRIBUTING: Describe commonly used tags; introduce Reported-at.

This is partly documentation of how patches are tagged in practice in Open
vSwitch.  The bits at the end about "Reported-at:" and "VMware-BZ:" are
an attempt to influence future practices; I cannot say how successful they
will be.

I am not sure whether these key-value pairs at the end of commit messages
are actually commonly called "tags".  I'm happy to use a different term if
that one seems wrong.

Reported-by: Flavio Leitner <fbl@redhat.com>
Reported-at: http://openvswitch.org/pipermail/dev/2014-June/040952.html
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Flavio Leitner <fbl@redhat.com>
Acked-by: YAMAMOTO Takashi <yamamoto@valinux.co.jp>

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>

tests/ofproto-dpif.at: Add delay before stopping time.

Add 2 second sleep before time stop to allow internal ports to
reconnect in case the initial connection attempt failed.

Bond tests sometimes persistently fail without these delays.

Signed-off-by: Jarno Rajahalme <jrajahalme@nicira.com>
Acked-by: Andy Zhou <azhou@nicira.com>

revalidator: Use xcache when revalidation is required.

One of the reasons that xlate_cache was introduced was to ensure that
statistics were attributed to the correct rules and interfaces according
to the flow that was installed into the datapath, rather than according
to the current state of the flow table.

This patch makes the revalidators use the xlate_cache to attribute stats
when full revalidation is required, as the statistics belong to the old
ruleset. However, when revalidating, the rules may have changed while
leaving the datapath flows intact, so we still re-create the xcache at
that point.

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

revalidator: Refactor ukey creation/lookup.

This patch refactors the code around ukey creation and lookup to
simplify the code for callers. A new function ukey_acquire() combines
these functions and attempts to acquire a lock on the ukey. Failure to
acquire a lock on the ukey is usually a sign that another thread is
handling the same flow concurrently, which means the flow does not need
to be handled anyway.

Signed-off-by: Joe Stringer <joestringer@nicira.com>
Acked-by: Ethan Jackson <ethan@nicira.com>

tests: Rename "ofproto-if"->"ofproto-dpif"

It's unclear why these tests are named "ofproto-if..." unlike all of the
other ofproto-dpif tests. Rename them to be more consistent.

Signed-off-by: Joe Stringer <joestringer@nicira.com>
Acked-by: Simon Horman <horms@verge.net.au>

dpif-netdev: Upcall: Remove an extra memcpy of packet data.

When a bridge of datatype type netdev receives a packet, it
copies the packet from the NIC to a buffer in userspace.
Currently, when making an upcall, the packet is again copied
to the upcall's buffer. However, this extra copy is not
necessary when the datapath exists in userspace as the upcall
can directly access the packet data.

This patch eliminates this extra copy of the packet data in
most cases. In cases where the packet may still be used later
by callers of dp_netdev_execute_actions, making a copy of the
packet data is still necessary.

This patch also adds a dpdk_buf field to 'struct ofpbuf' when
using DPDK. This field holds a pointer to the allocated DPDK
buffer in the rte_mempool. Thus, an upcall packet ofpbuf
allocated on the stack can now share data and free memory of
a rte_mempool allocated ofpbuf.

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

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>

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>

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>

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>

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>

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>

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>

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>

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>

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>

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

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>

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>

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>

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>

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>

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>

timeval: Import ctypes Python library within a try statement.

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>

timeval: Use monotonic time in OVS Python timeval library.

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>

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

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>

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>

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>

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>

ofproto-provider: Fix typo in comment.

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

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>

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>

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>

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>

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>

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>

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>

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>

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>