[ovs-dev] [PATCH 2/5] upcall: Defer ukey deletion until after pushing stats.

Ben Pfaff blp at nicira.com
Tue Feb 25 23:16:10 UTC 2014


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 <joestringer at 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;



More information about the dev mailing list