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>
guarded-list: New data structure for thread-safe queue.

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

Signed-off-by: Ben Pfaff <blp@nicira.com>
classifier: Allow CLS_CURSOR_FOR_EACH to use a const-qualified iterator.

Signed-off-by: Ben Pfaff <blp@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>
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>
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>
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>
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>
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>
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>
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>
ofproto: Refactor eviction cases to use common code.

Signed-off-by: Ben Pfaff <blp@nicira.com>
ofproto: Add global locking around flow table changes.

This makes 'ofproto_mutex' protect the flow table well enough that threads
other than the main one can realistically modify flows.

I need to look at the interface between ofproto and connmgr: I think that
there might need to be some locking there too.

Signed-off-by: Ben Pfaff <blp@nicira.com>
ofproto: Remove redundant cls parameter from a few functions.

Previously this parameter was useful for Clang locking annotations
but it isn't actually a locking requirement anymore, so remove
the parameter.

Signed-off-by: Ben Pfaff <blp@nicira.com>
ofproto-dpif: Move "learn" actions into individual threads.

Before OVS introduced threading, any given ``learn'' action took effect in
the flow table before the next incoming flow was set up.  This meant that
if a packet came in, used ``learn'' to set up a flow to handle replies, and
then a reply came in, the reply would always hit the flow set up by the
``learn'' action.  Until now, with the threading implementation, though,
the effects of ``learn'' actions happen asynchronously via a queue, which
means that the reply can arrive before the flow to handle it is set up.
This introduced an unacceptable regression in important use cases.

This commit fixes the problem by switching back to executing learn actions
before forwarding the packet that triggered it.

I imagine that there is considerable opportunity for optimization here.

Bug #19147.
Bug #19244.
Signed-off-by: Ben Pfaff <blp@nicira.com>