From: Ben Pfaff Date: Mon, 10 Jan 2011 21:12:12 +0000 (-0800) Subject: datapath: Change listing ports to use an iterator concept. X-Git-Url: https://pintos-os.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=b0ec0f279ea4537cb421eba807254aba8f1971f2;p=openvswitch datapath: Change listing ports to use an iterator concept. 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. The Netlink version won't need to take the starting port number from userspace, since Netlink sockets can keep track of that state as part of their "dump" feature. Signed-off-by: Ben Pfaff Acked-by: Jesse Gross --- diff --git a/datapath/datapath.c b/datapath/datapath.c index 3837e920..26f04440 100644 --- a/datapath/datapath.c +++ b/datapath/datapath.c @@ -1397,37 +1397,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 = get_vport_protected(dp, 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) @@ -1552,8 +1542,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: @@ -1600,19 +1590,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) @@ -1839,8 +1828,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 4da630ca..eca6d8b3 100644 --- a/datapath/odp-compat.h +++ b/datapath/odp-compat.h @@ -15,7 +15,7 @@ #include "openvswitch/datapath-protocol.h" #include -#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 76612290..dd717c09 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) @@ -187,9 +187,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 5c2c7550..ac0444d8 100644 --- a/lib/dpif-linux.c +++ b/lib/dpif-linux.c @@ -324,25 +324,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 @@ -585,7 +597,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 73ab1e5f..c870f667 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 bb7b2a87..649a4927 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 b54ecaa9..708b5347 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 3e72539c..7277647c 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 d6ed1601..1e1621af 100644 --- a/ofproto/ofproto.c +++ b/ofproto/ofproto.c @@ -1536,12 +1536,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); @@ -1549,11 +1548,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); @@ -1783,26 +1780,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 f7409fb1..23733fde 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 ef2a22a9..0f857093 100644 --- a/vswitchd/bridge.c +++ b/vswitchd/bridge.c @@ -1,4 +1,4 @@ -/* Copyright (c) 2008, 2009, 2010 Nicira Networks +/* Copyright (c) 2008, 2009, 2010, 2011 Nicira Networks * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -606,38 +606,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 { + char *type; /* Network device type, e.g. "system". */ + uint32_t port_no; /* Port number within datapath. */ + }; 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 *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. */ @@ -647,10 +649,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 *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). */ @@ -658,7 +663,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; } @@ -732,9 +737,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 *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) { @@ -2030,8 +2040,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. */ @@ -2044,19 +2054,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)); } @@ -2067,7 +2075,6 @@ bridge_fetch_dp_ifaces(struct bridge *br) : -1)); } } - free(dpif_ports); } /* Bridge packet processing functions. */