Merge pull request #1839 from private-octopus/long-random-hash

Improve hash functions used by picohash tables

Author: huitema

CMakeLists.txt

@@ -8,7 +8,7 @@ else()
 endif()
 
 project(picoquic
-        VERSION 1.1.30.2
+        VERSION 1.1.31.0
         DESCRIPTION "picoquic library"
         LANGUAGES C CXX)
 
@@ -105,6 +105,7 @@ set(PICOQUIC_LIBRARY_FILES
     picoquic/sacks.c
     picoquic/sender.c
     picoquic/sim_link.c
+    picoquic/siphash.c
     picoquic/sockloop.c
     picoquic/spinbit.c
     picoquic/ticket_store.c
@@ -124,7 +125,8 @@ set(PICOQUIC_CORE_HEADERS
      picoquic/picoquic_logger.h
      picoquic/picoquic_binlog.h
      picoquic/picoquic_config.h
-     picoquic/picoquic_lb.h)
+     picoquic/picoquic_lb.h
+     picoquic/siphash.h)
 
 set(LOGLIB_LIBRARY_FILES
     loglib/autoqlog.c

UnitTest1/unittest1.cpp

@@ -48,6 +48,20 @@ namespace UnitTest1
             Assert::AreEqual(ret, 0);
         }
 
+        TEST_METHOD(picohash_bytes)
+        {
+            int ret = picohash_bytes_test();
+
+            Assert::AreEqual(ret, 0);
+        }
+
+        TEST_METHOD(siphash)
+        {
+            int ret = siphash_test();
+
+            Assert::AreEqual(ret, 0);
+        }
+
         TEST_METHOD(picolog_basic)
         {
             int ret = picolog_basic_test();

loglib/cidset.c

@@ -24,10 +24,10 @@
 #include "picoquic_utils.h"
 
 /* Hash and compare for CNX hash tables */
-static uint64_t picoquic_cid_hash(const void* key)
+static uint64_t picoquic_cid_hash(const void* key, const uint8_t * hash_seed)
 {
     const picoquic_connection_id_t* cid = (const picoquic_connection_id_t*)key;
-    return picoquic_connection_id_hash(cid);
+    return picoquic_connection_id_hash(cid, hash_seed);
 }
 
 static int picoquic_cid_compare(const void* key0, const void* key1)

picoquic/picohash.c

@@ -23,14 +23,17 @@
  * Basic hash implementation, like we have seen tons off already.
  */
 #include "picohash.h"
+#include "siphash.h"
 #include <stdlib.h>
 #include <string.h>
 
 picohash_table* picohash_create_ex(size_t nb_bin,
-    uint64_t (*picohash_hash)(const void*),
+    uint64_t (*picohash_hash)(const void*, const uint8_t *),
     int (*picohash_compare)(const void*, const void*),
-    picohash_item * (*picohash_key_to_item)(const void*))
+    picohash_item * (*picohash_key_to_item)(const void*),
+    const uint8_t* hash_seed)
 {
+    static const uint8_t null_seed[16] = { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 };
     picohash_table* t = (picohash_table*)malloc(sizeof(picohash_table));
     size_t items_length = sizeof(picohash_item*) * nb_bin;
     t->hash_bin = NULL;
@@ -48,21 +51,22 @@ picohash_table* picohash_create_ex(size_t nb_bin,
         t->picohash_hash = picohash_hash;
         t->picohash_compare = picohash_compare;
         t->picohash_key_to_item = picohash_key_to_item;
+        t->hash_seed = (hash_seed == NULL)? null_seed: hash_seed;
     }
 
     return t;
 }
 
 picohash_table* picohash_create(size_t nb_bin,
-    uint64_t(*picohash_hash)(const void*),
+    uint64_t(*picohash_hash)(const void*, const uint8_t*),
     int (*picohash_compare)(const void*, const void*))
 {
-    return picohash_create_ex(nb_bin, picohash_hash, picohash_compare, NULL);
+    return picohash_create_ex(nb_bin, picohash_hash, picohash_compare, NULL, NULL);
 }
 
 picohash_item* picohash_retrieve(picohash_table* hash_table, const void* key)
 {
-    uint64_t hash = hash_table->picohash_hash(key);
+    uint64_t hash = hash_table->picohash_hash(key, hash_table->hash_seed);
     uint32_t bin = (uint32_t)(hash % hash_table->nb_bin);
     picohash_item* item = hash_table->hash_bin[bin];
 
@@ -79,7 +83,7 @@ picohash_item* picohash_retrieve(picohash_table* hash_table, const void* key)
 
 int picohash_insert(picohash_table* hash_table, const void* key)
 {
-    uint64_t hash = hash_table->picohash_hash(key);
+    uint64_t hash = hash_table->picohash_hash(key, hash_table->hash_seed);
     uint32_t bin = (uint32_t)(hash % hash_table->nb_bin);
     int ret = 0;
     picohash_item* item;
@@ -150,19 +154,21 @@ void picohash_delete_key(picohash_table* hash_table, void* key, int delete_key_t
 
 void picohash_delete(picohash_table* hash_table, int delete_key_too)
 {
-    for (uint32_t i = 0; i < hash_table->nb_bin; i++) {
-        picohash_item* item = hash_table->hash_bin[i];
-        while (item != NULL) {
-            picohash_item* tmp = item;
-            const void* key_to_delete = tmp->key;
-
-            item = item->next_in_bin;
-
-            if (hash_table->picohash_key_to_item == NULL) {
-                free(tmp);
-            }
-            if (delete_key_too) {
-                free((void*)key_to_delete);
+    if (hash_table->count > 0) {
+        for (uint32_t i = 0; i < hash_table->nb_bin; i++) {
+            picohash_item* item = hash_table->hash_bin[i];
+            while (item != NULL) {
+                picohash_item* tmp = item;
+                const void* key_to_delete = tmp->key;
+
+                item = item->next_in_bin;
+
+                if (hash_table->picohash_key_to_item == NULL) {
+                    free(tmp);
+                }
+                if (delete_key_too) {
+                    free((void*)key_to_delete);
+                }
             }
         }
     }
@@ -171,23 +177,43 @@ void picohash_delete(picohash_table* hash_table, int delete_key_too)
     free(hash_table);
 }
 
-uint64_t picohash_hash_mix(uint64_t hash, uint64_t h2)
-{
-    h2 ^= (hash << 17) ^ (hash >> 37);
-    hash ^= ((h2 << 31) ^ (h2 >> 17));
-
-    return hash;
-}
-
-uint64_t picohash_bytes(const uint8_t* key, uint32_t length)
+uint64_t picohash_bytes(const uint8_t* bytes, size_t length, const uint8_t* hash_seed)
 {
-    uint64_t hash = 0xDEADBEEF;
+    uint64_t hash =
+        ((uint64_t)hash_seed[8]) +
+        (((uint64_t)hash_seed[9]) << 8) +
+        (((uint64_t)hash_seed[10]) << 16) +
+        (((uint64_t)hash_seed[11]) << 24) +
+        (((uint64_t)hash_seed[12]) << 32) +
+        (((uint64_t)hash_seed[13]) << 40) +
+        (((uint64_t)hash_seed[14]) << 48) +
+        (((uint64_t)hash_seed[15]) << 56);
+    int rotate = 11;
 
     for (uint32_t i = 0; i < length; i++) {
-        hash ^= key[i];
-        hash ^= ((hash << 31) ^ (hash >> 17));
+        hash ^= bytes[i];
+        hash ^= hash_seed[i & 15];
+        hash ^= (hash << 8);
+        hash += (hash >> rotate);
+        rotate = (int)(hash & 31) + 11;
     }
-
+    hash ^= (hash >> rotate);
     return hash;
 }
 
+uint64_t picohash_siphash(const uint8_t* bytes, size_t length, const uint8_t* hash_seed)
+{
+    uint8_t sip_out[8];
+    uint64_t hash;
+    (void)siphash(bytes, length, hash_seed, sip_out, 8);
+    hash =
+        (uint64_t)sip_out[0] +
+        (((uint64_t)sip_out[1]) << 8) +
+        (((uint64_t)sip_out[2]) << 16) +
+        (((uint64_t)sip_out[3]) << 24) +
+        (((uint64_t)sip_out[4]) << 32) +
+        (((uint64_t)sip_out[5]) << 40) +
+        (((uint64_t)sip_out[6]) << 48) +
+        (((uint64_t)sip_out[7]) << 56);
+    return hash;
+}

picoquic/picohash.h

@@ -44,19 +44,21 @@ typedef struct picohash_table {
     picohash_item** hash_bin;
     size_t nb_bin;
     size_t count;
-    uint64_t (*picohash_hash)(const void*);
+    const uint8_t* hash_seed;
+    uint64_t (*picohash_hash)(const void*, const uint8_t*);
     int (*picohash_compare)(const void*, const void*);
     picohash_item* (*picohash_key_to_item)(const void*);
 } picohash_table;
 
 picohash_table* picohash_create(size_t nb_bin,
-    uint64_t (*picohash_hash)(const void*),
+    uint64_t (*picohash_hash)(const void*, const uint8_t*),
     int (*picohash_compute)(const void*, const void*));
 
 picohash_table* picohash_create_ex(size_t nb_bin,
-    uint64_t(*picohash_hash)(const void*),
+    uint64_t(*picohash_hash)(const void*, const uint8_t*),
     int (*picohash_compare)(const void*, const void*),
-    picohash_item* (*picohash_key_to_item)(const void*));
+    picohash_item* (*picohash_key_to_item)(const void*),
+    const uint8_t* hash_seed);
 
 picohash_item* picohash_retrieve(picohash_table* hash_table, const void* key);
 
@@ -68,9 +70,9 @@ void picohash_delete_key(picohash_table* hash_table, void* key, int delete_key_t
 
 void picohash_delete(picohash_table* hash_table, int delete_key_too);
 
-uint64_t picohash_hash_mix(uint64_t hash, uint64_t h2);
+uint64_t picohash_bytes(const uint8_t* key, size_t length, const uint8_t* hash_seed);
 
-uint64_t picohash_bytes(const uint8_t* key, uint32_t length);
+uint64_t picohash_siphash(const uint8_t* bytes, size_t length, const uint8_t* hash_seed);
 
 #ifdef __cplusplus
 }

picoquic/picoquic.h

@@ -40,7 +40,7 @@
 extern "C" {
 #endif
 
-#define PICOQUIC_VERSION "1.1.30.2"
+#define PICOQUIC_VERSION "1.1.31.0"
 #define PICOQUIC_ERROR_CLASS 0x400
 #define PICOQUIC_ERROR_DUPLICATE (PICOQUIC_ERROR_CLASS + 1)
 #define PICOQUIC_ERROR_AEAD_CHECK (PICOQUIC_ERROR_CLASS + 3)

picoquic/picoquic.vcxproj

@@ -175,6 +175,7 @@
     <ClCompile Include="sender.c" />
     <ClCompile Include="bbr.c" />
     <ClCompile Include="sim_link.c" />
+    <ClCompile Include="siphash.c" />
     <ClCompile Include="sockloop.c" />
     <ClCompile Include="spinbit.c" />
     <ClCompile Include="ticket_store.c" />

picoquic/picoquic.vcxproj.filters

@@ -138,6 +138,9 @@
     <ClCompile Include="pacing.c">
       <Filter>Source Files</Filter>
     </ClCompile>
+    <ClCompile Include="siphash.c">
+      <Filter>Source Files</Filter>
+    </ClCompile>
   </ItemGroup>
   <ItemGroup>
     <ClInclude Include="picoquic.h">

picoquic/picoquic_internal.h

@@ -613,7 +613,7 @@ typedef struct st_picoquic_quic_t {
     uint8_t reset_seed[PICOQUIC_RESET_SECRET_SIZE];
     uint8_t retry_seed[PICOQUIC_RETRY_SECRET_SIZE];
     uint64_t* p_simulated_time;
-    uint8_t hash_seed[8];
+    uint8_t hash_seed[16];
     char const* ticket_file_name;
     char const* token_file_name;
     picoquic_stored_ticket_t * p_first_ticket;

picoquic/picoquic_utils.h

@@ -90,9 +90,10 @@ uint8_t picoquic_format_connection_id(uint8_t* bytes, size_t bytes_max, picoquic
 uint8_t picoquic_parse_connection_id(const uint8_t* bytes, uint8_t len, picoquic_connection_id_t *cnx_id);
 int picoquic_is_connection_id_null(const picoquic_connection_id_t * cnx_id);
 int picoquic_compare_connection_id(const picoquic_connection_id_t * cnx_id1, const picoquic_connection_id_t * cnx_id2);
-uint64_t picoquic_connection_id_hash(const picoquic_connection_id_t * cid);
+uint64_t picoquic_connection_id_hash(const picoquic_connection_id_t * cid, const uint8_t * hash_seed);
 uint64_t picoquic_val64_connection_id(picoquic_connection_id_t cnx_id);
-uint64_t picoquic_hash_addr(const struct sockaddr* addr);
+size_t picoquic_hash_addr_bytes(const struct sockaddr* addr, uint8_t* bytes);
+uint64_t picoquic_hash_addr(const struct sockaddr* addr, const uint8_t* hash_seed);
 size_t picoquic_parse_hexa(char const* hex_input, size_t input_length, uint8_t* bin_output, size_t output_max);
 uint8_t picoquic_parse_connection_id_hexa(char const * hex_input, size_t input_length, picoquic_connection_id_t * cnx_id);
 int picoquic_print_connection_id_hexa(char* buf, size_t buf_len, const picoquic_connection_id_t* cnxid);