Seems fine to me. Out of curiosity: Is this compatible with the 1.0
spec? I wonder if anyone is relying on the old behavior? Seems
unlikely I suppose.
Ethan
On Fri, Aug 19, 2011 at 15:28, Ben Pfaff <b...@nicira.com> wrote:
> I finally found a good use for hard timeouts in OpenFlow, but they require
> a slight reinterpretation of the meaning of hard timeouts. Until now, a
> hard timeout meant that a flow would be removed the specified number of
> seconds after a flow was created. Intervening modifications with
> OFPFC_MODIFY(_STRICT) had no effect on the hard timeout; the flow would
> still be deleted the specified number of seconds after its original
> creation.
>
> This commit changes the effect of OFPFC_MODIFY(_STRICT). Now, modifying
> a flow resets its hard timeout counter. A flow will time out the specified
> number of seconds after creation or after the last time it is modified,
> whichever comes later.
> ---
> ofproto/ofproto-dpif.c | 2 +-
> ofproto/ofproto-provider.h | 3 ++-
> ofproto/ofproto.c | 8 ++++++--
> 3 files changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> index 3130da4..2669bf0 100644
> --- a/ofproto/ofproto-dpif.c
> +++ b/ofproto/ofproto-dpif.c
> @@ -1992,7 +1992,7 @@ rule_expire(struct rule_dpif *rule)
> /* Has 'rule' expired? */
> now = time_msec();
> if (rule->up.hard_timeout
> - && now > rule->up.created + rule->up.hard_timeout * 1000) {
> + && now > rule->up.modified + rule->up.hard_timeout * 1000) {
> reason = OFPRR_HARD_TIMEOUT;
> } else if (rule->up.idle_timeout && list_is_empty(&rule->facets)
> && now > rule->used + rule->up.idle_timeout * 1000) {
> diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
> index 9c53657..e56c8d1 100644
> --- a/ofproto/ofproto-provider.h
> +++ b/ofproto/ofproto-provider.h
> @@ -106,8 +106,9 @@ struct rule {
> ovs_be64 flow_cookie; /* Controller-issued identifier. */
>
> long long int created; /* Creation time. */
> + long long int modified; /* Time of last modification. */
> uint16_t idle_timeout; /* In seconds from time of last use. */
> - uint16_t hard_timeout; /* In seconds from time of creation. */
> + uint16_t hard_timeout; /* In seconds from last modification. */
> uint8_t table_id; /* Index in ofproto's 'tables' array. */
> bool send_flow_removed; /* Send a flow removed message? */
>
> diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
> index d35e1d8..b7e31d2 100644
> --- a/ofproto/ofproto.c
> +++ b/ofproto/ofproto.c
> @@ -2233,7 +2233,7 @@ add_flow(struct ofproto *ofproto, struct ofconn *ofconn,
> rule->cr = fm->cr;
> rule->pending = NULL;
> rule->flow_cookie = fm->cookie;
> - rule->created = time_msec();
> + rule->created = rule->modified = time_msec();
> rule->idle_timeout = fm->idle_timeout;
> rule->hard_timeout = fm->hard_timeout;
> rule->table_id = table - ofproto->tables;
> @@ -2296,6 +2296,8 @@ modify_flows__(struct ofproto *ofproto, struct ofconn
> *ofconn,
> rule->actions = ofputil_actions_clone(fm->actions, fm->n_actions);
> rule->n_actions = fm->n_actions;
> rule->ofproto->ofproto_class->rule_modify_actions(rule);
> + } else {
> + rule->modified = time_msec();
> }
> rule->flow_cookie = fm->cookie;
> }
> @@ -2904,7 +2906,9 @@ ofoperation_complete(struct ofoperation *op, int error)
> break;
>
> case OFOPERATION_MODIFY:
> - if (error) {
> + if (!error) {
> + rule->modified = time_msec();
> + } else {
> free(rule->actions);
> rule->actions = op->actions;
> rule->n_actions = op->n_actions;
> --
> 1.7.4.4
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
