bridge: Eliminate direct dependencies on dpif.
authorBen Pfaff <blp@nicira.com>
Wed, 4 May 2011 17:15:31 +0000 (10:15 -0700)
committerBen Pfaff <blp@nicira.com>
Wed, 4 May 2011 17:15:31 +0000 (10:15 -0700)
The 'ofp_portp' argument of the new function ofproto_port_add() is always
set to NULL in this commit, but a future commit will use nonnull values.

ofproto/ofproto.c
ofproto/ofproto.h
utilities/ovs-openflowd.c
vswitchd/bridge.c

index 6aceedb7e8e36046b65275f0f00ff295db815aec..45c51253a9c86bce917a83b49290aa392e4a06ce 100644 (file)
@@ -358,7 +358,7 @@ ofproto_create(const char *datapath, const char *datapath_type,
     ofproto_unixctl_init();
 
     /* Connect to datapath and start listening for messages. */
-    error = dpif_open(datapath, datapath_type, &dpif);
+    error = dpif_create_and_open(datapath, datapath_type, &dpif);
     if (error) {
         VLOG_ERR("failed to open datapath %s: %s", datapath, strerror(error));
         return error;
@@ -675,8 +675,8 @@ ofproto_get_snoops(const struct ofproto *ofproto, struct sset *snoops)
     connmgr_get_snoops(ofproto->connmgr, snoops);
 }
 
-void
-ofproto_destroy(struct ofproto *p)
+static void
+ofproto_destroy__(struct ofproto *p, bool delete)
 {
     struct ofport *ofport, *next_ofport;
 
@@ -691,7 +691,15 @@ ofproto_destroy(struct ofproto *p)
     classifier_destroy(&p->cls);
     hmap_destroy(&p->facets);
 
+    if (delete) {
+        int error = dpif_delete(p->dpif);
+        if (error && error != ENOENT) {
+            VLOG_ERR("bridge %s: failed to destroy (%s)",
+                     dpif_name(p->dpif), strerror(error));
+        }
+    }
     dpif_close(p->dpif);
+
     netdev_monitor_destroy(p->netdev_monitor);
     HMAP_FOR_EACH_SAFE (ofport, next_ofport, hmap_node, &p->ports) {
         hmap_remove(&p->ports, &ofport->hmap_node);
@@ -715,6 +723,18 @@ ofproto_destroy(struct ofproto *p)
     free(p);
 }
 
+void
+ofproto_destroy(struct ofproto *p)
+{
+    ofproto_destroy__(p, false);
+}
+
+void
+ofproto_destroy_and_delete(struct ofproto *p)
+{
+    ofproto_destroy__(p, true);
+}
+
 int
 ofproto_run(struct ofproto *p)
 {
@@ -893,7 +913,134 @@ ofproto_free_ofproto_controller_info(struct shash *info)
     shash_destroy(info);
 }
 
-/* Deletes port number 'odp_port' from the datapath for 'ofproto'.
+/* Makes a deep copy of 'old' into 'port'. */
+void
+ofproto_port_clone(struct ofproto_port *port, const struct ofproto_port *old)
+{
+    port->name = xstrdup(old->name);
+    port->type = xstrdup(old->type);
+    port->ofp_port = old->ofp_port;
+}
+
+/* Frees memory allocated to members of 'ofproto_port'.
+ *
+ * Do not call this function on an ofproto_port obtained from
+ * ofproto_port_dump_next(): that function retains ownership of the data in the
+ * ofproto_port. */
+void
+ofproto_port_destroy(struct ofproto_port *ofproto_port)
+{
+    free(ofproto_port->name);
+    free(ofproto_port->type);
+}
+
+/* Converts a dpif_port into an ofproto_port.
+ *
+ * This only makes a shallow copy, so make sure that the dpif_port doesn't get
+ * freed while the ofproto_port is still in use.  You can choose to free the
+ * ofproto_port instead of the dpif_port. */
+static void
+ofproto_port_from_dpif_port(struct ofproto_port *ofproto_port,
+                            struct dpif_port *dpif_port)
+{
+    ofproto_port->name = dpif_port->name;
+    ofproto_port->type = dpif_port->type;
+    ofproto_port->ofp_port = odp_port_to_ofp_port(dpif_port->port_no);
+}
+
+/* Initializes 'dump' to begin dumping the ports in an ofproto.
+ *
+ * This function provides no status indication.  An error status for the entire
+ * dump operation is provided when it is completed by calling
+ * ofproto_port_dump_done().
+ */
+void
+ofproto_port_dump_start(struct ofproto_port_dump *dump,
+                        const struct ofproto *ofproto)
+{
+    struct dpif_port_dump *dpif_dump;
+
+    dump->state = dpif_dump = xmalloc(sizeof *dpif_dump);
+    dpif_port_dump_start(dpif_dump, ofproto->dpif);
+}
+
+/* Attempts to retrieve another port from 'dump', which must have been created
+ * with ofproto_port_dump_start().  On success, stores a new ofproto_port into
+ * 'port' and returns true.  On failure, returns false.
+ *
+ * Failure might indicate an actual error or merely that the last port has been
+ * dumped.  An error status for the entire dump operation is provided when it
+ * is completed by calling ofproto_port_dump_done().
+ *
+ * The ofproto owns the data stored in 'port'.  It will remain valid until at
+ * least the next time 'dump' is passed to ofproto_port_dump_next() or
+ * ofproto_port_dump_done(). */
+bool
+ofproto_port_dump_next(struct ofproto_port_dump *dump,
+                       struct ofproto_port *port)
+{
+    struct dpif_port_dump *dpif_dump = dump->state;
+    struct dpif_port dpif_port;
+    bool ok;
+
+    ok = dpif_port_dump_next(dpif_dump, &dpif_port);
+    if (ok) {
+        ofproto_port_from_dpif_port(port, &dpif_port);
+    }
+    return ok;
+}
+
+/* Completes port table dump operation 'dump', which must have been created
+ * with ofproto_port_dump_start().  Returns 0 if the dump operation was
+ * error-free, otherwise a positive errno value describing the problem. */
+int
+ofproto_port_dump_done(struct ofproto_port_dump *dump)
+{
+    struct dpif_port_dump *dpif_dump = dump->state;
+    int error = dpif_port_dump_done(dpif_dump);
+    free(dpif_dump);
+    return error;
+}
+
+/* Attempts to add 'netdev' as a port on 'ofproto'.  If successful, returns 0
+ * and sets '*ofp_portp' to the new port's port OpenFlow number (if 'ofp_portp'
+ * is non-null).  On failure, returns a positive errno value and sets
+ * '*ofp_portp' to OFPP_NONE (if 'ofp_portp' is non-null). */
+int
+ofproto_port_add(struct ofproto *ofproto, struct netdev *netdev,
+                 uint16_t *ofp_portp)
+{
+    uint16_t odp_port;
+    int error;
+
+    error = dpif_port_add(ofproto->dpif, netdev, &odp_port);
+    if (ofp_portp) {
+        *ofp_portp = error ? OFPP_NONE : odp_port_to_ofp_port(odp_port);
+    }
+    return error;
+}
+
+/* Looks up a port named 'devname' in 'ofproto'.  On success, returns 0 and
+ * initializes '*port' appropriately; on failure, returns a positive errno
+ * value.
+ *
+ * The caller owns the data in 'port' and must free it with
+ * ofproto_port_destroy() when it is no longer needed. */
+int
+ofproto_port_query_by_name(const struct ofproto *ofproto, const char *devname,
+                           struct ofproto_port *port)
+{
+    struct dpif_port dpif_port;
+    int error;
+
+    error = dpif_port_query_by_name(ofproto->dpif, devname, &dpif_port);
+    if (!error) {
+        ofproto_port_from_dpif_port(port, &dpif_port);
+    }
+    return error;
+}
+
+/* Deletes port number 'ofp_port' from the datapath for 'ofproto'.
  *
  * This is almost the same as calling dpif_port_del() directly on the
  * datapath, but it also makes 'ofproto' close its open netdev for the port
@@ -904,8 +1051,9 @@ ofproto_free_ofproto_controller_info(struct shash *info)
  *
  * Returns 0 if successful, otherwise a positive errno. */
 int
-ofproto_port_del(struct ofproto *ofproto, uint16_t odp_port)
+ofproto_port_del(struct ofproto *ofproto, uint16_t ofp_port)
 {
+    uint32_t odp_port = ofp_port_to_odp_port(ofp_port);
     struct ofport *ofport = get_port(ofproto, odp_port);
     const char *name = ofport ? netdev_get_name(ofport->netdev) : "<unknown>";
     int error;
@@ -3022,6 +3170,15 @@ ofproto_get_all_flows(struct ofproto *p, struct ds *results)
     }
 }
 
+/* Obtains the NetFlow engine type and engine ID for 'ofproto' into
+ * '*engine_type' and '*engine_id', respectively. */
+void
+ofproto_get_netflow_ids(const struct ofproto *ofproto,
+                        uint8_t *engine_type, uint8_t *engine_id)
+{
+    dpif_get_netflow_ids(ofproto->dpif, engine_type, engine_id);
+}
+
 static void
 query_aggregate_stats(struct ofproto *ofproto, struct cls_rule *target,
                       ovs_be16 out_port, uint8_t table_id,
index de5b92d1947d519e127fef259881534ff931387d..4860ba36441f006857aa97fc8921bc942bea83ab 100644 (file)
@@ -33,6 +33,8 @@ extern "C" {
 
 struct cfm;
 struct cls_rule;
+struct dpif_port;
+struct netdev;
 struct nlattr;
 struct ofhooks;
 struct ofproto;
@@ -97,15 +99,54 @@ int ofproto_create(const char *datapath, const char *datapath_type,
                    const struct ofhooks *, void *aux,
                    struct ofproto **ofprotop);
 void ofproto_destroy(struct ofproto *);
+void ofproto_destroy_and_delete(struct ofproto *);
 int ofproto_run(struct ofproto *);
 int ofproto_run1(struct ofproto *);
 int ofproto_run2(struct ofproto *, bool revalidate_all);
 void ofproto_wait(struct ofproto *);
 bool ofproto_is_alive(const struct ofproto *);
 
-int ofproto_port_del(struct ofproto *, uint16_t odp_port);
+/* A port within an OpenFlow switch.
+ *
+ * 'name' and 'type' are suitable for passing to netdev_open(). */
+struct ofproto_port {
+    char *name;                 /* Network device name, e.g. "eth0". */
+    char *type;                 /* Network device type, e.g. "system". */
+    uint16_t ofp_port;          /* OpenFlow port number. */
+};
+void ofproto_port_clone(struct ofproto_port *, const struct ofproto_port *);
+void ofproto_port_destroy(struct ofproto_port *);
+
+struct ofproto_port_dump {
+    const struct ofproto *ofproto;
+    int error;
+    void *state;
+};
+void ofproto_port_dump_start(struct ofproto_port_dump *,
+                             const struct ofproto *);
+bool ofproto_port_dump_next(struct ofproto_port_dump *, struct ofproto_port *);
+int ofproto_port_dump_done(struct ofproto_port_dump *);
+
+/* Iterates through each DPIF_PORT in OFPROTO, using DUMP as state.
+ *
+ * Arguments all have pointer type.
+ *
+ * If you break out of the loop, then you need to free the dump structure by
+ * hand using ofproto_port_dump_done(). */
+#define OFPROTO_PORT_FOR_EACH(OFPROTO_PORT, DUMP, OFPROTO)  \
+    for (ofproto_port_dump_start(DUMP, OFPROTO);            \
+         (ofproto_port_dump_next(DUMP, OFPROTO_PORT)        \
+          ? true                                            \
+          : (ofproto_port_dump_done(DUMP), false));         \
+        )
+
+int ofproto_port_add(struct ofproto *, struct netdev *, uint16_t *ofp_portp);
+int ofproto_port_del(struct ofproto *, uint16_t ofp_port);
 bool ofproto_port_is_floodable(struct ofproto *, uint16_t odp_port);
 
+int ofproto_port_query_by_name(const struct ofproto *, const char *devname,
+                               struct ofproto_port *);
+
 /* Top-level configuration. */
 void ofproto_set_datapath_id(struct ofproto *, uint64_t datapath_id);
 void ofproto_set_controllers(struct ofproto *,
@@ -138,6 +179,8 @@ void ofproto_get_listeners(const struct ofproto *, struct sset *);
 bool ofproto_has_snoops(const struct ofproto *);
 void ofproto_get_snoops(const struct ofproto *, struct sset *);
 void ofproto_get_all_flows(struct ofproto *p, struct ds *);
+void ofproto_get_netflow_ids(const struct ofproto *,
+                             uint8_t *engine_type, uint8_t *engine_id);
 
 /* Functions for use by ofproto implementation modules, not by clients. */
 void ofproto_add_flow(struct ofproto *, const struct cls_rule *,
index 86f5ca5926e6b755e7b6a99d5a819f74a81abe7a..f54eb355d30694b51cb48d6266b26784d26166d4 100644 (file)
@@ -93,7 +93,6 @@ main(int argc, char *argv[])
     struct ofproto *ofproto;
     struct ofsettings s;
     int error;
-    struct dpif *dpif;
     struct netflow_options nf_options;
     const char *port;
     bool exiting;
@@ -116,9 +115,10 @@ main(int argc, char *argv[])
     VLOG_INFO("Open vSwitch version %s", VERSION BUILDNR);
     VLOG_INFO("OpenFlow protocol version 0x%02x", OFP_VERSION);
 
-    error = dpif_create_and_open(s.dp_name, s.dp_type, &dpif);
+    error = ofproto_create(s.dp_name, s.dp_type, NULL, NULL, &ofproto);
     if (error) {
-        VLOG_FATAL("could not create datapath (%s)", strerror(error));
+        VLOG_FATAL("could not initialize OpenFlow switch (%s)",
+                   strerror(error));
     }
 
     /* Add ports to the datapath if requested by the user. */
@@ -131,7 +131,7 @@ main(int argc, char *argv[])
                        port, strerror(error));
         }
 
-        error = dpif_port_add(dpif, netdev, NULL);
+        error = ofproto_port_add(ofproto, netdev, NULL);
         if (error) {
             VLOG_FATAL("failed to add %s as a port (%s)",
                        port, strerror(error));
@@ -140,12 +140,7 @@ main(int argc, char *argv[])
         netdev_close(netdev);
     }
 
-    /* Start OpenFlow processing. */
-    error = ofproto_create(s.dp_name, s.dp_type, NULL, NULL, &ofproto);
-    if (error) {
-        VLOG_FATAL("could not initialize openflow switch (%s)",
-                   strerror(error));
-    }
+    /* Configure OpenFlow switch. */
     if (s.datapath_id) {
         ofproto_set_datapath_id(ofproto, s.datapath_id);
     }
@@ -188,7 +183,7 @@ main(int argc, char *argv[])
         poll_block();
     }
 
-    dpif_close(dpif);
+    ofproto_destroy(ofproto);
 
     return 0;
 }
index ecde7160c0d258c6b6d68413c723aaf21b10d07b..898c455c95444dee0e992d712982b1a9a0d5bed9 100644 (file)
@@ -171,12 +171,9 @@ struct bridge {
     /* OpenFlow switch processing. */
     struct ofproto *ofproto;    /* OpenFlow switch. */
 
-    /* Kernel datapath information. */
-    struct dpif *dpif;          /* Datapath. */
-    struct hmap ifaces;         /* "struct iface"s indexed by dp_ifidx. */
-
     /* Bridge ports. */
     struct hmap ports;          /* "struct port"s indexed by name. */
+    struct hmap ifaces;         /* "struct iface"s indexed by dp_ifidx. */
     struct hmap iface_by_name;  /* "struct iface"s indexed by name. */
 
     /* Bonding. */
@@ -542,18 +539,19 @@ 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. */
     LIST_FOR_EACH (br, node, &all_bridges) {
-        struct dpif_port_dump dump;
+        struct ofproto_port ofproto_port;
+        struct ofproto_port_dump dump;
         struct shash want_ifaces;
-        struct dpif_port dpif_port;
 
         bridge_get_all_ifaces(br, &want_ifaces);
-        DPIF_PORT_FOR_EACH (&dpif_port, &dump, br->dpif) {
-            if (!shash_find(&want_ifaces, dpif_port.name)
-                && strcmp(dpif_port.name, br->name)) {
-                int retval = dpif_port_del(br->dpif, dpif_port.port_no);
+        OFPROTO_PORT_FOR_EACH (&ofproto_port, &dump, br->ofproto) {
+            if (!shash_find(&want_ifaces, ofproto_port.name)
+                && strcmp(ofproto_port.name, br->name)) {
+                int retval = ofproto_port_del(br->ofproto,
+                                              ofproto_port.ofp_port);
                 if (retval) {
                     VLOG_WARN("bridge %s: failed to remove %s interface (%s)",
-                              br->name, dpif_port.name, strerror(retval));
+                              br->name, ofproto_port.name, strerror(retval));
                 }
             }
         }
@@ -561,15 +559,15 @@ bridge_reconfigure(const struct ovsrec_open_vswitch *ovs_cfg)
     }
     LIST_FOR_EACH (br, node, &all_bridges) {
         struct shash cur_ifaces, want_ifaces;
-        struct dpif_port_dump dump;
-        struct dpif_port dpif_port;
+        struct ofproto_port ofproto_port;
+        struct ofproto_port_dump dump;
 
         /* Get the set of interfaces currently in this datapath. */
         shash_init(&cur_ifaces);
-        DPIF_PORT_FOR_EACH (&dpif_port, &dump, br->dpif) {
-            struct dpif_port *port_info = xmalloc(sizeof *port_info);
-            dpif_port_clone(port_info, &dpif_port);
-            shash_add(&cur_ifaces, dpif_port.name, port_info);
+        OFPROTO_PORT_FOR_EACH (&ofproto_port, &dump, br->ofproto) {
+            struct ofproto_port *port_info = xmalloc(sizeof *port_info);
+            ofproto_port_clone(port_info, &ofproto_port);
+            shash_add(&cur_ifaces, ofproto_port.name, port_info);
         }
 
         /* Get the set of interfaces we want on this datapath. */
@@ -579,25 +577,26 @@ bridge_reconfigure(const struct ovsrec_open_vswitch *ovs_cfg)
         SHASH_FOR_EACH (node, &want_ifaces) {
             const char *if_name = node->name;
             struct iface *iface = node->data;
-            struct dpif_port *dpif_port;
+            struct ofproto_port *ofproto_port;
             const char *type;
             int error;
 
             type = iface ? iface->type : "internal";
-            dpif_port = shash_find_data(&cur_ifaces, if_name);
+            ofproto_port = shash_find_data(&cur_ifaces, if_name);
 
             /* If we have a port or a netdev already, and it's not the type we
              * want, then delete the port (if any) and close the netdev (if
              * any). */
-            if ((dpif_port && strcmp(dpif_port->type, type))
+            if ((ofproto_port && strcmp(ofproto_port->type, type))
                 || (iface && iface->netdev
                     && strcmp(type, netdev_get_type(iface->netdev)))) {
-                if (dpif_port) {
-                    error = ofproto_port_del(br->ofproto, dpif_port->port_no);
+                if (ofproto_port) {
+                    error = ofproto_port_del(br->ofproto,
+                                             ofproto_port->ofp_port);
                     if (error) {
                         continue;
                     }
-                    dpif_port = NULL;
+                    ofproto_port = NULL;
                 }
                 if (iface) {
                     if (iface->port->bond) {
@@ -614,7 +613,7 @@ bridge_reconfigure(const struct ovsrec_open_vswitch *ovs_cfg)
 
             /* If the port doesn't exist or we don't have the netdev open,
              * we need to do more work. */
-            if (!dpif_port || (iface && !iface->netdev)) {
+            if (!ofproto_port || (iface && !iface->netdev)) {
                 struct netdev_options options;
                 struct netdev *netdev;
                 struct shash args;
@@ -641,8 +640,8 @@ bridge_reconfigure(const struct ovsrec_open_vswitch *ovs_cfg)
                 }
 
                 /* Then add the port if we haven't already. */
-                if (!dpif_port) {
-                    error = dpif_port_add(br->dpif, netdev, NULL);
+                if (!ofproto_port) {
+                    error = ofproto_port_add(br->ofproto, netdev, NULL);
                     if (error) {
                         netdev_close(netdev);
                         if (error == EFBIG) {
@@ -676,8 +675,8 @@ bridge_reconfigure(const struct ovsrec_open_vswitch *ovs_cfg)
         shash_destroy(&want_ifaces);
 
         SHASH_FOR_EACH (node, &cur_ifaces) {
-            struct dpif_port *port_info = node->data;
-            dpif_port_destroy(port_info);
+            struct ofproto_port *port_info = node->data;
+            ofproto_port_destroy(port_info);
             free(port_info);
         }
         shash_destroy(&cur_ifaces);
@@ -752,7 +751,8 @@ bridge_reconfigure(const struct ovsrec_open_vswitch *ovs_cfg)
 
             memset(&opts, 0, sizeof opts);
 
-            dpif_get_netflow_ids(br->dpif, &opts.engine_type, &opts.engine_id);
+            ofproto_get_netflow_ids(br->ofproto,
+                                    &opts.engine_type, &opts.engine_id);
             if (nf_cfg->engine_type) {
                 opts.engine_type = *nf_cfg->engine_type;
             }
@@ -1638,20 +1638,11 @@ bridge_create(const struct ovsrec_bridge *br_cfg)
     assert(!bridge_lookup(br_cfg->name));
     br = xzalloc(sizeof *br);
 
-    error = dpif_create_and_open(br_cfg->name, br_cfg->datapath_type,
-                                 &br->dpif);
-    if (error) {
-        free(br);
-        return NULL;
-    }
-
     error = ofproto_create(br_cfg->name, br_cfg->datapath_type, &bridge_ofhooks,
                            br, &br->ofproto);
     if (error) {
         VLOG_ERR("failed to create switch %s: %s", br_cfg->name,
                  strerror(error));
-        dpif_delete(br->dpif);
-        dpif_close(br->dpif);
         free(br);
         return NULL;
     }
@@ -1679,7 +1670,6 @@ bridge_destroy(struct bridge *br)
 {
     if (br) {
         struct port *port, *next;
-        int error;
         int i;
 
         HMAP_FOR_EACH_SAFE (port, next, hmap_node, &br->ports) {
@@ -1689,13 +1679,7 @@ bridge_destroy(struct bridge *br)
             mirror_destroy(br->mirrors[i]);
         }
         list_remove(&br->node);
-        ofproto_destroy(br->ofproto);
-        error = dpif_delete(br->dpif);
-        if (error && error != ENOENT) {
-            VLOG_ERR("bridge %s: failed to destroy (%s)",
-                     br->name, strerror(error));
-        }
-        dpif_close(br->dpif);
+        ofproto_destroy_and_delete(br->ofproto);
         mac_learning_destroy(br->ml);
         hmap_destroy(&br->ifaces);
         hmap_destroy(&br->ports);
@@ -1826,16 +1810,18 @@ bridge_reconfigure_one(struct bridge *br)
                       br->name, name);
         }
     }
-    if (!shash_find(&new_ports, br->name)) {
-        struct dpif_port dpif_port;
+    if (bridge_get_controllers(br, NULL)
+        && !shash_find(&new_ports, br->name)) {
+        struct ofproto_port local_port;
         char *type;
+        int error;
 
         VLOG_WARN("bridge %s: no port named %s, synthesizing one",
                   br->name, br->name);
 
-        dpif_port_query_by_number(br->dpif, ODPP_LOCAL, &dpif_port);
-        type = xstrdup(dpif_port.type ? dpif_port.type : "internal");
-        dpif_port_destroy(&dpif_port);
+        error = ofproto_port_query_by_name(br->ofproto, br->name, &local_port);
+        type = xstrdup(error ? "internal" : local_port.type);
+        ofproto_port_destroy(&local_port);
 
         br->synth_local_port.interfaces = &br->synth_local_ifacep;
         br->synth_local_port.n_interfaces = 1;
@@ -2073,8 +2059,8 @@ bridge_get_all_ifaces(const struct bridge *br, struct shash *ifaces)
 static void
 bridge_fetch_dp_ifaces(struct bridge *br)
 {
-    struct dpif_port_dump dump;
-    struct dpif_port dpif_port;
+    struct ofproto_port ofproto_port;
+    struct ofproto_port_dump dump;
     struct port *port;
 
     /* Reset all interface numbers. */
@@ -2087,17 +2073,19 @@ bridge_fetch_dp_ifaces(struct bridge *br)
     }
     hmap_clear(&br->ifaces);
 
-    DPIF_PORT_FOR_EACH (&dpif_port, &dump, br->dpif) {
-        struct iface *iface = iface_lookup(br, dpif_port.name);
+    OFPROTO_PORT_FOR_EACH (&ofproto_port, &dump, br->ofproto) {
+        struct iface *iface = iface_lookup(br, ofproto_port.name);
         if (iface) {
+            uint32_t odp_port = ofp_port_to_odp_port(ofproto_port.ofp_port);
+
             if (iface->dp_ifidx >= 0) {
                 VLOG_WARN("bridge %s: interface %s reported twice",
-                          br->name, dpif_port.name);
-            } else if (iface_from_dp_ifidx(br, dpif_port.port_no)) {
+                          br->name, ofproto_port.name);
+            } else if (iface_from_dp_ifidx(br, odp_port)) {
                 VLOG_WARN("bridge %s: interface %"PRIu16" reported twice",
-                          br->name, dpif_port.port_no);
+                          br->name, odp_port);
             } else {
-                iface->dp_ifidx = dpif_port.port_no;
+                iface->dp_ifidx = odp_port;
                 hmap_insert(&br->ifaces, &iface->dp_ifidx_node,
                             hash_int(iface->dp_ifidx, 0));
             }