[ovs-dev] [next 34/35] ofproto: Complete abstraction by adding enumeration and deletion functions.

Ben Pfaff blp at nicira.com
Tue Apr 26 16:25:00 UTC 2011


This eliminates the final reference from bridge.c directly into the dpif
layer, which will make it easier to change the implementation of ofproto
to support other lower layers.
---
 lib/dpif.h        |    3 +--
 ofproto/ofproto.c |   44 ++++++++++++++++++++++++++++++++++++++++++++
 ofproto/ofproto.h |    6 ++++++
 vswitchd/bridge.c |   50 ++++++++++++++++++++++----------------------------
 4 files changed, 73 insertions(+), 30 deletions(-)

diff --git a/lib/dpif.h b/lib/dpif.h
index a4e5568..447ccd9 100644
--- a/lib/dpif.h
+++ b/lib/dpif.h
@@ -40,6 +40,7 @@ struct dpif_class;
 int dp_register_provider(const struct dpif_class *);
 int dp_unregister_provider(const char *type);
 void dp_enumerate_types(struct sset *types);
+const char *dpif_normalize_type(const char *);
 
 int dp_enumerate_names(const char *type, struct sset *names);
 void dp_parse_name(const char *datapath_name, char **name, char **type);
@@ -55,8 +56,6 @@ void dpif_wait(struct dpif *);
 const char *dpif_name(const struct dpif *);
 const char *dpif_base_name(const struct dpif *);
 
-const char *dpif_normalize_type(const char *);
-
 int dpif_delete(struct dpif *);
 
 int dpif_get_dp_stats(const struct dpif *, struct odp_stats *);
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index b07c155..fef2482 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -409,6 +409,36 @@ static bool is_admissible(struct ofproto *, const struct flow *,
 
 static void ofproto_unixctl_init(void);
 
+/* Clears 'types' and enumerates all registered ofproto types into it.  The
+ * caller must first initialize the sset. */
+void
+ofproto_enumerate_types(struct sset *types)
+{
+    dp_enumerate_types(types);
+}
+
+/* Returns the fully spelled out name for the given ofproto 'type'.
+ *
+ * Normalized type string can be compared with strcmp().  Unnormalized type
+ * string might be the same even if they have different spellings. */
+const char *
+ofproto_normalize_type(const char *type)
+{
+    return dpif_normalize_type(type);
+}
+
+/* Clears 'names' and enumerates the names of all known created ofprotos with
+ * the given 'type'.  The caller must first initialize the sset.  Returns 0 if
+ * successful, otherwise a positive errno value.
+ *
+ * Some kinds of datapaths might not be practically enumerable.  This is not
+ * considered an error. */
+int
+ofproto_enumerate_names(const char *type, struct sset *names)
+{
+    return dp_enumerate_names(type, names);
+}
+
 int
 ofproto_create(const char *datapath, const char *datapath_type,
                struct ofproto **ofprotop)
@@ -1327,6 +1357,20 @@ ofproto_destroy(struct ofproto *p)
     free(p);
 }
 
+int
+ofproto_delete(const char *name, const char *type)
+{
+    struct dpif *dpif;
+    int error;
+
+    error = dpif_open(name, type, &dpif);
+    if (!error) {
+        error = dpif_delete(dpif);
+        dpif_close(dpif);
+    }
+    return error;
+}
+
 static void
 process_port_change(struct ofproto *ofproto, int error, char *devname)
 {
diff --git a/ofproto/ofproto.h b/ofproto/ofproto.h
index b069d13..c20ec53 100644
--- a/ofproto/ofproto.h
+++ b/ofproto/ofproto.h
@@ -92,11 +92,17 @@ struct ofproto_controller {
 #define DEFAULT_SERIAL_DESC "None"
 #define DEFAULT_DP_DESC "None"
 
+void ofproto_enumerate_types(struct sset *types);
+const char *ofproto_normalize_type(const char *);
+
+int ofproto_enumerate_names(const char *type, struct sset *names);
 void ofproto_parse_name(const char *name, char **dp_name, char **dp_type);
 
 int ofproto_create(const char *datapath, const char *datapath_type,
                    struct ofproto **ofprotop);
 void ofproto_destroy(struct ofproto *);
+int ofproto_delete(const char *name, const char *type);
+
 int ofproto_run(struct ofproto *);
 void ofproto_wait(struct ofproto *);
 bool ofproto_is_alive(const struct ofproto *);
diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
index 747b21c..635aa21 100644
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -38,7 +38,6 @@
 #include "coverage.h"
 #include "daemon.h"
 #include "dirs.h"
-#include "dpif.h"
 #include "dynamic-string.h"
 #include "flow.h"
 #include "hash.h"
@@ -158,8 +157,8 @@ static long long int stats_timer = LLONG_MIN;
 static long long int db_limiter = LLONG_MIN;
 
 static void add_del_bridges(const struct ovsrec_open_vswitch *);
-static void bridge_del_dps(void);
-static bool bridge_add_dp(struct bridge *);
+static void bridge_del_ofprotos(void);
+static bool bridge_add_ofprotos(struct bridge *);
 static void bridge_create(const struct ovsrec_bridge *);
 static void bridge_destroy(struct bridge *);
 static struct bridge *bridge_lookup(const char *name);
@@ -395,7 +394,7 @@ bridge_reconfigure(const struct ovsrec_open_vswitch *ovs_cfg)
      * that port already belongs to a different datapath, so we must do all
      * port deletions before any port additions.  A datapath always has a
      * "local port" so we must delete not-configured datapaths too. */
-    bridge_del_dps();
+    bridge_del_ofprotos();
     HMAP_FOR_EACH (br, node, &all_bridges) {
         if (br->ofproto) {
             bridge_del_ofproto_ports(br);
@@ -409,7 +408,7 @@ bridge_reconfigure(const struct ovsrec_open_vswitch *ovs_cfg)
      * has at least one iface, every "struct iface" has a valid ofp_port and
      * netdev. */
     HMAP_FOR_EACH_SAFE (br, next, node, &all_bridges) {
-        if (!br->ofproto && !bridge_add_dp(br)) {
+        if (!br->ofproto && !bridge_add_ofprotos(br)) {
             bridge_destroy(br);
         }
     }
@@ -451,40 +450,35 @@ bridge_reconfigure(const struct ovsrec_open_vswitch *ovs_cfg)
     daemonize_complete();
 }
 
-/* Iterate over all system dpifs and delete any of them that do not have a
+/* Iterate over all ofprotos and delete any of them that do not have a
  * configured bridge or that are the wrong type. */
 static void
-bridge_del_dps(void)
+bridge_del_ofprotos(void)
 {
-    struct sset dpif_names;
-    struct sset dpif_types;
+    struct sset names;
+    struct sset types;
     const char *type;
 
-    sset_init(&dpif_names);
-    sset_init(&dpif_types);
-    dp_enumerate_types(&dpif_types);
-    SSET_FOR_EACH (type, &dpif_types) {
+    sset_init(&names);
+    sset_init(&types);
+    ofproto_enumerate_types(&types);
+    SSET_FOR_EACH (type, &types) {
         const char *name;
 
-        dp_enumerate_names(type, &dpif_names);
-        SSET_FOR_EACH (name, &dpif_names) {
+        ofproto_enumerate_names(type, &names);
+        SSET_FOR_EACH (name, &names) {
             struct bridge *br = bridge_lookup(name);
             if (!br || strcmp(type, br->type)) {
-                struct dpif *dpif;
-
-                if (!dpif_open(name, type, &dpif)) {
-                    dpif_delete(dpif);
-                    dpif_close(dpif);
-                }
+                ofproto_delete(name, type);
             }
         }
     }
-    sset_destroy(&dpif_names);
-    sset_destroy(&dpif_types);
+    sset_destroy(&names);
+    sset_destroy(&types);
 }
 
 static bool
-bridge_add_dp(struct bridge *br)
+bridge_add_ofprotos(struct bridge *br)
 {
     int error = ofproto_create(br->name, br->type, &br->ofproto);
     if (error) {
@@ -743,8 +737,8 @@ add_del_bridges(const struct ovsrec_open_vswitch *cfg)
      * Update 'cfg' of bridges that still exist. */
     HMAP_FOR_EACH_SAFE (br, next, node, &all_bridges) {
         br->cfg = shash_find_data(&new_br, br->name);
-        if (!br->cfg || strcmp(br->type,
-                               dpif_normalize_type(br->cfg->datapath_type))) {
+        if (!br->cfg || strcmp(br->type, ofproto_normalize_type(
+                                   br->cfg->datapath_type))) {
             bridge_destroy(br);
         }
     }
@@ -864,7 +858,7 @@ bridge_refresh_ofp_port(struct bridge *br)
     }
 }
 
-/* Add a dpif port for any "struct iface" that doesn't have one.
+/* Add an ofproto port for any "struct iface" that doesn't have one.
  * Delete any "struct iface" for which this fails.
  * Delete any "struct port" that thereby ends up with no ifaces. */
 static void
@@ -1665,7 +1659,7 @@ bridge_create(const struct ovsrec_bridge *br_cfg)
     br = xzalloc(sizeof *br);
 
     br->name = xstrdup(br_cfg->name);
-    br->type = xstrdup(dpif_normalize_type(br_cfg->datapath_type));
+    br->type = xstrdup(ofproto_normalize_type(br_cfg->datapath_type));
     br->cfg = br_cfg;
     eth_addr_nicira_random(br->default_ea);
 
-- 
1.7.4.4




More information about the dev mailing list