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 10/10] siphash: Use incremental rather than all-at-once siphash functions
Date: Sat, 23 Sep 2023 00:06:30 +1000	[thread overview]
Message-ID: <20230922140630.3184256-11-david@gibson.dropbear.id.au> (raw)
In-Reply-To: <20230922140630.3184256-1-david@gibson.dropbear.id.au>

We have a bunch of variants of the siphash functions for different data
sizes.  The callers, in tcp.c, need to pack the various values they want to
hash into a temporary structure, then call the appropriate version.  We can
avoid the copy into the temporary by directly using the incremental
siphash functions.

The length specific hash functions also have an undocumented constraint
that the data pointer they take must, in fact, be aligned to avoid
unaligned accesses, which may cause crashes on some architectures.

So, prefer the incremental approach and remove the length-specific
functions.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 Makefile     |   2 +-
 inany.h      |  16 ++++++-
 siphash.c    | 121 ---------------------------------------------------
 tcp.c        |  32 +++++---------
 tcp_splice.c |   1 +
 5 files changed, 27 insertions(+), 145 deletions(-)
 delete mode 100644 siphash.c

diff --git a/Makefile b/Makefile
index 4435bd6..ec3c3fb 100644
--- a/Makefile
+++ b/Makefile
@@ -45,7 +45,7 @@ FLAGS += -DDUAL_STACK_SOCKETS=$(DUAL_STACK_SOCKETS)
 
 PASST_SRCS = arch.c arp.c checksum.c conf.c dhcp.c dhcpv6.c icmp.c igmp.c \
 	isolation.c lineread.c log.c mld.c ndp.c netlink.c packet.c passt.c \
-	pasta.c pcap.c siphash.c tap.c tcp.c tcp_splice.c udp.c util.c
+	pasta.c pcap.c tap.c tcp.c tcp_splice.c udp.c util.c
 QRAP_SRCS = qrap.c
 SRCS = $(PASST_SRCS) $(QRAP_SRCS)
 
diff --git a/inany.h b/inany.h
index aadb20b..266d101 100644
--- a/inany.h
+++ b/inany.h
@@ -14,8 +14,9 @@
  * @v4mapped.zero:	All zero-bits for an IPv4 address
  * @v4mapped.one:	All one-bits for an IPv4 address
  * @v4mapped.a4:	If @a6 is an IPv4 mapped address, the IPv4 address
+ * @u64:		As an array of u64s (solely for hashing)
  *
- * @v4mapped shouldn't be accessed except via helpers.
+ * @v4mapped and @u64 shouldn't be accessed except via helpers.
  */
 union inany_addr {
 	struct in6_addr a6;
@@ -24,7 +25,9 @@ union inany_addr {
 		uint8_t one[2];
 		struct in_addr a4;
 	} v4mapped;
+	uint64_t u64[2];
 };
+static_assert(sizeof(union inany_addr) == sizeof(struct in6_addr));
 
 /** inany_v4 - Extract IPv4 address, if present, from IPv[46] address
  * @addr:	IPv4 or IPv6 address
@@ -94,4 +97,15 @@ static inline void inany_from_sockaddr(union inany_addr *aa, in_port_t *port,
 	}
 }
 
+/** inany_siphash_feed- Fold IPv[46] address into an in-progress siphash
+ * @state:	siphash state
+ * @aa:		inany to hash
+ */
+static inline void inany_siphash_feed(struct siphash_state *state,
+				      const union inany_addr *aa)
+{
+	siphash_feed(state, aa->u64[0]);
+	siphash_feed(state, aa->u64[1]);
+}
+
 #endif /* INANY_H */
diff --git a/siphash.c b/siphash.c
deleted file mode 100644
index d2b068c..0000000
--- a/siphash.c
+++ /dev/null
@@ -1,121 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0-or-later
-
-/* PASST - Plug A Simple Socket Transport
- *  for qemu/UNIX domain socket mode
- *
- * PASTA - Pack A Subtle Tap Abstraction
- *  for network namespace/tap device mode
- *
- * siphash.c - SipHash routines
- *
- * Copyright (c) 2020-2021 Red Hat GmbH
- * Author: Stefano Brivio <sbrivio@redhat.com>
- */
-
-#include <stddef.h>
-#include <stdint.h>
-
-#include "siphash.h"
-
-/**
- * siphash_8b() - Table index or timestamp offset for TCP over IPv4 (8 bytes in)
- * @in:		Input data (remote address and two ports, or two addresses)
- * @k:		Hash function key, 128 bits
- *
- * Return: the 64-bit hash output
- */
-/* NOLINTNEXTLINE(clang-diagnostic-unknown-attributes) */
-__attribute__((optimize("-fno-strict-aliasing")))	/* See csum_16b() */
-/* cppcheck-suppress unusedFunction */
-uint64_t siphash_8b(const uint8_t *in, const uint64_t *k)
-{
-	struct siphash_state state = SIPHASH_INIT(k);
-
-	siphash_feed(&state, *(uint64_t *)in);
-
-	return siphash_final(&state, 8, 0);
-}
-
-/**
- * siphash_12b() - Initial sequence number for TCP over IPv4 (12 bytes in)
- * @in:		Input data (two addresses, two ports)
- * @k:		Hash function key, 128 bits
- *
- * Return: the 64-bit hash output
- */
-/* NOLINTNEXTLINE(clang-diagnostic-unknown-attributes) */
-__attribute__((optimize("-fno-strict-aliasing")))	/* See csum_16b() */
-/* cppcheck-suppress unusedFunction */
-uint64_t siphash_12b(const uint8_t *in, const uint64_t *k)
-{
-	struct siphash_state state = SIPHASH_INIT(k);
-	uint32_t *in32 = (uint32_t *)in;
-
-	siphash_feed(&state, (uint64_t)(*(in32 + 1)) << 32 | *in32);
-
-	return siphash_final(&state, 12, *(in32 + 2));
-}
-
-/**
- * siphash_20b() - Table index for TCP over IPv6 (20 bytes in)
- * @in:		Input data (remote address, two ports)
- * @k:		Hash function key, 128 bits
- *
- * Return: the 64-bit hash output
- */
-/* NOLINTNEXTLINE(clang-diagnostic-unknown-attributes) */
-__attribute__((optimize("-fno-strict-aliasing")))	/* See csum_16b() */
-uint64_t siphash_20b(const uint8_t *in, const uint64_t *k)
-{
-	struct siphash_state state = SIPHASH_INIT(k);
-	uint32_t *in32 = (uint32_t *)in;
-	int i;
-
-	for (i = 0; i < 2; i++, in32 += 2)
-		siphash_feed(&state, (uint64_t)(*(in32 + 1)) << 32 | *in32);
-
-	return siphash_final(&state, 20, *in32);
-}
-
-/**
- * siphash_32b() - Timestamp offset for TCP over IPv6 (32 bytes in)
- * @in:		Input data (two addresses)
- * @k:		Hash function key, 128 bits
- *
- * Return: the 64-bit hash output
- */
-/* NOLINTNEXTLINE(clang-diagnostic-unknown-attributes) */
-__attribute__((optimize("-fno-strict-aliasing")))	/* See csum_16b() */
-/* cppcheck-suppress unusedFunction */
-uint64_t siphash_32b(const uint8_t *in, const uint64_t *k)
-{
-	struct siphash_state state = SIPHASH_INIT(k);
-	uint64_t *in64 = (uint64_t *)in;
-	int i;
-
-	for (i = 0; i < 4; i++, in64++)
-		siphash_feed(&state, *in64);
-
-	return siphash_final(&state, 32, 0);
-}
-
-/**
- * siphash_36b() - Initial sequence number for TCP over IPv6 (36 bytes in)
- * @in:		Input data (two addresses, two ports)
- * @k:		Hash function key, 128 bits
- *
- * Return: the 64-bit hash output
- */
-/* NOLINTNEXTLINE(clang-diagnostic-unknown-attributes) */
-__attribute__((optimize("-fno-strict-aliasing")))	/* See csum_16b() */
-uint64_t siphash_36b(const uint8_t *in, const uint64_t *k)
-{
-	struct siphash_state state = SIPHASH_INIT(k);
-	uint32_t *in32 = (uint32_t *)in;
-	int i;
-
-	for (i = 0; i < 4; i++, in32 += 2)
-		siphash_feed(&state, (uint64_t)(*(in32 + 1)) << 32 | *in32);
-
-	return siphash_final(&state, 36, *in32);
-}
diff --git a/tcp.c b/tcp.c
index 9f28020..18ceed1 100644
--- a/tcp.c
+++ b/tcp.c
@@ -1165,18 +1165,13 @@ static int tcp_hash_match(const struct tcp_tap_conn *conn,
 static unsigned int tcp_hash(const struct ctx *c, const union inany_addr *faddr,
 			     in_port_t eport, in_port_t fport)
 {
-	struct {
-		union inany_addr faddr;
-		in_port_t eport;
-		in_port_t fport;
-	} __attribute__((__packed__)) in = {
-		*faddr, eport, fport
-	};
-	uint64_t b = 0;
+	struct siphash_state state = SIPHASH_INIT(c->tcp.hash_secret);
+	uint64_t hash;
 
-	b = siphash_20b((uint8_t *)&in, c->tcp.hash_secret);
+	inany_siphash_feed(&state, faddr);
+	hash = siphash_final(&state, 20, (uint64_t)eport << 16 | fport);
 
-	return (unsigned int)(b % TCP_HASH_TABLE_SIZE);
+	return (unsigned int)(hash % TCP_HASH_TABLE_SIZE);
 }
 
 /**
@@ -1815,17 +1810,8 @@ static void tcp_clamp_window(const struct ctx *c, struct tcp_tap_conn *conn,
 static void tcp_seq_init(const struct ctx *c, struct tcp_tap_conn *conn,
 			 const struct timespec *now)
 {
+	struct siphash_state state = SIPHASH_INIT(c->tcp.hash_secret);
 	union inany_addr aany;
-	struct {
-		union inany_addr src;
-		in_port_t srcport;
-		union inany_addr dst;
-		in_port_t dstport;
-	} __attribute__((__packed__)) in = {
-		.src = conn->faddr,
-		.srcport = conn->fport,
-		.dstport = conn->eport,
-	};
 	uint64_t hash;
 	uint32_t ns;
 
@@ -1833,9 +1819,11 @@ static void tcp_seq_init(const struct ctx *c, struct tcp_tap_conn *conn,
 		inany_from_af(&aany, AF_INET, &c->ip4.addr);
 	else
 		inany_from_af(&aany, AF_INET6, &c->ip6.addr);
-	in.dst = aany;
 
-	hash = siphash_36b((uint8_t *)&in, c->tcp.hash_secret);
+	inany_siphash_feed(&state, &conn->faddr);
+	inany_siphash_feed(&state, &aany);
+	hash = siphash_final(&state, 36,
+			     (uint64_t)conn->fport << 16 | conn->eport);
 
 	/* 32ns ticks, overflows 32 bits every 137s */
 	ns = (now->tv_sec * 1000000000 + now->tv_nsec) >> 5;
diff --git a/tcp_splice.c b/tcp_splice.c
index 5b36975..3b98260 100644
--- a/tcp_splice.c
+++ b/tcp_splice.c
@@ -52,6 +52,7 @@
 #include "passt.h"
 #include "log.h"
 #include "tcp_splice.h"
+#include "siphash.h"
 #include "inany.h"
 
 #include "tcp_conn.h"
-- 
@@ -52,6 +52,7 @@
 #include "passt.h"
 #include "log.h"
 #include "tcp_splice.h"
+#include "siphash.h"
 #include "inany.h"
 
 #include "tcp_conn.h"
-- 
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 ` [PATCH 04/10] siphash: Clean up hash finalisation with posthash_final() function David Gibson
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 ` David Gibson [this message]
2023-09-26  6:23   ` [PATCH 10/10] siphash: Use incremental rather than all-at-once siphash functions 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-11-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).