ofproto: Document that ->rule_construct() should uninitialize victim rules.

The comments didn't say how this should work, so this clarifies it.

diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
index c9d74eeb7a556903ef199ded66f25f7eafa4f580..82844186fa3f6688c2f9fca00c656655cd39fe81 100644 (file)
--- a/ofproto/ofproto-provider.h
+++ b/ofproto/ofproto-provider.h
@@ -686,7 +686,8 @@ struct ofproto_class {
      *
      *   - 'rule' is replacing an existing rule in its flow table that had the
      *     same matching criteria and priority.  In this case,
-     *     ofoperation_get_victim(rule) returns the rule being replaced.
+     *     ofoperation_get_victim(rule) returns the rule being replaced (the
+     *     "victim" rule).
      *
      * ->rule_construct() should set the following in motion:
      *
@@ -706,9 +707,13 @@ struct ofproto_class {
      *   - If the rule is valid, update the datapath flow table, adding the new
      *     rule or replacing the existing one.
      *
+     *   - If 'rule' is replacing an existing rule, uninitialize any derived
+     *     state for the victim rule, as in step 5 in the "Life Cycle"
+     *     described above.
+     *
      * (On failure, the ofproto code will roll back the insertion from the flow
-     * table, either removing 'rule' or replacing it by the flow that was
-     * originally in its place.)
+     * table, either removing 'rule' or replacing it by the victim rule if
+     * there is one.)
      *
      * ->rule_construct() must act in one of the following ways:
      *

diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 849a376aec395c72c9b158976bc814362dd650d2..15bcd13502492ca9dce35cf6f9b1f3d7c4957318 100644 (file)
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -2828,8 +2828,29 @@ ofoperation_destroy(struct ofoperation *op)
  * indicate success or an OpenFlow error code (constructed with
  * e.g. ofp_mkerr()).
  *
- * If 'op' is a "delete flow" operation, 'error' must be 0.  That is, flow
- * deletions are not allowed to fail.
+ * If 'error' is 0, indicating success, the operation will be committed
+ * permanently to the flow table.  There is one interesting subcase:
+ *
+ *   - If 'op' is an "add flow" operation that is replacing an existing rule in
+ *     the flow table (the "victim" rule) by a new one, then the caller must
+ *     have uninitialized any derived state in the victim rule, as in step 5 in
+ *     the "Life Cycle" in ofproto/ofproto-provider.h.  ofoperation_complete()
+ *     performs steps 6 and 7 for the victim rule, most notably by calling its
+ *     ->rule_dealloc() function.
+ *
+ * If 'error' is nonzero, then generally the operation will be rolled back:
+ *
+ *   - If 'op' is an "add flow" operation, ofproto removes the new rule or
+ *     restores the original rule.  The caller must have uninitialized any
+ *     derived state in the new rule, as in step 5 of in the "Life Cycle" in
+ *     ofproto/ofproto-provider.h.  ofoperation_complete() performs steps 6 and
+ *     and 7 for the new rule, calling its ->rule_dealloc() function.
+ *
+ *   - If 'op' is a "modify flow" operation, ofproto restores the original
+ *     actions.
+ *
+ *   - 'op' must not be a "delete flow" operation.  Removing a rule is not
+ *     allowed to fail.  It must always succeed.
  *
  * Please see the large comment in ofproto/ofproto-provider.h titled
  * "Asynchronous Operation Support" for more information. */