-
Notifications
You must be signed in to change notification settings - Fork 24
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Increase plugin performance #2
Comments
Hello @acouvreur , did you try with containous/whoami or nginx (with hardcoded return 200) as a WAF backend? How does go handle connections towards the WAF? Does it reuse them or establishes new one for each request? |
I used it with containous/whoami as WAF backend
It actually creates a new request that it forwards to the WAF. Then depending on the response, the plugin sends the initial request to subsequent middlewares / service. How can I reuse the the request ? |
Unfortunately I don't know golang at all 😅 I just saw many syn/fin between traefik and the waf container. I found this article stating the response body should be read before being closed in order to reuse the connection, not sure if accurate or feasible: https://golang.cafe/blog/how-to-reuse-http-connections-in-go.html |
Hi @acouvreur Unfortunately I am a Python dev, not Go (Go is on my list of things to learn for this year), so not quite sure why that additional 60ms average latency when calling Modsecurity via WAF vs when calling directly. Even without Go knowledge my first guess would be HTTP (TCP) connection reuse - or rather the lack of it. I saw this line, where you are deferring the closure of the body, but reading this the author is recommending closing the body completely before issuing a new HTTP request (and not deferring it):
Would you mind testing that whether that helps? Although it seems other Middlewares are also using |
+1 excited about a WAF middleware for Traefik! Thanks for your work @acouvreur, this is awesome. My understanding is that traefik-modsecurity-plugin/modsecurity.go Lines 97 to 105 in 19cdb47
|
Thanks all for your interest, I just tested it without the defer and it seems that it doesn't affect the throughput that much. Before
After - defer resp.Body.Close()
+ resp.Body.Close()
|
I'm also very interested in using this plugin, so following and hopefully able to contribute when time allows |
EDIT: Traefik uses Yaegi, and unfortunately Yaegi does not support CGo :-( |
I made some modifications, just to be sure that Go is re-cycling HTTP client connections: diff --git a/modsecurity.go b/modsecurity.go
index 42b24b2..59fb335 100644
--- a/modsecurity.go
+++ b/modsecurity.go
@@ -94,12 +94,14 @@ func (a *Modsecurity) ServeHTTP(rw http.ResponseWriter, req *http.Request) {
http.Error(rw, "", http.StatusBadGateway)
return
}
- defer resp.Body.Close()
if resp.StatusCode >= 400 {
forwardResponse(resp, rw)
+ resp.Body.Close()
return
}
+ io.Copy(ioutil.Discard, resp.Body)
+ resp.Body.Close()
a.next.ServeHTTP(rw, req)
} I have these values now: Website with WAF:
Website without WAF:
Benchmarking the WAF directly:
So I was able to lower the "ratio of slowness" from 12.2x (see below my previous values) to ~5.5x. I tried also to avoid DNS lookups for Also, I tried to switch to Previous values for Website + WAF:
|
That is some great progress in performance!!! I am glad to see, that making Go to re-cycle the http connections (switching |
Yup, it has helped a lot. That line, plus |
Hi @acouvreur and @Enrico204 I am interested in using this middleware, but I am not yet a Go developer (learning now). It is good to see progress being made regarding performance. I see that @Enrico204 you tried to use fasthttp. I was going to suggest this as well, but did not know it was unsupported. I did some digging, and it seems that Traefik realize there is demand for fasthttp support, and are considering replacing net/http with fasthttp in the next major version. This could mean a significant performance increase is on the horizon. |
Unfortunately the problem is that fasthttp is not compatible with the plugin system (Yaegi), so inside plugins we'll be limited to |
Benchmark using https://github.com/codesenberg/bombardier
Traefik access without WAF
Traefik access with WAF
The WAF makes the Throughput almost 20x times slower.
Or all combined ?
This is the test for direct access through WAF, no Traefik
So the plugin might have room for improvement.
The text was updated successfully, but these errors were encountered: