Hello!

On Tue, Jul 12, 2022 at 06:59:39PM +0400, Sergey Kandaurov wrote:

> > On 8 Jul 2022, at 04:35, Maxim Dounin <mdounin@mdounin.ru> wrote:
> >
> > On Thu, Jul 07, 2022 at 07:49:51PM +0400, Sergey Kandaurov wrote:
> >
> >>>> @@ -4250,7 +4269,27 @@ ngx_resolver_export(ngx_resolver_t *r, n
> >>>> }
> >>>>
> >>>> i = 0;
> >>>> - d = rotate ? ngx_random() % n : 0;
> >>>> +
> >>>> + switch (r->prefer) {
> >>>> +
> >>>> +#if (NGX_HAVE_INET6)
> >>>> + case NGX_RESOLVE_PREFER_A:
> >>>> + d = 0;
> >>>> + break;
> >>>> +
> >>>> + case NGX_RESOLVE_PREFER_AAAA:
> >>>> + d = rn->naddrs6;
> >>>> +
> >>>> + if (d == n) {
> >>>> + d = 0;
> >>>> + }
> >>>> +
> >>>> + break;
> >>>> +#endif
> >>>> +
> >>>> + default:
> >>>> + d = rotate ? ngx_random() % n : 0;
> >>>> + }
> >>>
> >>> With this code, a configuration like this:
> >>>
> >>> resolver ... prefer=ipv4;
> >>> set $foo "";
> >>> proxy_pass http://example.com$foo;
> >>>
> >>> will result in only IPv4 addresses being used assuming successful
> >>> connections, and IPv6 addresses being used only as a backup. This
> >>> looks quite different from the current behaviour, as well as from
> >>> what we do with
> >>>
> >>> proxy_pass http://example.com;
> >>>
> >>> when using system resolver.
> >>
> >> Can you please elaborate, what specific concerns are you referring to?
> >> The prefer option implements exactly the expected behaviour:
> >> first, a flat array is populated with preferred addresses
> >> (IPv4 for "prefer=ipv4", if any), then - with the rest, such as IPv6.
> >> The API user iterates though them until she gets a "successful" address.
> >>
> >> If the name is in the resolver cache, then rotation is also applied.
> >> The default nginx resolver behaviour is to rotate resolved addresses
> >> regardless of address families. Unlike this, in case of "prefer=ipv4",
> >> addresses are rotated within address families, that is, AFs are sorted:
> >> ipv4_x, ipv4_y, ipv4_z; ipv6_x, ipv6_y, ipv6_z
> >>
> >> This is close to how system resolver is used with getaddrinfo(), which
> >> depends on a preference and, if applicable, AF/address reachability.
> >
> > Try the two above configurations with a name which resolves to
> > 127.0.0.1 and ::1, and with both addresses responding on port 80.
> > Configuration without variables (using system resolver) will
> > balance requests between both addresses, regardless of system
> > resolver settings. Configuration with variables and resolver with
> > "prefer=ipv4" will use only the IPv4 address.
> >
> > server {
> > listen localhost:8080;
> >
> > location /dynamic/ {
> > resolver 8.8.8.8 prefer=ipv4;
> > set $foo "";
> > proxy_pass http://test.mdounin.ru:8081$foo;
> > }
> >
> > location /static/ {
> > proxy_pass http://test.mdounin.ru:8082;
> > }
> > }
> >
> > server {
> > listen test.mdounin.ru:8081;
> > listen test.mdounin.ru:8082;
> > return 200 $server_addr\n;
> > }
> >
> > Static configuration without variables uses both addresses:
> >
> > $ curl http://127.0.0.1:8080/static/
> > 127.0.0.1
> > $ curl http://127.0.0.1:8080/static/
> > ::1
> > $ curl http://127.0.0.1:8080/static/
> > 127.0.0.1
> > $ curl http://127.0.0.1:8080/static/
> > ::1
> >
> > Dynamic configuration with "prefer=ipv4" will only use IPv4 (IPv6
> > addresses will be used only in case of errors):
> >
> > $ curl http://127.0.0.1:8080/dynamic/
> > 127.0.0.1
> > $ curl http://127.0.0.1:8080/dynamic/
> > 127.0.0.1
> > $ curl http://127.0.0.1:8080/dynamic/
> > 127.0.0.1
> > $ curl http://127.0.0.1:8080/dynamic/
> > 127.0.0.1
> >
> > [...]
>
> Thanks for clarification.
>
> >
> >>> Not sure we want to introduce such behaviour. While it might be
> >>> closer to what RFC 6724 recommends for clients, it is clearly not
> >>> in line with how we handle multiple upstream addresses in general,
> >>> and certainly will confuse users. If we want to introduce this,
> >>> it probably should be at least consistent within resolver vs.
> >>> system resolver cases.
> >>
> >> If you refer to how we balance though multiple addresses in upstream
> >> implicitly defined with proxy_pass vs. proxy_pass with variable, then
> >> I tend to agree with you. In implicitly defined upstream, addresses
> >> are selected with rr balancer, which eventually makes them tried all.
> >> OTOH, the prefer option used for proxy_pass with variable, factually
> >> moves the unprefer addresses to backup, and since the upstream group
> >> isn't preserved across requests, this makes them almost never tried.
> >> But this is how proxy_pass with variable is used to work.
> >
> > Yes, I refer to the difference in handling of multiple upstream
> > addresses which is introduced with this change. Right now there
> > are no difference in the behaviour of static proxy_pass (without
> > variables) and dynamic one (with variables). With "prefer=ipv4"
> > as implemented the difference appears, and this certainly breaks
> > POLA.
> >
> > One possible option would be to change "prefer=" to rotate all
> > addresses, so proxy_pass will try them all. With this approach,
> > "prefer=ipv4" would be exactly equivalent to the default behaviour
> > (on the first resolution, resolver returns list of all IPv4
> > addresses, followed by all IPv6 addresses, and then addresses are
> > rotated) and "prefer=ipv6" would use the reverse order on the
> > first resolution (IPv6 followed by IPv4). Not sure it is at all
> > needed though (but might be still beneficial for tasks like OCSP
> > server resolution).
>
> Updated patch to rotate regardless of preference.
> Below is hg diff -w on top off of the previous one, for clarity:
>
> diff -r d9a8c2d87055 src/core/ngx_resolver.c
> --- a/src/core/ngx_resolver.c Wed Jul 06 17:10:24 2022 +0400
> +++ b/src/core/ngx_resolver.c Tue Jul 12 18:55:56 2022 +0400
> @@ -4270,6 +4270,10 @@ ngx_resolver_export(ngx_resolver_t *r, n
>
> i = 0;
>
> + if (rotate) {
> + d = ngx_random() % n;
> +
> + } else {
> switch (r->prefer) {
>
> #if (NGX_HAVE_INET6)
> @@ -4288,7 +4292,8 @@ ngx_resolver_export(ngx_resolver_t *r, n
> #endif
>
> default:
> - d = rotate ? ngx_random() % n : 0;
> + d = 0;
> + }
> }
>
> if (rn->naddrs) {
>
> Personally, I think such patch has a little sense. Keeping preference
> is reasonable e.g. to avoid connections over certain address family,
> should they be slow/tunneled or otherwise uneven, which I believe is
> quite common in practice. Such way, they would be always tried as a
> last resort and indeed can be seen as a backup option.
> In that sense, I'm rather skeptical about rotation behaviour in static
> proxy_pass (by round-robin means) that doesn't distinguish address families
> and apparently is a legacy from times (4d68c486fcb0) before URLs obtained
> IPv6 support in eaf95350d75c.
> Changing it requires more investment though and certainly breaks POLA.
> OTOH, with "ipv4=" introduction, such unpreferred connections can be
> simply disabled.

I generally agree. For now, the best option seems to postpone the
"prefer=" patch.

[...]

> i = 0;
> - d = rotate ? ngx_random() % n : 0;
> +
> + if (rotate) {
> + d = ngx_random() % n;
> +
> + } else {
> + switch (r->prefer) {
> +
> +#if (NGX_HAVE_INET6)
> + case NGX_RESOLVE_PREFER_A:
> + d = 0;
> + break;
> +
> + case NGX_RESOLVE_PREFER_AAAA:
> + d = rn->naddrs6;
> +
> + if (d == n) {
> + d = 0;
> + }
> +
> + break;
> +#endif
> +
> + default:
> + d = 0;

Just a side note: in this form, NGX_RESOLVE_PREFER_A is exactly
equivalent to no preference, so r->prefer can be represented with
just one bit.

--
Maxim Dounin
http://mdounin.ru/
_______________________________________________
nginx-devel mailing list -- nginx-devel@nginx.org
To unsubscribe send an email to nginx-devel-leave@nginx.org