On Wed, Oct 31, 2018 at 3:35 AM Ben Pfaff <b...@ovn.org> wrote:

> The key for a "struct ovn_chassis_qdisc_queues" is a Chassis UUID and a
> queue_id, but only the UUID was being hashed, so if there was more than one
> per chassis then they'd all end up in the same hash bucket, which is
> needlessly inefficient.  (And if there's only one per chassis then why do
> we bother allocating them at all?)
>
> Found by inspection.
>
> Signed-off-by: Ben Pfaff <b...@ovn.org>
>

Acked-by: Numan Siddique <nusid...@redhat.com>


> ---
>  ovn/northd/ovn-northd.c | 18 +++++++++++++-----
>  1 file changed, 13 insertions(+), 5 deletions(-)
>
> diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
> index bc07a40bd0f8..6cb0b6cc410a 100644
> --- a/ovn/northd/ovn-northd.c
> +++ b/ovn/northd/ovn-northd.c
> @@ -323,6 +323,12 @@ struct ovn_chassis_qdisc_queues {
>      struct uuid chassis_uuid;
>  };
>
> +static uint32_t
> +hash_chassis_queue(const struct uuid *chassis_uuid, uint32_t queue_id)
> +{
> +    return hash_2words(uuid_hash(chassis_uuid), queue_id);
> +}
> +
>  static void
>  destroy_chassis_queues(struct hmap *set)
>  {
> @@ -340,7 +346,8 @@ add_chassis_queue(struct hmap *set, struct uuid
> *chassis_uuid,
>      struct ovn_chassis_qdisc_queues *node = xmalloc(sizeof *node);
>      node->queue_id = queue_id;
>      node->chassis_uuid = *chassis_uuid;
> -    hmap_insert(set, &node->key_node, uuid_hash(chassis_uuid));
> +    hmap_insert(set, &node->key_node,
> +                hash_chassis_queue(chassis_uuid, queue_id));
>  }
>
>  static bool
> @@ -348,7 +355,8 @@ chassis_queueid_in_use(const struct hmap *set, struct
> uuid *chassis_uuid,
>                         uint32_t queue_id)
>  {
>      const struct ovn_chassis_qdisc_queues *node;
> -    HMAP_FOR_EACH_WITH_HASH (node, key_node, uuid_hash(chassis_uuid),
> set) {
> +    HMAP_FOR_EACH_WITH_HASH (node, key_node,
> +                             hash_chassis_queue(chassis_uuid, queue_id),
> set) {
>          if (uuid_equals(chassis_uuid, &node->chassis_uuid)
>              && node->queue_id == queue_id) {
>              return true;
> @@ -378,11 +386,11 @@ static void
>  free_chassis_queueid(struct hmap *set, struct sbrec_chassis *chassis,
>                       uint32_t queue_id)
>  {
> +    const struct uuid *chassis_uuid = &chassis->header_.uuid;
>      struct ovn_chassis_qdisc_queues *node;
>      HMAP_FOR_EACH_WITH_HASH (node, key_node,
> -                             uuid_hash(&chassis->header_.uuid),
> -                             set) {
> -        if (uuid_equals(&chassis->header_.uuid, &node->chassis_uuid)
> +                             hash_chassis_queue(chassis_uuid, queue_id),
> set) {
> +        if (uuid_equals(chassis_uuid, &node->chassis_uuid)
>              && node->queue_id == queue_id) {
>              hmap_remove(set, &node->key_node);
>              break;
> --
> 2.16.1
>
> _______________________________________________
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to