Depending on the driver implementation it can take up to 2 seconds before
offloaded flow statistics are updated. This is causing a problem with
min-revalidate-pps, as old statistic values are used during this period.
This fix will wait for at least 2 seconds before assuming no packets
where received during this period.
Signed-off-by: Eelco Chaudron <echau...@redhat.com>
---
Changes:
- v2: Use existing ukey->offloaded so the revalidate_missed_dp_flow case is
also covered.
ofproto/ofproto-dpif-upcall.c | 25 +++++++++++++++----------
1 file changed, 15 insertions(+), 10 deletions(-)
diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index ad9635496..c395adbc6 100644
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -2094,10 +2094,11 @@ ukey_delete(struct umap *umap, struct udpif_key *ukey)
}
static bool
-should_revalidate(const struct udpif *udpif, uint64_t packets,
- long long int used)
+should_revalidate(const struct udpif *udpif, struct udpif_key *ukey,
+ uint64_t packets)
{
long long int metric, now, duration;
+ long long int used = ukey->stats.used;
if (!used) {
/* Always revalidate the first time a flow is dumped. */
@@ -2124,8 +2125,12 @@ should_revalidate(const struct udpif *udpif, uint64_t
packets,
duration = now - used;
metric = duration / packets;
- if (metric < 1000 / ofproto_min_revalidate_pps) {
- /* The flow is receiving more than min-revalidate-pps, so keep it. */
+ if (metric < 1000 / ofproto_min_revalidate_pps ||
+ (ukey->offloaded && duration < 2000)) {
+ /* The flow is receiving more than min-revalidate-pps, so keep it.
+ * Or it's a hardware offloaded flow that might take up to 2 seconds
+ * to update its statistics. Until we are sure the statistics had a
+ * chance to be updated, also keep it. */
return true;
}
return false;
@@ -2323,7 +2328,7 @@ static enum reval_result
revalidate_ukey(struct udpif *udpif, struct udpif_key *ukey,
const struct dpif_flow_stats *stats,
struct ofpbuf *odp_actions, uint64_t reval_seq,
- struct recirc_refs *recircs, bool offloaded)
+ struct recirc_refs *recircs)
OVS_REQUIRES(ukey->mutex)
{
bool need_revalidate = ukey->reval_seq != reval_seq;
@@ -2342,7 +2347,7 @@ revalidate_ukey(struct udpif *udpif, struct udpif_key
*ukey,
: 0);
if (need_revalidate) {
- if (should_revalidate(udpif, push.n_packets, ukey->stats.used)) {
+ if (should_revalidate(udpif, ukey, push.n_packets)) {
if (!ukey->xcache) {
ukey->xcache = xlate_cache_new();
} else {
@@ -2358,7 +2363,7 @@ revalidate_ukey(struct udpif *udpif, struct udpif_key
*ukey,
/* Stats for deleted flows will be attributed upon flow deletion. Skip. */
if (result != UKEY_DELETE) {
- xlate_push_stats(ukey->xcache, &push, offloaded);
+ xlate_push_stats(ukey->xcache, &push, ukey->offloaded);
ukey->stats = *stats;
ukey->reval_seq = reval_seq;
}
@@ -2758,6 +2763,7 @@ revalidate(struct revalidator *revalidator)
continue;
}
+ ukey->offloaded = f->attrs.offloaded;
already_dumped = ukey->dump_seq == dump_seq;
if (already_dumped) {
/* The flow has already been handled during this flow dump
@@ -2789,8 +2795,7 @@ revalidate(struct revalidator *revalidator)
result = UKEY_DELETE;
} else {
result = revalidate_ukey(udpif, ukey, &stats, &odp_actions,
- reval_seq, &recircs,
- f->attrs.offloaded);
+ reval_seq, &recircs);
}
ukey->dump_seq = dump_seq;
@@ -2875,7 +2880,7 @@ revalidator_sweep__(struct revalidator *revalidator, bool
purge)
COVERAGE_INC(revalidate_missed_dp_flow);
memset(&stats, 0, sizeof stats);
result = revalidate_ukey(udpif, ukey, &stats, &odp_actions,
- reval_seq, &recircs, false);
+ reval_seq, &recircs);
}
if (result != UKEY_KEEP) {
/* Clears 'recircs' if filled by revalidate_ukey(). */
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
