> On 8 Jul 2022, at 04:35, Maxim Dounin <mdounin@mdounin.ru> wrote:
>
> Hello!
>
> 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.

# HG changeset patch
# User Ruslan Ermilov <ru@nginx.com>
# Date 1657113024 -14400
# Wed Jul 06 17:10:24 2022 +0400
# Node ID 496a9b8d14048295723bd3e21ef4a9a1129cb97c
# Parent d8c0fb82b7a8d3f958b13db06f2f911d95a41644
The "prefer=" parameter of the "resolver" directive.

diff -r d8c0fb82b7a8 -r 496a9b8d1404 src/core/ngx_resolver.c
--- a/src/core/ngx_resolver.c Wed Jul 06 17:09:40 2022 +0400
+++ b/src/core/ngx_resolver.c Wed Jul 06 17:10:24 2022 +0400
@@ -227,6 +227,7 @@ ngx_resolver_create(ngx_conf_t *cf, ngx_
}

#if (NGX_HAVE_INET6)
+
if (ngx_strncmp(names[i].data, "ipv4=", 5) == 0) {

if (ngx_strcmp(&names[i].data[5], "on") == 0) {
@@ -260,6 +261,24 @@ ngx_resolver_create(ngx_conf_t *cf, ngx_

continue;
}
+
+ if (ngx_strncmp(names[i].data, "prefer=", 7) == 0) {
+
+ if (ngx_strcmp(&names[i].data[7], "ipv4") == 0) {
+ r->prefer = NGX_RESOLVE_PREFER_A;
+
+ } else if (ngx_strcmp(&names[i].data[7], "ipv6") == 0) {
+ r->prefer = NGX_RESOLVE_PREFER_AAAA;
+
+ } else {
+ ngx_conf_log_error(NGX_LOG_EMERG, cf, 0,
+ "invalid parameter: %V", &names[i]);
+ return NULL;
+ }
+
+ continue;
+ }
+
#endif

ngx_memzero(&u, sizeof(ngx_url_t));
@@ -4250,7 +4269,32 @@ ngx_resolver_export(ngx_resolver_t *r, n
}

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;
+ }
+ }

if (rn->naddrs) {
j = rotate ? ngx_random() % rn->naddrs : 0;
diff -r d8c0fb82b7a8 -r 496a9b8d1404 src/core/ngx_resolver.h
--- a/src/core/ngx_resolver.h Wed Jul 06 17:09:40 2022 +0400
+++ b/src/core/ngx_resolver.h Wed Jul 06 17:10:24 2022 +0400
@@ -36,6 +36,9 @@

#define NGX_RESOLVER_MAX_RECURSION 50

+#define NGX_RESOLVE_PREFER_A 1
+#define NGX_RESOLVE_PREFER_AAAA 2
+

typedef struct ngx_resolver_s ngx_resolver_t;

@@ -175,6 +178,8 @@ struct ngx_resolver_s {
ngx_queue_t srv_expire_queue;
ngx_queue_t addr_expire_queue;

+ unsigned prefer:2;
+
unsigned ipv4:1;

#if (NGX_HAVE_INET6)

--
Sergey Kandaurov

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