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 <[email protected]>
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
[email protected]
http://openvswitch.org/mailman/listinfo/dev
