Hi,

> -----Original Message-----
> From: Maxim Dounin <mdounin@mdounin.ru>
> Sent: Thursday, June 16, 2022 18:51
>
> Hello!
>
> On Thu, Jun 16, 2022 at 08:26:48AM +0000, Pavel Pautov via nginx-devel wrote:
>
> > Looks like, we've made a full circle then... I've replied to
> > that suggestion already and in last e-mail (with patch) I note
> > that moving additional logic into the ngx_http_proxy_set_ssl()
> > has its own drawbacks, but I can certainly move more stuff into
> > it.
> >
> > So do you envision something like "ngx_http_proxy_set_ssl(cf,
> > conf, prev, reuse_ssl)"? As previously we've established that
> > directives merging stays out of ngx_http_proxy_set_ssl (and
> > reuse_ssl calculation has to happen before it).
>
> I don't think further attempts to explain how to write readable
> code worth the effort.
>

Too bad.

> Please see the patch below. Review and testing appreciated.
>

The patch seems to contradict some previously discussed points. For example, it can actually increase memory usage in certain configurations (say, when proxy_ssl_* directives in location override server level directives or when proxy_pass "https://..." is absent). Or you don't consider this an issue anymore?

More importantly, it doesn't really solve the use case from #1234, i.e. http level proxy_ssl_* directives with many servers (as by default http level values are remain unset and thus are not equal to server level values).

Also, you effectively compare some directives by value (instead of checking the presence), so it might be surprising to the user that repeating some directives on inner level increases memory consumption and repeating others doesn't.

My last patch already addresses all of above...

Also, it would be nice to avoid all this copy-paste and have the same optimization in the stream module.

> # HG changeset patch
> # User Maxim Dounin <mdounin@mdounin.ru>
> # Date 1655429915 -10800
> # Fri Jun 17 04:38:35 2022 +0300
> # Node ID e4a0eeb3edba037f0d090023a2242bda2f8dcb03
> # Parent e23a385cd0ec866a3eb1d8c9c956991e1ed50d78
> Upstream: optimized use of SSL contexts (ticket #1234).
[...]
>
> #if (NGX_HTTP_SSL)
> - u->ssl = (glcf->upstream.ssl != NULL);
> + u->ssl = glcf->ssl;

I like this change, with it additional 'shared_ssl' pointer can be removed from my patch.

_______________________________________________
nginx-devel mailing list -- nginx-devel@nginx.org
To unsubscribe send an email to nginx-devel-leave@nginx.org