From 994c997345100f1868d8fbee508443475c556439 Mon Sep 17 00:00:00 2001 From: Ben Pfaff Date: Mon, 26 Aug 2013 13:08:30 -0700 Subject: [PATCH 1/1] ofproto-dpif-xlate: Fix fin_timeout to make rules expirable. 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 Acked-by: Ethan Jackson --- 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 54fabfe43..eeccbc573 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 d8b6a796c..72ba3bed9 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 709fd0751..4e3efbeb9 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) -- 2.43.0