Created attachment 224911 [details]
dynamic tls patch
- Re-add dynamic tls patch from cloudflare after 1.20.0 update
- Bump PORTREVISION
this patch has been reviewed by NGINX Development team and rejected.
I'd not recommend to add it back to the www/nginx port.
It's off by default. You are not required to use it if you do not wish to.
A commit in branch main references this bug:
Author: Ryan Steinmetz <zi@FreeBSD.org>
AuthorDate: 2021-06-02 13:37:21 +0000
Commit: Ryan Steinmetz <zi@FreeBSD.org>
CommitDate: 2021-06-02 13:37:21 +0000
www/nginx: Restore lost dynamic tls patch
Approved by: maintainer timeout (2+ weeks)
www/nginx/Makefile.extmod | 2 +
www/nginx/files/extra-patch-dynamic-tls (new) | 225 ++++++++++++++++++++++++++
2 files changed, 227 insertions(+)
(In reply to commit-hook from comment #3)
pls revert. osa has clearly said here that the patch was rejected by the NGINX team. thank you. This commit does not have my approval.
(In reply to Sergey A. Osokin from comment #1)
Why Nginx doesn't approve this patch?
This PR was to restore a patch that was lost during the upgrade to 1.20.0.
No maintainer activity happened for almost 3 weeks, it was automatically approved via the maintainer timeout:
There is no harm by having an additional patch present.
(In reply to Christos Chatzaras from comment #5)
Short answer is it's possible to reach the same goal with
(In reply to Sergey A. Osokin from comment #7)
If I remember correctly this patch improves TTFB. Don't know if it has any disadvantages.
(In reply to Christos Chatzaras from comment #8)
Thanks for the update, Christos.
Could you provide test results that show/measure such improvements.
(In reply to Ryan Steinmetz from comment #6)
There was a reason why the patch was removed.
But okay, I will remove it again in the next update
Perhaps there would be value in you providing a reason to for it to no longer be an OPTION?
So far, "nginx rejected it" and "there is a reason it was removed" is all that I've seen--no actual rationale for preventing users from having it as an option was provided.
The only (transient) reason that I've been able to come up with is that the patch no longer cleanly applied to 1.20.0 because two structs were renamed in the newer release. However, that was a trivial fix and I opened a fresh PR with updated patch to address the issue.
I've never seen such a passionate resistance to an optional nginx addon/patch, so this all strikes me as very weird. To me, it seems like this particular patch was singled out for some ulterior motive.
Cloudflare's writeup provides more details:
As well as a rationale why statically configuring ssl_buffer_size to 4k can be improved upon:
And lastly, this was approved/added by the maintainer in 1.18.x: