Welcome! Log In Create A New Profile

Advanced

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

Sergey Kandaurov
June 16, 2022 11:18AM
> On 23 Feb 2022, at 08:11, Ruslan Ermilov <ru@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@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

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

[PATCH] Add ipv4=off option in resolver like ipv6=off (ticket #1330)

Lukas Lihotzki via nginx-devel 537 January 19, 2022 01:50PM

Re: [PATCH] Add ipv4=off option in resolver like ipv6=off (ticket #1330)

ru@nginx.com 74 February 16, 2022 07:32AM

Re: [PATCH] Add ipv4=off option in resolver like ipv6=off (ticket #1330)

ru@nginx.com 83 February 22, 2022 11:10PM

Re: [PATCH] Add ipv4=off option in resolver like ipv6=off (ticket #1330)

Lukas Lihotzki via nginx-devel 29 May 28, 2022 12:30PM

Re: [PATCH] Add ipv4=off option in resolver like ipv6=off (ticket #1330)

Sergey Kandaurov 13 June 15, 2022 06:28AM

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

Sergey Kandaurov 21 June 16, 2022 11:18AM



Sorry, you do not have permission to post/reply in this forum.

Online Users

Guests: 133
Record Number of Users: 6 on February 13, 2018
Record Number of Guests: 421 on December 02, 2018
Powered by nginx      Powered by FreeBSD      PHP Powered      Powered by MariaDB      ipv6 ready