[ovs-dev] [next 15/35] bridge: Change all_bridges from list to hmap (indexed by name).
Ethan Jackson
ethan at nicira.com
Mon May 2 22:23:47 UTC 2011
Looks Good.
Ethan
On Tue, Apr 26, 2011 at 09:24, Ben Pfaff <blp at nicira.com> wrote:
> This is more convenient for looking up a bridge by name. That makes
> reconfiguration a little bit simpler, because there is no longer a need to
> build a temporary index of existing bridges. I don't see any downsides.
> ---
> vswitchd/bridge.c | 56 ++++++++++++++++++++++------------------------------
> 1 files changed, 24 insertions(+), 32 deletions(-)
>
> diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
> index ca0c0af..16e1c7e 100644
> --- a/vswitchd/bridge.c
> +++ b/vswitchd/bridge.c
> @@ -161,7 +161,7 @@ struct port {
> };
>
> struct bridge {
> - struct list node; /* Node in global list of bridges. */
> + struct hmap_node node; /* In 'all_bridges'. */
> char *name; /* User-specified arbitrary name. */
> struct mac_learning *ml; /* MAC learning table. */
> uint8_t ea[ETH_ADDR_LEN]; /* Bridge Ethernet Address. */
> @@ -191,8 +191,8 @@ struct bridge {
> struct ovsrec_interface *synth_local_ifacep;
> };
>
> -/* List of all bridges. */
> -static struct list all_bridges = LIST_INITIALIZER(&all_bridges);
> +/* All bridges, indexed by name. */
> +static struct hmap all_bridges = HMAP_INITIALIZER(&all_bridges);
>
> /* OVSDB IDL used to obtain configuration. */
> static struct ovsdb_idl *idl;
> @@ -357,7 +357,7 @@ bridge_exit(void)
> {
> struct bridge *br, *next_br;
>
> - LIST_FOR_EACH_SAFE (br, next_br, node, &all_bridges) {
> + HMAP_FOR_EACH_SAFE (br, next_br, node, &all_bridges) {
> bridge_destroy(br);
> }
> ovsdb_idl_destroy(idl);
> @@ -476,10 +476,10 @@ collect_in_band_managers(const struct ovsrec_open_vswitch *ovs_cfg,
> static void
> bridge_reconfigure(const struct ovsrec_open_vswitch *ovs_cfg)
> {
> - struct shash old_br, new_br;
> struct shash_node *node;
> struct bridge *br, *next;
> struct sockaddr_in *managers;
> + struct shash new_br;
> size_t n_managers;
> size_t i;
> int sflow_bridge_number;
> @@ -489,11 +489,7 @@ bridge_reconfigure(const struct ovsrec_open_vswitch *ovs_cfg)
> collect_in_band_managers(ovs_cfg, &managers, &n_managers);
>
> /* Collect old and new bridges. */
> - shash_init(&old_br);
> shash_init(&new_br);
> - LIST_FOR_EACH (br, node, &all_bridges) {
> - shash_add(&old_br, br->name, br);
> - }
> for (i = 0; i < ovs_cfg->n_bridges; i++) {
> const struct ovsrec_bridge *br_cfg = ovs_cfg->bridges[i];
> if (!shash_add_once(&new_br, br_cfg->name, br_cfg)) {
> @@ -502,18 +498,15 @@ bridge_reconfigure(const struct ovsrec_open_vswitch *ovs_cfg)
> }
>
> /* Get rid of deleted bridges and add new bridges. */
> - LIST_FOR_EACH_SAFE (br, next, node, &all_bridges) {
> - struct ovsrec_bridge *br_cfg = shash_find_data(&new_br, br->name);
> - if (br_cfg) {
> - br->cfg = br_cfg;
> - } else {
> + HMAP_FOR_EACH_SAFE (br, next, node, &all_bridges) {
> + br->cfg = shash_find_data(&new_br, br->name);
> + if (!br->cfg) {
> bridge_destroy(br);
> }
> }
> SHASH_FOR_EACH (node, &new_br) {
> - const char *br_name = node->name;
> const struct ovsrec_bridge *br_cfg = node->data;
> - br = shash_find_data(&old_br, br_name);
> + struct bridge *br = bridge_lookup(node->name);
> if (br) {
> /* If the bridge datapath type has changed, we need to tear it
> * down and recreate. */
> @@ -525,11 +518,10 @@ bridge_reconfigure(const struct ovsrec_open_vswitch *ovs_cfg)
> bridge_create(br_cfg);
> }
> }
> - shash_destroy(&old_br);
> shash_destroy(&new_br);
>
> /* Reconfigure all bridges. */
> - LIST_FOR_EACH (br, node, &all_bridges) {
> + HMAP_FOR_EACH (br, node, &all_bridges) {
> bridge_reconfigure_one(br);
> }
>
> @@ -538,7 +530,7 @@ bridge_reconfigure(const struct ovsrec_open_vswitch *ovs_cfg)
> * The kernel will reject any attempt to add a given port to a datapath if
> * 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) {
> + HMAP_FOR_EACH (br, node, &all_bridges) {
> struct ofproto_port ofproto_port;
> struct ofproto_port_dump dump;
> struct shash want_ifaces;
> @@ -557,7 +549,7 @@ bridge_reconfigure(const struct ovsrec_open_vswitch *ovs_cfg)
> }
> shash_destroy(&want_ifaces);
> }
> - LIST_FOR_EACH (br, node, &all_bridges) {
> + HMAP_FOR_EACH (br, node, &all_bridges) {
> struct shash cur_ifaces, want_ifaces;
> struct ofproto_port ofproto_port;
> struct ofproto_port_dump dump;
> @@ -682,7 +674,7 @@ bridge_reconfigure(const struct ovsrec_open_vswitch *ovs_cfg)
> shash_destroy(&cur_ifaces);
> }
> sflow_bridge_number = 0;
> - LIST_FOR_EACH (br, node, &all_bridges) {
> + HMAP_FOR_EACH (br, node, &all_bridges) {
> uint8_t ea[ETH_ADDR_LEN];
> uint64_t dpid;
> struct iface *local_iface;
> @@ -853,7 +845,7 @@ bridge_reconfigure(const struct ovsrec_open_vswitch *ovs_cfg)
> * the datapath ID before the controller. */
> bridge_reconfigure_remotes(br, managers, n_managers);
> }
> - LIST_FOR_EACH (br, node, &all_bridges) {
> + HMAP_FOR_EACH (br, node, &all_bridges) {
> struct port *port;
>
> br->has_bonded_ports = false;
> @@ -878,7 +870,7 @@ bridge_reconfigure(const struct ovsrec_open_vswitch *ovs_cfg)
>
> /* Some reconfiguration operations require the bridge to have been run at
> * least once. */
> - LIST_FOR_EACH (br, node, &all_bridges) {
> + HMAP_FOR_EACH (br, node, &all_bridges) {
> struct iface *iface;
>
> bridge_run_one(br);
> @@ -1354,7 +1346,7 @@ bridge_run(void)
>
> /* Let each bridge do the work that it needs to do. */
> datapath_destroyed = false;
> - LIST_FOR_EACH (br, node, &all_bridges) {
> + HMAP_FOR_EACH (br, node, &all_bridges) {
> int error = bridge_run_one(br);
> if (error) {
> static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
> @@ -1406,7 +1398,7 @@ bridge_run(void)
> struct ovsdb_idl_txn *txn;
>
> txn = ovsdb_idl_txn_create(idl);
> - LIST_FOR_EACH (br, node, &all_bridges) {
> + HMAP_FOR_EACH (br, node, &all_bridges) {
> struct port *port;
>
> HMAP_FOR_EACH (port, hmap_node, &br->ports) {
> @@ -1432,7 +1424,7 @@ bridge_run(void)
> bool changed = false;
>
> txn = ovsdb_idl_txn_create(idl);
> - LIST_FOR_EACH (br, node, &all_bridges) {
> + HMAP_FOR_EACH (br, node, &all_bridges) {
> struct port *port;
>
> HMAP_FOR_EACH (port, hmap_node, &br->ports) {
> @@ -1459,7 +1451,7 @@ bridge_wait(void)
> {
> struct bridge *br;
>
> - LIST_FOR_EACH (br, node, &all_bridges) {
> + HMAP_FOR_EACH (br, node, &all_bridges) {
> struct port *port;
>
> ofproto_wait(br->ofproto);
> @@ -1661,7 +1653,7 @@ bridge_create(const struct ovsrec_bridge *br_cfg)
>
> br->flush = false;
>
> - list_push_back(&all_bridges, &br->node);
> + hmap_insert(&all_bridges, &br->node, hash_string(br->name, 0));
>
> VLOG_INFO("bridge %s: created", br->name);
>
> @@ -1681,7 +1673,7 @@ bridge_destroy(struct bridge *br)
> for (i = 0; i < MAX_MIRRORS; i++) {
> mirror_destroy(br->mirrors[i]);
> }
> - list_remove(&br->node);
> + hmap_remove(&all_bridges, &br->node);
> ofproto_destroy_and_delete(br->ofproto);
> mac_learning_destroy(br->ml);
> hmap_destroy(&br->ifaces);
> @@ -1698,7 +1690,7 @@ bridge_lookup(const char *name)
> {
> struct bridge *br;
>
> - LIST_FOR_EACH (br, node, &all_bridges) {
> + HMAP_FOR_EACH_WITH_HASH (br, node, hash_string(name, 0), &all_bridges) {
> if (!strcmp(br->name, name)) {
> return br;
> }
> @@ -1744,7 +1736,7 @@ bridge_unixctl_reconnect(struct unixctl_conn *conn,
> }
> ofproto_reconnect_controllers(br->ofproto);
> } else {
> - LIST_FOR_EACH (br, node, &all_bridges) {
> + HMAP_FOR_EACH (br, node, &all_bridges) {
> ofproto_reconnect_controllers(br->ofproto);
> }
> }
> @@ -3289,7 +3281,7 @@ iface_find(const char *name)
> {
> const struct bridge *br;
>
> - LIST_FOR_EACH (br, node, &all_bridges) {
> + HMAP_FOR_EACH (br, node, &all_bridges) {
> struct iface *iface = iface_lookup(br, name);
>
> if (iface) {
> --
> 1.7.4.4
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
>
More information about the dev
mailing list