From 74949fb0914d7d496efadf51ef2dd81e64b1b7d0 Mon Sep 17 00:00:00 2001 From: Hayder <96513935+jadidbourbaki@users.noreply.github.com> Date: Fri, 29 Mar 2024 12:56:18 -0400 Subject: [PATCH] reverseproxy: Use xxhash instead of fnv32 for LB (#6203) * Added Faster Non-cryptographic Hash Function for Load Balancing * Ran golangci-lint * Updated hash version and hash return type --- .../reverseproxy/selectionpolicies.go | 11 +- .../reverseproxy/selectionpolicies_test.go | 156 +++++++++--------- 2 files changed, 84 insertions(+), 83 deletions(-) diff --git a/modules/caddyhttp/reverseproxy/selectionpolicies.go b/modules/caddyhttp/reverseproxy/selectionpolicies.go index b6f807c1..e61b3e0f 100644 --- a/modules/caddyhttp/reverseproxy/selectionpolicies.go +++ b/modules/caddyhttp/reverseproxy/selectionpolicies.go @@ -20,7 +20,6 @@ import ( "encoding/hex" "encoding/json" "fmt" - "hash/fnv" weakrand "math/rand" "net" "net/http" @@ -28,6 +27,8 @@ import ( "strings" "sync/atomic" + "github.com/cespare/xxhash/v2" + "github.com/caddyserver/caddy/v2" "github.com/caddyserver/caddy/v2/caddyconfig" "github.com/caddyserver/caddy/v2/caddyconfig/caddyfile" @@ -807,7 +808,7 @@ func hostByHashing(pool []*Upstream, s string) *Upstream { // see https://medium.com/i0exception/rendezvous-hashing-8c00e2fb58b0, // https://randorithms.com/2020/12/26/rendezvous-hashing.html, // and https://en.wikipedia.org/wiki/Rendezvous_hashing. - var highestHash uint32 + var highestHash uint64 var upstream *Upstream for _, up := range pool { if !up.Available() { @@ -823,10 +824,10 @@ func hostByHashing(pool []*Upstream, s string) *Upstream { } // hash calculates a fast hash based on s. -func hash(s string) uint32 { - h := fnv.New32a() +func hash(s string) uint64 { + h := xxhash.New() _, _ = h.Write([]byte(s)) - return h.Sum32() + return h.Sum64() } func loadFallbackPolicy(d *caddyfile.Dispenser) (json.RawMessage, error) { diff --git a/modules/caddyhttp/reverseproxy/selectionpolicies_test.go b/modules/caddyhttp/reverseproxy/selectionpolicies_test.go index d7e79626..1adb1cb2 100644 --- a/modules/caddyhttp/reverseproxy/selectionpolicies_test.go +++ b/modules/caddyhttp/reverseproxy/selectionpolicies_test.go @@ -157,8 +157,8 @@ func TestIPHashPolicy(t *testing.T) { // We should be able to predict where every request is routed. req.RemoteAddr = "172.0.0.1:80" h := ipHash.Select(pool, req, nil) - if h != pool[1] { - t.Error("Expected ip hash policy host to be the second host.") + if h != pool[0] { + t.Error("Expected ip hash policy host to be the first host.") } req.RemoteAddr = "172.0.0.2:80" h = ipHash.Select(pool, req, nil) @@ -167,8 +167,8 @@ func TestIPHashPolicy(t *testing.T) { } req.RemoteAddr = "172.0.0.3:80" h = ipHash.Select(pool, req, nil) - if h != pool[1] { - t.Error("Expected ip hash policy host to be the second host.") + if h != pool[0] { + t.Error("Expected ip hash policy host to be the first host.") } req.RemoteAddr = "172.0.0.4:80" h = ipHash.Select(pool, req, nil) @@ -179,8 +179,8 @@ func TestIPHashPolicy(t *testing.T) { // we should get the same results without a port req.RemoteAddr = "172.0.0.1" h = ipHash.Select(pool, req, nil) - if h != pool[1] { - t.Error("Expected ip hash policy host to be the second host.") + if h != pool[0] { + t.Error("Expected ip hash policy host to be the first host.") } req.RemoteAddr = "172.0.0.2" h = ipHash.Select(pool, req, nil) @@ -189,8 +189,8 @@ func TestIPHashPolicy(t *testing.T) { } req.RemoteAddr = "172.0.0.3" h = ipHash.Select(pool, req, nil) - if h != pool[1] { - t.Error("Expected ip hash policy host to be the second host.") + if h != pool[0] { + t.Error("Expected ip hash policy host to be the first host.") } req.RemoteAddr = "172.0.0.4" h = ipHash.Select(pool, req, nil) @@ -203,8 +203,8 @@ func TestIPHashPolicy(t *testing.T) { req.RemoteAddr = "172.0.0.4" pool[1].setHealthy(false) h = ipHash.Select(pool, req, nil) - if h != pool[0] { - t.Error("Expected ip hash policy host to be the first host.") + if h != pool[2] { + t.Error("Expected ip hash policy host to be the third host.") } req.RemoteAddr = "172.0.0.2" @@ -217,8 +217,8 @@ func TestIPHashPolicy(t *testing.T) { req.RemoteAddr = "172.0.0.3" pool[2].setHealthy(false) h = ipHash.Select(pool, req, nil) - if h != pool[1] { - t.Error("Expected ip hash policy host to be the second host.") + if h != pool[0] { + t.Error("Expected ip hash policy host to be the first host.") } req.RemoteAddr = "172.0.0.4" h = ipHash.Select(pool, req, nil) @@ -300,8 +300,8 @@ func TestClientIPHashPolicy(t *testing.T) { // We should be able to predict where every request is routed. caddyhttp.SetVar(req.Context(), caddyhttp.ClientIPVarKey, "172.0.0.1:80") h := ipHash.Select(pool, req, nil) - if h != pool[1] { - t.Error("Expected ip hash policy host to be the second host.") + if h != pool[0] { + t.Error("Expected ip hash policy host to be the first host.") } caddyhttp.SetVar(req.Context(), caddyhttp.ClientIPVarKey, "172.0.0.2:80") h = ipHash.Select(pool, req, nil) @@ -310,8 +310,8 @@ func TestClientIPHashPolicy(t *testing.T) { } caddyhttp.SetVar(req.Context(), caddyhttp.ClientIPVarKey, "172.0.0.3:80") h = ipHash.Select(pool, req, nil) - if h != pool[1] { - t.Error("Expected ip hash policy host to be the second host.") + if h != pool[0] { + t.Error("Expected ip hash policy host to be the first host.") } caddyhttp.SetVar(req.Context(), caddyhttp.ClientIPVarKey, "172.0.0.4:80") h = ipHash.Select(pool, req, nil) @@ -322,8 +322,8 @@ func TestClientIPHashPolicy(t *testing.T) { // we should get the same results without a port caddyhttp.SetVar(req.Context(), caddyhttp.ClientIPVarKey, "172.0.0.1") h = ipHash.Select(pool, req, nil) - if h != pool[1] { - t.Error("Expected ip hash policy host to be the second host.") + if h != pool[0] { + t.Error("Expected ip hash policy host to be the first host.") } caddyhttp.SetVar(req.Context(), caddyhttp.ClientIPVarKey, "172.0.0.2") h = ipHash.Select(pool, req, nil) @@ -332,8 +332,8 @@ func TestClientIPHashPolicy(t *testing.T) { } caddyhttp.SetVar(req.Context(), caddyhttp.ClientIPVarKey, "172.0.0.3") h = ipHash.Select(pool, req, nil) - if h != pool[1] { - t.Error("Expected ip hash policy host to be the second host.") + if h != pool[0] { + t.Error("Expected ip hash policy host to be the first host.") } caddyhttp.SetVar(req.Context(), caddyhttp.ClientIPVarKey, "172.0.0.4") h = ipHash.Select(pool, req, nil) @@ -346,8 +346,8 @@ func TestClientIPHashPolicy(t *testing.T) { caddyhttp.SetVar(req.Context(), caddyhttp.ClientIPVarKey, "172.0.0.4") pool[1].setHealthy(false) h = ipHash.Select(pool, req, nil) - if h != pool[0] { - t.Error("Expected ip hash policy host to be the first host.") + if h != pool[2] { + t.Error("Expected ip hash policy host to be the third host.") } caddyhttp.SetVar(req.Context(), caddyhttp.ClientIPVarKey, "172.0.0.2") @@ -360,8 +360,8 @@ func TestClientIPHashPolicy(t *testing.T) { caddyhttp.SetVar(req.Context(), caddyhttp.ClientIPVarKey, "172.0.0.3") pool[2].setHealthy(false) h = ipHash.Select(pool, req, nil) - if h != pool[1] { - t.Error("Expected ip hash policy host to be the second host.") + if h != pool[0] { + t.Error("Expected ip hash policy host to be the first host.") } caddyhttp.SetVar(req.Context(), caddyhttp.ClientIPVarKey, "172.0.0.4") h = ipHash.Select(pool, req, nil) @@ -470,21 +470,21 @@ func TestQueryHashPolicy(t *testing.T) { request = httptest.NewRequest(http.MethodGet, "/?foo=100000", nil) h = queryPolicy.Select(pool, request, nil) - if h != pool[0] { - t.Error("Expected query policy host to be the first host.") + if h != pool[1] { + t.Error("Expected query policy host to be the second host.") } request = httptest.NewRequest(http.MethodGet, "/?foo=1", nil) pool[0].setHealthy(false) h = queryPolicy.Select(pool, request, nil) - if h != pool[1] { - t.Error("Expected query policy host to be the second host.") + if h != pool[2] { + t.Error("Expected query policy host to be the third host.") } request = httptest.NewRequest(http.MethodGet, "/?foo=100000", nil) h = queryPolicy.Select(pool, request, nil) - if h != pool[2] { - t.Error("Expected query policy host to be the third host.") + if h != pool[1] { + t.Error("Expected query policy host to be the second host.") } // We should be able to resize the host pool and still be able to predict @@ -533,14 +533,14 @@ func TestURIHashPolicy(t *testing.T) { request := httptest.NewRequest(http.MethodGet, "/test", nil) h := uriPolicy.Select(pool, request, nil) - if h != pool[1] { - t.Error("Expected uri policy host to be the second host.") + if h != pool[2] { + t.Error("Expected uri policy host to be the third host.") } pool[2].setHealthy(false) h = uriPolicy.Select(pool, request, nil) - if h != pool[1] { - t.Error("Expected uri policy host to be the second host.") + if h != pool[0] { + t.Error("Expected uri policy host to be the first host.") } request = httptest.NewRequest(http.MethodGet, "/test_2", nil) @@ -691,54 +691,54 @@ func TestCookieHashPolicy(t *testing.T) { } func TestCookieHashPolicyWithSecureRequest(t *testing.T) { - ctx, cancel := caddy.NewContext(caddy.Context{Context: context.Background()}) - defer cancel() - cookieHashPolicy := CookieHashSelection{} - if err := cookieHashPolicy.Provision(ctx); err != nil { - t.Errorf("Provision error: %v", err) - t.FailNow() - } + ctx, cancel := caddy.NewContext(caddy.Context{Context: context.Background()}) + defer cancel() + cookieHashPolicy := CookieHashSelection{} + if err := cookieHashPolicy.Provision(ctx); err != nil { + t.Errorf("Provision error: %v", err) + t.FailNow() + } - pool := testPool() - pool[0].Dial = "localhost:8080" - pool[1].Dial = "localhost:8081" - pool[2].Dial = "localhost:8082" - pool[0].setHealthy(true) - pool[1].setHealthy(false) - pool[2].setHealthy(false) + pool := testPool() + pool[0].Dial = "localhost:8080" + pool[1].Dial = "localhost:8081" + pool[2].Dial = "localhost:8082" + pool[0].setHealthy(true) + pool[1].setHealthy(false) + pool[2].setHealthy(false) - // Create a test server that serves HTTPS requests - ts := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - h := cookieHashPolicy.Select(pool, r, w) - if h != pool[0] { - t.Error("Expected cookieHashPolicy host to be the first only available host.") - } - })) - defer ts.Close() + // Create a test server that serves HTTPS requests + ts := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + h := cookieHashPolicy.Select(pool, r, w) + if h != pool[0] { + t.Error("Expected cookieHashPolicy host to be the first only available host.") + } + })) + defer ts.Close() - // Make a new HTTPS request to the test server - client := ts.Client() - request, err := http.NewRequest(http.MethodGet, ts.URL+"/test", nil) - if err != nil { - t.Fatal(err) - } - response, err := client.Do(request) - if err != nil { - t.Fatal(err) - } + // Make a new HTTPS request to the test server + client := ts.Client() + request, err := http.NewRequest(http.MethodGet, ts.URL+"/test", nil) + if err != nil { + t.Fatal(err) + } + response, err := client.Do(request) + if err != nil { + t.Fatal(err) + } - // Check if the cookie set is Secure and has SameSiteNone mode - cookies := response.Cookies() - if len(cookies) == 0 { - t.Fatal("Expected a cookie to be set") - } - cookie := cookies[0] - if !cookie.Secure { - t.Error("Expected cookie Secure attribute to be true when request is secure") - } - if cookie.SameSite != http.SameSiteNoneMode { - t.Error("Expected cookie SameSite attribute to be None when request is secure") - } + // Check if the cookie set is Secure and has SameSiteNone mode + cookies := response.Cookies() + if len(cookies) == 0 { + t.Fatal("Expected a cookie to be set") + } + cookie := cookies[0] + if !cookie.Secure { + t.Error("Expected cookie Secure attribute to be true when request is secure") + } + if cookie.SameSite != http.SameSiteNoneMode { + t.Error("Expected cookie SameSite attribute to be None when request is secure") + } } func TestCookieHashPolicyWithFirstFallback(t *testing.T) {