[ovs-dev] [PATCH v2 2/2] dpif-netdev: Implement batched flow dumping.

[Ryan Wilson]

Previously, flows were retrieved one by one when dumping flows for
datapaths of type 'netdev'. This increased contention for the dump's
mutex, negatively affecting revalidator performance.

This patch retrieves batches of flows when dumping flows for datapaths
of type 'netdev'.

Signed-off-by: Ryan Wilson <wryan at nicira.com>
---
v2: Addressed Joe's comments, split into 2 patches
---
 lib/dpif-netdev.c |   82 +++++++++++++++++++++++++++++------------------------
 1 file changed, 45 insertions(+), 37 deletions(-)

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 805af9a..b61e132 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -1423,8 +1423,8 @@ dpif_netdev_flow_dump_destroy(struct dpif_flow_dump *dump_)
 struct dpif_netdev_flow_dump_thread {
     struct dpif_flow_dump_thread up;
     struct dpif_netdev_flow_dump *dump;
-    struct odputil_keybuf keybuf;
-    struct odputil_keybuf maskbuf;
+    struct odputil_keybuf *keybuf;
+    struct odputil_keybuf *maskbuf;
 };
 
 static struct dpif_netdev_flow_dump_thread *
@@ -1442,6 +1442,8 @@ dpif_netdev_flow_dump_thread_create(struct dpif_flow_dump *dump_, int max_flows)
     thread = xmalloc(sizeof *thread);
     dpif_flow_dump_thread_init(&thread->up, &dump->up, max_flows);
     thread->dump = dump;
+    thread->keybuf = xmalloc(max_flows * sizeof *thread->keybuf);
+    thread->maskbuf = xmalloc(max_flows * sizeof *thread->maskbuf);
     return &thread->up;
 }
 
@@ -1451,29 +1453,34 @@ dpif_netdev_flow_dump_thread_destroy(struct dpif_flow_dump_thread *thread_)
     struct dpif_netdev_flow_dump_thread *thread
         = dpif_netdev_flow_dump_thread_cast(thread_);
 
+    free(thread->keybuf);
+    free(thread->maskbuf);
     free(thread);
 }
 
 /* XXX the caller must use 'actions' without quiescing */
 static int
 dpif_netdev_flow_dump_next(struct dpif_flow_dump_thread *thread_,
-                           struct dpif_flow *f, int max_flows OVS_UNUSED)
+                           struct dpif_flow *f, int max_flows)
 {
     struct dpif_netdev_flow_dump_thread *thread
         = dpif_netdev_flow_dump_thread_cast(thread_);
     struct dpif_netdev_flow_dump *dump = thread->dump;
     struct dpif_netdev *dpif = dpif_netdev_cast(thread->up.dpif);
     struct dp_netdev *dp = get_dp_netdev(&dpif->dpif);
-    struct dp_netdev_flow *netdev_flow;
-    struct flow_wildcards wc;
-    struct dp_netdev_actions *dp_actions;
-    struct ofpbuf buf;
-    int error;
+    int n_flows = 0;
 
     ovs_mutex_lock(&dump->mutex);
-    error = dump->status;
-    if (!error) {
+
+    while (n_flows < max_flows && !dump->status) {
+        struct odputil_keybuf *maskbuf = &thread->maskbuf[n_flows];
+        struct odputil_keybuf *keybuf = &thread->keybuf[n_flows];
+        struct dpif_flow *flow = &f[n_flows];
+        struct dp_netdev_flow *netdev_flow;
+        struct flow_wildcards wc;
+        struct dp_netdev_actions *dp_actions;
         struct hmap_node *node;
+        struct ofpbuf buf;
 
         fat_rwlock_rdlock(&dp->cls.rwlock);
         node = hmap_at_position(&dp->flow_table, &dump->bucket, &dump->offset);
@@ -1482,40 +1489,41 @@ dpif_netdev_flow_dump_next(struct dpif_flow_dump_thread *thread_,
         }
         fat_rwlock_unlock(&dp->cls.rwlock);
         if (!node) {
-            dump->status = error = EOF;
+            dump->status = EOF;
+            break;
         }
-    }
-    ovs_mutex_unlock(&dump->mutex);
-    if (error) {
-        return 0;
-    }
 
-    minimask_expand(&netdev_flow->cr.match.mask, &wc);
+        minimask_expand(&netdev_flow->cr.match.mask, &wc);
 
-    /* Key. */
-    ofpbuf_use_stack(&buf, &thread->keybuf, sizeof thread->keybuf);
-    odp_flow_key_from_flow(&buf, &netdev_flow->flow, &wc.masks,
-                           netdev_flow->flow.in_port.odp_port, true);
-    f->key = ofpbuf_data(&buf);
-    f->key_len = ofpbuf_size(&buf);
+        /* Key. */
+        ofpbuf_use_stack(&buf, keybuf, sizeof *keybuf);
+        odp_flow_key_from_flow(&buf, &netdev_flow->flow, &wc.masks,
+                               netdev_flow->flow.in_port.odp_port, true);
+        flow->key = ofpbuf_data(&buf);
+        flow->key_len = ofpbuf_size(&buf);
 
-    /* Mask. */
-    ofpbuf_use_stack(&buf, &thread->maskbuf, sizeof thread->maskbuf);
-    odp_flow_key_from_mask(&buf, &wc.masks, &netdev_flow->flow,
-                           odp_to_u32(wc.masks.in_port.odp_port),
-                           SIZE_MAX, true);
-    f->mask = ofpbuf_data(&buf);
-    f->mask_len = ofpbuf_size(&buf);
+        /* Mask. */
+        ofpbuf_use_stack(&buf, maskbuf, sizeof *maskbuf);
+        odp_flow_key_from_mask(&buf, &wc.masks, &netdev_flow->flow,
+                               odp_to_u32(wc.masks.in_port.odp_port),
+                               SIZE_MAX, true);
+        flow->mask = ofpbuf_data(&buf);
+        flow->mask_len = ofpbuf_size(&buf);
 
-    /* Actions. */
-    dp_actions = dp_netdev_flow_get_actions(netdev_flow);
-    f->actions = dp_actions->actions;
-    f->actions_len = dp_actions->size;
+        /* Actions. */
+        dp_actions = dp_netdev_flow_get_actions(netdev_flow);
+        flow->actions = dp_actions->actions;
+        flow->actions_len = dp_actions->size;
 
-    /* Stats. */
-    get_dpif_flow_stats(netdev_flow, &f->stats);
+        /* Stats. */
+        get_dpif_flow_stats(netdev_flow, &flow->stats);
+
+        n_flows++;
+    }
+
+    ovs_mutex_unlock(&dump->mutex);
 
-    return 1;
+    return n_flows;
 }
 
 static int
-- 
1.7.9.5