public inbox for passt-dev@passt.top
 help / color / mirror / code / Atom feed
From: David Gibson <david@gibson.dropbear.id.au>
To: Stefano Brivio <sbrivio@redhat.com>, passt-dev@passt.top
Cc: David Gibson <david@gibson.dropbear.id.au>
Subject: [PATCH 04/10] siphash: Clean up hash finalisation with posthash_final() function
Date: Sat, 23 Sep 2023 00:06:24 +1000	[thread overview]
Message-ID: <20230922140630.3184256-5-david@gibson.dropbear.id.au> (raw)
In-Reply-To: <20230922140630.3184256-1-david@gibson.dropbear.id.au>

The POSTAMBLE macro implements the finalisation steps of SipHash.  It
relies on some variables in the environment, including returning the final
hash value that way.  That isn't great hygeine.

In addition the PREAMBLE macro takes a length parameter which is used only
to initialize the 'b' value that's not used until the finalisation and is
also sometimes modified in a non-obvious way by the callers.

The 'b' value is always composed from the total length of the hash input
plus up to 7 bytes of "tail" data - that is the remainder of the hash input
after a multiple of 8 bytes has been consumed.

Simplify all this by replacing the POSTAMBLE macro with a siphash_final()
function which takes the length and tail data as parameters and returns the
final hash value.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 siphash.c | 58 +++++++++++++++++++++++++++----------------------------
 1 file changed, 28 insertions(+), 30 deletions(-)

diff --git a/siphash.c b/siphash.c
index 716ab62..ec39793 100644
--- a/siphash.c
+++ b/siphash.c
@@ -51,16 +51,16 @@
  *
  */
 
+#include <stddef.h>
 #include <stdint.h>
 
 #include "siphash.h"
 
 #define ROTL(x, b) (uint64_t)(((x) << (b)) | ((x) >> (64 - (b))))
 
-#define PREAMBLE(len)							  \
+#define PREAMBLE							  \
 	uint64_t v[4] = { 0x736f6d6570736575ULL, 0x646f72616e646f6dULL,	  \
 			  0x6c7967656e657261ULL, 0x7465646279746573ULL }; \
-	uint64_t b = (uint64_t)(len) << 56;				  \
 	int __i;							  \
 									  \
 	do {								  \
@@ -103,13 +103,21 @@ static inline void siphash_feed(uint64_t *v, uint64_t in)
 	v[0] ^= in;
 }
 
-#define POSTAMBLE							  \
-	do {								  \
-		siphash_feed(v, b);					  \
-		v[2] ^= 0xff;						  \
-		sipround(v, 4);						  \
-		b = (v[0] ^ v[1]) ^ (v[2] ^ v[3]);			  \
-	} while (0)
+/**
+ * siphash_final - Finalize SipHash calculations
+ * @v:		siphash state (4 x 64-bit integers)
+ * @len:	Total length of input data
+ * @tail:	Final data for the hash (<= 7 bytes)
+ */
+static inline uint64_t siphash_final(uint64_t *v, size_t len, uint64_t tail)
+{
+	uint64_t b = (uint64_t)(len) << 56 | tail;
+
+	siphash_feed(v, b);
+	v[2] ^= 0xff;
+	sipround(v, 4);
+	return v[0] ^ v[1] ^ v[2] ^ v[3];
+}
 
 /**
  * siphash_8b() - Table index or timestamp offset for TCP over IPv4 (8 bytes in)
@@ -132,11 +140,11 @@ __attribute__((optimize("-fno-strict-aliasing")))
 /* cppcheck-suppress unusedFunction */
 uint64_t siphash_8b(const uint8_t *in, const uint64_t *k)
 {
-	PREAMBLE(8);
+	PREAMBLE;
 	siphash_feed(v, *(uint64_t *)in);
-	POSTAMBLE;
 
-	return b;
+
+	return siphash_final(v, 8, 0);
 }
 
 /**
@@ -153,12 +161,10 @@ uint64_t siphash_12b(const uint8_t *in, const uint64_t *k)
 {
 	uint32_t *in32 = (uint32_t *)in;
 
-	PREAMBLE(12);
+	PREAMBLE;
 	siphash_feed(v, (uint64_t)(*(in32 + 1)) << 32 | *in32);
-	b |= *(in32 + 2);
-	POSTAMBLE;
 
-	return b;
+	return siphash_final(v, 12, *(in32 + 2));
 }
 
 /**
@@ -175,15 +181,12 @@ uint64_t siphash_20b(const uint8_t *in, const uint64_t *k)
 	uint32_t *in32 = (uint32_t *)in;
 	int i;
 
-	PREAMBLE(20);
+	PREAMBLE;
 
 	for (i = 0; i < 2; i++, in32 += 2)
 		siphash_feed(v, (uint64_t)(*(in32 + 1)) << 32 | *in32);
 
-	b |= *in32;
-	POSTAMBLE;
-
-	return b;
+	return siphash_final(v, 20, *in32);
 }
 
 /**
@@ -201,14 +204,12 @@ uint64_t siphash_32b(const uint8_t *in, const uint64_t *k)
 	uint64_t *in64 = (uint64_t *)in;
 	int i;
 
-	PREAMBLE(32);
+	PREAMBLE;
 
 	for (i = 0; i < 4; i++, in64++)
 		siphash_feed(v, *in64);
 
-	POSTAMBLE;
-
-	return b;
+	return siphash_final(v, 32, 0);
 }
 
 /**
@@ -225,13 +226,10 @@ uint64_t siphash_36b(const uint8_t *in, const uint64_t *k)
 	uint32_t *in32 = (uint32_t *)in;
 	int i;
 
-	PREAMBLE(36);
+	PREAMBLE;
 
 	for (i = 0; i < 4; i++, in32 += 2)
 		siphash_feed(v, (uint64_t)(*(in32 + 1)) << 32 | *in32);
 
-	b |= *in32;
-	POSTAMBLE;
-
-	return b;
+	return siphash_final(v, 36, *in32);
 }
-- 
@@ -51,16 +51,16 @@
  *
  */
 
+#include <stddef.h>
 #include <stdint.h>
 
 #include "siphash.h"
 
 #define ROTL(x, b) (uint64_t)(((x) << (b)) | ((x) >> (64 - (b))))
 
-#define PREAMBLE(len)							  \
+#define PREAMBLE							  \
 	uint64_t v[4] = { 0x736f6d6570736575ULL, 0x646f72616e646f6dULL,	  \
 			  0x6c7967656e657261ULL, 0x7465646279746573ULL }; \
-	uint64_t b = (uint64_t)(len) << 56;				  \
 	int __i;							  \
 									  \
 	do {								  \
@@ -103,13 +103,21 @@ static inline void siphash_feed(uint64_t *v, uint64_t in)
 	v[0] ^= in;
 }
 
-#define POSTAMBLE							  \
-	do {								  \
-		siphash_feed(v, b);					  \
-		v[2] ^= 0xff;						  \
-		sipround(v, 4);						  \
-		b = (v[0] ^ v[1]) ^ (v[2] ^ v[3]);			  \
-	} while (0)
+/**
+ * siphash_final - Finalize SipHash calculations
+ * @v:		siphash state (4 x 64-bit integers)
+ * @len:	Total length of input data
+ * @tail:	Final data for the hash (<= 7 bytes)
+ */
+static inline uint64_t siphash_final(uint64_t *v, size_t len, uint64_t tail)
+{
+	uint64_t b = (uint64_t)(len) << 56 | tail;
+
+	siphash_feed(v, b);
+	v[2] ^= 0xff;
+	sipround(v, 4);
+	return v[0] ^ v[1] ^ v[2] ^ v[3];
+}
 
 /**
  * siphash_8b() - Table index or timestamp offset for TCP over IPv4 (8 bytes in)
@@ -132,11 +140,11 @@ __attribute__((optimize("-fno-strict-aliasing")))
 /* cppcheck-suppress unusedFunction */
 uint64_t siphash_8b(const uint8_t *in, const uint64_t *k)
 {
-	PREAMBLE(8);
+	PREAMBLE;
 	siphash_feed(v, *(uint64_t *)in);
-	POSTAMBLE;
 
-	return b;
+
+	return siphash_final(v, 8, 0);
 }
 
 /**
@@ -153,12 +161,10 @@ uint64_t siphash_12b(const uint8_t *in, const uint64_t *k)
 {
 	uint32_t *in32 = (uint32_t *)in;
 
-	PREAMBLE(12);
+	PREAMBLE;
 	siphash_feed(v, (uint64_t)(*(in32 + 1)) << 32 | *in32);
-	b |= *(in32 + 2);
-	POSTAMBLE;
 
-	return b;
+	return siphash_final(v, 12, *(in32 + 2));
 }
 
 /**
@@ -175,15 +181,12 @@ uint64_t siphash_20b(const uint8_t *in, const uint64_t *k)
 	uint32_t *in32 = (uint32_t *)in;
 	int i;
 
-	PREAMBLE(20);
+	PREAMBLE;
 
 	for (i = 0; i < 2; i++, in32 += 2)
 		siphash_feed(v, (uint64_t)(*(in32 + 1)) << 32 | *in32);
 
-	b |= *in32;
-	POSTAMBLE;
-
-	return b;
+	return siphash_final(v, 20, *in32);
 }
 
 /**
@@ -201,14 +204,12 @@ uint64_t siphash_32b(const uint8_t *in, const uint64_t *k)
 	uint64_t *in64 = (uint64_t *)in;
 	int i;
 
-	PREAMBLE(32);
+	PREAMBLE;
 
 	for (i = 0; i < 4; i++, in64++)
 		siphash_feed(v, *in64);
 
-	POSTAMBLE;
-
-	return b;
+	return siphash_final(v, 32, 0);
 }
 
 /**
@@ -225,13 +226,10 @@ uint64_t siphash_36b(const uint8_t *in, const uint64_t *k)
 	uint32_t *in32 = (uint32_t *)in;
 	int i;
 
-	PREAMBLE(36);
+	PREAMBLE;
 
 	for (i = 0; i < 4; i++, in32 += 2)
 		siphash_feed(v, (uint64_t)(*(in32 + 1)) << 32 | *in32);
 
-	b |= *in32;
-	POSTAMBLE;
-
-	return b;
+	return siphash_final(v, 36, *in32);
 }
-- 
2.41.0


  parent reply	other threads:[~2023-09-22 14:06 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-09-22 14:06 [PATCH 00/10] siphash: cleanups and fixes David Gibson
2023-09-22 14:06 ` [PATCH 01/10] siphash: Make siphash functions consistently return 64-bit results David Gibson
2023-09-22 14:06 ` [PATCH 02/10] siphash: Make sip round calculations an inline function rather than macro David Gibson
2023-09-22 14:06 ` [PATCH 03/10] siphash: Add siphash_feed() helper David Gibson
2023-09-22 14:06 ` David Gibson [this message]
2023-09-22 14:06 ` [PATCH 05/10] siphash: Fix bug in state initialisation David Gibson
2023-09-22 14:06 ` [PATCH 06/10] siphash: Use more hygienic state initialiser David Gibson
2023-09-27 17:04   ` Stefano Brivio
2023-09-28  1:20     ` David Gibson
2023-09-29 15:19       ` Stefano Brivio
2023-09-22 14:06 ` [PATCH 07/10] siphash: Use specific structure for internal state David Gibson
2023-09-22 14:06 ` [PATCH 08/10] siphash: Make internal helpers public David Gibson
2023-09-22 14:06 ` [PATCH 09/10] siphash, checksum: Move TBAA explanation to checksum.c David Gibson
2023-09-22 14:06 ` [PATCH 10/10] siphash: Use incremental rather than all-at-once siphash functions David Gibson
2023-09-26  6:23   ` David Gibson
2023-09-26  7:02     ` David Gibson
2023-09-27 17:05       ` Stefano Brivio

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230922140630.3184256-5-david@gibson.dropbear.id.au \
    --to=david@gibson.dropbear.id.au \
    --cc=passt-dev@passt.top \
    --cc=sbrivio@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://passt.top/passt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for IMAP folder(s).