I suspect you may have found that the later patches make use of this
structure in more locations?
Either way, fine by me.
On 25 February 2014 15:16, Ben Pfaff <b...@nicira.com> wrote:
> On Tue, Feb 11, 2014 at 01:55:33PM -0800, Joe Stringer wrote:
> > It is possible for a datapath to dump the same flow twice, for instance
> > if the flow is the last in a batch of flows to be dumped, then a new
> > flow is inserted into the same bucket before the flow dumper fetches
> > another batch.
> >
> > In this case, datapath flow stats may be duplicated: The revalidator
> > records the stats from the first flow, using the ukey to get the stats
> > delta. The ukey is deleted, then the revalidator reads the second
> > (duplicate) flow and cannot lookup the ukey for the delta. As such, it
> > will push the stats as-is.
> >
> > This patch reduces the likelihood of such stats duplications by
> > deferring ukey deletion until after stats are pushed for deleted flows.
> >
> > Signed-off-by: Joe Stringer <joestrin...@nicira.com>
>
> Thanks, I'm adding this to my queue for application.
>
> Since struct dump_op is only used inside revalidate_udumps() I decided
> to move the declaration inside revalidate_udumps() too, for clarity:
>
> diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
> index 254a59d..18784ec 100644
> --- a/ofproto/ofproto-dpif-upcall.c
> +++ b/ofproto/ofproto-dpif-upcall.c
> @@ -1376,18 +1376,18 @@ exit:
> return ok;
> }
>
> -struct dump_op {
> - struct udpif_key *ukey;
> - struct udpif_flow_dump *udump;
> - struct dpif_flow_stats stats; /* Stats for 'op'. */
> - struct dpif_op op; /* Flow del operation. */
> -};
> -
> static void
> revalidate_udumps(struct revalidator *revalidator, struct list *udumps)
> {
> struct udpif *udpif = revalidator->udpif;
>
> + struct dump_op {
> + struct udpif_key *ukey;
> + struct udpif_flow_dump *udump;
> + struct dpif_flow_stats stats; /* Stats for 'op'. */
> + struct dpif_op op; /* Flow del operation. */
> + };
> +
> struct dump_op ops[REVALIDATE_MAX_BATCH];
> struct dpif_op *opsp[REVALIDATE_MAX_BATCH];
> struct udpif_flow_dump *udump, *next_udump;
>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
