[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