[ovs-dev] [next 13/35] bridge: Change 'iface_by_name' from shash to hmap.

[Ethan Jackson]

This seems fine to me.  I'm not completely convinced of it's value, as
a bit of extra memory stored in the shash is not a big deal.  That
said, I think it's fine.

Ethan

On Tue, Apr 26, 2011 at 09:24, Ben Pfaff <blp at nicira.com> wrote:
> This avoids having duplicate copies of interface names (inside the shash)
> and it isn't any harder to work with.
> ---
>  vswitchd/bridge.c |   23 ++++++++++++++++-------
>  1 files changed, 16 insertions(+), 7 deletions(-)
>
> diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
> index f6b7289..3f6a137 100644
> --- a/vswitchd/bridge.c
> +++ b/vswitchd/bridge.c
> @@ -99,6 +99,7 @@ static void dst_set_free(struct dst_set *);
>  struct iface {
>     /* These members are always valid. */
>     struct list port_elem;      /* Element in struct port's "ifaces" list. */
> +    struct hmap_node name_node; /* In struct bridge's "iface_by_name" hmap. */
>     struct port *port;          /* Containing port. */
>     char *name;                 /* Host network device name. */
>     tag_type tag;               /* Tag associated with this interface. */
> @@ -176,7 +177,7 @@ struct bridge {
>
>     /* Bridge ports. */
>     struct hmap ports;          /* "struct port"s indexed by name. */
> -    struct shash iface_by_name; /* "struct iface"s indexed by name. */
> +    struct hmap iface_by_name;  /* "struct iface"s indexed by name. */
>
>     /* Bonding. */
>     bool has_bonded_ports;
> @@ -1665,7 +1666,7 @@ bridge_create(const struct ovsrec_bridge *br_cfg)
>
>     hmap_init(&br->ports);
>     hmap_init(&br->ifaces);
> -    shash_init(&br->iface_by_name);
> +    hmap_init(&br->iface_by_name);
>
>     br->flush = false;
>
> @@ -1701,7 +1702,7 @@ bridge_destroy(struct bridge *br)
>         mac_learning_destroy(br->ml);
>         hmap_destroy(&br->ifaces);
>         hmap_destroy(&br->ports);
> -        shash_destroy(&br->iface_by_name);
> +        hmap_destroy(&br->iface_by_name);
>         free(br->synth_local_iface.type);
>         free(br->name);
>         free(br);
> @@ -3238,7 +3239,7 @@ iface_create(struct port *port, const struct ovsrec_interface *if_cfg)
>     iface->netdev = NULL;
>     iface->cfg = if_cfg;
>
> -    shash_add_assert(&br->iface_by_name, iface->name, iface);
> +    hmap_insert(&br->iface_by_name, &iface->name_node, hash_string(name, 0));
>
>     list_push_back(&port->ifaces, &iface->port_elem);
>
> @@ -3264,13 +3265,12 @@ iface_destroy(struct iface *iface)
>             lacp_slave_unregister(port->lacp, iface);
>         }
>
> -        shash_find_and_delete_assert(&br->iface_by_name, iface->name);
> -
>         if (iface->dp_ifidx >= 0) {
>             hmap_remove(&br->ifaces, &iface->dp_ifidx_node);
>         }
>
>         list_remove(&iface->port_elem);
> +        hmap_remove(&br->iface_by_name, &iface->name_node);
>
>         netdev_close(iface->netdev);
>
> @@ -3284,7 +3284,16 @@ iface_destroy(struct iface *iface)
>  static struct iface *
>  iface_lookup(const struct bridge *br, const char *name)
>  {
> -    return shash_find_data(&br->iface_by_name, name);
> +    struct iface *iface;
> +
> +    HMAP_FOR_EACH_WITH_HASH (iface, name_node, hash_string(name, 0),
> +                             &br->iface_by_name) {
> +        if (!strcmp(iface->name, name)) {
> +            return iface;
> +        }
> +    }
> +
> +    return NULL;
>  }
>
>  static struct iface *
> --
> 1.7.4.4
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
>