Convert bgpd to use siphash

2014-12-04 Thread Dimitris Papastamos
Hi,

This is my attempt at converting bgpd to use siphash.  This does
*not* link as is because the userland implementation of siphash
is currently missing.

In certain cases where multiple data is hashed in succession, it might
have been better to use a siphash context and call SipHash24_Update().

Any feedback?

Cheers,
Dimitris

Index: Makefile
===
RCS file: /cvs/src/usr.sbin/bgpd/Makefile,v
retrieving revision 1.29
diff -u -p -r1.29 Makefile
--- Makefile26 May 2010 16:44:32 -  1.29
+++ Makefile4 Dec 2014 17:30:00 -
@@ -11,8 +11,8 @@ CFLAGS+= -Wmissing-declarations
 CFLAGS+= -Wshadow -Wpointer-arith -Wcast-qual
 CFLAGS+= -Wsign-compare
 YFLAGS=
-LDADD+=-lutil
-DPADD+= ${LIBUTIL}
+LDADD+= -lcrypto -lutil
+DPADD+= ${LIBCRYPTO} ${LIBUTIL}
 MAN= bgpd.8 bgpd.conf.5
 
 .include bsd.prog.mk
Index: rde.h
===
RCS file: /cvs/src/usr.sbin/bgpd/rde.h,v
retrieving revision 1.147
diff -u -p -r1.147 rde.h
--- rde.h   14 Aug 2013 20:34:26 -  1.147
+++ rde.h   4 Dec 2014 17:30:00 -
@@ -131,7 +131,7 @@ struct attr {
LIST_ENTRY(attr) entry;
u_char  *data;
int  refcnt;
-   u_int32_thash;
+   u_int64_thash;
u_int16_tlen;
u_int8_t flags;
u_int8_t type;
Index: rde_attr.c
===
RCS file: /cvs/src/usr.sbin/bgpd/rde_attr.c,v
retrieving revision 1.92
diff -u -p -r1.92 rde_attr.c
--- rde_attr.c  8 Oct 2014 16:15:37 -   1.92
+++ rde_attr.c  4 Dec 2014 17:30:00 -
@@ -17,9 +17,10 @@
  */
 
 #include sys/types.h
-#include sys/hash.h
 #include sys/queue.h
 
+#include crypto/siphash.h
+
 #include netinet/in.h
 
 #include limits.h
@@ -316,17 +317,20 @@ struct attr *
 attr_alloc(u_int8_t flags, u_int8_t type, const void *data, u_int16_t len)
 {
struct attr *a;
+   SIPHASH_KEY  key;
 
a = calloc(1, sizeof(struct attr));
if (a == NULL)
fatal(attr_optadd);
rdemem.attr_cnt++;
 
+   arc4random_buf(key, sizeof(key));
+
flags = ~ATTR_DEFMASK; /* normalize mask */
a-flags = flags;
-   a-hash = hash32_buf(flags, sizeof(flags), HASHINIT);
+   a-hash = SipHash24(key, flags, sizeof(flags));
a-type = type;
-   a-hash = hash32_buf(type, sizeof(type), a-hash);
+   a-hash = SipHash24(key, type, sizeof(type));
a-len = len;
if (len != 0) {
if ((a-data = malloc(len)) == NULL)
@@ -338,8 +342,8 @@ attr_alloc(u_int8_t flags, u_int8_t type
} else
a-data = NULL;
 
-   a-hash = hash32_buf(len, sizeof(len), a-hash);
-   a-hash = hash32_buf(a-data, a-len, a-hash);
+   a-hash = SipHash24(key, len, sizeof(len));
+   a-hash = SipHash24(key, a-data, a-len);
LIST_INSERT_HEAD(ATTR_HASH(a-hash), a, entry);
 
return (a);
@@ -350,13 +354,15 @@ attr_lookup(u_int8_t flags, u_int8_t typ
 {
struct attr_list*head;
struct attr *a;
-   u_int32_thash;
+   u_int64_thash;
+   SIPHASH_KEY  key;
 
+   arc4random_buf(key, sizeof(key));
flags = ~ATTR_DEFMASK; /* normalize mask */
-   hash = hash32_buf(flags, sizeof(flags), HASHINIT);
-   hash = hash32_buf(type, sizeof(type), hash);
-   hash = hash32_buf(len, sizeof(len), hash);
-   hash = hash32_buf(data, len, hash);
+   hash = SipHash24(key, flags, sizeof(flags));
+   hash = SipHash24(key, type, sizeof(type));
+   hash = SipHash24(key, len, sizeof(len));
+   hash = SipHash24(key, data, len);
head = ATTR_HASH(hash);
 
LIST_FOREACH(a, head, entry) {
@@ -483,6 +489,7 @@ aspath_get(void *data, u_int16_t len)
 {
struct aspath_list  *head;
struct aspath   *aspath;
+   SIPHASH_KEY  key;
 
/* The aspath must already have been checked for correctness. */
aspath = aspath_lookup(data, len);
@@ -499,9 +506,9 @@ aspath_get(void *data, u_int16_t len)
aspath-ascnt = aspath_count(data, len);
memcpy(aspath-data, data, len);
 
+   arc4random_buf(key, sizeof(key));
/* link */
-   head = ASPATH_HASH(hash32_buf(aspath-data, aspath-len,
-   HASHINIT));
+   head = ASPATH_HASH(SipHash24(key, aspath-data, aspath-len));
LIST_INSERT_HEAD(head, aspath, entry);
}
aspath-refcnt++;
@@ -829,9 +836,11 @@ aspath_lookup(const void *data, u_int16_
 {
struct aspath_list  *head;
struct aspath   *aspath;
-   u_int32_t

Re: Convert bgpd to use siphash

2014-12-04 Thread Dimitris Papastamos
Admittedly it would have been better to store the key in the struct
and then use arc4random_buf() only once or so.  I am not familiar with the
code so I will investigate this option.



Re: Convert bgpd to use siphash

2014-12-04 Thread Ted Unangst
On Thu, Dec 04, 2014 at 17:32, Dimitris Papastamos wrote:
 Hi,
 
 This is my attempt at converting bgpd to use siphash.  This does
 *not* link as is because the userland implementation of siphash
 is currently missing.

Indeed. That should be coming in a few days. I have a diff ready.

 
 + arc4random_buf(key, sizeof(key));
 +
 flags = ~ATTR_DEFMASK;   /* normalize mask */
 a-flags = flags;
 - a-hash = hash32_buf(flags, sizeof(flags), HASHINIT);
 + a-hash = SipHash24(key, flags, sizeof(flags));

Ah, so this won't work. That's not how siphash works.

The key needs to last as long as the hash table does, which is
generally as long as the program. Different keys result in different
hashes, which makes it hard to look things up later.

Good start, though perhaps we should wait a bit until libc is ready to
go. It slipped my mind that siphash needs to be available before it
can be used when I sent that email about hash.h.



Re: Convert bgpd to use siphash

2014-12-04 Thread Stuart Henderson
On 2014/12/04 17:41, Dimitris Papastamos wrote:
 Admittedly it would have been better to store the key in the struct
 and then use arc4random_buf() only once or so.  I am not familiar with the
 code so I will investigate this option.
 

I suspect attr_alloc() is going to be a rather hot function, and bgpd is
already rather slow in some path processing related things so I think we
need to tread carefully in this area.



Re: Convert bgpd to use siphash

2014-12-04 Thread Dimitris Papastamos
On Thu, Dec 04, 2014 at 12:52:04PM -0500, Ted Unangst wrote:
 Ah, so this won't work. That's not how siphash works.
 
 The key needs to last as long as the hash table does, which is
 generally as long as the program. Different keys result in different
 hashes, which makes it hard to look things up later.

Ah, of course.  I will post another diff after siphash hits userspace.