[PATCH] The "sort=" parameter of the "resolver" directive

[Sergey Kandaurov]

> On 23 Feb 2022, at 08:11, Ruslan Ermilov <ru at nginx.com> wrote:
> 
> src/core/ngx_resolver.c |  38 +++++++++++++++++++++++++++++++++++++-
> src/core/ngx_resolver.h |   5 +++++
> 2 files changed, 42 insertions(+), 1 deletions(-)
> 
> 
> # HG changeset patch
> # User Ruslan Ermilov <ru at nginx.com>
> # Date 1645589387 -10800
> #      Wed Feb 23 07:09:47 2022 +0300
> # Node ID 8db4bbd67840e8bebb23f9c6d10c0f633552e616
> # Parent  1c19779448db2309d607c74e2628ff98f84569ff
> The "sort=" parameter of the "resolver" directive.
> 

IMHO, the name isn't enough self-documenting in this context.
It can be misinterpreted as sorting addresses, not families.
I'd prefer the parameter name "prefer" here.

> diff --git a/src/core/ngx_resolver.c b/src/core/ngx_resolver.c
> --- a/src/core/ngx_resolver.c
> +++ b/src/core/ngx_resolver.c
> @@ -266,6 +266,27 @@ ngx_resolver_create(ngx_conf_t *cf, ngx_
>         }
> #endif
> 
> +        if (ngx_strncmp(names[i].data, "sort=", 5) == 0) {
> +
> +            if (ngx_strcasecmp(&names[i].data[5], (u_char *) "ipv4") == 0) {

The same question raises about case-sensitivity.
I prefer to make it case-sensitive, like other similar directive parameters.

> +                r->sort = NGX_RESOLVE_A_FIRST;
> +
> +#if (NGX_HAVE_INET6)
> +            } else if (ngx_strcasecmp(&names[i].data[5], (u_char *) "ipv6")
> +                       == 0)
> +            {
> +                r->sort = NGX_RESOLVE_AAAA_FIRST;
> +#endif
> +
> +            } else {
> +                ngx_conf_log_error(NGX_LOG_EMERG, cf, 0,
> +                                   "invalid parameter: %V", &names[i]);
> +                return NULL;
> +            }
> +
> +            continue;
> +        }
> +

What about to put the whole block under NGX_HAVE_INET6?
If built without ipv6 support it makes same little sense
as for "ipv6" and "ipv4" parameters that already there.

>         ngx_memzero(&u, sizeof(ngx_url_t));
> 
>         u.url = names[i];
> @@ -4253,7 +4274,22 @@ ngx_resolver_export(ngx_resolver_t *r, n
>     }
> 
>     i = 0;
> -    d = rotate ? ngx_random() % n : 0;
> +
> +    if (r->sort == NGX_RESOLVE_A_FIRST) {
> +        d = 0;
> +
> +#if (NGX_HAVE_INET6)
> +    } else if (r->sort == NGX_RESOLVE_AAAA_FIRST) {
> +        d = rn->naddrs6;
> +
> +        if (d == n) {
> +            d = 0;
> +        }
> +#endif
> +
> +    } else {
> +        d = rotate ? ngx_random() % n : 0;
> +    }
> 
>     if (rn->naddrs) {
>         j = rotate ? ngx_random() % rn->naddrs : 0;
> diff --git a/src/core/ngx_resolver.h b/src/core/ngx_resolver.h
> --- a/src/core/ngx_resolver.h
> +++ b/src/core/ngx_resolver.h
> @@ -36,6 +36,9 @@
> 
> #define NGX_RESOLVER_MAX_RECURSION    50
> 
> +#define NGX_RESOLVE_A_FIRST           1
> +#define NGX_RESOLVE_AAAA_FIRST        2
> +

I'd adjust name for better sorting.

> 
> typedef struct ngx_resolver_s  ngx_resolver_t;
> 
> @@ -185,6 +188,8 @@ struct ngx_resolver_s {
>     ngx_queue_t               addr6_expire_queue;
> #endif
> 
> +    ngx_uint_t                sort;
> +

Since the previous patch introduces ipv4/ipv6 bit fields,
it makes sense to make this one a bit field as well.

>     time_t                    resend_timeout;
>     time_t                    tcp_timeout;
>     time_t                    expire;
> 

diff --git a/src/core/ngx_resolver.c b/src/core/ngx_resolver.c
--- a/src/core/ngx_resolver.c
+++ b/src/core/ngx_resolver.c
@@ -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,22 @@ ngx_resolver_export(ngx_resolver_t *r, n
     }
 
     i = 0;
-    d = rotate ? ngx_random() % n : 0;
+
+    if (r->prefer == NGX_RESOLVE_PREFER_A) {
+        d = 0;
+
+#if (NGX_HAVE_INET6)
+    } else if (r->prefer == NGX_RESOLVE_PREFER_AAAA) {
+        d = rn->naddrs6;
+
+        if (d == n) {
+            d = 0;
+        }
+#endif
+
+    } else {
+        d = rotate ? ngx_random() % n : 0;
+    }
 
     if (rn->naddrs) {
         j = rotate ? ngx_random() % rn->naddrs : 0;
diff --git a/src/core/ngx_resolver.h b/src/core/ngx_resolver.h
--- a/src/core/ngx_resolver.h
+++ b/src/core/ngx_resolver.h
@@ -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