nginx -t disrupts streamed udp traffic

[bartebor@…]

nginx -t creates sockets with socket option SO_REUSEADDR. This, at least in linux, allows UDP sockets to bind successfully. This makes some of the streamed traffic lost regardless of the state of SO_REUSEPORT being recommended setting in listen directive. This effect is more pronounced with large configurations, where testing may take a few seconds.

I think SO_REUSEADDR should be disabled (at least for UDP sockets) in the same way SO_REUSEPORT is in testing mode.

I tested this with nginx 1.18.x, but relevant code is still there in more recent versions.

Sample config (saved to $(pwd)/etc/nginx/nginx.conf):

# load_module "/usr/lib/nginx/modules/ngx_stream_module.so"; # if not compiled in
daemon off;
worker_processes 1;
pid /tmp/nginx.pid;

error_log /dev/stderr error;


events {
        worker_connections 1024;
}

stream {
        upstream uudp {
                server 127.0.0.1:4444 down;
        }

        server {
                listen 127.0.0.1:3333 udp reuseport;

                proxy_pass uudp;
        }
}

Now, we start nginx and then test the config:

$ /usr/sbin/nginx  -p  $(pwd) -c $(pwd)/etc/nginx/nginx.conf &
[1] 1659171
$ strace -esocket,setsockopt,bind /usr/sbin/nginx  -p  $(pwd) -c $(pwd)/etc/nginx/nginx.conf -t
nginx: the configuration file /tmp/etc/nginx/nginx.conf syntax is ok
socket(AF_INET, SOCK_DGRAM, IPPROTO_IP) = 4
setsockopt(4, SOL_SOCKET, SO_REUSEADDR, [1], 4) = 0
bind(4, {sa_family=AF_INET, sin_port=htons(3333), sin_addr=inet_addr("127.0.0.1")}, 16) = 0
nginx: configuration file /tmp/etc/nginx/nginx.conf test is successful
+++ exited with 0 +++

As strace shows there is SO_REUSEADDR set, SO_REUSEPORT is not set and bind is successful.

With simple patch disabling SO_REUSEADDR in test mode:

diff -ur nginx.orig/src/core/ngx_connection.c nginx.new/src/core/ngx_connection.c
--- nginx.orig/src/core/ngx_connection.c        2021-05-18 16:41:55.627284469 +0200
+++ nginx.new/src/core/ngx_connection.c 2021-05-18 16:44:11.321172445 +0200
@@ -495,7 +495,7 @@
                 return NGX_ERROR;
             }
 
-            if (setsockopt(s, SOL_SOCKET, SO_REUSEADDR,
+            if (!ngx_test_config && setsockopt(s, SOL_SOCKET, SO_REUSEADDR,
                            (const void *) &reuseaddr, sizeof(int))
                 == -1)
             {

the strace output is:

$ strace -esocket,setsockopt,bind /usr/sbin/nginx  -p  $(pwd) -c $(pwd)/etc/nginx/nginx.conf -t
nginx: the configuration file /tmp/etc/nginx/nginx.conf syntax is ok
socket(AF_INET, SOCK_DGRAM, IPPROTO_IP) = 4
setsockopt(4, SOL_SOCKET, SO_REUSEADDR, [1], 4) = 0
bind(4, {sa_family=AF_INET, sin_port=htons(3333), sin_addr=inet_addr("127.0.0.1")}, 16) = -1 EADDRINUSE (Address already in use)
nginx: configuration file /tmp/etc/nginx/nginx.conf test is successful
+++ exited with 0 +++

[Maxim Dounin]

I don't think that not using SO_REUSEADDR when testing the configuration at all is a good idea. On the other hand, given the Linux handling of SO_REUSEADDR, it is probably better to avoid on UDP sockets when testing configuration. Please test if the following patch works for you:

# HG changeset patch
# User Maxim Dounin <mdounin@mdounin.ru>
# Date 1621816657 -10800
#      Mon May 24 03:37:37 2021 +0300
# Node ID 3a34a0ba05e5e39c24b0e928b50250d0c1549261
# Parent  47945173813f5185e37385be3df5c02037be870d
Core: disabled SO_REUSEADDR on UDP sockets while testing config.

On Linux, SO_REUSEADDR allows completely duplicate UDP sockets, so using
SO_REUSEADDR when testing configuration results in packets being dropped
if there is an existing traffic on the sockets being tested (ticket #2187).
While dropped packets are expected with UDP, it is better to avoid this
when possible.

With this change, SO_REUSEADDR is no longer set on datagram sockets when
testing configuration.

diff --git a/src/core/ngx_connection.c b/src/core/ngx_connection.c
--- a/src/core/ngx_connection.c
+++ b/src/core/ngx_connection.c
@@ -495,21 +495,24 @@ ngx_open_listening_sockets(ngx_cycle_t *
                 return NGX_ERROR;
             }
 
-            if (setsockopt(s, SOL_SOCKET, SO_REUSEADDR,
-                           (const void *) &reuseaddr, sizeof(int))
-                == -1)
-            {
-                ngx_log_error(NGX_LOG_EMERG, log, ngx_socket_errno,
-                              "setsockopt(SO_REUSEADDR) %V failed",
-                              &ls[i].addr_text);
+            if (ls[i].type != SOCK_DGRAM || !ngx_test_config) {
 
-                if (ngx_close_socket(s) == -1) {
+                if (setsockopt(s, SOL_SOCKET, SO_REUSEADDR,
+                               (const void *) &reuseaddr, sizeof(int))
+                    == -1)
+                {
                     ngx_log_error(NGX_LOG_EMERG, log, ngx_socket_errno,
-                                  ngx_close_socket_n " %V failed",
+                                  "setsockopt(SO_REUSEADDR) %V failed",
                                   &ls[i].addr_text);
-                }
 
-                return NGX_ERROR;
+                    if (ngx_close_socket(s) == -1) {
+                        ngx_log_error(NGX_LOG_EMERG, log, ngx_socket_errno,
+                                      ngx_close_socket_n " %V failed",
+                                      &ls[i].addr_text);
+                    }
+
+                    return NGX_ERROR;
+                }
             }
 
 #if (NGX_HAVE_REUSEPORT)

Just in case, ignoring indentation changes the patch is as simple as:

diff --git a/src/core/ngx_connection.c b/src/core/ngx_connection.c
--- a/src/core/ngx_connection.c
+++ b/src/core/ngx_connection.c
@@ -495,6 +495,8 @@ ngx_open_listening_sockets(ngx_cycle_t *
                 return NGX_ERROR;
             }
 
+            if (ls[i].type != SOCK_DGRAM || !ngx_test_config) {
+
             if (setsockopt(s, SOL_SOCKET, SO_REUSEADDR,
                            (const void *) &reuseaddr, sizeof(int))
                 == -1)
@@ -511,6 +513,7 @@ ngx_open_listening_sockets(ngx_cycle_t *
 
                 return NGX_ERROR;
             }
+            }
 
 #if (NGX_HAVE_REUSEPORT)
 

[bartebor@…]

Thanks for your reply. The patch you provided fixes the issue. Running nginx -t does not interfere with running instance and UDP traffic is not lost. As for TCP SO_REUSEADDR alone is not enough to bind succesfully, so it makes no harm there.

[Maxim Dounin <mdounin@…>]

In 7868:46815874bcc6/nginx:

Core: disabled SO_REUSEADDR on UDP sockets while testing config.

On Linux, SO_REUSEADDR allows completely duplicate UDP sockets, so using
SO_REUSEADDR when testing configuration results in packets being dropped
if there is an existing traffic on the sockets being tested (ticket #2187).
While dropped packets are expected with UDP, it is better to avoid this
when possible.

With this change, SO_REUSEADDR is no longer set on datagram sockets when
testing configuration.

[Maxim Dounin]

Patch committed. Thanks for reporting this and for testing.