From: Claudio Jeker Subject: Re: inline syn cache hash function To: Alexander Bluhm Cc: tech@openbsd.org Date: Tue, 12 Aug 2025 14:16:03 +0200 On Tue, Aug 12, 2025 at 01:56:35PM +0200, Alexander Bluhm wrote: > On Tue, Aug 12, 2025 at 07:27:48AM +0200, Claudio Jeker wrote: > > On Sat, Aug 09, 2025 at 01:40:12PM +0200, Alexander Bluhm wrote: > > > Hi, > > > > > > I would like to convert these nested macros into am inline function > > > to calculate the has for the TCP SYN cache. > > > > > > ok? > > > > Why is this not using siphash? I do not trust these hash function. > > This hash seems to be trivially reversible and so it is possible to attack > > it. > > Because SIP hash is slow and nobody made a performance analysis for > TCP SYN. Nobody did a security analysis for the TCP SYN hash either or even looking if it is a good hash function. > We have SIP hash in inet PCB, where it can take significant amount > of CPU time. When sending short UDP packets from Linux to OpenBSD > the hash function takes 11.3% CPU time. > > http://bluhm.genua.de/perform/results/2025-07-30T08:32:36Z/2025-07-30T00%3A00%3A00Z/btrace/netbench.pl_-v_-B1000000000_-b1000000_-d1_-f0_-i3_-N10_-cperform%40lt13_-a10.3.46.40_-t10_udpbench-btrace-kstack.0.svg?s=in_pcbhash I do not trust flamegraphs. They misreport a lot of things because of the way dt(4) works. Also a lot of the time can be attributed to slow string functions. > The trivial SYN hash can be attacked, but the SYN cache will regularly > reshuffle its random. I have implemented this to prevent timing > attacks on the length of the hash buckets. > > Maybe xxhash could be an alternative between slow SIP hash and trvial > hashes. https://github.com/Cyan4973/xxHash Maybe. > > > Index: netinet/tcp_input.c > > > =================================================================== > > > RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/tcp_input.c,v > > > diff -u -p -r1.457 tcp_input.c > > > --- netinet/tcp_input.c 24 Jul 2025 21:34:07 -0000 1.457 > > > +++ netinet/tcp_input.c 8 Aug 2025 15:17:55 -0000 > > > @@ -3216,42 +3216,38 @@ struct syn_cache_set tcp_syn_cache[2]; > > > int tcp_syn_cache_active; /* [S] */ > > > struct mutex syn_cache_mtx = MUTEX_INITIALIZER(IPL_SOFTNET); > > > > > > -#define SYN_HASH(sa, sp, dp, rand) \ > > > - (((sa)->s_addr ^ (rand)[0]) * \ > > > - (((((u_int32_t)(dp))<<16) + ((u_int32_t)(sp))) ^ (rand)[4])) > > > -#ifndef INET6 > > > -#define SYN_HASHALL(hash, src, dst, rand) \ > > > -do { \ > > > - hash = SYN_HASH(&satosin_const(src)->sin_addr, \ > > > - satosin_const(src)->sin_port, \ > > > - satosin_const(dst)->sin_port, (rand)); \ > > > -} while (/*CONSTCOND*/ 0) > > > -#else > > > -#define SYN_HASH6(sa, sp, dp, rand) \ > > > - (((sa)->s6_addr32[0] ^ (rand)[0]) * \ > > > - ((sa)->s6_addr32[1] ^ (rand)[1]) * \ > > > - ((sa)->s6_addr32[2] ^ (rand)[2]) * \ > > > - ((sa)->s6_addr32[3] ^ (rand)[3]) * \ > > > - (((((u_int32_t)(dp))<<16) + ((u_int32_t)(sp))) ^ (rand)[4])) > > > - > > > -#define SYN_HASHALL(hash, src, dst, rand) \ > > > -do { \ > > > - switch ((src)->sa_family) { \ > > > - case AF_INET: \ > > > - hash = SYN_HASH(&satosin_const(src)->sin_addr, \ > > > - satosin_const(src)->sin_port, \ > > > - satosin_const(dst)->sin_port, (rand)); \ > > > - break; \ > > > - case AF_INET6: \ > > > - hash = SYN_HASH6(&satosin6_const(src)->sin6_addr, \ > > > - satosin6_const(src)->sin6_port, \ > > > - satosin6_const(dst)->sin6_port, (rand)); \ > > > - break; \ > > > - default: \ > > > - hash = 0; \ > > > - } \ > > > -} while (/*CONSTCOND*/0) > > > -#endif /* INET6 */ > > > +static inline uint32_t > > > +syn_cache_hash(const struct sockaddr *src, const struct sockaddr *dst, > > > + uint32_t rand[]) > > > +{ > > > + switch (src->sa_family) { > > > + case AF_INET: { > > > + uint32_t src_port = satosin_const(src)->sin_port; > > > + uint32_t dst_port = satosin_const(dst)->sin_port; > > > + const in_addr_t *src_addr = > > > + &satosin_const(src)->sin_addr.s_addr; > > > + > > > + return ((((dst_port << 16) + src_port) ^ rand[4]) * > > > + (*src_addr ^ rand[0])); > > > + } > > > +#ifdef INET6 > > > + case AF_INET6: { > > > + uint32_t src_port = satosin6_const(src)->sin6_port; > > > + uint32_t dst_port = satosin6_const(dst)->sin6_port; > > > + const uint32_t *src_addr6 = > > > + satosin6_const(src)->sin6_addr.s6_addr32; > > > + > > > + return ((((dst_port << 16) + src_port) ^ rand[4]) * > > > + (src_addr6[0] ^ rand[0]) * > > > + (src_addr6[1] ^ rand[1]) * > > > + (src_addr6[2] ^ rand[2]) * > > > + (src_addr6[3] ^ rand[3])); > > > + } > > > +#endif > > > + default: > > > + unhandled_af(src->sa_family); > > > + } > > > +} > > > > > > void > > > syn_cache_rm(struct syn_cache *sc) > > > @@ -3345,7 +3341,7 @@ syn_cache_insert(struct syn_cache *sc, s > > > tcpstat_inc(tcps_sc_seedrandom); > > > } > > > > > > - SYN_HASHALL(sc->sc_hash, &sc->sc_src.sa, &sc->sc_dst.sa, > > > + sc->sc_hash = syn_cache_hash(&sc->sc_src.sa, &sc->sc_dst.sa, > > > set->scs_random); > > > scp = &set->scs_buckethead[sc->sc_hash % set->scs_size]; > > > sc->sc_buckethead = scp; > > > @@ -3564,7 +3560,7 @@ syn_cache_lookup(const struct sockaddr * > > > for (i = 0; i < 2; i++) { > > > if (sets[i]->scs_count == 0) > > > continue; > > > - SYN_HASHALL(hash, src, dst, sets[i]->scs_random); > > > + hash = syn_cache_hash(src, dst, sets[i]->scs_random); > > > scp = &sets[i]->scs_buckethead[hash % sets[i]->scs_size]; > > > *headp = scp; > > > TAILQ_FOREACH(sc, &scp->sch_bucket, sc_bucketq) { > > > > > > > -- > > :wq Claudio > -- :wq Claudio