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