[ovs-dev] [PATCH 3/5] upcall: Refactor ukey creation and dump handling

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


On Tue, Feb 25, 2014 at 03:41:36PM -0800, Ben Pfaff wrote:
> On Tue, Feb 25, 2014 at 03:25:35PM -0800, Ben Pfaff wrote:
> > On Tue, Feb 11, 2014 at 01:55:34PM -0800, Joe Stringer wrote:
> > > This splits out functions for re-use by later patches, and compacts the
> > > udump revalidation code.
> > > 
> > > Co-authored-by: Ethan Jackson <ethan at nicira.com>
> > > Signed-off-by: Joe Stringer <joestringer at nicira.com>
> > 
> > I have this queued up to apply, thanks!
> 
> It looks like we can avoid more code duplication in the following
> patch by folding in the following.  I'm tentatively doing that.

Ditto for the following.

diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index 1bced67..343181b 100644
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -1412,8 +1412,10 @@ dump_op_init(struct dump_op *op, const struct nlattr *key, size_t key_len,
 }
 
 static void
-push_dump_ops(struct udpif *udpif, struct dump_op *ops, size_t n_ops)
+push_dump_ops(struct revalidator *revalidator,
+              struct dump_op *ops, size_t n_ops)
 {
+    struct udpif *udpif = revalidator->udpif;
     struct dpif_op *opsp[REVALIDATE_MAX_BATCH];
     size_t i;
 
@@ -1463,6 +1465,18 @@ push_dump_ops(struct udpif *udpif, struct dump_op *ops, size_t n_ops)
             }
         }
     }
+
+    for (i = 0; i < n_ops; i++) {
+        struct udpif_key *ukey = ops[i].ukey;
+
+        /* Look up the ukey to prevent double-free in case 'ops' contains a
+         * given ukey more than once (which can happen if the datapath dumps a
+         * given flow more than once). */
+        ukey = ukey_lookup(revalidator, ops[i].udump);
+        if (ukey) {
+            ukey_delete(revalidator, ukey);
+        }
+    }
 }
 
 static void
@@ -1472,7 +1486,7 @@ revalidate_udumps(struct revalidator *revalidator, struct list *udumps)
 
     struct dump_op ops[REVALIDATE_MAX_BATCH];
     struct udpif_flow_dump *udump, *next_udump;
-    size_t n_ops, i, n_flows;
+    size_t n_ops, n_flows;
     unsigned int flow_limit;
     long long int max_idle;
     bool must_del;
@@ -1524,17 +1538,7 @@ revalidate_udumps(struct revalidator *revalidator, struct list *udumps)
         free(udump);
     }
 
-    push_dump_ops(udpif, ops, n_ops);
-    for (i = 0; i < n_ops; i++) {
-        struct udpif_key *ukey = ops[i].ukey;
-
-        /* Look up the ukey to prevent double-free if the datapath dumps the
-         * same flow twice. */
-        ukey = ukey_lookup(revalidator, ops[i].udump);
-        if (ukey) {
-            ukey_delete(revalidator, ukey);
-        }
-    }
+    push_dump_ops(revalidator, ops, n_ops);
 
     LIST_FOR_EACH_SAFE (udump, next_udump, list_node, udumps) {
         list_remove(&udump->list_node);



More information about the dev mailing list