ofproto: Fully construct rules before putting them in the classifier.
authorBen Pfaff <blp@nicira.com>
Mon, 26 Aug 2013 19:45:55 +0000 (12:45 -0700)
committerBen Pfaff <blp@nicira.com>
Wed, 28 Aug 2013 04:45:37 +0000 (21:45 -0700)
commit8037acb42c04aa50011a2b34d1fe4a715c73a4b7
tree102131618d5d5ebc85e56008c3e14de7285e4881
parent7061a497b10b0c830cebcd463ceac053ba81dea5
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>
ofproto/ofproto-dpif.c
ofproto/ofproto-provider.h
ofproto/ofproto.c