[ovs-dev] [PATCH 4/5] upcall: Delete flows that were not recently dumped.

[Joe Stringer]

Looks good.


On 25 February 2014 15:53, Ben Pfaff <blp at nicira.com> wrote:

> 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 <joestringer at 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
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openvswitch.org/pipermail/ovs-dev/attachments/20140225/4d851fe8/attachment-0005.html>