Acked-by: Ethan Jackson <et...@nicira.com>
On Mon, Aug 26, 2013 at 5:10 PM, Ben Pfaff <b...@nicira.com> wrote:
> In an Open vSwitch flow table that has a configured maximum number of
> flows, flows that have an idle or hard timeout, or both, are expirable,
> and flows with neither are permanent. The fin_timeout action can change
> a flow that has no idle or hard timeout into one that has either one or
> both, which should make a permanent flow into an expirable one, but the
> implementation was buggy and did not actually make the flow expirable.
> This commit fixes the problem.
>
> This commit also moves most of the implementation of fin_timeout from
> ofproto-dpif-xlate into ofproto, because this seems to better respect the
> layering.
>
> Signed-off-by: Ben Pfaff <b...@nicira.com>
> ---
> ofproto/ofproto-dpif-xlate.c | 24 ++----------------------
> ofproto/ofproto-provider.h | 3 +++
> ofproto/ofproto.c | 41 +++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 46 insertions(+), 22 deletions(-)
>
> diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
> index 54fabfe..eeccbc5 100644
> --- a/ofproto/ofproto-dpif-xlate.c
> +++ b/ofproto/ofproto-dpif-xlate.c
> @@ -2111,33 +2111,13 @@ xlate_learn_action(struct xlate_ctx *ctx,
> ofproto_dpif_flow_mod(ctx->xbridge->ofproto, fm);
> }
>
> -/* Reduces '*timeout' to no more than 'max'. A value of zero in either case
> - * means "infinite". */
> -static void
> -reduce_timeout(uint16_t max, uint16_t *timeout)
> -{
> - if (max && (!*timeout || *timeout > max)) {
> - *timeout = max;
> - }
> -}
> -
> static void
> xlate_fin_timeout(struct xlate_ctx *ctx,
> const struct ofpact_fin_timeout *oft)
> {
> if (ctx->xin->tcp_flags & (TCP_FIN | TCP_RST) && ctx->rule) {
> - struct rule_dpif *rule = ctx->rule;
> -
> - ovs_mutex_lock(&rule->up.ofproto->expirable_mutex);
> - if (list_is_empty(&rule->up.expirable)) {
> - list_insert(&rule->up.ofproto->expirable, &rule->up.expirable);
> - }
> - ovs_mutex_unlock(&rule->up.ofproto->expirable_mutex);
> -
> - ovs_mutex_lock(&rule->up.timeout_mutex);
> - reduce_timeout(oft->fin_idle_timeout, &rule->up.idle_timeout);
> - reduce_timeout(oft->fin_hard_timeout, &rule->up.hard_timeout);
> - ovs_mutex_unlock(&rule->up.timeout_mutex);
> + ofproto_rule_reduce_timeouts(&ctx->rule->up, oft->fin_idle_timeout,
> + oft->fin_hard_timeout);
> }
> }
>
> diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
> index d8b6a79..72ba3be 100644
> --- a/ofproto/ofproto-provider.h
> +++ b/ofproto/ofproto-provider.h
> @@ -281,6 +281,9 @@ void ofproto_rule_expire(struct rule *rule, uint8_t
> reason)
> OVS_RELEASES(rule->evict);
> void ofproto_rule_destroy(struct ofproto *, struct classifier *cls,
> struct rule *) OVS_REQ_WRLOCK(cls->rwlock);
> +void ofproto_rule_reduce_timeouts(struct rule *rule, uint16_t idle_timeout,
> + uint16_t hard_timeout)
> + OVS_EXCLUDED(rule->ofproto->expirable_mutex, rule->timeout_mutex);
>
> bool ofproto_rule_has_out_port(const struct rule *, ofp_port_t out_port);
>
> diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
> index 709fd07..4e3efbe 100644
> --- a/ofproto/ofproto.c
> +++ b/ofproto/ofproto.c
> @@ -186,6 +186,7 @@ static bool choose_rule_to_evict(struct oftable *table,
> struct rule **rulep)
> OVS_TRY_WRLOCK(true, (*rulep)->evict);
> static void ofproto_evict(struct ofproto *);
> static uint32_t rule_eviction_priority(struct rule *);
> +static void eviction_group_add_rule(struct rule *rule);
>
> /* ofport. */
> static void ofport_destroy__(struct ofport *);
> @@ -3785,6 +3786,46 @@ ofproto_rule_expire(struct rule *rule, uint8_t reason)
> ofproto->ofproto_class->rule_destruct(rule);
> ofopgroup_submit(group);
> }
> +
> +/* Reduces '*timeout' to no more than 'max'. A value of zero in either case
> + * means "infinite". */
> +static void
> +reduce_timeout(uint16_t max, uint16_t *timeout)
> +{
> + if (max && (!*timeout || *timeout > max)) {
> + *timeout = max;
> + }
> +}
> +
> +/* If 'idle_timeout' is nonzero, and 'rule' has no idle timeout or an idle
> + * timeout greater than 'idle_timeout', lowers 'rule''s idle timeout to
> + * 'idle_timeout' seconds. Similarly for 'hard_timeout'.
> + *
> + * Suitable for implementing OFPACT_FIN_TIMEOUT. */
> +void
> +ofproto_rule_reduce_timeouts(struct rule *rule,
> + uint16_t idle_timeout, uint16_t hard_timeout)
> + OVS_EXCLUDED(rule->ofproto->expirable_mutex, rule->timeout_mutex)
> +{
> + if (!idle_timeout && !hard_timeout) {
> + return;
> + }
> +
> + ovs_mutex_lock(&rule->ofproto->expirable_mutex);
> + if (list_is_empty(&rule->expirable)) {
> + list_insert(&rule->ofproto->expirable, &rule->expirable);
> + }
> + ovs_mutex_unlock(&rule->ofproto->expirable_mutex);
> +
> + ovs_mutex_lock(&rule->timeout_mutex);
> + reduce_timeout(idle_timeout, &rule->idle_timeout);
> + reduce_timeout(hard_timeout, &rule->hard_timeout);
> + ovs_mutex_unlock(&rule->timeout_mutex);
> +
> + if (!rule->eviction_group) {
> + eviction_group_add_rule(rule);
> + }
> +}
>
> static enum ofperr
> handle_flow_mod(struct ofconn *ofconn, const struct ofp_header *oh)
> --
> 1.7.10.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
