Nest: Parametric network hashes

Currently, all fib hash tables use the same hashing function. This leads
to a situation where feeding routes through a pipe from one table to
another causes significant number of collisions, as routes are fed in the
order of increasing hash values, but dst tables are sized based on the
number of stored routes.

The patch makes fib hashing function parametric and chooses random
parameter for each table. Also generally improves quality of hashing
functions.

Unfortunately, while this patch fixes the issue with initial collisions,
having different hashing functions leads to 2x slowdown of pipe feeding,
presumably due to worse cache behavior in dst tables. Also, the original
issue significantly affects just the initial part of feed, when the dst
table is small, so even ideal fix would not improve that much.

Therefore, no merge for this patch.

diff --git a/lib/birdlib.h b/lib/birdlib.h
index 81d4908acd9d1bf188622b505298dc33f780597c..f20c4640861b4c4148db6c82e0895876753cbcee 100644 (file)
--- a/lib/birdlib.h
+++ b/lib/birdlib.h
@@ -201,4 +201,23 @@ u32 random_u32(void);
 void random_init(void);
 void random_bytes(void *buf, size_t size);
 
+
+/* Hashing */
+
+/* Constant parameter for non-parametrized hashes */
+#define HASH_PARAM 2902958171u
+
+/* Precomputed powers of HASH_PARAM */
+#define HASH_PARAM1 ((u64) HASH_PARAM)
+#define HASH_PARAM2 (HASH_PARAM1 * HASH_PARAM)
+#define HASH_PARAM3 (HASH_PARAM2 * HASH_PARAM)
+#define HASH_PARAM4 (HASH_PARAM3 * HASH_PARAM)
+
+/* Reduce intermediate 64-bit value to final 32-bit value */
+static inline u32 hash_value(u64 a)
+{ return ((u32) a) ^ ((u32) (a >> 32)); }
+
+u32 random_hash_param(void);
+
+
 #endif

diff --git a/lib/bitops.h b/lib/bitops.h
index 0beda2a3525c3fe99bcac0b0c55e3311e5dbea3b..53830a54df40581ff1e6cdf431d39743b86d12d2 100644 (file)
--- a/lib/bitops.h
+++ b/lib/bitops.h
@@ -25,7 +25,17 @@ uint u32_masklen(u32 x);
 
 u32 u32_log2(u32 v);
 
-static inline u32 u32_hash(u32 v) { return v * 2902958171u; }
+static inline u64 u32_hash0(u32 v, u32 p, u64 acc)
+{ return (acc + v) * p; }
+
+static inline u32 u32_hash(u32 v)
+{ return hash_value(u32_hash0(v, HASH_PARAM, 0)); }
+
+static inline u64 u64_hash0(u64 v, u32 p, u64 acc)
+{ return u32_hash0(v >> 32, p, u32_hash0(v, p, acc)); }
+
+static inline u32 u64_hash(u64 v)
+{ return hash_value(u64_hash0(v, HASH_PARAM, 0)); }
 
 static inline u8 u32_popcount(u32 v) { return __builtin_popcount(v); }
 static inline u8 u64_popcount(u64 v) { return __builtin_popcountll(v); }

diff --git a/lib/ip.h b/lib/ip.h
index 9eef2e1633d361488c5d706dc660afef83b29b07..3a98b06141671d88328bb4d6284f8777fdbac634 100644 (file)
--- a/lib/ip.h
+++ b/lib/ip.h
@@ -194,14 +194,28 @@ static inline int ipa_nonzero2(ip_addr a)
  *     Hash and compare functions
  */
 
+static inline u64 ip4_hash0(ip4_addr a, u32 p, u64 acc)
+{ return (acc + _I(a)) * p; }
+
 static inline u32 ip4_hash(ip4_addr a)
-{ return u32_hash(_I(a)); }
+{ return hash_value(ip4_hash0(a, HASH_PARAM, 0)); }
+
+static inline u64 ip6_hash0(ip6_addr a, u32 p, u64 acc)
+{
+  acc += _I0(a); acc *= p;
+  acc += _I1(a); acc *= p;
+  acc += _I2(a); acc *= p;
+  acc += _I3(a); acc *= p;
+  return acc;
+}
 
 static inline u32 ip6_hash(ip6_addr a)
 {
-  /* Returns a 32-bit hash key, although low-order bits are not mixed */
-  u32 x = _I0(a) ^ _I1(a) ^ _I2(a) ^ _I3(a);
-  return x ^ (x << 16) ^ (x << 24);
+  /* Equivalent of ip6_hash0(a, HASH_PARAM, 0) */
+  return hash_value(_I0(a) * HASH_PARAM4 +
+                   _I1(a) * HASH_PARAM3 +
+                   _I2(a) * HASH_PARAM2 +
+                   _I3(a) * HASH_PARAM1);
 }
 
 static inline int ip4_compare(ip4_addr a, ip4_addr b)

diff --git a/lib/net.c b/lib/net.c
index 976ddbcc7b841c8a82dd69db6d8af96eae736a35..e8cab0db99ff9102599dbfd6093c7ad3be309442 100644 (file)
--- a/lib/net.c
+++ b/lib/net.c
@@ -171,23 +171,23 @@ net_compare(const net_addr *a, const net_addr *b)
   return 0;
 }
 
-#define NET_HASH(a,t) net_hash_##t((const net_addr_##t *) a)
+#define NET_HASH(a, p, t) net_hash_##t((const net_addr_##t *) a, p)
 
 u32
-net_hash(const net_addr *n)
+net_hash(const net_addr *n, u32 p)
 {
   switch (n->type)
   {
-  case NET_IP4: return NET_HASH(n, ip4);
-  case NET_IP6: return NET_HASH(n, ip6);
-  case NET_VPN4: return NET_HASH(n, vpn4);
-  case NET_VPN6: return NET_HASH(n, vpn6);
-  case NET_ROA4: return NET_HASH(n, roa4);
-  case NET_ROA6: return NET_HASH(n, roa6);
-  case NET_FLOW4: return NET_HASH(n, flow4);
-  case NET_FLOW6: return NET_HASH(n, flow6);
-  case NET_IP6_SADR: return NET_HASH(n, ip6_sadr);
-  case NET_MPLS: return NET_HASH(n, mpls);
+  case NET_IP4: return NET_HASH(n, p, ip4);
+  case NET_IP6: return NET_HASH(n, p, ip6);
+  case NET_VPN4: return NET_HASH(n, p, vpn4);
+  case NET_VPN6: return NET_HASH(n, p, vpn6);
+  case NET_ROA4: return NET_HASH(n, p, roa4);
+  case NET_ROA6: return NET_HASH(n, p, roa6);
+  case NET_FLOW4: return NET_HASH(n, p, flow4);
+  case NET_FLOW6: return NET_HASH(n, p, flow6);
+  case NET_IP6_SADR: return NET_HASH(n, p, ip6_sadr);
+  case NET_MPLS: return NET_HASH(n, p, mpls);
   default: bug("invalid type");
   }
 }

diff --git a/lib/net.h b/lib/net.h
index 9f4a00ada2e6ebef086a20f9165eda5ca56ce374..67aef4ff41fe03930c717f0c1a44eea5deb622e5 100644 (file)
--- a/lib/net.h
+++ b/lib/net.h
@@ -479,41 +479,43 @@ static inline void net_copy_mpls(net_addr_mpls *dst, const net_addr_mpls *src)
 { memcpy(dst, src, sizeof(net_addr_mpls)); }
 
 
-/* XXXX */
-static inline u32 u64_hash(u64 a)
-{ return u32_hash(a); }
+static inline u64 px4_hash0(ip4_addr prefix, u32 pxlen, u32 p)
+{ return ip4_hash0(prefix, p, 0) ^ (pxlen << 26); }
 
-static inline u32 net_hash_ip4(const net_addr_ip4 *n)
-{ return ip4_hash(n->prefix) ^ ((u32) n->pxlen << 26); }
+static inline u64 px6_hash0(ip6_addr prefix, u32 pxlen, u32 p)
+{ return ip6_hash0(prefix, p, 0) ^ (pxlen << 26); }
 
-static inline u32 net_hash_ip6(const net_addr_ip6 *n)
-{ return ip6_hash(n->prefix) ^ ((u32) n->pxlen << 26); }
+static inline u32 net_hash_ip4(const net_addr_ip4 *n, u32 p)
+{ return hash_value(px4_hash0(n->prefix, n->pxlen, p)); }
 
-static inline u32 net_hash_vpn4(const net_addr_vpn4 *n)
-{ return ip4_hash(n->prefix) ^ ((u32) n->pxlen << 26) ^ u64_hash(n->rd); }
+static inline u32 net_hash_ip6(const net_addr_ip6 *n, u32 p)
+{ return hash_value(px6_hash0(n->prefix, n->pxlen, p)); }
 
-static inline u32 net_hash_vpn6(const net_addr_vpn6 *n)
-{ return ip6_hash(n->prefix) ^ ((u32) n->pxlen << 26) ^ u64_hash(n->rd); }
+static inline u32 net_hash_vpn4(const net_addr_vpn4 *n, u32 p)
+{ return hash_value(u64_hash0(n->rd, p, px4_hash0(n->prefix, n->pxlen, p))); }
 
-static inline u32 net_hash_roa4(const net_addr_roa4 *n)
-{ return ip4_hash(n->prefix) ^ ((u32) n->pxlen << 26); }
+static inline u32 net_hash_vpn6(const net_addr_vpn6 *n, u32 p)
+{ return hash_value(u64_hash0(n->rd, p, px6_hash0(n->prefix, n->pxlen, p))); }
 
-static inline u32 net_hash_roa6(const net_addr_roa6 *n)
-{ return ip6_hash(n->prefix) ^ ((u32) n->pxlen << 26); }
+static inline u32 net_hash_roa4(const net_addr_roa4 *n, u32 p)
+{ return hash_value(px4_hash0(n->prefix, n->pxlen, p)); }
 
-static inline u32 net_hash_flow4(const net_addr_flow4 *n)
-{ return ip4_hash(n->prefix) ^ ((u32) n->pxlen << 26); }
+static inline u32 net_hash_roa6(const net_addr_roa6 *n, u32 p)
+{ return hash_value(px6_hash0(n->prefix, n->pxlen, p)); }
 
-static inline u32 net_hash_flow6(const net_addr_flow6 *n)
-{ return ip6_hash(n->prefix) ^ ((u32) n->pxlen << 26); }
+static inline u32 net_hash_flow4(const net_addr_flow4 *n, u32 p)
+{ return hash_value(px4_hash0(n->prefix, n->pxlen, p)); }
 
-static inline u32 net_hash_ip6_sadr(const net_addr_ip6_sadr *n)
-{ return net_hash_ip6((net_addr_ip6 *) n); }
+static inline u32 net_hash_flow6(const net_addr_flow6 *n, u32 p)
+{ return hash_value(px6_hash0(n->prefix, n->pxlen, p)); }
 
-static inline u32 net_hash_mpls(const net_addr_mpls *n)
-{ return n->label; }
+static inline u32 net_hash_ip6_sadr(const net_addr_ip6_sadr *n, u32 p)
+{ return hash_value(px6_hash0(n->dst_prefix, n->dst_pxlen, p)); }
 
-u32 net_hash(const net_addr *a);
+static inline u32 net_hash_mpls(const net_addr_mpls *n, u32 p)
+{ return hash_value(u32_hash0(n->label, p, 0)); }
+
+u32 net_hash(const net_addr *a, u32 p);
 
 
 static inline int net_validate_px4(const ip4_addr prefix, uint pxlen)

diff --git a/nest/route.h b/nest/route.h
index 395139a31975fdec8826776aad201b447f36901f..2d9d4e79489839a366a86dab5c4ec18d8e689384 100644 (file)
--- a/nest/route.h
+++ b/nest/route.h
@@ -58,6 +58,7 @@ struct fib {
   pool *fib_pool;                      /* Pool holding all our data */
   slab *fib_slab;                      /* Slab holding all fib nodes */
   struct fib_node **hash_table;                /* Node hash table */
+  u32  hash_param;                     /* Parameter for hash function */
   uint hash_size;                      /* Number of hash table entries (a power of two) */
   uint hash_order;                     /* Binary logarithm of hash_size */
   uint hash_shift;                     /* 32 - hash_order */

diff --git a/nest/rt-fib.c b/nest/rt-fib.c
index 1690a8f6f2bf1de9cc28526b91a5caa8afcad165..e4b092b7b4e722c5f86fbeab0ad0d74c3e7cec67 100644 (file)
--- a/nest/rt-fib.c
+++ b/nest/rt-fib.c
@@ -159,6 +159,7 @@ fib_init(struct fib *f, pool *p, uint addr_type, uint node_size, uint node_offse
   f->addr_type = addr_type;
   f->node_size = node_size;
   f->node_offset = node_offset;
+  f->hash_param = random_hash_param();
   f->hash_order = hash_order;
   fib_ht_alloc(f);
   bzero(f->hash_table, f->hash_size * sizeof(struct fib_node *));
@@ -213,7 +214,8 @@ fib_rehash(struct fib *f, int step)
 #define CAST(t) (const net_addr_##t *)
 #define CAST2(t) (net_addr_##t *)
 
-#define FIB_HASH(f,a,t) (net_hash_##t(CAST(t) a) >> f->hash_shift)
+#define FIB_HASH0(f,a,t) (net_hash_##t(CAST(t) a, f->hash_param))
+#define FIB_HASH(f,a,t) (FIB_HASH0(f, a, t) >> f->hash_shift)
 
 #define FIB_FIND(f,a,t)                                                        \
   ({                                                                   \
@@ -225,11 +227,11 @@ fib_rehash(struct fib *f, int step)
 
 #define FIB_INSERT(f,a,e,t)                                            \
   ({                                                                   \
-  u32 h = net_hash_##t(CAST(t) a);                                     \
+  u32 h = FIB_HASH0(f, a, t);                                          \
   struct fib_node **ee = f->hash_table + (h >> f->hash_shift);         \
   struct fib_node *g;                                                  \
                                                                        \
-  while ((g = *ee) && (net_hash_##t(CAST(t) g->addr) < h))             \
+  while ((g = *ee) && (FIB_HASH0(f, g->addr, t) < h))                  \
     ee = &g->next;                                                     \
                                                                        \
   net_copy_##t(CAST2(t) e->addr, CAST(t) a);                           \
@@ -242,7 +244,7 @@ static inline u32
 fib_hash(struct fib *f, const net_addr *a)
 {
   /* Same as FIB_HASH() */
-  return net_hash(a) >> f->hash_shift;
+  return net_hash(a, f->hash_param) >> f->hash_shift;
 }
 
 void *

diff --git a/proto/bgp/attrs.c b/proto/bgp/attrs.c
index a97871735aac8e616454b1e717a1481401f4853c..769c89624d17ff52711c89a430519f5361ce3ea9 100644 (file)
--- a/proto/bgp/attrs.c
+++ b/proto/bgp/attrs.c
@@ -1609,6 +1609,7 @@ void
 bgp_init_prefix_table(struct bgp_channel *c)
 {
   HASH_INIT(c->prefix_hash, c->pool, 8);
+  c->prefix_param = random_hash_param();
 
   uint alen = net_addr_length[c->c.net_type];
   c->prefix_slab = alen ? sl_new(c->pool, sizeof(struct bgp_prefix) + alen) : NULL;
@@ -1626,8 +1627,7 @@ bgp_free_prefix_table(struct bgp_channel *c)
 static struct bgp_prefix *
 bgp_get_prefix(struct bgp_channel *c, net_addr *net, u32 path_id)
 {
-  /* We must use a different hash function than the rtable */
-  u32 hash = u32_hash(net_hash(net) ^ u32_hash(path_id));
+  u32 hash = net_hash(net, c->prefix_param) ^ u32_hash(path_id);
   struct bgp_prefix *px = HASH_FIND(c->prefix_hash, PXH, net, path_id, hash);
 
   if (px)

diff --git a/proto/bgp/bgp.h b/proto/bgp/bgp.h
index 4969c0b9c0d508ca281ac5e23f95a69079e66cb2..fbd90c2abea963dffdbf498d4a5784d4b534013b 100644 (file)
--- a/proto/bgp/bgp.h
+++ b/proto/bgp/bgp.h
@@ -352,6 +352,7 @@ struct bgp_channel {
   list bucket_queue;                   /* Queue of buckets to send (struct bgp_bucket) */
 
   HASH(struct bgp_prefix) prefix_hash; /* Prefixes to be sent */
+  u32 prefix_param;                    /* Parameter for prefix hash function */
   slab *prefix_slab;                   /* Slab holding prefix nodes */
 
   ip_addr next_hop_addr;               /* Local address for NEXT_HOP attribute */

diff --git a/sysdep/unix/random.c b/sysdep/unix/random.c
index 4e64e56b8a9c0a2468196ca709cc073d1e6ef3af..1d0e6a22953d9a685e2b5c435b9f31d00fe3738c 100644 (file)
--- a/sysdep/unix/random.c
+++ b/sysdep/unix/random.c
@@ -24,13 +24,27 @@
 u32
 random_u32(void)
 {
-  long int rand_low, rand_high;
+  u32 rand_low, rand_high;
 
   rand_low = random();
   rand_high = random();
   return (rand_low & 0xffff) | ((rand_high & 0xffff) << 16);
 }
 
+/* Generate random hash parameter (odd, bits roughly balanced) */
+u32
+random_hash_param(void)
+{
+  while (1)
+  {
+    u32 p = random_u32() | 1;
+    u32 c = u32_popcount(p);
+
+    if ((c >= 12) && (c <= 20))
+      return p;
+  }
+}
+
 
 /* If there is no getrandom() / getentropy(), use /dev/urandom */
 #if !defined(HAVE_GETRANDOM) && !defined(HAVE_GETENTROPY)