On Tue, Feb 11, 2014 at 01:55:35PM -0800, Joe Stringer wrote:
> Previously, we would clean up the ukeys whose flow was not seen in the
> most recent dump, while leaving the flow in the datapath. In the
> unlikely case that the datapath fails to dump a flow that still exists
> in the datapath, this would cause double-counting of those flow stats.
>
> This is currently very rare to see due to batching of datapath flow
> deletion, but is more easily observable with upcoming patches which
> modify the batch size based on dpif implementation.
>
> Signed-off-by: Joe Stringer <joestrin...@nicira.com>
I think this is cleaner when push_dump_ops() does more. It becomes
just this:
diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index 343181b..bd781ed 100644
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -1549,15 +1549,32 @@ revalidate_udumps(struct revalidator *revalidator,
struct list *udumps)
static void
revalidator_sweep(struct revalidator *revalidator)
{
+ struct dump_op ops[REVALIDATE_MAX_BATCH];
struct udpif_key *ukey, *next;
+ size_t n_ops;
+
+ n_ops = 0;
HMAP_FOR_EACH_SAFE (ukey, next, hmap_node, &revalidator->ukeys) {
if (ukey->mark) {
ukey->mark = false;
} else {
- ukey_delete(revalidator, ukey);
+ struct dump_op *op = &ops[n_ops++];
+
+ /* If we have previously seen a flow in the datapath, but didn't
+ * see it during the most recent dump, delete it. This allows us
+ * to clean up the ukey and keep the statistics consistent. */
+ dump_op_init(op, ukey->key, ukey->key_len, ukey, NULL);
+ if (n_ops == REVALIDATE_MAX_BATCH) {
+ push_dump_ops(revalidator, ops, n_ops);
+ n_ops = 0;
+ }
}
}
+
+ if (n_ops) {
+ push_dump_ops(revalidator, ops, n_ops);
+ }
}
static void
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
