Please see inline.
Thanks,
-Hao
On Thu, Sep 8, 2011 at 4:27 PM, Ben Pfaff <b...@nicira.com> wrote:
> 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.
>
I don't see ofoperation_complete() calls ->rule_destruct() in case of "add
flow" operation when 'error' is nonzero.
> + *
> + * - 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
