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>
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>
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>
ofproto: Correct comments.

Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Ethan Jackson <ethan@nicira.com>
ofproto: Avoid gratuitous memory allocation and free.

Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Ethan Jackson <ethan@nicira.com>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>