[ovs-dev] [netlink v3 08/16] datapath: Change listing ports to use an iterator concept.

Ben Pfaff blp at nicira.com
Thu Dec 30 00:56:29 UTC 2010


One of the goals for Open vSwitch is to decouple kernel and userspace
software, so that either one can be upgraded or rolled back independent of
the other.  To do this in full generality, it must be possible to add new
features to the kernel vport layer without changing userspace software.  In
turn, that means that the odp_port structure must become variable-length.
This does not, however, fit in well with the ODP_PORT_LIST ioctl in its
current form, because that would require userspace to know how much space
to allocate for each port in advance, or to allocate as much space as
could possibly be needed.  Neither choice is very attractive.

This commit prepares for a different solution, by replacing ODP_PORT_LIST
by a new ioctl ODP_VPORT_DUMP that retrieves information about a single
vport from the datapath on each call.  It is much cleaner to allocate the
maximum amount of space for a single vport than to do so for possibly a
large number of vports.

It would be faster to retrieve a number of vports in batch instead of just
one at a time, but that will naturally happen later when the kernel
datapath interface is changed to use Netlink, so this patch does not bother
with it.

Signed-off-by: Ben Pfaff <blp at nicira.com>
---
 datapath/datapath.c                     |   65 +++++++++-------------
 datapath/odp-compat.h                   |    8 +-
 include/openvswitch/datapath-protocol.h |   18 +++++-
 lib/dpif-linux.c                        |   42 +++++++++-----
 lib/dpif-netdev.c                       |   44 +++++++++++----
 lib/dpif-provider.h                     |   23 ++++++--
 lib/dpif.c                              |   92 ++++++++++++++++---------------
 lib/dpif.h                              |   23 +++++++-
 ofproto/ofproto.c                       |   31 +++-------
 utilities/ovs-dpctl.c                   |   29 ++--------
 vswitchd/bridge.c                       |   71 +++++++++++++-----------
 11 files changed, 246 insertions(+), 200 deletions(-)

diff --git a/datapath/datapath.c b/datapath/datapath.c
index 64b5193..b0dfb80 100644
--- a/datapath/datapath.c
+++ b/datapath/datapath.c
@@ -1389,37 +1389,27 @@ static int query_port(struct datapath *dp, struct odp_port __user *uport)
 	return put_port(vport, uport);
 }
 
-static int do_list_ports(struct datapath *dp, struct odp_port __user *uports,
-			 int n_ports)
+static int do_dump_port(struct datapath *dp, struct odp_vport_dump *dump)
 {
-	int idx = 0;
-	if (n_ports) {
-		struct vport *p;
-
-		list_for_each_entry_rcu (p, &dp->port_list, node) {
-			if (put_port(p, &uports[idx]))
-				return -EFAULT;
-			if (idx++ >= n_ports)
-				break;
-		}
+	u32 port_no;
+
+	for (port_no = dump->port_no; port_no < DP_MAX_PORTS; port_no++) {
+		struct vport *vport = dp->ports[port_no];
+		if (vport)
+			return put_port(vport, (struct odp_port __force __user*)dump->port);
 	}
-	return idx;
+
+	return put_user('\0', (char __force __user*)&dump->port->devname[0]);
 }
 
-static int list_ports(struct datapath *dp, struct odp_portvec __user *upv)
+static int dump_port(struct datapath *dp, struct odp_vport_dump __user *udump)
 {
-	struct odp_portvec pv;
-	int retval;
+	struct odp_vport_dump dump;
 
-	if (copy_from_user(&pv, upv, sizeof pv))
+	if (copy_from_user(&dump, udump, sizeof dump))
 		return -EFAULT;
 
-	retval = do_list_ports(dp, (struct odp_port __user __force *)pv.ports,
-			       pv.n_ports);
-	if (retval < 0)
-		return retval;
-
-	return put_user(retval, &upv->n_ports);
+	return do_dump_port(dp, &dump);
 }
 
 static int get_listen_mask(const struct file *f)
@@ -1544,8 +1534,8 @@ static long openvswitch_ioctl(struct file *f, unsigned int cmd,
 		err = query_port(dp, (struct odp_port __user *)argp);
 		break;
 
-	case ODP_VPORT_LIST:
-		err = list_ports(dp, (struct odp_portvec __user *)argp);
+	case ODP_VPORT_DUMP:
+		err = dump_port(dp, (struct odp_vport_dump __user *)argp);
 		break;
 
 	case ODP_FLOW_FLUSH:
@@ -1592,19 +1582,18 @@ static int dp_has_packet_of_interest(struct datapath *dp, int listeners)
 }
 
 #ifdef CONFIG_COMPAT
-static int compat_list_ports(struct datapath *dp, struct compat_odp_portvec __user *upv)
+static int compat_dump_port(struct datapath *dp, struct compat_odp_vport_dump __user *compat)
 {
-	struct compat_odp_portvec pv;
-	int retval;
+	struct odp_vport_dump dump;
+	compat_uptr_t port;
 
-	if (copy_from_user(&pv, upv, sizeof pv))
+	if (!access_ok(VERIFY_READ, compat, sizeof(struct compat_odp_vport_dump)) ||
+	    __get_user(port, &compat->port) ||
+	    __get_user(dump.port_no, &compat->port_no))
 		return -EFAULT;
 
-	retval = do_list_ports(dp, compat_ptr(pv.ports), pv.n_ports);
-	if (retval < 0)
-		return retval;
-
-	return put_user(retval, &upv->n_ports);
+	dump.port = (struct odp_port __force *)compat_ptr(port);
+	return do_dump_port(dp, &dump);
 }
 
 static int compat_get_flow(struct odp_flow *flow, const struct compat_odp_flow __user *compat)
@@ -1668,7 +1657,7 @@ static int compat_del_flow(struct datapath *dp, struct compat_odp_flow __user *u
 	if (compat_get_flow(&uf, ufp))
 		return -EFAULT;
 
-	flow = do_del_flow(dp, uf.key, uf.key_len);
+	flow = do_del_flow(dp, (const struct nlattr __force __user*)uf.key, uf.key_len);
 	if (IS_ERR(flow))
 		return PTR_ERR(flow);
 
@@ -1694,7 +1683,7 @@ static int compat_query_flows(struct datapath *dp,
 		if (compat_get_flow(&uf, ufp))
 			return -EFAULT;
 
-		error = flow_copy_from_user(&key, uf.key, uf.key_len);
+		error = flow_copy_from_user(&key, (const struct nlattr __force __user*)uf.key, uf.key_len);
 		if (error)
 			return error;
 
@@ -1831,8 +1820,8 @@ static long openvswitch_compat_ioctl(struct file *f, unsigned int cmd, unsigned
 		goto exit;
 
 	switch (cmd) {
-	case ODP_VPORT_LIST32:
-		err = compat_list_ports(dp, compat_ptr(argp));
+	case ODP_VPORT_DUMP32:
+		err = compat_dump_port(dp, compat_ptr(argp));
 		break;
 
 	case ODP_FLOW_PUT32:
diff --git a/datapath/odp-compat.h b/datapath/odp-compat.h
index 4da630c..eca6d8b 100644
--- a/datapath/odp-compat.h
+++ b/datapath/odp-compat.h
@@ -15,7 +15,7 @@
 #include "openvswitch/datapath-protocol.h"
 #include <linux/compat.h>
 
-#define ODP_VPORT_LIST32	_IOWR('O', 10, struct compat_odp_portvec)
+#define ODP_VPORT_DUMP32	_IOWR('O', 10, struct compat_odp_vport_dump)
 #define ODP_FLOW_GET32		_IOWR('O', 13, struct compat_odp_flowvec)
 #define ODP_FLOW_PUT32		_IOWR('O', 14, struct compat_odp_flow)
 #define ODP_FLOW_DUMP32		_IOWR('O', 15, struct compat_odp_flow_dump)
@@ -23,9 +23,9 @@
 #define ODP_EXECUTE32		_IOR('O', 18, struct compat_odp_execute)
 #define ODP_FLOW_DEL32		_IOWR('O', 17, struct compat_odp_flow)
 
-struct compat_odp_portvec {
-	compat_uptr_t ports;
-	u32 n_ports;
+struct compat_odp_vport_dump {
+	compat_uptr_t port;
+	u32 port_no;
 };
 
 struct compat_odp_flow {
diff --git a/include/openvswitch/datapath-protocol.h b/include/openvswitch/datapath-protocol.h
index aee83c5..5c0adbf 100644
--- a/include/openvswitch/datapath-protocol.h
+++ b/include/openvswitch/datapath-protocol.h
@@ -85,7 +85,7 @@
 #define ODP_VPORT_ATTACH        _IOR('O', 7, struct odp_port)
 #define ODP_VPORT_DETACH        _IOR('O', 8, int)
 #define ODP_VPORT_QUERY         _IOWR('O', 9, struct odp_port)
-#define ODP_VPORT_LIST          _IOWR('O', 10, struct odp_portvec)
+#define ODP_VPORT_DUMP          _IOWR('O', 10, struct odp_vport_dump)
 
 #define ODP_FLOW_GET            _IOWR('O', 13, struct odp_flowvec)
 #define ODP_FLOW_PUT            _IOWR('O', 14, struct odp_flow)
@@ -184,9 +184,19 @@ struct odp_port {
     __aligned_u64 config[VPORT_CONFIG_SIZE / 8]; /* type-specific */
 };
 
-struct odp_portvec {
-    struct odp_port *ports;
-    uint32_t n_ports;
+/**
+ * struct odp_vport_dump - ODP_VPORT_DUMP argument.
+ * @port: Points to port structure to fill in.
+ * @port_no: Minimum port number of interest.
+ *
+ * Used to iterate through vports one at a time.  The kernel fills in @port
+ * with the information for the configured port with the smallest port number
+ * greater than or equal to @port_no.  If there is no such port, it sets
+ * @port->devname to the empty string.
+ */
+struct odp_vport_dump {
+    struct odp_port *port;
+    uint32_t port_no;
 };
 
 struct odp_flow_stats {
diff --git a/lib/dpif-linux.c b/lib/dpif-linux.c
index ea4cb75..2908964 100644
--- a/lib/dpif-linux.c
+++ b/lib/dpif-linux.c
@@ -323,25 +323,37 @@ dpif_linux_flow_flush(struct dpif *dpif_)
 }
 
 static int
-dpif_linux_port_list(const struct dpif *dpif_, struct odp_port *ports, int n)
+dpif_linux_port_dump_start(const struct dpif *dpif OVS_UNUSED, void **statep)
 {
-    struct odp_portvec pv;
-    unsigned int i;
+    *statep = xzalloc(sizeof(struct odp_vport_dump));
+    return 0;
+}
+
+static int
+dpif_linux_port_dump_next(const struct dpif *dpif, void *state,
+                          struct odp_port *port)
+{
+    struct odp_vport_dump *dump = state;
     int error;
 
-    pv.ports = ports;
-    pv.n_ports = n;
-    error = do_ioctl(dpif_, ODP_VPORT_LIST, &pv);
+    dump->port = port;
+    error = do_ioctl(dpif, ODP_VPORT_DUMP, dump);
     if (error) {
-        return -error;
-    }
-
-    for (i = 0; i < pv.n_ports; i++) {
-        struct odp_port *port = &pv.ports[i];
-
+        return error;
+    } else if (port->devname[0] == '\0') {
+        return EOF;
+    } else {
+        dump->port_no = port->port + 1;
         translate_vport_type_to_netdev_type(port);
+        return 0;
     }
-    return pv.n_ports;
+}
+
+static int
+dpif_linux_port_dump_done(const struct dpif *dpif OVS_UNUSED, void *state)
+{
+    free(state);
+    return 0;
 }
 
 static int
@@ -582,7 +594,9 @@ const struct dpif_class dpif_linux_class = {
     dpif_linux_port_del,
     dpif_linux_port_query_by_number,
     dpif_linux_port_query_by_name,
-    dpif_linux_port_list,
+    dpif_linux_port_dump_start,
+    dpif_linux_port_dump_next,
+    dpif_linux_port_dump_done,
     dpif_linux_port_poll,
     dpif_linux_port_poll_wait,
     dpif_linux_flow_get,
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 788eac2..9e71e8c 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -539,23 +539,41 @@ dpif_netdev_flow_flush(struct dpif *dpif)
     return 0;
 }
 
+struct dp_netdev_port_state {
+    uint32_t port_no;
+};
+
+static int
+dpif_netdev_port_dump_start(const struct dpif *dpif OVS_UNUSED, void **statep)
+{
+    *statep = xzalloc(sizeof(struct dp_netdev_port_state));
+    return 0;
+}
+
 static int
-dpif_netdev_port_list(const struct dpif *dpif, struct odp_port *ports, int n)
+dpif_netdev_port_dump_next(const struct dpif *dpif, void *state_,
+                           struct odp_port *odp_port)
 {
+    struct dp_netdev_port_state *state = state_;
     struct dp_netdev *dp = get_dp_netdev(dpif);
-    struct dp_netdev_port *port;
-    int i;
+    uint32_t port_no;
 
-    i = 0;
-    LIST_FOR_EACH (port, node, &dp->port_list) {
-        struct odp_port *odp_port = &ports[i];
-        if (i >= n) {
-            break;
+    for (port_no = state->port_no; port_no < MAX_PORTS; port_no++) {
+        struct dp_netdev_port *port = dp->ports[port_no];
+        if (port) {
+            answer_port_query(port, odp_port);
+            state->port_no = port_no + 1;
+            return 0;
         }
-        answer_port_query(port, odp_port);
-        i++;
     }
-    return dp->n_ports;
+    return EOF;
+}
+
+static int
+dpif_netdev_port_dump_done(const struct dpif *dpif OVS_UNUSED, void *state)
+{
+    free(state);
+    return 0;
 }
 
 static int
@@ -1362,7 +1380,9 @@ const struct dpif_class dpif_netdev_class = {
     dpif_netdev_port_del,
     dpif_netdev_port_query_by_number,
     dpif_netdev_port_query_by_name,
-    dpif_netdev_port_list,
+    dpif_netdev_port_dump_start,
+    dpif_netdev_port_dump_next,
+    dpif_netdev_port_dump_done,
     dpif_netdev_port_poll,
     dpif_netdev_port_poll_wait,
     dpif_netdev_flow_get,
diff --git a/lib/dpif-provider.h b/lib/dpif-provider.h
index bb7b2a8..649a492 100644
--- a/lib/dpif-provider.h
+++ b/lib/dpif-provider.h
@@ -153,11 +153,24 @@ struct dpif_class {
     int (*port_query_by_name)(const struct dpif *dpif, const char *devname,
                               struct odp_port *port);
 
-    /* Stores in 'ports' information about up to 'n' ports attached to 'dpif',
-     * in no particular order.  Returns the number of ports attached to 'dpif'
-     * (not the number stored), if successful, otherwise a negative errno
-     * value. */
-    int (*port_list)(const struct dpif *dpif, struct odp_port *ports, int n);
+    /* Attempts to begin dumping the ports in a dpif.  On success, returns 0
+     * and initializes '*statep' with any data needed for iteration.  On
+     * failure, returns a positive errno value. */
+    int (*port_dump_start)(const struct dpif *dpif, void **statep);
+
+    /* Attempts to retrieve another port from 'dpif' for 'state', which was
+     * initialized by a successful call to the 'port_dump_start' function for
+     * 'dpif'.  On success, stores a new odp_port into 'port' and returns 0.
+     * Returns EOF if the end of the port table has been reached, or a positive
+     * errno value on error.  This function will not be called again once it
+     * returns nonzero once for a given iteration (but the 'port_dump_done'
+     * function will be called afterward). */
+    int (*port_dump_next)(const struct dpif *dpif, void *state,
+                          struct odp_port *port);
+
+    /* Releases resources from 'dpif' for 'state', which was initialized by a
+     * successful call to the 'port_dump_start' function for 'dpif'.  */
+    int (*port_dump_done)(const struct dpif *dpif, void *state);
 
     /* Polls for changes in the set of ports in 'dpif'.  If the set of ports in
      * 'dpif' has changed, then this function should do one of the
diff --git a/lib/dpif.c b/lib/dpif.c
index b54ecaa..708b534 100644
--- a/lib/dpif.c
+++ b/lib/dpif.c
@@ -556,60 +556,62 @@ dpif_port_get_name(struct dpif *dpif, uint16_t port_no,
     return error;
 }
 
-/* Obtains a list of all the ports in 'dpif'.
+/* Initializes 'dump' to begin dumping the ports in a dpif.
  *
- * If successful, returns 0 and sets '*portsp' to point to an array of
- * appropriately initialized port structures and '*n_portsp' to the number of
- * ports in the array.  The caller is responsible for freeing '*portp' by
- * calling free().
+ * This function provides no status indication.  An error status for the entire
+ * dump operation is provided when it is completed by calling
+ * dpif_port_dump_done().
+ */
+void
+dpif_port_dump_start(struct dpif_port_dump *dump, const struct dpif *dpif)
+{
+    dump->dpif = dpif;
+    dump->error = dpif->dpif_class->port_dump_start(dpif, &dump->state);
+    log_operation(dpif, "port_dump_start", dump->error);
+}
+
+/* Attempts to retrieve another port from 'dump', which must have been
+ * initialized with dpif_port_dump_start().  On success, stores a new odp_port
+ * into 'port' and returns true.  On failure, returns false.
  *
- * On failure, returns a positive errno value and sets '*portsp' to NULL and
- * '*n_portsp' to 0. */
-int
-dpif_port_list(const struct dpif *dpif,
-               struct odp_port **portsp, size_t *n_portsp)
+ * 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 dpif_port_dump_done(). */
+bool
+dpif_port_dump_next(struct dpif_port_dump *dump, struct odp_port *port)
 {
-    struct odp_port *ports;
-    size_t n_ports = 0;
-    int error;
+    const struct dpif *dpif = dump->dpif;
 
-    for (;;) {
-        struct odp_stats stats;
-        int retval;
+    if (dump->error) {
+        return false;
+    }
 
-        error = dpif_get_dp_stats(dpif, &stats);
-        if (error) {
-            goto exit;
-        }
+    dump->error = dpif->dpif_class->port_dump_next(dpif, dump->state, port);
+    if (dump->error == EOF) {
+        VLOG_DBG_RL(&dpmsg_rl, "%s: dumped all ports", dpif_name(dpif));
+    } else {
+        log_operation(dpif, "port_dump_next", dump->error);
+    }
 
-        ports = xcalloc(stats.n_ports, sizeof *ports);
-        retval = dpif->dpif_class->port_list(dpif, ports, stats.n_ports);
-        if (retval < 0) {
-            /* Hard error. */
-            error = -retval;
-            free(ports);
-            goto exit;
-        } else if (retval <= stats.n_ports) {
-            /* Success. */
-            error = 0;
-            n_ports = retval;
-            goto exit;
-        } else {
-            /* Soft error: port count increased behind our back.  Try again. */
-            free(ports);
-        }
+    if (dump->error) {
+        dpif->dpif_class->port_dump_done(dpif, dump->state);
+        return false;
     }
+    return true;
+}
 
-exit:
-    if (error) {
-        *portsp = NULL;
-        *n_portsp = 0;
-    } else {
-        *portsp = ports;
-        *n_portsp = n_ports;
+/* Completes port table dump operation 'dump', which must have been initialized
+ * with dpif_port_dump_start().  Returns 0 if the dump operation was
+ * error-free, otherwise a positive errno value describing the problem. */
+int
+dpif_port_dump_done(struct dpif_port_dump *dump)
+{
+    const struct dpif *dpif = dump->dpif;
+    if (!dump->error) {
+        dump->error = dpif->dpif_class->port_dump_done(dpif, dump->state);
+        log_operation(dpif, "port_dump_done", dump->error);
     }
-    log_operation(dpif, "port_list", error);
-    return error;
+    return dump->error == EOF ? 0 : dump->error;
 }
 
 /* Polls for changes in the set of ports in 'dpif'.  If the set of ports in
diff --git a/lib/dpif.h b/lib/dpif.h
index 3e72539..7277647 100644
--- a/lib/dpif.h
+++ b/lib/dpif.h
@@ -69,7 +69,28 @@ int dpif_port_query_by_name(const struct dpif *, const char *devname,
                             struct odp_port *);
 int dpif_port_get_name(struct dpif *, uint16_t port_no,
                        char *name, size_t name_size);
-int dpif_port_list(const struct dpif *, struct odp_port **, size_t *n_ports);
+
+struct dpif_port_dump {
+    const struct dpif *dpif;
+    int error;
+    void *state;
+};
+void dpif_port_dump_start(struct dpif_port_dump *, const struct dpif *);
+bool dpif_port_dump_next(struct dpif_port_dump *, struct odp_port *);
+int dpif_port_dump_done(struct dpif_port_dump *);
+
+/* Iterates through each ODP_PORT in DPIF, 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 dpif_port_dump_done(). */
+#define DPIF_PORT_FOR_EACH(ODP_PORT, DUMP, DPIF)    \
+    for (dpif_port_dump_start(DUMP, DPIF);          \
+         (dpif_port_dump_next(DUMP, ODP_PORT)       \
+          ? true                                    \
+          : (dpif_port_dump_done(DUMP), false));    \
+        )
 
 int dpif_port_poll(const struct dpif *, char **devnamep);
 void dpif_port_poll_wait(const struct dpif *);
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 87e9d46..6f6d565 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -1482,12 +1482,11 @@ ofproto_flush_flows(struct ofproto *ofproto)
 static void
 reinit_ports(struct ofproto *p)
 {
+    struct dpif_port_dump dump;
     struct shash_node *node;
     struct shash devnames;
     struct ofport *ofport;
-    struct odp_port *odp_ports;
-    size_t n_odp_ports;
-    size_t i;
+    struct odp_port odp_port;
 
     COVERAGE_INC(ofproto_reinit_ports);
 
@@ -1495,11 +1494,9 @@ reinit_ports(struct ofproto *p)
     HMAP_FOR_EACH (ofport, hmap_node, &p->ports) {
         shash_add_once (&devnames, ofport->opp.name, NULL);
     }
-    dpif_port_list(p->dpif, &odp_ports, &n_odp_ports);
-    for (i = 0; i < n_odp_ports; i++) {
-        shash_add_once (&devnames, odp_ports[i].devname, NULL);
+    DPIF_PORT_FOR_EACH (&odp_port, &dump, p->dpif) {
+        shash_add_once (&devnames, odp_port.devname, NULL);
     }
-    free(odp_ports);
 
     SHASH_FOR_EACH (node, &devnames) {
         update_port(p, node->name);
@@ -1729,26 +1726,18 @@ update_port(struct ofproto *p, const char *devname)
 static int
 init_ports(struct ofproto *p)
 {
-    struct odp_port *ports;
-    size_t n_ports;
-    size_t i;
-    int error;
-
-    error = dpif_port_list(p->dpif, &ports, &n_ports);
-    if (error) {
-        return error;
-    }
+    struct dpif_port_dump dump;
+    struct odp_port odp_port;
 
-    for (i = 0; i < n_ports; i++) {
-        const struct odp_port *odp_port = &ports[i];
-        if (!ofport_conflicts(p, odp_port)) {
-            struct ofport *ofport = make_ofport(odp_port);
+    DPIF_PORT_FOR_EACH (&odp_port, &dump, p->dpif) {
+        if (!ofport_conflicts(p, &odp_port)) {
+            struct ofport *ofport = make_ofport(&odp_port);
             if (ofport) {
                 ofport_install(p, ofport);
             }
         }
     }
-    free(ports);
+
     return 0;
 }
 
diff --git a/utilities/ovs-dpctl.c b/utilities/ovs-dpctl.c
index 0291c0a..a824608 100644
--- a/utilities/ovs-dpctl.c
+++ b/utilities/ovs-dpctl.c
@@ -214,21 +214,6 @@ do_del_dp(int argc OVS_UNUSED, char *argv[])
     dpif_close(dpif);
 }
 
-static int
-compare_ports(const void *a_, const void *b_)
-{
-    const struct odp_port *a = a_;
-    const struct odp_port *b = b_;
-    return a->port < b->port ? -1 : a->port > b->port;
-}
-
-static void
-query_ports(struct dpif *dpif, struct odp_port **ports, size_t *n_ports)
-{
-    run(dpif_port_list(dpif, ports, n_ports), "listing ports");
-    qsort(*ports, *n_ports, sizeof **ports, compare_ports);
-}
-
 static void
 do_add_if(int argc OVS_UNUSED, char *argv[])
 {
@@ -346,10 +331,9 @@ do_del_if(int argc OVS_UNUSED, char *argv[])
 static void
 show_dpif(struct dpif *dpif)
 {
-    struct odp_port *ports;
+    struct dpif_port_dump dump;
+    struct odp_port odp_port;
     struct odp_stats stats;
-    size_t n_ports;
-    size_t i;
 
     printf("%s:\n", dpif_name(dpif));
     if (!dpif_get_dp_stats(dpif, &stats)) {
@@ -366,19 +350,16 @@ show_dpif(struct dpif *dpif)
         printf("\tqueues: max-miss:%"PRIu16", max-action:%"PRIu16"\n",
                stats.max_miss_queue, stats.max_action_queue);
     }
-    query_ports(dpif, &ports, &n_ports);
-    for (i = 0; i < n_ports; i++) {
-        const struct odp_port *p = &ports[i];
+    DPIF_PORT_FOR_EACH (&odp_port, &dump, dpif) {
         struct ds ds;
 
-        printf("\tport %u: %s", p->port, p->devname);
+        printf("\tport %u: %s", odp_port.port, odp_port.devname);
 
         ds_init(&ds);
-        format_odp_port_type(&ds, p);
+        format_odp_port_type(&ds, &odp_port);
         printf("%s\n", ds_cstr(&ds));
         ds_destroy(&ds);
     }
-    free(ports);
     dpif_close(dpif);
 }
 
diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
index 96a24fd..0cb3b9e 100644
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -592,38 +592,40 @@ 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 odp_port *dpif_ports;
-        size_t n_dpif_ports;
+        struct dpif_port_dump dump;
         struct shash want_ifaces;
+        struct odp_port odp_port;
 
-        dpif_port_list(br->dpif, &dpif_ports, &n_dpif_ports);
         bridge_get_all_ifaces(br, &want_ifaces);
-        for (i = 0; i < n_dpif_ports; i++) {
-            const struct odp_port *p = &dpif_ports[i];
-            if (!shash_find(&want_ifaces, p->devname)
-                && strcmp(p->devname, br->name)) {
-                int retval = dpif_port_del(br->dpif, p->port);
+        DPIF_PORT_FOR_EACH (&odp_port, &dump, br->dpif) {
+            if (!shash_find(&want_ifaces, odp_port.devname)
+                && strcmp(odp_port.devname, br->name)) {
+                int retval = dpif_port_del(br->dpif, odp_port.port);
                 if (retval) {
                     VLOG_ERR("failed to remove %s interface from %s: %s",
-                             p->devname, dpif_name(br->dpif),
+                             odp_port.devname, dpif_name(br->dpif),
                              strerror(retval));
                 }
             }
         }
         shash_destroy(&want_ifaces);
-        free(dpif_ports);
     }
     LIST_FOR_EACH (br, node, &all_bridges) {
-        struct odp_port *dpif_ports;
-        size_t n_dpif_ports;
+        struct dpif_port_info {
+            uint32_t port_no;
+            char *type;
+        };
         struct shash cur_ifaces, want_ifaces;
+        struct dpif_port_dump dump;
+        struct odp_port odp_port;
 
         /* Get the set of interfaces currently in this datapath. */
-        dpif_port_list(br->dpif, &dpif_ports, &n_dpif_ports);
         shash_init(&cur_ifaces);
-        for (i = 0; i < n_dpif_ports; i++) {
-            const char *name = dpif_ports[i].devname;
-            shash_add_once(&cur_ifaces, name, &dpif_ports[i]);
+        DPIF_PORT_FOR_EACH (&odp_port, &dump, br->dpif) {
+            struct dpif_port_info *port_info = xmalloc(sizeof *port_info);
+            port_info->port_no = odp_port.port;
+            port_info->type = xstrdup(odp_port.type);
+            shash_add(&cur_ifaces, odp_port.devname, port_info);
         }
 
         /* Get the set of interfaces we want on this datapath. */
@@ -633,10 +635,13 @@ 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 odp_port *dpif_port = shash_find_data(&cur_ifaces, if_name);
-            const char *type = iface ? iface->type : "internal";
+            struct dpif_port_info *dpif_port;
+            const char *type;
             int error;
 
+            type = iface ? iface->type : "internal";
+            dpif_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). */
@@ -644,7 +649,7 @@ bridge_reconfigure(const struct ovsrec_open_vswitch *ovs_cfg)
                 || (iface && iface->netdev
                     && strcmp(type, netdev_get_type(iface->netdev)))) {
                 if (dpif_port) {
-                    error = ofproto_port_del(br->ofproto, dpif_port->port);
+                    error = ofproto_port_del(br->ofproto, dpif_port->port_no);
                     if (error) {
                         continue;
                     }
@@ -718,9 +723,14 @@ bridge_reconfigure(const struct ovsrec_open_vswitch *ovs_cfg)
                 shash_destroy(&args);
             }
         }
-        free(dpif_ports);
-        shash_destroy(&cur_ifaces);
         shash_destroy(&want_ifaces);
+
+        SHASH_FOR_EACH (node, &cur_ifaces) {
+            struct dpif_port_info *port_info = node->data;
+            free(port_info->type);
+            free(port_info);
+        }
+        shash_destroy(&cur_ifaces);
     }
     sflow_bridge_number = 0;
     LIST_FOR_EACH (br, node, &all_bridges) {
@@ -1903,8 +1913,8 @@ bridge_get_all_ifaces(const struct bridge *br, struct shash *ifaces)
 static void
 bridge_fetch_dp_ifaces(struct bridge *br)
 {
-    struct odp_port *dpif_ports;
-    size_t n_dpif_ports;
+    struct dpif_port_dump dump;
+    struct odp_port odp_port;
     size_t i, j;
 
     /* Reset all interface numbers. */
@@ -1917,19 +1927,17 @@ bridge_fetch_dp_ifaces(struct bridge *br)
     }
     hmap_clear(&br->ifaces);
 
-    dpif_port_list(br->dpif, &dpif_ports, &n_dpif_ports);
-    for (i = 0; i < n_dpif_ports; i++) {
-        struct odp_port *p = &dpif_ports[i];
-        struct iface *iface = iface_lookup(br, p->devname);
+    DPIF_PORT_FOR_EACH (&odp_port, &dump, br->dpif) {
+        struct iface *iface = iface_lookup(br, odp_port.devname);
         if (iface) {
             if (iface->dp_ifidx >= 0) {
                 VLOG_WARN("%s reported interface %s twice",
-                          dpif_name(br->dpif), p->devname);
-            } else if (iface_from_dp_ifidx(br, p->port)) {
+                          dpif_name(br->dpif), odp_port.devname);
+            } else if (iface_from_dp_ifidx(br, odp_port.port)) {
                 VLOG_WARN("%s reported interface %"PRIu16" twice",
-                          dpif_name(br->dpif), p->port);
+                          dpif_name(br->dpif), odp_port.port);
             } else {
-                iface->dp_ifidx = p->port;
+                iface->dp_ifidx = odp_port.port;
                 hmap_insert(&br->ifaces, &iface->dp_ifidx_node,
                             hash_int(iface->dp_ifidx, 0));
             }
@@ -1940,7 +1948,6 @@ bridge_fetch_dp_ifaces(struct bridge *br)
                               : -1));
         }
     }
-    free(dpif_ports);
 }
 
 /* Bridge packet processing functions. */
-- 
1.7.1





More information about the dev mailing list