ofproto: Make OFPFC_ADD internally modify a rule instead of swapping.
authorBen Pfaff <blp@nicira.com>
Tue, 27 Aug 2013 19:53:10 +0000 (12:53 -0700)
committerBen Pfaff <blp@nicira.com>
Tue, 27 Aug 2013 20:23:02 +0000 (13:23 -0700)
commitb277c7cc6984ef0aa233d5fed48db8a4d6af2210
treed66797cf10530c1a8803f36b746e88bf25ac56ad
parent0fb88c18fb26dcbe353501d346ac03295d552b36
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>
OPENFLOW-1.1+
ofproto/ofproto-dpif.c
ofproto/ofproto-provider.h
ofproto/ofproto.c