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>