Call hash_finish properly in hashing sequeces that didn't do that.
This should prevent suboptimal hashing results.
Reported-by: Ilya Maximets <i.maxim...@ovn.org>
Signed-off-by: Ales Musil <amu...@redhat.com>
---
controller/ecmp-next-hop-monitor.c | 20 +++++++++++-------
controller/pinctrl.c | 33 +++++++++++++++---------------
northd/en-ecmp-nexthop.c | 16 +++++++++++----
3 files changed, 42 insertions(+), 27 deletions(-)
diff --git a/controller/ecmp-next-hop-monitor.c
b/controller/ecmp-next-hop-monitor.c
index 4dd5aa542..39ea46a20 100644
--- a/controller/ecmp-next-hop-monitor.c
+++ b/controller/ecmp-next-hop-monitor.c
@@ -62,6 +62,17 @@ void ecmp_nexthop_destroy(void)
ecmp_nexthop_destroy_map(&ecmp_nexthop);
}
+static uint32_t
+ecmp_nexthop_hash(const char *nexthop, const char *mac,
+ const uint16_t zone_id)
+{
+ uint32_t hash = hash_string(nexthop, 0);
+ hash = hash_add(hash, hash_string(mac, 0));
+ hash = hash_add(hash, zone_id);
+
+ return hash_finish(hash, 64);
+}
+
static struct ecmp_nexthop_data *
ecmp_nexthop_alloc_entry(const char *nexthop, const char *mac,
const uint16_t zone_id, struct hmap *map)
@@ -71,10 +82,7 @@ ecmp_nexthop_alloc_entry(const char *nexthop, const char
*mac,
e->mac = xstrdup(mac);
e->zone_id = zone_id;
- uint32_t hash = hash_string(nexthop, 0);
- hash = hash_add(hash, hash_string(mac, 0));
- hash = hash_add(hash, zone_id);
- hmap_insert(map, &e->hmap_node, hash);
+ hmap_insert(map, &e->hmap_node, ecmp_nexthop_hash(nexthop, mac, zone_id));
return e;
}
@@ -83,9 +91,7 @@ static struct ecmp_nexthop_data *
ecmp_nexthop_find_entry(const char *nexthop, const char *mac,
const uint16_t zone_id, const struct hmap *map)
{
- uint32_t hash = hash_string(nexthop, 0);
- hash = hash_add(hash, hash_string(mac, 0));
- hash = hash_add(hash, zone_id);
+ uint32_t hash = ecmp_nexthop_hash(nexthop, mac, zone_id);
struct ecmp_nexthop_data *e;
HMAP_FOR_EACH_WITH_HASH (e, hmap_node, hash, map) {
diff --git a/controller/pinctrl.c b/controller/pinctrl.c
index 400cb59e4..684a8ae5c 100644
--- a/controller/pinctrl.c
+++ b/controller/pinctrl.c
@@ -4910,15 +4910,24 @@ destroy_send_arps_nds(void)
hmap_destroy(&send_arp_nd_data);
}
+static uint32_t
+arp_nd_data_get_hash(const struct in6_addr *dst_ip, const uint32_t dp_key,
+ const uint32_t port_key)
+{
+ uint32_t hash = 0;
+ hash = hash_add_in6_addr(hash, dst_ip);
+ hash = hash_add(hash, dp_key);
+ hash = hash_add(hash, port_key);
+
+ return hash_finish(hash, 24);
+}
+
static struct arp_nd_data *
arp_nd_find_data(const struct sbrec_port_binding *pb,
const struct in6_addr *nexthop)
{
- uint32_t hash = 0;
-
- hash = hash_add_in6_addr(hash, nexthop);
- hash = hash_add(hash, pb->datapath->tunnel_key);
- hash = hash_add(hash, pb->tunnel_key);
+ uint32_t hash = arp_nd_data_get_hash(nexthop, pb->datapath->tunnel_key,
+ pb->tunnel_key);
struct arp_nd_data *e;
HMAP_FOR_EACH_WITH_HASH (e, hmap_node, hash, &send_arp_nd_data) {
@@ -4931,15 +4940,6 @@ arp_nd_find_data(const struct sbrec_port_binding *pb,
return NULL;
}
-static uint32_t
-arp_nd_data_get_hash(struct arp_nd_data *e)
-{
- uint32_t hash = 0;
- hash = hash_add_in6_addr(hash, &e->dst_ip);
- hash = hash_add(hash, e->dp_key);
- return hash_add(hash, e->port_key);
-}
-
static struct arp_nd_data *
arp_nd_alloc_data(const struct eth_addr ea,
struct in6_addr src_ip, struct in6_addr dst_ip,
@@ -4954,7 +4954,7 @@ arp_nd_alloc_data(const struct eth_addr ea,
e->dp_key = pb->datapath->tunnel_key;
e->port_key = pb->tunnel_key;
- uint32_t hash = arp_nd_data_get_hash(e);
+ uint32_t hash = arp_nd_data_get_hash(&e->dst_ip, e->dp_key, e->port_key);
hmap_insert(&send_arp_nd_data, &e->hmap_node, hash);
notify_pinctrl_handler();
@@ -4977,7 +4977,8 @@ arp_nd_sync_data(const struct sbrec_ecmp_nexthop_table
*ecmp_nh_table)
&nexthop);
if (e) {
hmap_remove(&send_arp_nd_data, &e->hmap_node);
- uint32_t hash = arp_nd_data_get_hash(e);
+ uint32_t hash = arp_nd_data_get_hash(&e->dst_ip, e->dp_key,
+ e->port_key);
hmap_insert(&arp_nd_active, &e->hmap_node, hash);
}
}
diff --git a/northd/en-ecmp-nexthop.c b/northd/en-ecmp-nexthop.c
index 7e9ed4f34..673e4aa4a 100644
--- a/northd/en-ecmp-nexthop.c
+++ b/northd/en-ecmp-nexthop.c
@@ -39,6 +39,15 @@ struct ecmp_nexthop_data {
const struct sbrec_ecmp_nexthop *sb_ecmp_nh;
};
+static uint32_t
+ecmp_nexthop_hash(const char *nexthop, const uint32_t port_key)
+{
+ uint32_t hash = hash_string(nexthop, 0);
+ hash = hash_add(hash, port_key);
+
+ return hash_finish(hash, 64);
+}
+
static struct ecmp_nexthop_data *
ecmp_nexthop_insert_entry(const struct sbrec_ecmp_nexthop *sb_ecmp_nh,
struct hmap *map)
@@ -46,8 +55,8 @@ ecmp_nexthop_insert_entry(const struct sbrec_ecmp_nexthop
*sb_ecmp_nh,
struct ecmp_nexthop_data *e = xmalloc(sizeof *e);
e->sb_ecmp_nh = sb_ecmp_nh;
- uint32_t hash = hash_string(sb_ecmp_nh->nexthop, 0);
- hash = hash_add(hash, hash_int(sb_ecmp_nh->port->tunnel_key, 0));
+ uint32_t hash = ecmp_nexthop_hash(sb_ecmp_nh->nexthop,
+ sb_ecmp_nh->port->tunnel_key);
hmap_insert(map, &e->hmap_node, hash);
return e;
@@ -58,8 +67,7 @@ ecmp_nexthop_find_entry(const char *nexthop,
const struct sbrec_port_binding *port,
struct hmap *map)
{
- uint32_t hash = hash_string(nexthop, 0);
- hash = hash_add(hash, hash_int(port->tunnel_key, 0));
+ uint32_t hash = ecmp_nexthop_hash(nexthop, port->tunnel_key);
struct ecmp_nexthop_data *e;
HMAP_FOR_EACH_WITH_HASH (e, hmap_node, hash, map) {
--
2.50.1
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
