Remove duplicate call to subfacet_create().

There were two calls to subfacet_create(), which is redundant.  The first
one seemed the one to remove, as there is a subsequent test on the facet's
subfacet list being empty, which is never true after a call to
subfacet_create().  The correctness of this needs to be checked, however.

Signed-off-by: Jarno Rajahalme <jrajahalme@nicira.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
ofproto-dpif-upcall.c: Fix indentation.

Signed-off-by: Jarno Rajahalme <jrajahalme@nicira.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
ofproto-dpif: Fix thread safety annotation on rule_dpif_lookup_in_table().

New Clang versions raise warnings about the incorrect old annotation.

I first noticed these warnings with Clang 1:3.4~svn188890-1~exp1.
I previously used version 1:3.4~svn187484-1~exp1.

Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Jarno Rajahalme <jrajahalme@nicira.com>
ofproto-dpif-xlate: Refactor xlate_table_action() to avoid Clang warnings.

I get a bunch of thread-safety warnings with the latest Clang without this
patch, because Clang is smart enough to see locking and unlocking but not
smart enough to figure out the relationships.  This refactoring avoids the
warnings.

I first noticed these warnings with Clang 1:3.4~svn188890-1~exp1.
I previously used version 1:3.4~svn187484-1~exp1.

Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Jarno Rajahalme <jrajahalme@nicira.com>
meta-flow: Use correct OF1.2+ errors for invalid fields in actions.

OFPERR_OFPBAC_BAD_ARGUMENT is not as specific as the errors provided by
OpenFlow 1.2 and later.

Some of these errors needed Nicira extension numbers for use with OpenFlow
1.0 and 1.1, so this commit also adds those.

Some of these errors had poor explanations likely to confuse users, so this
commits improves them.

Some of the errors had the wrong names, so this commit fixes them.

Reported-by: Jean Tourrilhes <jt@hpl.hp.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Jean Tourrilhes <jt@hpl.hp.com>
Acked-by: Jarno Rajahalme <jrajahalme@nicira.com>
ofproto: remove the extra reset to make code clear to read.

Signed-off-by: pritesh <pritesh.kothari@cisco.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Jarno Rajahalme <jrajahalme@nicira.com>
ofproto: Change max_ports and alloc_port_no to uint16_t type.

The declaration of 'max_ports' and 'alloc_port_no' as ofp_port_t
adds unwanted complexity to coding.  This commit changes them back
to uint16_t type.

Signed-off-by: Alex Wang <alexw@nicira.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
dpif: Change get_max_ports() to return uint32_t.

The declaration of 'get_max_ports()' to return odp_port_t adds
unwanted complexity to coding.  This commit changes it back to
return uint32_t type.

Signed-off-by: Alex Wang <alexw@nicira.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
ovs-atomic: Add native Clang implementation.

With this implementation I get warnings with Clang on GNU/Linux when the
previous patch is not applied.  This ought to make it easier to avoid
introducing new problems in the future even without building on FreeBSD.

Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Jarno Rajahalme <jrajahalme@nicira.com>
ovs-save: Do not depend on hardcoded set of $PATH for utilities.

Reported-by: Jian Qiu <swordqiu@gmail.com>
Signed-off-by: Gurucharan Shetty <gshetty@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
ofproto-dpif.at: Remove push_vlan from an OF1.0 test.

Remove push_vlan from an OF1.0 test, as it requires OF1.1+ support, but was
silently discarded.  A later patch will make this test to fail due to
validation of usable OpenFlow protocol versions while parsing actions.

It should be noted that existing controllers may depend on the silently
discarded push_vlan being accepted during the action validation code at the
switch, as OpenFlow 1.0 supports setting a vlan ID, which will implicitly
push a vlan header, if it did not exist already.

Signed-off-by: Jarno Rajahalme <jrajahalme@nicira.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
datapath: optimize flow compare and mask functions

Make sure the sw_flow_key structure and valid mask boundaries are always
machine word aligned. Optimize the flow compare and mask operations
using machine word size operations. This patch improves throughput on
average by 15% when CPU is the bottleneck of forwarding packets.

This patch is inspired by ideas and code from a patch submitted by Peter
Klausler titled "replace memcmp() with specialized comparator".
However, The original patch only optimizes for architectures
support unaligned machine word access. This patch optimizes for all
architectures.

Signed-off-by: Andy Zhou <azhou@nicira.com>
Signed-off-by: Jesse Gross <jesse@nicira.com>
Fix typos in a couple of OpenFlow related comments.

Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Ethan Jackson <ethan@nicira.com>
classifier: New function cls_rule_move().

This function will acquire its first user in an upcoming commit.

Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Ethan Jackson <ethan@nicira.com>
ofproto-dpif-xlate: Fix fin_timeout to make rules expirable.

In an Open vSwitch flow table that has a configured maximum number of
flows, flows that have an idle or hard timeout, or both, are expirable,
and flows with neither are permanent.  The fin_timeout action can change
a flow that has no idle or hard timeout into one that has either one or
both, which should make a permanent flow into an expirable one, but the
implementation was buggy and did not actually make the flow expirable.
This commit fixes the problem.

This commit also moves most of the implementation of fin_timeout from
ofproto-dpif-xlate into ofproto, because this seems to better respect the
layering.

Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Ethan Jackson <ethan@nicira.com>
ofp-util: Abstract flow_mod OFPFF_* flags.

The OFPFF_* flags used in flow_mods are just confusing enough that it
seems worthwhile to try to abstract them out.  In particular:

    * OFPFF_EMERG was introduced in OF1.0, deleted in OF1.1, and then
      its bit was reused for a different purpose in OF1.2.

    * OFPFF_RESET_COUNTS was introduced in OF1.2 but the semantics that it
      specifies are implied by "add" commands in earlier versions, so
      proper translation requires the OpenFlow version number and flow_mod
      command.

This commit does the abstraction.

Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Ethan Jackson <ethan@nicira.com>
ofproto: Make OFPFC_ADD internally modify a rule instead of swapping.

add_flow() in ofproto.c has a race: it adds a new flow to the flow table
before calling ->rule_construct().  That means that (in ofproto-dpif) the
new flow is visible to the forwarder threads before gets properly
initialized.

One solution would be to lock the flow table across the entire operation,
but this is not desirable:

   * We would need a write-lock but this would be expensive for
     implementing "learn" flow_mods that often don't really modify anything
     at all.

   * The code would need to keep the lock across a couple of differen calls
     into the client, which seems undesirable if it can be avoided.

   * The code in add_flow() is difficult to understand already.

Instead, I've decided to refactor add_flow() to simplify it and make it
easier to understand.  I think this will also improve the potential for
concurrency.

This commit starts off by collapsing two different cases together into
(almost) one.  In particular, OpenFlow has two ways to modify a flow with a
"flow_mod" command.  You can use an "add" flow_mod to replace the flow, or
you can use a "modify" flow_mod to change it.  The differences in semantics
are minor, but until now Open vSwitch has treated them quite differently.
This commit merges both cases, treating them as variants of what was
previously a "modify".  The advantage is that add_flow() no longer has to
deal with two flows at a time in the "add" case (the old flow being
deleted, the new flow replacing that one).  This does not fix the race, but
it makes it easier to deal with in a later commit.

Transforming "add" into a form of "modify" requires that "modify" be able
to reset flow statistic counters.  OpenFlow 1.2 and later make this an
optional flag in a "flow_mod".  Until now, we haven't implemented that
feature of OF1.2+, but we get it pretty much for free with this
refactoring, so this commit also adds that OF1.2+ feature too.

Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Ethan Jackson <ethan@nicira.com>
datapath: add rounddown() definition into compatibility code

rounddown() was not available in older kenrel, such as kernel 2.6.32.61.
Add it to the compatibility code.

Signed-off-by: Andy Zhou <azhou@nicira.com>
Signed-off-by: Jesse Gross <jesse@nicira.com>
ofproto: Fully construct rules before putting them in the classifier.

add_flow() in ofproto.c has a race: it adds a new flow to the flow table
before calling ->rule_construct().  That means that (in ofproto-dpif) the
new flow is visible to the forwarder threads before gets properly
initialized.

One solution would be to lock the flow table across the entire operation,
but this is not desirable:

   * We would need a write-lock but this would be expensive for
     implementing "learn" flow_mods that often don't really modify anything
     at all.

   * The code would need to keep the lock across a couple of different calls
     into the client, which seems undesirable if it can be avoided.

   * The code in add_flow() is difficult to understand already.

Instead, I've decided to refactor add_flow() to simplify it and make it
easier to understand.  I think this will also improve the potential for
concurrency.

This commit completes the initial refactoring and solves the race.  It makes
two key changes:

    1. It disentangles insertion of a new flow from evicting some existing
       flow to make room for it (if necessary).  Instead, if inserting a
       new flow would make the flow table overfull, it evicts a flow and
       commits that operation before it attempts the insertion.  This is
       a user-visible change in behavior, in that previously a flow table
       insertion could never cause the number of flows in the flow table
       to decrease, and now it theoretically could if the eviction succeeds
       but the insertion fails.  However, I do not think that this is a
       serious problem.

    2. It adds two new steps to the life cycle of a rule.  Previously,
       rules had "alloc", "construct", "destruct", and "dealloc" steps,
       like other ofproto objects do.  This adds "insert" and "delete"
       steps between "construct" and "destruct".  The new steps are
       intended to handle the actual insertion and deletion into the
       datapath flow table.

Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Ethan Jackson <ethan@nicira.com>
netdev: Fix deadlock when netdev_dump_queues() callback calls into netdev.

We have a call chain like this:

    iface_configure_qos() calls
        netdev_dump_queues(), which calls
            netdev_linux_dump_queues(), which calls back through 'cb' to
                qos_unixctl_show_cb(), which calls
                    netdev_delete_queue(), which calls
                        netdev_linux_delete_queue().

Both netdev_dump_queues() and netdev_linux_delete_queue() take the same
mutex in the same netdev, which deadlocks.

This commit fixes the problem by getting rid of the callback.

netdev_linux_dump_queue_stats() would benefit from the same treatment but
it's less urgent because I don't see any callbacks from that function that
call back into a netdev function.

Bug #19319.
Reported-by: Scott Hendricks <shendricks@vmware.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>