Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 23 Jan 2015 00:06:36 +0000 (UTC)
From:      Will Andrews <will@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r277544 - head/sys/net
Message-ID:  <201501230006.t0N06a2E000567@svn.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: will
Date: Fri Jan 23 00:06:35 2015
New Revision: 277544
URL: https://svnweb.freebsd.org/changeset/base/277544

Log:
  Improve the distribution of LAGG port traffic.
  
  I edited the original change to retain the use of arc4random() as a seed for
  the hashing as a very basic defense against intentional lagg port selection.
  
  The author's original commit message (edited slightly):
  
  sys/net/ieee8023ad_lacp.c
  sys/net/if_lagg.c
  	In lagg_hashmbuf, use the FNV hash instead of the old
  	hash32_buf.  The hash32 family of functions operate one octet
  	at a time, and when run on a string s of length n, their output
  	is equivalent to :
  
  		   ----- i=n-1
  		   \
  	       n    \           (n-i-1)              32
  	( seed^  +  /        33^        * s[i] ) % 2^
  		   /
  		   ----- i=0
  
  	The problem is that the last five bytes of input don't get
  	multiplied by sufficiently many powers of 33 to rollover 2^32.
  	That means that changing the last few bytes (but obviously not
  	the very last) of input will always change the value of the
  	hash by a multiple of 33.  In the case of lagg_hashmbuf() with
  	ipv4 input, the last four bytes are the TCP or UDP port
  	numbers.  Since the output of lagg_hashmbuf is always taken
  	modulo the port count, and 3 is a common port count for a lagg,
  	that's bad.  It means that the UDP or TCP source port will
  	never affect which lagg member is selected on a 3-port lagg.
  
  	At 10Gbps, I was not able to measure any difference in CPU
  	consumption between the old and new hash.
  
  Submitted by:	asomers (original commit)
  Reviewed by:	emaste, glebius
  MFC after:	1 week
  Sponsored by:	Spectra Logic
  MFSpectraBSD:	1001723 on 2013/08/28 (original)
  		1114258 on 2015/01/22 (edit)

Modified:
  head/sys/net/ieee8023ad_lacp.c
  head/sys/net/if_lagg.c

Modified: head/sys/net/ieee8023ad_lacp.c
==============================================================================
--- head/sys/net/ieee8023ad_lacp.c	Fri Jan 23 00:02:26 2015	(r277543)
+++ head/sys/net/ieee8023ad_lacp.c	Fri Jan 23 00:06:35 2015	(r277544)
@@ -35,6 +35,7 @@ __FBSDID("$FreeBSD$");
 #include <sys/eventhandler.h>
 #include <sys/mbuf.h>
 #include <sys/systm.h>
+#include <sys/fnv_hash.h>
 #include <sys/malloc.h>
 #include <sys/kernel.h> /* hz */
 #include <sys/socket.h> /* for net/if.h */
@@ -757,13 +758,16 @@ void
 lacp_attach(struct lagg_softc *sc)
 {
 	struct lacp_softc *lsc;
+	uint32_t seed;
 
 	lsc = malloc(sizeof(struct lacp_softc), M_DEVBUF, M_WAITOK | M_ZERO);
 
 	sc->sc_psc = lsc;
 	lsc->lsc_softc = sc;
 
-	lsc->lsc_hashkey = arc4random();
+	seed = arc4random();
+	lsc->lsc_hashkey = FNV1_32_INIT;
+	lsc->lsc_hashkey = fnv_32_buf(&seed, sizeof(seed), lsc->lsc_hashkey);
 	lsc->lsc_active_aggregator = NULL;
 	lsc->lsc_strict_mode = 1;
 	LACP_LOCK_INIT(lsc);

Modified: head/sys/net/if_lagg.c
==============================================================================
--- head/sys/net/if_lagg.c	Fri Jan 23 00:02:26 2015	(r277543)
+++ head/sys/net/if_lagg.c	Fri Jan 23 00:06:35 2015	(r277544)
@@ -36,7 +36,7 @@ __FBSDID("$FreeBSD$");
 #include <sys/priv.h>
 #include <sys/systm.h>
 #include <sys/proc.h>
-#include <sys/hash.h>
+#include <sys/fnv_hash.h>
 #include <sys/lock.h>
 #include <sys/rmlock.h>
 #include <sys/taskqueue.h>
@@ -1853,13 +1853,13 @@ lagg_hashmbuf(struct lagg_softc *sc, str
 	eh = mtod(m, struct ether_header *);
 	etype = ntohs(eh->ether_type);
 	if (sc->sc_flags & LAGG_F_HASHL2) {
-		p = hash32_buf(&eh->ether_shost, ETHER_ADDR_LEN, p);
-		p = hash32_buf(&eh->ether_dhost, ETHER_ADDR_LEN, p);
+		p = fnv_32_buf(&eh->ether_shost, ETHER_ADDR_LEN, p);
+		p = fnv_32_buf(&eh->ether_dhost, ETHER_ADDR_LEN, p);
 	}
 
 	/* Special handling for encapsulating VLAN frames */
 	if ((m->m_flags & M_VLANTAG) && (sc->sc_flags & LAGG_F_HASHL2)) {
-		p = hash32_buf(&m->m_pkthdr.ether_vtag,
+		p = fnv_32_buf(&m->m_pkthdr.ether_vtag,
 		    sizeof(m->m_pkthdr.ether_vtag), p);
 	} else if (etype == ETHERTYPE_VLAN) {
 		vlan = lagg_gethdr(m, off,  sizeof(*vlan), &buf);
@@ -1867,7 +1867,7 @@ lagg_hashmbuf(struct lagg_softc *sc, str
 			goto out;
 
 		if (sc->sc_flags & LAGG_F_HASHL2)
-			p = hash32_buf(&vlan->evl_tag, sizeof(vlan->evl_tag), p);
+			p = fnv_32_buf(&vlan->evl_tag, sizeof(vlan->evl_tag), p);
 		etype = ntohs(vlan->evl_proto);
 		off += sizeof(*vlan) - sizeof(*eh);
 	}
@@ -1880,8 +1880,8 @@ lagg_hashmbuf(struct lagg_softc *sc, str
 			goto out;
 
 		if (sc->sc_flags & LAGG_F_HASHL3) {
-			p = hash32_buf(&ip->ip_src, sizeof(struct in_addr), p);
-			p = hash32_buf(&ip->ip_dst, sizeof(struct in_addr), p);
+			p = fnv_32_buf(&ip->ip_src, sizeof(struct in_addr), p);
+			p = fnv_32_buf(&ip->ip_dst, sizeof(struct in_addr), p);
 		}
 		if (!(sc->sc_flags & LAGG_F_HASHL4))
 			break;
@@ -1896,7 +1896,7 @@ lagg_hashmbuf(struct lagg_softc *sc, str
 				ports = lagg_gethdr(m, off, sizeof(*ports), &buf);
 				if (ports == NULL)
 					break;
-				p = hash32_buf(ports, sizeof(*ports), p);
+				p = fnv_32_buf(ports, sizeof(*ports), p);
 				break;
 		}
 		break;
@@ -1909,10 +1909,10 @@ lagg_hashmbuf(struct lagg_softc *sc, str
 		if (ip6 == NULL)
 			goto out;
 
-		p = hash32_buf(&ip6->ip6_src, sizeof(struct in6_addr), p);
-		p = hash32_buf(&ip6->ip6_dst, sizeof(struct in6_addr), p);
+		p = fnv_32_buf(&ip6->ip6_src, sizeof(struct in6_addr), p);
+		p = fnv_32_buf(&ip6->ip6_dst, sizeof(struct in6_addr), p);
 		flow = ip6->ip6_flow & IPV6_FLOWLABEL_MASK;
-		p = hash32_buf(&flow, sizeof(flow), p);	/* IPv6 flow label */
+		p = fnv_32_buf(&flow, sizeof(flow), p);	/* IPv6 flow label */
 		break;
 #endif
 	}
@@ -2087,12 +2087,15 @@ lagg_lb_attach(struct lagg_softc *sc)
 {
 	struct lagg_port *lp;
 	struct lagg_lb *lb;
+	uint32_t seed;
 
 	lb = malloc(sizeof(struct lagg_lb), M_DEVBUF, M_WAITOK | M_ZERO);
 
 	sc->sc_capabilities = IFCAP_LAGG_FULLDUPLEX;
 
-	lb->lb_key = arc4random();
+	seed = arc4random();
+	lb->lb_key = FNV1_32_INIT;
+	lb->lb_key = fnv_32_buf(&seed, sizeof(seed), lb->lb_key);
 	sc->sc_psc = lb;
 
 	SLIST_FOREACH(lp, &sc->sc_ports, lp_entries)



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201501230006.t0N06a2E000567>