Use new rule's flags to determine whether stats should be forwarded
from the old, modified rule to the new rule. This captures the fact
that prior to OpenFlow 1.2, which defines the reset counts flag, the
reset counts semantics was assumed by default. However, in that case
the reset counts flag is only present in the new flow, not on the
corresponding flow mod.
Having the above fixed revealed that the 'used' timestamp was not
forwarded from the old rule to the new rule when counts were not being
forwarded. Fix this by always forwarding the 'used' timestamp.
Fixes: 39c9459355 ("Use classifier versioning.")
Signed-off-by: Jarno Rajahalme <ja...@ovn.org>
---
ofproto/ofproto-dpif.c | 38 +++++++++++++++++++++++++++++++-------
ofproto/ofproto-provider.h | 7 ++++---
ofproto/ofproto.c | 9 +++++----
3 files changed, 40 insertions(+), 14 deletions(-)
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 0f00cef..d57068d 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -96,6 +96,8 @@ struct rule_dpif {
* 'rule->new_rule->stats_mutex' must be held together, acquire them in that
* order, */
struct rule_dpif *new_rule OVS_GUARDED;
+ bool forward_counts OVS_GUARDED; /* Forward counts? 'used' time will be
+ * forwarded in all cases. */
/* If non-zero then the recirculation id that has
* been allocated for use with this rule.
@@ -3832,17 +3834,30 @@ ofproto_dpif_execute_actions(struct ofproto_dpif
*ofproto,
ofpacts_len, 0, 0, 0, packet);
}
+static void
+rule_dpif_credit_stats__(struct rule_dpif *rule,
+ const struct dpif_flow_stats *stats,
+ bool credit_counts)
+ OVS_REQUIRES(rule->stats_mutex)
+{
+ if (credit_counts) {
+ rule->stats.n_packets += stats->n_packets;
+ rule->stats.n_bytes += stats->n_bytes;
+ }
+ rule->stats.used = MAX(rule->stats.used, stats->used);
+}
+
void
rule_dpif_credit_stats(struct rule_dpif *rule,
const struct dpif_flow_stats *stats)
{
ovs_mutex_lock(&rule->stats_mutex);
if (OVS_UNLIKELY(rule->new_rule)) {
- rule_dpif_credit_stats(rule->new_rule, stats);
+ ovs_mutex_lock(&rule->new_rule->stats_mutex);
+ rule_dpif_credit_stats__(rule->new_rule, stats, rule->forward_counts);
+ ovs_mutex_unlock(&rule->new_rule->stats_mutex);
} else {
- rule->stats.n_packets += stats->n_packets;
- rule->stats.n_bytes += stats->n_bytes;
- rule->stats.used = MAX(rule->stats.used, stats->used);
+ rule_dpif_credit_stats__(rule, stats, true);
}
ovs_mutex_unlock(&rule->stats_mutex);
}
@@ -4171,17 +4186,18 @@ rule_construct(struct rule *rule_)
rule->stats.used = rule->up.modified;
rule->recirc_id = 0;
rule->new_rule = NULL;
+ rule->forward_counts = false;
return 0;
}
static void
-rule_insert(struct rule *rule_, struct rule *old_rule_, bool forward_stats)
+rule_insert(struct rule *rule_, struct rule *old_rule_, bool forward_counts)
OVS_REQUIRES(ofproto_mutex)
{
struct rule_dpif *rule = rule_dpif_cast(rule_);
- if (old_rule_ && forward_stats) {
+ if (old_rule_) {
struct rule_dpif *old_rule = rule_dpif_cast(old_rule_);
ovs_assert(!old_rule->new_rule);
@@ -4193,7 +4209,15 @@ rule_insert(struct rule *rule_, struct rule *old_rule_,
bool forward_stats)
ovs_mutex_lock(&old_rule->stats_mutex);
ovs_mutex_lock(&rule->stats_mutex);
old_rule->new_rule = rule; /* Forward future stats. */
- rule->stats = old_rule->stats; /* Transfer stats to the new rule. */
+ old_rule->forward_counts = forward_counts;
+
+ if (forward_counts) {
+ rule->stats = old_rule->stats; /* Transfer stats to the new
+ * rule. */
+ } else {
+ /* Used timestamp must be forwarded whenever a rule is modified. */
+ rule->stats.used = old_rule->stats.used;
+ }
ovs_mutex_unlock(&rule->stats_mutex);
ovs_mutex_unlock(&old_rule->stats_mutex);
}
diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
index c6300e7..0610e78 100644
--- a/ofproto/ofproto-provider.h
+++ b/ofproto/ofproto-provider.h
@@ -1311,8 +1311,9 @@ struct ofproto_class {
* must add the new rule to the datapath flow table and return only after
* this is complete. The 'new_rule' may be a duplicate of an 'old_rule'.
* In this case the 'old_rule' is non-null, and the implementation should
- * forward rule statistics from the 'old_rule' to the 'new_rule' if
- * 'forward_stats' is 'true'. This may not fail.
+ * forward rule statistics counts from the 'old_rule' to the 'new_rule' if
+ * 'forward_counts' is 'true', 'used' timestamp is always forwarded. This
+ * may not fail.
*
*
* Deletion
@@ -1336,7 +1337,7 @@ struct ofproto_class {
enum ofperr (*rule_construct)(struct rule *rule)
/* OVS_REQUIRES(ofproto_mutex) */;
void (*rule_insert)(struct rule *rule, struct rule *old_rule,
- bool forward_stats)
+ bool forward_counts)
/* OVS_REQUIRES(ofproto_mutex) */;
void (*rule_delete)(struct rule *rule) /* OVS_REQUIRES(ofproto_mutex) */;
void (*rule_destruct)(struct rule *rule);
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 8ebbad0..7f63925 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -5071,17 +5071,18 @@ replace_rule_finish(struct ofproto *ofproto, struct
ofputil_flow_mod *fm,
struct ovs_list *dead_cookies)
OVS_REQUIRES(ofproto_mutex)
{
- bool forward_stats = !(fm->flags & OFPUTIL_FF_RESET_COUNTS);
+ bool forward_counts = !(new_rule->flags & OFPUTIL_FF_RESET_COUNTS);
struct rule *replaced_rule;
replaced_rule = fm->delete_reason != OFPRR_EVICTION ? old_rule : NULL;
/* Insert the new flow to the ofproto provider. A non-NULL 'replaced_rule'
* is a duplicate rule the 'new_rule' is replacing. The provider should
- * link the stats from the old rule to the new one if 'forward_stats' is
- * 'true'. The 'replaced_rule' will be deleted right after this call. */
+ * link the packet and byte counts from the old rule to the new one if
+ * 'forward_counts' is 'true'. The 'replaced_rule' will be deleted right
+ * after this call. */
ofproto->ofproto_class->rule_insert(new_rule, replaced_rule,
- forward_stats);
+ forward_counts);
learned_cookies_inc(ofproto, rule_get_actions(new_rule));
if (old_rule) {
--
2.1.4
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
