daemon-windows: Recognize --no-chdir option for windows.

The option won't have any effect on the running of the daemon.
Recognizing the option lets us avoid if else conditions in unit
tests.

Signed-off-by: Gurucharan Shetty <gshetty@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
run-ryu: Use unix socket rather than patch ports

My understanding of the implementation of patch ports is that they
are rather special, being handled as a special case inside
compose_output_action__() and do not exist in the datapath.

A side effect of the implementation of patch ports (though perhaps not the
portion mentioned above) is that the OFPUTIL_PC_PORT_DOWN bit may not be
set via a port mod message. In particular, the call to
netdev_turn_flags_on() in update_port_config() fails.

There is a test provided by Ryu that test this via port mod and thus fails.

While that test could be modified or the results ignored it seems to me
that it would be best if ryu-check used ports which were more fully
featured and not special cases.

Thus this patch moves run-ryu to use unix socket backed ports rather than
patch ports.

I believe a more significant problem with the use of patch ports
is that they will require (more) special case code in order to correctly
handle recirculation. As Ryu provides many tests that exercise
recirculation for MPLS it would be nice if they could be used to exercise
recirculation for MPLS (which I have provided patches for separately[1])
without the need to add more special-case code for that purpose.

I believe that patch ports are also incompatible with recirculation for
bonding, which has already been merged, though I have not verified that
and it is not strictly related to this patch as I do not believe that Ryu
provides any tests to exercise that case.

The key problem with patch ports in the context of recirculation is that
the ofproto and in_port may change during translation.  And this
information is lost by the time that execution occurs.

Furthermore the new in_port will not exist in the datapath as it is a
patch port. That particular problem may be addressed by executing the
actions in user-space, I have posted patches to provide infrastructure
for that[1].

Overall it is not clear to me that the complexity of supporting
recirculation for patch-ports would have sufficient pay-off.

[1] [PATCH v3 00/16] Flow-Based Recirculation for MPLS

Signed-off-by: Simon Horman <horms@verge.net.au>
Signed-off-by: Ben Pfaff <blp@nicira.com>
lib/util: Input validation in str_to_uint

This function returns true when 's' is negative or greater than UINT_MAX. Also,
the representation of 'int' and 'unsigned int' is implementation dependent, so
converting [INT_MAX..UINT_MAX] values with str_to_int is fragile.
Instead, we should convert straight to 'long long' and do a boundary check
before returning the converted value.
This patch also move the function to the .c file as it's not-trivial now, and
deletes the other str_to_u* functions as they are not used.

Signed-off-by: Zoltan Kiss <zoltan.kiss@citrix.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
ofproto-dpif: Improve code clarity and comments on recirc changes to rule_dpif_lookup()

This patch improves the code readability and comments on the
recirculation related changes to rule_dpif_lookup() base on off-line
discussions with Jarno.  There is no behavior changes.

Signed-off-by: Andy Zhou <azhou@nicira.com>
Acked-by: Ethan Jackson <ethan@nicira.com>
revalidator: Prevent handling the same flow twice.

When the datapath flow table is modified while a flow dump operation is
in progress, it is possible for the same flow to be dumped twice. In
such cases, revalidators may perform redundant work, or attempt to
delete the same flow twice.

This was causing intermittent testsuite failures for test #670 -
"ofproto-dpif, active-backup bonding" where a flow (that had not
previously been dumped) was dumped, revalidated and deleted twice.

The logs show errors 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 adds a 'flow_exists' field to 'struct udpif_key' to track
whether the flow is (in progress) to be deleted. After doing a ukey
lookup, we check whether ukey->mark or ukey->flow indicates that the
flow has already been handled. If it has already been handled, we skip
handling the flow again.

We also defer ukey cleanup for flows that fail revalidation, so that the
ukey will still exist if the same flow is dumped twice. This allows the
above logic to work in this case.

Signed-off-by: Joe Stringer <joestringer@nicira.com>
Acked-by: Alex Wang <alexw@nicira.com>
bridge: When ports disappear from a datapath, add them back.

Before commit 2a73b1d73d4bdb (bridge: Reconfigure in single pass.), if a
port disappeared, for one reason or another, from a datapath, the next
bridge reconfiguration pass would notice and, if the port was still
configured in the database, add the port back to the datapath.  That
commit, however, removed the logic from bridge_refresh_ofp_port() that
did that and failed to add the same logic to the replacement function
bridge_delete_or_reconfigure_ports().  This commit fixes the problem.

To see this problem on a Linux kernel system:

ovs-vsctl add-br br0                             # 1
tunctl -t tap                                    # 2
ovs-vsctl add-port br0 tap                       # 3
ovs-dpctl show                                   # 4
tunctl -d tap                                    # 5
ovs-dpctl show                                   # 6
tunctl -t tap                                    # 7
ovs-vsctl del-port tap -- add-port br0 tap       # 8
ovs-dpctl show                                   # 9

Steps 1-4 create a bridge and a tap and add it to the bridge and
demonstrate that the tap is part of the datapath.  Step 5 and 6 delete
the tap and demonstrate that it has therefore disappeared from the
datapath.  Step 7 recreates a tap with the same name, and step 8
forces ovs-vswitchd to reconfigure.  Step 9 shows the effect of the
fix: without the fix, the new tap is not added back to the datapath;
with this fix, it is.

Special thanks to Gurucharan Shetty <gshetty@nicira.com> for finding a
simple reproduction case and then bisecting to find the commit that
introduced the problem.

Bug #1238467.
Reported-by: Ronald Lee <ronaldlee@vmware.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Ethan Jackson <ethan@nicira.com>
ofproto-dpif-upcall: Remove the flow_dumper thread.

Previously, we had a separate flow_dumper thread that fetched flows from
the datapath to distribute to revalidator threads. This patch takes the
logic for dumping and pushes it into the revalidator threads, resulting
in simpler code with similar performance to the current code.

One thread, the "leader", is responsible for beginning and ending each
flow dump, maintaining the flow_limit, and checking whether the
revalidator threads need to exit. All revalidator threads dump,
revalidate, delete datapath flows and garbage collect ukeys.

Co-authored-by: Joe Stringer <joestringer@nicira.com>
Signed-off-by: Joe Stringer <joestringer@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
hmap_random_node: Improve distribution

Improve random distribution for an hmap with a small number of nodes
with the expense of the increased cpu cost.
It would be a fair trade-off because the situation is rather common
for bond, which is currently the only consumer of this API in tree.

Consider 2 items, 4 buckets, no collision.

    bucket 0   item 0
    bucket 1
    bucket 2
    bucket 3   item 1

The old algorithm picks item 0 if rand % 4 == 0.  (25%)
Otherwise it picks item 1.  (75%)

This change makes them 50%.

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

This allows rules to be used without taking references while RCU
protected.

The last step of destroying an ofproto also needs to be postponed, as
the rule destruction requires the class structure to be available at
the postponed destruction callback.

Signed-off-by: Jarno Rajahalme <jrajahalme@nicira.com>
Acked-by: Ethan Jackson <ethan@nicira.com>
ofproto: Make taking rule reference conditional on lookup.

Prior to this paths the rule lookup functions have always taken a
reference on the found rule before returning.  Make this conditional,
so that unnecessary refs/unrefs can be avoided in a later patch.

Signed-off-by: Jarno Rajahalme <jrajahalme@nicira.com>
Acked-by: Ethan Jackson <ethan@nicira.com>
ofproto: Reduce taking rule references.

Only take reference to a looked up rule when needed.

This reduces the total CPU utilization of rule_ref/unref calls by 80%,
from 5% of total server CPU capacity to 1% in a netperf TCP_CRR
test stressing the userspace.

Signed-off-by: Jarno Rajahalme <jrajahalme@nicira.com>
Acked-by: Ethan Jackson <ethan@nicira.com>
ofproto-dpif-xlate: Identify STP BPDUs more specifically.

Apart from STP, EVB extension of LLDP as well as IEEE 802.1QBG use the
Nearest Customer Bridge (NCB) DMAC which has a value of 0180.c200.0000.
STP can be distinguished by Ethertype from these protocols.

Signed-off-by: Padmanabhan Krishnan <kprad1@yahoo.com>
[blp@nicira.com rewrote the details of the patch]
Signed-off-by: Ben Pfaff <blp@nicira.com>
Tested-by: Padmanabhan Krishnan <kprad1@yahoo.com>
daemon: Rename daemon.c as daemon-unix.c

An upcoming commit re-introduces daemon.c to have
common functions across daemon-unix.c and daemon-windows.c

Signed-off-by: Gurucharan Shetty <gshetty@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
daemon: Close standard file descriptors after detach for windows.

In the unit tests, we check for some logs stored in stderr. In case
of windows, unit tests fail because the child writes additional information
into stderr because it does not have it closed. This commit
closes standard file descriptors for windows too.

Because the functions related to closing file descriptors is common
for both windows and unix, add it to the common daemonization file
daemon.c

Signed-off-by: Gurucharan Shetty <gshetty@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
daemon: Move some common code to daemon.c

We have some common code between daemon-unix.c and
daemon-windows.c. Move them to daemon.c

Signed-off-by: Gurucharan Shetty <gshetty@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
bfd/cfm: Check status change before update status to database.

This commit adds boolean flag in bfd/cfm module for checking
status change.  If there is no status change, the current
update to OVS database will skip the bfd/cfm session.

In the experiment with 5K bfd sessions, when one session is
flapping at rate of every 0.3 second, this patch reduces the
cpu utilization of the ovs-vswitchd thread from 13 to 6.

Signed-off-by: Alex Wang <alexw@nicira.com>
Acked-by: Joe Stringer <joestringer@nicira.com>
FAQ: Explain what to do when building against a too-new kernel.

Also add references to this FAQ from INSTALL and configure.

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

This code path currently does not initialize
netdev_class_mutex.
dummy_enable
 ->netdev_dummy_register
   ->netdev_register_provider
     ->ovs_mutex_lock(&netdev_class_mutex)

ovsdb-server on windows crashes without it.

This commit adds a new initialization function.

Signed-off-by: Gurucharan Shetty <gshetty@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
ofproto: Don't destroy mutex before its use.

Currently, we are calling guarded_list_destroy()
to destroy a mutex and then go ahead and use it through
delete_group
 ->delete_group__
   ->handle_flow_mod__
    ->run_rule_executes
     ->guarded_list_pop_all

The group related unit tests cause ovs-vswitchd to crash
because of this (on windows).

Calling guarded_list_destroy() after delete_group() solves the
problem.

Signed-off-by: Gurucharan Shetty <gshetty@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
ofproto-dpif-upcall: Fix a bug.

Commit 7d170098 (ofproto-dpif-upcall: Remove the flow_dumper thread.)
initialized the memory barrier inside the udpif_start_threads() function.
However, the udpif_start_threads() function does not check the number of
revalidator threads specified in udpif.  So, when the number is zero, it
causes the error in barrier initialization.  This could happen when the
other_config:flow-restore-wait is set and the udpif_flush() is called.

This commit fixes the issue, by checking the specified number of threads
in udpif_start_threads().

Reported-by: Gurucharan Shetty <gshetty@nicira.com>
Signed-off-by: Alex Wang <alexw@nicira.com>
Acked-by: Gurucharan Shetty <gshetty@nicira.com>