Merge branch 'ondrej/remove-isc_hash_function' into 'main'

Use incremental hashing in the isc_sockaddr_hash() function

See merge request isc-projects/bind9!8298

lib/dns/dispatch.c

@@ -373,12 +373,15 @@ setup_socket(dns_dispatch_t *disp, dns_dispentry_t *resp,
 
 static uint32_t
 qid_hash(const dns_dispentry_t *dispentry) {
-	/*
+	isc_hash32_t hash;
-	 * TODO(OS): Add incremental isc_sockaddr_hash() function and then use
+
-	 * isc_hash32 API
+	isc_hash32_init(&hash);
-	 */
+
-	uint32_t hashval = isc_sockaddr_hash(&dispentry->peer, true);
+	isc_sockaddr_hash_ex(&hash, &dispentry->peer, true);
-	return (hashval ^ (((uint32_t)dispentry->id << 16) | dispentry->port));
+	isc_hash32_hash(&hash, &dispentry->id, sizeof(dispentry->id), true);
+	isc_hash32_hash(&hash, &dispentry->port, sizeof(dispentry->port), true);
+
+	return (isc_hash32_finalize(&hash));
 }
 
 static int

lib/isc/include/isc/hash.h

@@ -44,8 +44,6 @@ isc_hash_get_initializer(void);
 void
 isc_hash_set_initializer(const void *initializer);
 
-#define isc_hash_function isc_hash64
-
 void
 isc_hash32_init(isc_hash32_t *restrict state);
 void

lib/isc/include/isc/sockaddr.h

@@ -85,7 +85,16 @@ isc_sockaddr_eqaddrprefix(const isc_sockaddr_t *a, const isc_sockaddr_t *b,
  * If 'b''s scope is zero then 'a''s scope will be ignored.
  */
 
-unsigned int
+void
+isc_sockaddr_hash_ex(isc_hash32_t *hash, const isc_sockaddr_t *sockaddr,
+		     bool address_only);
+/*%<
+ * Add the hash of the sockaddr into the hash for incremental hashing
+ *
+ * See isc_sockaddr_hash() for details.
+ */
+
+uint32_t
 isc_sockaddr_hash(const isc_sockaddr_t *sockaddr, bool address_only);
 /*%<
  * Return a hash value for the socket address 'sockaddr'.  If 'address_only'

lib/isc/mem.c

@@ -216,11 +216,11 @@ add_trace_entry(isc_mem_t *mctx, const void *ptr, size_t size FLARG) {
 #ifdef __COVERITY__
 	/*
 	 * Use simple conversion from pointer to hash to avoid
-	 * tainting 'ptr' due to byte swap in isc_hash_function.
+	 * tainting 'ptr' due to byte swap in isc_hash32.
 	 */
 	hash = (uintptr_t)ptr >> 3;
 #else
-	hash = isc_hash_function(&ptr, sizeof(ptr), true);
+	hash = isc_hash32(&ptr, sizeof(ptr), true);
 #endif
 	idx = hash % DEBUG_TABLE_COUNT;
 
@@ -260,11 +260,11 @@ delete_trace_entry(isc_mem_t *mctx, const void *ptr, size_t size,
 #ifdef __COVERITY__
 	/*
 	 * Use simple conversion from pointer to hash to avoid
-	 * tainting 'ptr' due to byte swap in isc_hash_function.
+	 * tainting 'ptr' due to byte swap in isc_hash32.
 	 */
 	hash = (uintptr_t)ptr >> 3;
 #else
-	hash = isc_hash_function(&ptr, sizeof(ptr), true);
+	hash = isc_hash32(&ptr, sizeof(ptr), true);
 #endif
 	idx = hash % DEBUG_TABLE_COUNT;
 

lib/isc/sockaddr.c

@@ -187,51 +187,56 @@ isc_sockaddr_format(const isc_sockaddr_t *sa, char *array, unsigned int size) {
 	}
 }
 
-unsigned int
+void
-isc_sockaddr_hash(const isc_sockaddr_t *sockaddr, bool address_only) {
+isc_sockaddr_hash_ex(isc_hash32_t *hash, const isc_sockaddr_t *sockaddr,
-	unsigned int length = 0;
+		     bool address_only) {
-	const unsigned char *s = NULL;
+	REQUIRE(sockaddr != NULL);
-	unsigned int h = 0;
+
+	size_t len = 0;
+	const uint8_t *s = NULL;
 	unsigned int p = 0;
 	const struct in6_addr *in6;
 
-	REQUIRE(sockaddr != NULL);
-
 	switch (sockaddr->type.sa.sa_family) {
 	case AF_INET:
-		s = (const unsigned char *)&sockaddr->type.sin.sin_addr;
+		s = (const uint8_t *)&sockaddr->type.sin.sin_addr;
-		p = ntohs(sockaddr->type.sin.sin_port);
+		len = sizeof(sockaddr->type.sin.sin_addr.s_addr);
-		length = sizeof(sockaddr->type.sin.sin_addr.s_addr);
+		if (!address_only) {
+			p = ntohs(sockaddr->type.sin.sin_port);
+		}
 		break;
 	case AF_INET6:
 		in6 = &sockaddr->type.sin6.sin6_addr;
-		s = (const unsigned char *)in6;
+		s = (const uint8_t *)in6;
 		if (IN6_IS_ADDR_V4MAPPED(in6)) {
 			s += 12;
-			length = sizeof(sockaddr->type.sin.sin_addr.s_addr);
+			len = sizeof(sockaddr->type.sin.sin_addr.s_addr);
 		} else {
-			length = sizeof(sockaddr->type.sin6.sin6_addr);
+			len = sizeof(sockaddr->type.sin6.sin6_addr);
+		}
+		if (!address_only) {
+			p = ntohs(sockaddr->type.sin6.sin6_port);
 		}
-		p = ntohs(sockaddr->type.sin6.sin6_port);
 		break;
 	default:
-		UNEXPECTED_ERROR("unknown address family: %d",
+		UNREACHABLE();
-				 (int)sockaddr->type.sa.sa_family);
-		s = (const unsigned char *)&sockaddr->type;
-		length = sockaddr->length;
-		p = 0;
 	}
 
-	uint8_t buf[sizeof(struct sockaddr_storage) + sizeof(p)];
+	isc_hash32_hash(hash, s, len, true);
-	memmove(buf, s, length);
 	if (!address_only) {
-		memmove(buf + length, &p, sizeof(p));
+		isc_hash32_hash(hash, &p, sizeof(p), true);
-		h = isc_hash_function(buf, length + sizeof(p), true);
-	} else {
-		h = isc_hash_function(buf, length, true);
 	}
+}
 
-	return (h);
+uint32_t
+isc_sockaddr_hash(const isc_sockaddr_t *sockaddr, bool address_only) {
+	isc_hash32_t hash;
+
+	isc_hash32_init(&hash);
+
+	isc_sockaddr_hash_ex(&hash, sockaddr, address_only);
+
+	return (isc_hash32_finalize(&hash));
 }
 
 void

tests/isc/hash_test.c

@@ -34,60 +34,87 @@
 #include <tests/isc.h>
 
 /* Hash function test */
-ISC_RUN_TEST_IMPL(isc_hash_function) {
+ISC_RUN_TEST_IMPL(isc_hash32) {
-	unsigned int h1;
+	uint32_t h1;
-	unsigned int h2;
+	uint32_t h2;
-
-	UNUSED(state);
 
 	/* Immutability of hash function */
-	h1 = isc_hash_function(NULL, 0, true);
+	h1 = isc_hash32(NULL, 0, true);
-	h2 = isc_hash_function(NULL, 0, true);
+	h2 = isc_hash32(NULL, 0, true);
 
 	assert_int_equal(h1, h2);
 
 	/* Hash function characteristics */
-	h1 = isc_hash_function("Hello world", 12, true);
+	h1 = isc_hash32("Hello world", 12, true);
-	h2 = isc_hash_function("Hello world", 12, true);
+	h2 = isc_hash32("Hello world", 12, true);
 
 	assert_int_equal(h1, h2);
 
 	/* Case */
-	h1 = isc_hash_function("Hello world", 12, false);
+	h1 = isc_hash32("Hello world", 12, false);
-	h2 = isc_hash_function("heLLo WorLd", 12, false);
+	h2 = isc_hash32("heLLo WorLd", 12, false);
 
 	assert_int_equal(h1, h2);
 
 	/* Unequal */
-	h1 = isc_hash_function("Hello world", 12, true);
+	h1 = isc_hash32("Hello world", 12, true);
-	h2 = isc_hash_function("heLLo WorLd", 12, true);
+	h2 = isc_hash32("heLLo WorLd", 12, true);
+
+	assert_int_not_equal(h1, h2);
+}
+
+/* Hash function test */
+ISC_RUN_TEST_IMPL(isc_hash64) {
+	uint64_t h1;
+	uint64_t h2;
+
+	/* Immutability of hash function */
+	h1 = isc_hash64(NULL, 0, true);
+	h2 = isc_hash64(NULL, 0, true);
+
+	assert_int_equal(h1, h2);
+
+	/* Hash function characteristics */
+	h1 = isc_hash64("Hello world", 12, true);
+	h2 = isc_hash64("Hello world", 12, true);
+
+	assert_int_equal(h1, h2);
+
+	/* Case */
+	h1 = isc_hash64("Hello world", 12, false);
+	h2 = isc_hash64("heLLo WorLd", 12, false);
+
+	assert_int_equal(h1, h2);
+
+	/* Unequal */
+	h1 = isc_hash64("Hello world", 12, true);
+	h2 = isc_hash64("heLLo WorLd", 12, true);
 
 	assert_int_not_equal(h1, h2);
 }
 
 /* Hash function initializer test */
 ISC_RUN_TEST_IMPL(isc_hash_initializer) {
-	unsigned int h1;
+	uint64_t h1;
-	unsigned int h2;
+	uint64_t h2;
 
-	UNUSED(state);
+	h1 = isc_hash64("Hello world", 12, true);
-
+	h2 = isc_hash64("Hello world", 12, true);
-	h1 = isc_hash_function("Hello world", 12, true);
-	h2 = isc_hash_function("Hello world", 12, true);
 
 	assert_int_equal(h1, h2);
 
 	isc_hash_set_initializer(isc_hash_get_initializer());
 
 	/* Hash value must not change */
-	h2 = isc_hash_function("Hello world", 12, true);
+	h2 = isc_hash64("Hello world", 12, true);
 
 	assert_int_equal(h1, h2);
 }
 
 ISC_TEST_LIST_START
 
-ISC_TEST_ENTRY(isc_hash_function)
+ISC_TEST_ENTRY(isc_hash32)
+ISC_TEST_ENTRY(isc_hash64)
 ISC_TEST_ENTRY(isc_hash_initializer)
 
 ISC_TEST_LIST_END