cascardo/ovs.git
10 years agoofproto: Refactor eviction cases to use common code.
Ben Pfaff [Fri, 13 Sep 2013 03:30:56 +0000 (20:30 -0700)]
ofproto: Refactor eviction cases to use common code.

Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Ethan Jackson <ethan@nicira.com>
10 years agoofproto: New helper any_pending_ops().
Ben Pfaff [Wed, 11 Sep 2013 04:44:53 +0000 (21:44 -0700)]
ofproto: New helper any_pending_ops().

This function is trivial now but in an upcoming commit it will need to
become slightly more complicated, which makes writing it as a function
worthwhile.

Until then, this commit simplifies the logic, which was redundant since
the 'deletions' hmap always points into the 'pending' list anyway.

Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Ethan Jackson <ethan@nicira.com>
10 years agoofproto: Mark immutable members of struct rule 'const'.
Ben Pfaff [Wed, 11 Sep 2013 04:31:59 +0000 (21:31 -0700)]
ofproto: Mark immutable members of struct rule 'const'.

One difficult part of make flow_mods thread-safe is sorting out which
members of each structure can be modified under which locks and,
especially, documenting this in a way that makes it hard for programmers to
get it wrong later.  The compiler provides some tools for us, however, and
'const' is one of the nicer ones since it is standardized rather than part
of a compiler extension.

This commit makes use of 'const' to mark the immutable members of struct
rule, which is definitely the most confusing structure regarding thread
safety simply because it has so many members that use different forms of
synchronization.  It also adds a bunch of CONST_CASTs to allow these
members to be initialized and destroyed where necessary.

Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Ethan Jackson <ethan@nicira.com>
10 years agoofproto: Make some functions for rules private to ofproto.c.
Ben Pfaff [Wed, 11 Sep 2013 04:01:34 +0000 (21:01 -0700)]
ofproto: Make some functions for rules private to ofproto.c.

These aren't used outside this file.

Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Ethan Jackson <ethan@nicira.com>
10 years agoofproto: Remove ->timeout_mutex from struct rule (just use ->mutex).
Ben Pfaff [Wed, 11 Sep 2013 04:18:09 +0000 (21:18 -0700)]
ofproto: Remove ->timeout_mutex from struct rule (just use ->mutex).

I think that ->timeout_mutex and ->mutex were separate because the latter
(which was actually a rwlock named ->rwlock until recently) was held for
long periods of time, which meant that having a separate ->timeout_mutex
reduced lock contention.  But ->mutex is now only held briefly, so it seems
reasonable to just use it.

Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Ethan Jackson <ethan@nicira.com>
10 years agoofproto: Replace rwlock in struct rule by a mutex.
Ben Pfaff [Thu, 12 Sep 2013 07:28:49 +0000 (00:28 -0700)]
ofproto: Replace rwlock in struct rule by a mutex.

A rwlock is suitable when one expects long hold times so there is a need
for some parallelism for readers.  But when the lock is held briefly, it
makes more sense to use a mutex for two reasons.  First, a rwlock is more
complex than a mutex so one would expect it to be more expensive to
acquire.  Second, a rwlock is less fair than a mutex: as long as there are
any readers this blocks out writers.

At least, that's the behavior I intuitively expect, and a few looks around
the web suggest that I'm not the only one.

Previously, struct rule's rwlock was held for long periods, so using a
rwlock made sense.  Now it is held only briefly, so this commit replaces it
by a mutex.

Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Ethan Jackson <ethan@nicira.com>
10 years agoofproto: Protect index by cookie with ofproto_mutex.
Ben Pfaff [Mon, 9 Sep 2013 21:26:23 +0000 (14:26 -0700)]
ofproto: Protect index by cookie with ofproto_mutex.

The cookie index is only used for flow_mods, so it makes sense to use this
global mutex.

Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Ethan Jackson <ethan@nicira.com>
10 years agoofproto: Drop 'expirable_mutex' in favor of new global 'ofproto_mutex'.
Ben Pfaff [Thu, 12 Sep 2013 07:31:33 +0000 (00:31 -0700)]
ofproto: Drop 'expirable_mutex' in favor of new global 'ofproto_mutex'.

This is the first step toward making a global lock that protects everything
needed for updating the flow table.  This commit starts out by merging one
lock into the new one, and later commits will continue that process.

The mutex is initially a recursive one, because I wasn't sure that there
were no nested acquisitions at this stage, but a later commit will change
it to a regular error-checking mutex once it's settled down a bit.

Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Ethan Jackson <ethan@nicira.com>
10 years agoofproto: Move rule_execute() out of ofopgroup_complete().
Ben Pfaff [Thu, 12 Sep 2013 07:25:28 +0000 (00:25 -0700)]
ofproto: Move rule_execute() out of ofopgroup_complete().

One goal we're moving toward is to be able to execute "learn" actions
in each thread that handles packets that arrive on an interface just before
we forward those packets.  The planned strategy there is to have a global
lock that protects everything required to modify the flow table.  Generally
this works out well, but rule_execute() is a trouble spot.  That's because
it's called during a flow table modification (when that global lock is
held) which means that a "learn" action triggered by the executing the
packet would try to recursively modify the flow table and reacquire the
global lock.

I can see two ways out of this issue.  One would be to make the global lock
a recursive one, or otherwise deal with handling recursive flow_mods.  The
other is to just queue up flow_mods due to rule_execute(), which itself is
a corner case (it only happens when a flow_mod sent via OpenFlow includes
a buffer_id).  (I guess there could be other more radical solutions, like
just dropping packets that contain "learn" actions if they occur in this
situation.)

This commit implements the second solution because it seems less likely to
be wrong in a way that crashes the switch.

Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Ethan Jackson <ethan@nicira.com>
10 years agoclassifier: Allow CLS_CURSOR_FOR_EACH to use a const-qualified iterator.
Ben Pfaff [Tue, 10 Sep 2013 05:13:09 +0000 (22:13 -0700)]
classifier: Allow CLS_CURSOR_FOR_EACH to use a const-qualified iterator.

Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Ethan Jackson <ethan@nicira.com>
10 years agoguarded-list: New data structure for thread-safe queue.
Ben Pfaff [Fri, 13 Sep 2013 00:42:23 +0000 (17:42 -0700)]
guarded-list: New data structure for thread-safe queue.

We already had queues that were suitable for replacement by this data
structure, and I intend to add another one later on.

flow_miss_batch_ofproto_destroyed() did not work well with the guarded-list
structure (it required either adding a lot more functions or breaking the
abstraction) so I changed the caller to just use udpif_revalidate().

Checking reval_seq at the end of handle_miss_upcalls() also didn't work
well with the abstraction, so I decided that since this was a corner case
anyway it would be acceptable to just drop those in flow_miss_batch_next().

Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Ethan Jackson <ethan@nicira.com>
10 years agoofproto: Add a ref_count to "struct rule" to protect it from being freed.
Ben Pfaff [Thu, 12 Sep 2013 06:23:00 +0000 (23:23 -0700)]
ofproto: Add a ref_count to "struct rule" to protect it from being freed.

Taking a read-lock on the 'rwlock' member of struct rule prevents members
of the rule from changing.  This is a short-term use of the 'rwlock': one
takes the lock, reads some members, and releases the lock.

Taking a read-lock on the 'rwlock' also prevents the rule from being freed.
This is often a relatively long-term need.  For example, until now flow
translation has held the rwlock in xlate_table_action() across the entire
recursive translation, which can call into a great deal of different code
across multiple files.

This commit switches to using a reference count for this second purpose
of struct rule's rwlock.  This means that all the code that previously
held a read-lock on the rwlock across deep stacks of functions can now
simply keep a reference.

Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Ethan Jackson <ethan@nicira.com>
10 years agoofproto: Break actions out of rule into new rule_actions structure.
Ben Pfaff [Mon, 9 Sep 2013 20:05:52 +0000 (13:05 -0700)]
ofproto: Break actions out of rule into new rule_actions structure.

This permits code to ensure long-term access to a rule's actions
without holding a long-term lock on the rule's rwlock.

Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Ethan Jackson <ethan@nicira.com>
10 years agoofproto: Remove soon-to-be-invalid optimizations.
Ben Pfaff [Fri, 13 Sep 2013 04:06:46 +0000 (21:06 -0700)]
ofproto: Remove soon-to-be-invalid optimizations.

Until now, ofproto_add_flow() and ofproto_delete_flow() assumed that the
flow table could not change between its flow table check and its later
modification to the flow table.  This assumption will soon be untrue, so
remove it.

Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Ethan Jackson <ethan@nicira.com>
10 years agoofproto: Move function find_meter() into ofpacts as ofpacts_get_meter().
Ben Pfaff [Sat, 7 Sep 2013 02:52:14 +0000 (19:52 -0700)]
ofproto: Move function find_meter() into ofpacts as ofpacts_get_meter().

ofproto is too big anyway so we might as well move out code that can
reasonably live elsewhere.

Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Ethan Jackson <ethan@nicira.com>
10 years agoofproto: Merge ofproto_rule_delete() and ofproto_delete_rule().
Ben Pfaff [Fri, 30 Aug 2013 22:27:33 +0000 (15:27 -0700)]
ofproto: Merge ofproto_rule_delete() and ofproto_delete_rule().

These functions were identical but had different names (one just called
the other).  Make them a single function.

Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Ethan Jackson <ethan@nicira.com>
10 years agoofproto-dpif: Remove vestigial "clogged" feature.
Ben Pfaff [Sun, 1 Sep 2013 21:46:04 +0000 (14:46 -0700)]
ofproto-dpif: Remove vestigial "clogged" feature.

Before this commit, ofproto-dpif did not ordinarily ever defer
completion of a flow_mod, but it was possible to use "ovs-appctl
ofproto/clog" to force deferring completion, which allowed some basic
debugging of deferred completion.  I only ever used that feature once,
while doing initial debugging of the deferred completion feature.  I
intend in future commits to make changes that will make deferred
completion harder to implement in ofproto-dpif, so this commit removes
that feature in advance.

Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Ethan Jackson <ethan@nicira.com>
10 years agoofproto: Avoid gratuitous memory allocation and free.
Ben Pfaff [Sat, 7 Sep 2013 02:23:07 +0000 (19:23 -0700)]
ofproto: Avoid gratuitous memory allocation and free.

Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Ethan Jackson <ethan@nicira.com>
10 years agoofproto: Correct comments.
Ben Pfaff [Fri, 30 Aug 2013 22:20:08 +0000 (15:20 -0700)]
ofproto: Correct comments.

Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Ethan Jackson <ethan@nicira.com>
10 years agoofproto: Eliminate 'ofproto_node' member from struct rule.
Ben Pfaff [Wed, 11 Sep 2013 23:41:32 +0000 (16:41 -0700)]
ofproto: Eliminate 'ofproto_node' member from struct rule.

The ofproto_node member is convenient for collecting lists of rules, but
it is also challenging for concurrency because only a single thread at a
time can put a given rule on a list.  This commit eliminates the
ofproto_node member and introduces a new 'struct rule_collection' that
can be use in a thread-safe manner.

Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Ethan Jackson <ethan@nicira.com>
10 years agoofproto: Reduce number of "collect" functions taking lots of parameters.
Ben Pfaff [Wed, 11 Sep 2013 22:35:44 +0000 (15:35 -0700)]
ofproto: Reduce number of "collect" functions taking lots of parameters.

The long lists of parameters for all these "collect" functions was starting
to get to me.  This reduces the number of such functions to one.

Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Ethan Jackson <ethan@nicira.com>
10 years agoofproto: Factor code out of collect_rules_{loose,strict} into new helper.
Ben Pfaff [Fri, 6 Sep 2013 05:37:41 +0000 (22:37 -0700)]
ofproto: Factor code out of collect_rules_{loose,strict} into new helper.

Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Ethan Jackson <ethan@nicira.com>
10 years agoofproto: Use proper error code when meter_id is out of range.
Jarno Rajahalme [Sun, 8 Sep 2013 01:27:08 +0000 (18:27 -0700)]
ofproto: Use proper error code when meter_id is out of range.

Use OUT_OF_METERS when given meter_id is greater than what is supported
by the datapath.  Retain the INVALID_METER error code for the meter_ids
outside of the range supported by the specification.

Signed-off-by: Jarno Rajahalme <jrajahalme@nicira.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
10 years agoDelete meters in the right place.
Ben Pfaff [Fri, 13 Sep 2013 04:18:01 +0000 (21:18 -0700)]
Delete meters in the right place.

Signed-off-by: Jarno Rajahalme <jrajahalme@nicira.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
10 years agolist: New function list_move().
Ben Pfaff [Thu, 29 Aug 2013 20:06:21 +0000 (13:06 -0700)]
list: New function list_move().

An upcoming commit will add more users.

Signed-off-by: Ben Pfaff <blp@nicira.com>
Reviewed-by: Simon Horman <horms@verge.net.au>
10 years agoofproto: Avoid removing rules from meter lists twice.
Ben Pfaff [Thu, 29 Aug 2013 23:49:51 +0000 (16:49 -0700)]
ofproto: Avoid removing rules from meter lists twice.

Since the meter code was introduced, the oftable_remove_rule() function
(recently renamed oftable_remove_rule__()) has had two blocks of code to
remove a rule from its meter's list of rules that reference that meter.
This commit reduces that to one.

Also modifies oftable_remove_rule__() to maintain the invariant that
'rule' is in a meter's list if and only if
!list_is_empty(&rule->meter_list_node).  I don't believe that that fixes
a real bug, but it seems like a good idea.

(This seemed like a serious bug at first, but in fact list_remove() is
idempotent: calling it two times in a row has the same effect as calling
it once.)

Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Jarno Rajahalme <jrajahalme@nicira.com>
10 years agovswitchd: Clear bfd_status column when bfd is disabled or not supported
Alex Wang [Thu, 12 Sep 2013 19:51:53 +0000 (12:51 -0700)]
vswitchd: Clear bfd_status column when bfd is disabled or not supported

This commit makes vswitchd clear the 'bfd_status' column
in ovsdb when bfd is disabled or not supported.

Reported-by: Ansis Atteka <aatteka@nicira.com>
Signed-off-by: Alex Wang <alexw@nicira.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
10 years agobridge: Always call smap_destroy() after smap_init()
Alex Wang [Thu, 12 Sep 2013 18:54:16 +0000 (11:54 -0700)]
bridge: Always call smap_destroy() after smap_init()

This commit fixes a place in bridge.c where smap_destroy() is not
always called after smap_init().  Though there is no memory leak
now, it is necessary to fix it and prevent memory leak in the
future when smap_init() may be modified to allocate dynamic memory.

Reported-by: Ansis Atteka <aatteka@nicira.com>
Signed-off-by: Alex Wang <alexw@nicira.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
10 years agoofproto: Reduce default number of miss handlers.
Ethan Jackson [Tue, 10 Sep 2013 22:36:21 +0000 (15:36 -0700)]
ofproto: Reduce default number of miss handlers.

The default number of miss handlers should be the number of processors
minus two to account for the dispatcher and main threads.

Signed-off-by: Ethan Jackson <ethan@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
10 years agoofproto-dpif: Don't hold mac rwlock while calling send_packet().
Ethan Jackson [Fri, 6 Sep 2013 19:51:02 +0000 (12:51 -0700)]
ofproto-dpif: Don't hold mac rwlock while calling send_packet().

Holding the mac learning rwlock while calling send_packet() which
calls xlate_actions() is dangerous because somewhere deep in the chain
the same lock might be acquired causing a deadlock.  Though we haven't
run into this problem in testing, it seems prudent to refactor it.

Reported-by: Ben Pfaff <blp@nicira.com>
Signed-off-by: Ethan Jackson <ethan@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
10 years agoofproto: Fix core dump due to uninitialized cls_rule.
Ethan Jackson [Tue, 10 Sep 2013 20:29:30 +0000 (13:29 -0700)]
ofproto: Fix core dump due to uninitialized cls_rule.

Bug #19568.
Signed-off-by: Ethan Jackson <ethan@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
10 years agoofproto: Do not call ->rule_destruct() if ->rule_construct() failed.
Ben Pfaff [Fri, 30 Aug 2013 20:59:37 +0000 (13:59 -0700)]
ofproto: Do not call ->rule_destruct() if ->rule_construct() failed.

Found by inspection.

Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Ethan Jackson <ethan@nicira.com>
10 years agodatapath: flow: fix potential illegal memory access in __parse_flow_nlattrs
Daniel Borkmann [Mon, 9 Sep 2013 20:27:27 +0000 (13:27 -0700)]
datapath: flow: fix potential illegal memory access in __parse_flow_nlattrs

In function __parse_flow_nlattrs(), we check for condition
(type > OVS_KEY_ATTR_MAX) and if true, print an error, but we do
not return from this function as in other checks. It seems this
has been forgotten, as otherwise, we could access beyond the
memory of ovs_key_lens, which is of ovs_key_lens[OVS_KEY_ATTR_MAX + 1].
Hence, a maliciously prepared nla_type from user space could access
beyond this upper limit.

Introduced by 03f0d916a ("openvswitch: Mega flow implementation").

Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
Cc: Andy Zhou <azhou@nicira.com>
Signed-off-by: Jesse Gross <jesse@nicira.com>
10 years agoutil: Don't set thread name to empty
Guolin Yang [Mon, 9 Sep 2013 16:38:01 +0000 (09:38 -0700)]
util: Don't set thread name to empty

In monitor_daemon(), it set subprogram_name to "" which causes system crash
in some platform when trying to set the thread name to "".  This change set
thread name to program name in this case.

Signed-off-by: Guolin Yang <gyang@nicira.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
10 years agodatapath: Remove compat files.
Pravin B Shelar [Mon, 26 Aug 2013 18:19:10 +0000 (11:19 -0700)]
datapath: Remove compat files.

Signed-off-by: Pravin B Shelar <pshelar@nicira.com>
Acked-by: Jesse Gross <jesse@nicira.com>
10 years agodatapath: Remove reciprocal_div compat code.
Pravin B Shelar [Fri, 30 Aug 2013 17:42:53 +0000 (10:42 -0700)]
datapath: Remove reciprocal_div compat code.

Signed-off-by: Pravin B Shelar <pshelar@nicira.com>
Acked-by: Jesse Gross <jesse@nicira.com>
10 years agodatapath: Remove compat support for NLA_NUL_STRING
Pravin B Shelar [Wed, 28 Aug 2013 18:24:00 +0000 (11:24 -0700)]
datapath: Remove compat support for NLA_NUL_STRING

Signed-off-by: Pravin B Shelar <pshelar@nicira.com>
Acked-by: Jesse Gross <jesse@nicira.com>
10 years agodatapath: Cleanup compat support.
Pravin B Shelar [Tue, 27 Aug 2013 06:43:29 +0000 (23:43 -0700)]
datapath: Cleanup compat support.

cleanup various header file.

Signed-off-by: Pravin B Shelar <pshelar@nicira.com>
Acked-by: Jesse Gross <jesse@nicira.com>
10 years agodatapath: Cleanup netlink compat code.
Pravin B Shelar [Tue, 27 Aug 2013 06:53:17 +0000 (23:53 -0700)]
datapath: Cleanup netlink compat code.

Patch removes genl, netlink, rtnl compat code and dpif-linux
fallback-id compat code.

Signed-off-by: Pravin B Shelar <pshelar@nicira.com>
Acked-by: Jesse Gross <jesse@nicira.com>
10 years agodatapath: Remove vlan compat support
Pravin B Shelar [Wed, 4 Sep 2013 18:35:13 +0000 (11:35 -0700)]
datapath: Remove vlan compat support

Signed-off-by: Pravin B Shelar <pshelar@nicira.com>
Acked-by: Jesse Gross <jesse@nicira.com>
10 years agodatapath: Remove checksum compat support
Pravin B Shelar [Mon, 26 Aug 2013 18:18:07 +0000 (11:18 -0700)]
datapath: Remove checksum compat support

Signed-off-by: Pravin B Shelar <pshelar@nicira.com>
Acked-by: Jesse Gross <jesse@nicira.com>
10 years agodatapath: Remove skb->mark compat code.
Pravin B Shelar [Mon, 26 Aug 2013 20:27:02 +0000 (13:27 -0700)]
datapath: Remove skb->mark compat code.

Signed-off-by: Pravin B Shelar <pshelar@nicira.com>
Acked-by: Jesse Gross <jesse@nicira.com>
10 years agodatapath: Remove namespace compat support.
Pravin B Shelar [Mon, 26 Aug 2013 18:30:41 +0000 (11:30 -0700)]
datapath: Remove namespace compat support.

Signed-off-by: Pravin B Shelar <pshelar@nicira.com>
Acked-by: Jesse Gross <jesse@nicira.com>
10 years agodatapath: vport: Remove compat support
Pravin B Shelar [Tue, 27 Aug 2013 20:24:50 +0000 (13:24 -0700)]
datapath: vport: Remove compat support

Signed-off-by: Pravin B Shelar <pshelar@nicira.com>
Acked-by: Jesse Gross <jesse@nicira.com>
10 years agodatapath: Drop support for RHEL5 build
Pravin B Shelar [Tue, 3 Sep 2013 17:55:18 +0000 (10:55 -0700)]
datapath: Drop support for RHEL5 build

RHEL5 is based on kernel 2.6.18.

Signed-off-by: Pravin B Shelar <pshelar@nicira.com>
Acked-by: Jesse Gross <jesse@nicira.com>
10 years agodatapath: Move kernel version check to configure.
Pravin B Shelar [Tue, 3 Sep 2013 18:02:40 +0000 (11:02 -0700)]
datapath: Move kernel version check to configure.

Rather than having compile time check in datapath.c, its better
to check kernel version at configuration step.

Signed-off-by: Pravin B Shelar <pshelar@nicira.com>
Acked-by: Jesse Gross <jesse@nicira.com>
10 years agodatapath: Drop support for linux pre-2.6.32 kernel.
Pravin B Shelar [Fri, 6 Sep 2013 16:14:40 +0000 (09:14 -0700)]
datapath: Drop support for linux pre-2.6.32 kernel.

This makes datapath module much close to upstream datapath and
make code easy to understand.

Signed-off-by: Pravin B Shelar <pshelar@nicira.com>
Acked-by: Jesse Gross <jesse@nicira.com>
10 years agodatapath: Fix alignment of struct sw_flow_key.
Jesse Gross [Thu, 5 Sep 2013 20:01:58 +0000 (13:01 -0700)]
datapath: Fix alignment of struct sw_flow_key.

sw_flow_key alignment was declared as " __aligned(__alignof__(long))".
However, this breaks on the m68k architecture where long is 32 bit in
size but 16 bit aligned by default. This aligns to the size of a long to
ensure that we can always do comparsions in full long-sized chunks. It
also adds an additional build check to catch any reduction in alignment.

CC: Andy Zhou <azhou@nicira.com>
Reported-by: Fengguang Wu <fengguang.wu@intel.com>
Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
Signed-off-by: Jesse Gross <jesse@nicira.com>
10 years agoovs-ofctl: Correct formatting of instructions in manpage.
Simon Horman [Mon, 2 Sep 2013 02:23:16 +0000 (11:23 +0900)]
ovs-ofctl: Correct formatting of instructions in manpage.

Adjust formatting in ovs-ofctl manpage so that apply_actions, clear_actions
write_metadata and goto_table appear at the same level of indentation as
actions rather being indented as if they are arguments to the learn action.

Signed-off-by: Simon Horman <horms@verge.net.au>
Signed-off-by: Ben Pfaff <blp@nicira.com>
10 years agoofproto: Rename struct rule's evict lock and use it more widely.
Ethan Jackson [Wed, 4 Sep 2013 00:23:44 +0000 (17:23 -0700)]
ofproto: Rename struct rule's evict lock and use it more widely.

There are a few fields in struct rule which are accessible by
functions in ofproto-dpif and therefore need to accessed in a thread
safe manner.  This patch achieves this by generalizing the rules evict
rwlock and requiring a writelock to be held to edit them.

Signed-off-by: Ethan Jackson <ethan@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
10 years agoofproto-dpif: Don't manage eviction groups from threads.
Ethan Jackson [Tue, 3 Sep 2013 22:37:14 +0000 (15:37 -0700)]
ofproto-dpif: Don't manage eviction groups from threads.

Managing eviction groups from threads was going to be difficult to do
in a performant thread-safe manner, so this patch punts the problem to
the main thread.

Signed-off-by: Ethan Jackson <ethan@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
10 years agoofproto-dpif: Hide struct rule_dpif internally.
Ethan Jackson [Tue, 27 Aug 2013 20:17:11 +0000 (13:17 -0700)]
ofproto-dpif: Hide struct rule_dpif internally.

By hiding struct rule_dpif inside ofproto-dpif, it becomes very clear
which attributes are accessed by multiple threads and need to be
protected by locks.

Signed-off-by: Ethan Jackson <ethan@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
10 years agoofproto-dpif: Do not dpif_port_del() patch ports in port_del().
Ben Pfaff [Fri, 30 Aug 2013 03:55:10 +0000 (20:55 -0700)]
ofproto-dpif: Do not dpif_port_del() patch ports in port_del().

Patch ports don't have datapath ports so it doesn't make sense to try to
call dpif_port_del() on them.  If we do try, it will fail, which makes the
caller think that the port wasn't really deleted, which in turn keeps
ofproto from reporting the port deletion via OpenFlow.  This fixes the
problem.

Without this patch, the following commands, executed under "make sandbox",
will report the patch port addition in "ovs-ofctl monitor" output, but not
the patch port deletion.  With this patch, both the addition and deletion
will be reported.

    ovs-vsctl add-br br0 -- set bridge br0 datapath_type=netdev
    ovs-ofctl monitor br0 128 &
    ovs-vsctl add-port br0 patch1 \
        -- set interface patch1 type=patch options:peer=patch2 \
        -- add-port br0 patch2 \
        -- set interface patch2 type=patch options:peer=patch1
    ovs-vsctl del-port patch1
    ovs-vsctl del-port patch2

Reported-by: YAMAMOTO Takashi <yamamoto@valinux.co.jp>
Signed-off-by: Ben Pfaff <blp@nicira.com>
10 years agoofproto-dpif-xlate: Fix confusion between "no stp port" and "stp port 0".
Alex Wang [Wed, 4 Sep 2013 22:21:15 +0000 (15:21 -0700)]
ofproto-dpif-xlate: Fix confusion between "no stp port" and "stp port 0".

Commit 9d189a50e (ofproto-dpif-xlate: Pull STP xlation into
ofproto-dpif-xlate.) introduced the bug that considers 'stp_port_no'
of 0 as stp disabled on the port.  However 'stp_port_no' is
actually the index of the stp struct's port array and ranges
between [0, STP_MAX_PORTS).  So the bug allows the blocked
port keep transmitting packets and generates loop.

This commit fixes this bug.

Signed-off-by: Alex Wang <alexw@nicira.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
10 years agodpif: fix segfault in CONTROLLER action with sflow or ipfix setup
Romain Lenglet [Wed, 4 Sep 2013 01:58:37 +0000 (18:58 -0700)]
dpif: fix segfault in CONTROLLER action with sflow or ipfix setup

Signed-off-by: Romain Lenglet <rlenglet@vmware.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
10 years agoofproto: Convert units correctly in ofport_open().
Ben Pfaff [Wed, 4 Sep 2013 20:37:56 +0000 (13:37 -0700)]
ofproto: Convert units correctly in ofport_open().

netdev_features_to_bps() returns a speed in bps, but struct
ofputil_phy_port's curr_speed and max_speed are in kbps, so a conversion
is necessary.  This commit fixes the problem.

Reported-by: Benjamin Lunsky <benjamin.lunsky@netronome.com>
Tested-by: Benjamin Lunsky <benjamin.lunsky@netronome.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
10 years agoflow: Fix hypothetical memory leak in miniflow_move().
Ben Pfaff [Wed, 4 Sep 2013 19:33:06 +0000 (12:33 -0700)]
flow: Fix hypothetical memory leak in miniflow_move().

Ordinarily a miniflow will use its inline_values if its values can fit, but
there is nothing to prevent a small number of values from being stored
in malloc()'d memory.  If this happened, then miniflow_move() would leak
memory.  This commit fixes the problem.

This is a hypothetical problem.  I haven't seen it in practice.

Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Ethan Jackson <ethan@nicira.com>
10 years agoofproto: Fix memory leak in add_flow().
Ben Pfaff [Wed, 4 Sep 2013 19:33:43 +0000 (12:33 -0700)]
ofproto: Fix memory leak in add_flow().

Found by inspection.

Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Ethan Jackson <ethan@nicira.com>
10 years agoofproto-dpif-mirror: Fix memory leak in mbridge_unref().
Ben Pfaff [Wed, 4 Sep 2013 19:33:28 +0000 (12:33 -0700)]
ofproto-dpif-mirror: Fix memory leak in mbridge_unref().

Found by valgrind.

Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Ethan Jackson <ethan@nicira.com>
10 years agoflow: Fix memory leak in minimask_move().
Ben Pfaff [Wed, 4 Sep 2013 19:39:19 +0000 (12:39 -0700)]
flow: Fix memory leak in minimask_move().

Found by valgrind.

Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Ethan Jackson <ethan@nicira.com>
10 years agoofproto-dpif: Fix use-after-free deleting a bridge with active traffic.
Ben Pfaff [Wed, 4 Sep 2013 19:38:12 +0000 (12:38 -0700)]
ofproto-dpif: Fix use-after-free deleting a bridge with active traffic.

When a bridge that has active traffic is deleted, the bridge's ofproto can
be referenced by in-flight "flow_miss"es.  This commit fixes the problem
by destroying "flow_miss"es that reference the ofproto that is being
deleted.

I found the problem by adding and removing a bridge that had active traffic
(via hping3) while running under valgrind.

Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Ethan Jackson <ethan@nicira.com>
10 years agoofproto-dpif: Destroy facets on ofproto removal.
Ethan Jackson [Wed, 4 Sep 2013 16:27:31 +0000 (09:27 -0700)]
ofproto-dpif: Destroy facets on ofproto removal.

Facets maintain a pointer to their owning ofproto-dpif, and therefore
when an ofproto-dpif is destroyed all of their child facets should be
destroyed along with it.  If they aren't, it's possible a subfacet
looked up in the dpif-backer could access it's parent facet and
therefore a defunct parent ofproto causing a segmentation fault.

Bug #19421.
Bug #19423.
Diagnosed-by: Ben Pfaff <blp@nicira.com>
Signed-off-by: Ethan Jackson <ethan@nicira.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Ethan Jackson <ethan@nicira.com>
10 years agocfm: Fix a memory allocation bug in cfm_get_remove_mpids().
Guolin Yang [Wed, 4 Sep 2013 16:00:12 +0000 (09:00 -0700)]
cfm: Fix a memory allocation bug in cfm_get_remove_mpids().

In cfm, when allocating the rmp array, the size is not calculated correctly
which cause the remote MPID not correctly updated in ovsdb.

This bug was introduced by commit 13b1b2ae70 (cfm: Make the CFM module
thread safe.).

Signed-off-by: Guolin Yang <gyang@nicira.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
10 years agoofproto-dpif-xlate: Fix mac learning deadlock.
Ethan Jackson [Wed, 4 Sep 2013 00:34:00 +0000 (17:34 -0700)]
ofproto-dpif-xlate: Fix mac learning deadlock.

xlate_normal() held the mac_learning lock while calling
output_normal().  When running with patch ports, this could cause
xlate_actions() to be called again, possibly attempting to take a
write lock on the same learning table causing a deadlock.  This patch
solves the problem by holding the lock for a very brief period of
time.

Bug #19423.
Signed-off-by: Ethan Jackson <ethan@nicira.com>
Acked-by: Justin Pettit <jpettit@nicira.com>
10 years agoFix a bug in conversion between flow/mask and flow key
Guolin Yang [Fri, 30 Aug 2013 16:57:13 +0000 (09:57 -0700)]
Fix a bug in conversion between flow/mask and flow key

In odp_flow_key_from_flow__(), when converting ICMPv6 flow/mask
to flow/mask key, we should always use flow to check for whether
ND informaition is present or not. In mask case, both type and code
field should be masked, otherwise ND fields can be masked.

Similarly in reverse conversion (parse_l2_5_onward()), we should
have same check.

Signed-off-by: Guolin Yang <gyang@nicira.com>
[blp@nicira.com changed && to || in parse_l2_5_onward()
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Andy Zhou <azhou@nicira.com>
10 years agoutil: Include pthread_np.h on FreeBSD
Ed Maste [Fri, 30 Aug 2013 14:56:08 +0000 (10:56 -0400)]
util: Include pthread_np.h on FreeBSD

On FreeBSD pthread_set_name_np's prototype is provided by pthread_np.h.
As I believe it is the only platform to provide the "set_name" (with an
underscore) variant I hope it's fine to use the existing autoconf macro
HAVE_PTHREAD_SET_NAME_NP.

Signed-off-by: Ed Maste <emaste@freebsd.org>
Signed-off-by: Ben Pfaff <blp@nicira.com>
10 years agodatapath: Sync vxlan tunneling code with upstream ovs-vxlan.
Pravin B Shelar [Mon, 26 Aug 2013 18:05:35 +0000 (11:05 -0700)]
datapath: Sync vxlan tunneling code with upstream ovs-vxlan.

Upstream vxlan implementation was changed according to few
comments.  Following patch brings back those changes to out
of tree ovs module.

Signed-off-by: Pravin B Shelar <pshelar@nicira.com>
Acked-by: Jesse Gross <jesse@nicira.com>
10 years agoPrepare for 2.0.0.
Justin Pettit [Wed, 28 Aug 2013 23:13:41 +0000 (16:13 -0700)]
Prepare for 2.0.0.

We decided to call the next release 2.0 instead of 1.12.

Signed-off-by: Justin Pettit <jpettit@nicira.com>
10 years agoSet release date for 1.11.0.
Justin Pettit [Wed, 28 Aug 2013 21:32:27 +0000 (14:32 -0700)]
Set release date for 1.11.0.

Signed-off-by: Justin Pettit <jpettit@nicira.com>
10 years agoofproto-dpif-upcall: Batch upcalls.
Jarno Rajahalme [Wed, 28 Aug 2013 19:06:07 +0000 (12:06 -0700)]
ofproto-dpif-upcall: Batch upcalls.

Batching reduces overheads and enables upto 4 times the upcall processing
performance in a specialized test case.

Signed-off-by: Jarno Rajahalme <jrajahalme@nicira.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
10 years agonetdev-dummy: Stop overriding patch vport type with dummy.
YAMAMOTO Takashi [Wed, 28 Aug 2013 07:33:07 +0000 (16:33 +0900)]
netdev-dummy: Stop overriding patch vport type with dummy.

There's little point to override patch ports, which is implmeneted
purely in our userland process these days, with a dummy implementation.

This allows testing patch ports in "make sandbox" environment.

Signed-off-by: YAMAMOTO Takashi <yamt@mwd.biglobe.ne.jp>
Signed-off-by: Ben Pfaff <blp@nicira.com>
10 years agonetdev-bsd: fix crashes
YAMAMOTO Takashi [Wed, 28 Aug 2013 05:44:07 +0000 (14:44 +0900)]
netdev-bsd: fix crashes

fix a regression added by commit 89454bf477d1dc95357792677ccbd4d483ab42d8.
"netdev: Fix deadlock when netdev_dump_queues() callback calls into netdev."

Signed-off-by: YAMAMOTO Takashi <yamt@mwd.biglobe.ne.jp>
Signed-off-by: Ben Pfaff <blp@nicira.com>
10 years agopackets, pktbuf: Align L3 headers on 32-bit boundary.
Ben Pfaff [Wed, 28 Aug 2013 05:10:22 +0000 (22:10 -0700)]
packets, pktbuf: Align L3 headers on 32-bit boundary.

Memory access tends to be faster when data is properly aligned.

Signed-off-by: Ben Pfaff <blp@nicira.com>
10 years agopackets: Introduce IPv6 headers not aligned on a 32-bit boundary.
Ben Pfaff [Thu, 15 Aug 2013 18:07:24 +0000 (11:07 -0700)]
packets: Introduce IPv6 headers not aligned on a 32-bit boundary.

This fixes the same problem for IPv6 headers treated for other headers in
the previous commit.

Signed-off-by: Ben Pfaff <blp@nicira.com>
10 years agopackets: Do not assume that IPv4, TCP, or ARP headers are 32-bit aligned.
Ben Pfaff [Thu, 15 Aug 2013 17:47:39 +0000 (10:47 -0700)]
packets: Do not assume that IPv4, TCP, or ARP headers are 32-bit aligned.

Ethernet headers are 14 bytes long, so when the beginning of such a header
is 32-bit aligned, the following data is misaligned.  The usual trick to
fix that is to start the Ethernet header on an odd-numbered 16-bit
boundary.  That trick works OK for Open vSwitch, but there are two
problems:

   - OVS doesn't use that trick everywhere.  Maybe it should, but it's
     difficult to make sure that it does consistently because the CPUs
     most commonly used with OVS don't care about misalignment, so we
     only find problems when porting.

   - Some protocols (GRE, VXLAN) don't use that trick, so in such a case
     one can properly align the inner or outer L3/L4/L7 but not both.  (OVS
     userspace doesn't directly deal with such protocols yet, so this is
     just future-proofing.)

   - OpenFlow uses the alignment trick in a few places but not all of them.

This commit starts the adoption of what I hope will be a more robust way
to avoid misalignment problems and the resulting bus errors on RISC
architectures.  Instead of trying to ensure that 32-bit quantities are
always aligned, we always read them as if they were misaligned.  To ensure
that they are read this way, we change their types from 32-bit types to
pairs of 16-bit types.  (I don't know of any protocols that offset the
next header by an odd number of bytes, so a 16-bit alignment assumption
seems OK.)

The same would be necessary for 64-bit types in protocol headers, but we
don't yet have any protocol definitions with 64-bit types.

IPv6 protocol headers need the same treatment, but for those we rely on
structs provided by system headers, so I'll leave them for an upcoming
patch.

Signed-off-by: Ben Pfaff <blp@nicira.com>
10 years agoofproto: Remove obsolete comment and assertion.
Ben Pfaff [Wed, 14 Aug 2013 23:26:05 +0000 (16:26 -0700)]
ofproto: Remove obsolete comment and assertion.

At one time, the ofproto-dpif implementation of the 'rule_execute' member
function required, indirectly, at least struct(ofp10_packet_in) bytes of
headroom in the packet passed into it.  (This allowed constructing an
OFPT_PACKET_IN without allocating and copying a new ofpbuf.)  This
restriction has long been lifted, but rule_execute() had not yet caught
up.  This commit updates it.

Signed-off-by: Ben Pfaff <blp@nicira.com>
10 years agolib: Keep track of usable protocols while parsing.
Jarno Rajahalme [Wed, 21 Aug 2013 01:41:45 +0000 (18:41 -0700)]
lib: Keep track of usable protocols while parsing.

Keep track of usable protocols while parsing actions and matches,
rather than checking for them afterwards.  This fixes silently discarded
meter and goto table instructions when not explicitly specifying the
protocol to use.

Signed-off-by: Jarno Rajahalme <jrajahalme@nicira.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
10 years agonetdev: Fix deadlock when netdev_dump_queues() callback calls into netdev.
Ben Pfaff [Wed, 28 Aug 2013 00:15:53 +0000 (17:15 -0700)]
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>
10 years agoofproto: Fully construct rules before putting them in the classifier.
Ben Pfaff [Mon, 26 Aug 2013 19:45:55 +0000 (12:45 -0700)]
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>
10 years agodatapath: add rounddown() definition into compatibility code
Andy Zhou [Tue, 27 Aug 2013 23:19:53 +0000 (16:19 -0700)]
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>
10 years agoofproto: Make OFPFC_ADD internally modify a rule instead of swapping.
Ben Pfaff [Tue, 27 Aug 2013 19:53:10 +0000 (12:53 -0700)]
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>
10 years agoofp-util: Abstract flow_mod OFPFF_* flags.
Ben Pfaff [Mon, 26 Aug 2013 23:23:50 +0000 (16:23 -0700)]
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>
10 years agoofproto-dpif-xlate: Fix fin_timeout to make rules expirable.
Ben Pfaff [Mon, 26 Aug 2013 20:08:30 +0000 (13:08 -0700)]
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>
10 years agoclassifier: New function cls_rule_move().
Ben Pfaff [Tue, 27 Aug 2013 19:25:48 +0000 (12:25 -0700)]
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>
10 years agoFix typos in a couple of OpenFlow related comments.
Ben Pfaff [Mon, 26 Aug 2013 22:28:46 +0000 (15:28 -0700)]
Fix typos in a couple of OpenFlow related comments.

Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Ethan Jackson <ethan@nicira.com>
10 years agodatapath: optimize flow compare and mask functions
Andy Zhou [Tue, 27 Aug 2013 18:21:26 +0000 (11:21 -0700)]
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>
10 years agoofproto-dpif.at: Remove push_vlan from an OF1.0 test.
Jarno Rajahalme [Mon, 26 Aug 2013 20:39:37 +0000 (13:39 -0700)]
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>
10 years agoovs-save: Do not depend on hardcoded set of $PATH for utilities.
Gurucharan Shetty [Mon, 26 Aug 2013 17:44:18 +0000 (10:44 -0700)]
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>
10 years agoovs-atomic: Add native Clang implementation.
Ben Pfaff [Mon, 26 Aug 2013 20:03:02 +0000 (13:03 -0700)]
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>
10 years agodpif: Change get_max_ports() to return uint32_t.
Alex Wang [Thu, 22 Aug 2013 21:24:18 +0000 (14:24 -0700)]
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>
10 years agoofproto: Change max_ports and alloc_port_no to uint16_t type.
Alex Wang [Thu, 22 Aug 2013 21:24:17 +0000 (14:24 -0700)]
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>
10 years agoofproto: remove the extra reset to make code clear to read.
pritesh [Thu, 22 Aug 2013 20:59:32 +0000 (13:59 -0700)]
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>
10 years agometa-flow: Use correct OF1.2+ errors for invalid fields in actions.
Ben Pfaff [Fri, 23 Aug 2013 18:11:03 +0000 (11:11 -0700)]
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>
10 years agoofproto-dpif-xlate: Refactor xlate_table_action() to avoid Clang warnings.
Ben Pfaff [Fri, 23 Aug 2013 18:03:55 +0000 (11:03 -0700)]
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>
10 years agoofproto-dpif: Fix thread safety annotation on rule_dpif_lookup_in_table().
Ben Pfaff [Wed, 21 Aug 2013 18:27:25 +0000 (11:27 -0700)]
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>
10 years agoofproto-dpif-upcall.c: Fix indentation.
Jarno Rajahalme [Thu, 22 Aug 2013 23:01:41 +0000 (16:01 -0700)]
ofproto-dpif-upcall.c: Fix indentation.

Signed-off-by: Jarno Rajahalme <jrajahalme@nicira.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
10 years agoRemove duplicate call to subfacet_create().
Jarno Rajahalme [Thu, 22 Aug 2013 22:51:39 +0000 (15:51 -0700)]
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>
10 years agobfd: Implement forwarding_if_rx.
Alex Wang [Thu, 22 Aug 2013 22:44:23 +0000 (15:44 -0700)]
bfd: Implement forwarding_if_rx.

This commit adds a new boolean option "forwarding_if_rx" to bfd.

When forwarding_if_rx is true the interface will be considered
capable of packet I/O as long as there is packet received at
interface.  This is important in that when link becomes temporarily
conjested, consecutive BFD control packets can be lost.  And the
forwarding_if_rx can prevent link failover by detecting non-control
packets received at interface.

Signed-off-by: Alex Wang <alexw@nicira.com>
10 years agodatapath: Remove redundant EtherType check in vlan translation.
Jesse Gross [Thu, 22 Aug 2013 20:30:45 +0000 (13:30 -0700)]
datapath: Remove redundant EtherType check in vlan translation.

This was supposed to be part of the previous patch but I forgot
to commit it before pushing.

Signed-off-by: Jesse Gross <jesse@nicira.com>
10 years agodatapath: More strict vlan encap netlink check
Andy Zhou [Thu, 15 Aug 2013 21:16:52 +0000 (14:16 -0700)]
datapath: More strict vlan encap netlink check

Only parse the encap key field if eth_type is 802.1Q and
VLAN_TAG_PRESENT bit is set. Add a few more eror checks and logs.

Signed-off-by: Andy Zhou <azhou@nicira.com>
Signed-off-by: Jesse Gross <jesse@nicira.com>