The comments didn't say how this should work, so this clarifies it.
---
ofproto/ofproto-provider.h | 11 ++++++++---
ofproto/ofproto.c | 24 ++++++++++++++++++++++--
2 files changed, 30 insertions(+), 5 deletions(-)
diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
index c9d74ee..8284418 100644
--- 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 849a376..e42cfb5 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -2828,8 +2828,28 @@ 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. ofoperation_complete() performs steps 5, 6,
+ * and 7 for the new rule, most notably by calling its ->rule_destruct()
+ * and ->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. */
--
1.7.4.4
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
