[ovs-dev] [PATCH 2/3] ofproto: Cache result in dp_enumerate_type

Simon Horman horms at verge.net.au
Mon Jan 21 06:10:01 UTC 2013


It is not clear to me that the result calculated by dp_enumerate_type()
can ever change.

By caching the lookup made by dp_enumerate_types() calls to
shash_add() and shash_destroy() destroy are avoided reducing the use of
malloc(), free() and hash_bytes().

In my test environment this increased the rate at which packets could be
received from ~24.4kpps to ~25.6kpps.

Signed-off-by: Simon Horman <horms at verge.net.au>
---
 lib/dpif-netdev.c          |    7 ++-----
 lib/dpif.c                 |   16 ++++++++++------
 lib/dpif.h                 |    2 +-
 ofproto/ofproto-dpif.c     |    6 +++---
 ofproto/ofproto-provider.h |    6 ++----
 ofproto/ofproto.c          |   16 ++++++++--------
 utilities/ovs-dpctl.c      |   25 ++++++++-----------------
 7 files changed, 34 insertions(+), 44 deletions(-)

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index bec32c3..997dcd8 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -1337,17 +1337,14 @@ void
 dpif_dummy_register(bool override)
 {
     if (override) {
-        struct sset types;
+        const struct sset *types = dp_enumerate_types();
         const char *type;
 
-        sset_init(&types);
-        dp_enumerate_types(&types);
-        SSET_FOR_EACH (type, &types) {
+        SSET_FOR_EACH (type, types) {
             if (!dp_unregister_provider(type)) {
                 dpif_dummy_register__(type);
             }
         }
-        sset_destroy(&types);
     }
 
     dpif_dummy_register__("dummy");
diff --git a/lib/dpif.c b/lib/dpif.c
index 6aa52d5..dba6f11 100644
--- a/lib/dpif.c
+++ b/lib/dpif.c
@@ -174,18 +174,22 @@ dp_blacklist_provider(const char *type)
 
 /* Clears 'types' and enumerates the types of all currently registered datapath
  * providers into it.  The caller must first initialize the sset. */
-void
-dp_enumerate_types(struct sset *types)
+const struct sset *
+dp_enumerate_types(void)
 {
     struct shash_node *node;
+    static struct sset types = SSET_INITIALIZER(&types);
 
     dp_initialize();
-    sset_clear(types);
 
-    SHASH_FOR_EACH(node, &dpif_classes) {
-        const struct registered_dpif_class *registered_class = node->data;
-        sset_add(types, registered_class->dpif_class->type);
+    if (sset_is_empty(&types)) {
+        SHASH_FOR_EACH(node, &dpif_classes) {
+            const struct registered_dpif_class *registered_class = node->data;
+            sset_add(&types, registered_class->dpif_class->type);
+        }
     }
+
+    return &types;
 }
 
 /* Clears 'names' and enumerates the names of all known created datapaths with
diff --git a/lib/dpif.h b/lib/dpif.h
index a478db2..8c462d0 100644
--- a/lib/dpif.h
+++ b/lib/dpif.h
@@ -344,7 +344,7 @@ struct dpif_class;
 int dp_register_provider(const struct dpif_class *);
 int dp_unregister_provider(const char *type);
 void dp_blacklist_provider(const char *type);
-void dp_enumerate_types(struct sset *types);
+const struct sset *dp_enumerate_types(void);
 const char *dpif_normalize_type(const char *);
 
 int dp_enumerate_names(const char *type, struct sset *names);
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index b37b482..4eb51a4 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -750,10 +750,10 @@ init(const struct shash *iface_hints)
     }
 }
 
-static void
-enumerate_types(struct sset *types)
+static const struct sset *
+enumerate_types(void)
 {
-    dp_enumerate_types(types);
+    return dp_enumerate_types();
 }
 
 static int
diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
index f2274ef..0307f47 100644
--- a/ofproto/ofproto-provider.h
+++ b/ofproto/ofproto-provider.h
@@ -356,10 +356,8 @@ struct ofproto_class {
      * may choose to remove it all. */
     void (*init)(const struct shash *iface_hints);
 
-    /* Enumerates the types of all support ofproto types into 'types'.  The
-     * caller has already initialized 'types' and other ofproto classes might
-     * already have added names to it. */
-    void (*enumerate_types)(struct sset *types);
+    /* Enumerates the types of all support ofproto types into 'types'. */
+    const struct sset *(*enumerate_types)(void);
 
     /* Enumerates the names of all existing datapath of the specified 'type'
      * into 'names' 'all_dps'.  The caller has already initialized 'names' as
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 918b9b4..acf3b9a 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -271,13 +271,8 @@ ofproto_class_find__(const char *type)
 
     for (i = 0; i < n_ofproto_classes; i++) {
         const struct ofproto_class *class = ofproto_classes[i];
-        struct sset types;
-        bool found;
-
-        sset_init(&types);
-        class->enumerate_types(&types);
-        found = sset_contains(&types, type);
-        sset_destroy(&types);
+        const struct sset *types = class->enumerate_types();
+        bool found = sset_contains(types, type);
 
         if (found) {
             return class;
@@ -340,7 +335,12 @@ ofproto_enumerate_types(struct sset *types)
     size_t i;
 
     for (i = 0; i < n_ofproto_classes; i++) {
-        ofproto_classes[i]->enumerate_types(types);
+        const struct sset *types_ = ofproto_classes[i]->enumerate_types();
+        const char *name;
+
+        SSET_FOR_EACH(name, types_) {
+            sset_add(types, name);
+        }
     }
 }
 
diff --git a/utilities/ovs-dpctl.c b/utilities/ovs-dpctl.c
index b48b349..b96373b 100644
--- a/utilities/ovs-dpctl.c
+++ b/utilities/ovs-dpctl.c
@@ -203,14 +203,13 @@ static int if_up(const char *netdev_name)
 static char *
 get_one_dp(void)
 {
-    struct sset types;
+    const struct sset *types;
     const char *type;
     char *dp_name = NULL;
     size_t count = 0;
 
-    sset_init(&types);
-    dp_enumerate_types(&types);
-    SSET_FOR_EACH (type, &types) {
+    types = dp_enumerate_types();
+    SSET_FOR_EACH (type, types) {
         struct sset names;
 
         sset_init(&names);
@@ -222,7 +221,6 @@ get_one_dp(void)
         }
         sset_destroy(&names);
     }
-    sset_destroy(&types);
 
     if (!count) {
         ovs_fatal(0, "no datapaths exist");
@@ -642,12 +640,10 @@ dpctl_show(int argc, char *argv[])
             }
         }
     } else {
-        struct sset types;
+        const struct sset *types = dp_enumerate_types();
         const char *type;
 
-        sset_init(&types);
-        dp_enumerate_types(&types);
-        SSET_FOR_EACH (type, &types) {
+        SSET_FOR_EACH (type, types) {
             struct sset names;
             const char *name;
 
@@ -670,7 +666,6 @@ dpctl_show(int argc, char *argv[])
             }
             sset_destroy(&names);
         }
-        sset_destroy(&types);
     }
     if (failure) {
         exit(EXIT_FAILURE);
@@ -680,15 +675,12 @@ dpctl_show(int argc, char *argv[])
 static void
 dpctl_dump_dps(int argc OVS_UNUSED, char *argv[] OVS_UNUSED)
 {
-    struct sset dpif_names, dpif_types;
+    struct sset dpif_names;
+    const struct sset *dpif_types = dp_enumerate_types();
     const char *type;
     int error = 0;
 
-    sset_init(&dpif_names);
-    sset_init(&dpif_types);
-    dp_enumerate_types(&dpif_types);
-
-    SSET_FOR_EACH (type, &dpif_types) {
+    SSET_FOR_EACH (type, dpif_types) {
         const char *name;
         int retval;
 
@@ -707,7 +699,6 @@ dpctl_dump_dps(int argc OVS_UNUSED, char *argv[] OVS_UNUSED)
     }
 
     sset_destroy(&dpif_names);
-    sset_destroy(&dpif_types);
     if (error) {
         exit(EXIT_FAILURE);
     }
-- 
1.7.10.4




More information about the dev mailing list