[Openvpn-devel] Increase listen() backlog queue to 32

Message ID 20190815155319.28249-1-gert@greenie.muc.de
State Accepted
Headers show
Series
  • [Openvpn-devel] Increase listen() backlog queue to 32
Related show

Commit Message

Gert Doering Aug. 15, 2019, 3:53 p.m.
For reasons historically unknown, OpenVPN sets the listen() backlog
queue to "1", which signals the kernel "while there is one TCP connect
waiting for OpenVPN to handle it, refuse all others" - which, on
restarting a busy TCP server, will create connection issues.

The exact "best" value of the backlog queue is subject of discussion,
but for a server that is not extremely busy with many connections
coming in in parallel, there is no real difference between "10" or "500",
as long as it's "more than 1".

Found and debugged by "mjo" in Trac.

Trac: #1208

Signed-off-by: Gert Doering <gert@greenie.muc.de>
---
 src/openvpn/socket.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Antonio Quartulli Aug. 15, 2019, 8:21 p.m. | #1
Hi,

On 15/08/2019 17:53, Gert Doering wrote:
> For reasons historically unknown, OpenVPN sets the listen() backlog
> queue to "1", which signals the kernel "while there is one TCP connect
> waiting for OpenVPN to handle it, refuse all others" - which, on
> restarting a busy TCP server, will create connection issues.
> 
> The exact "best" value of the backlog queue is subject of discussion,
> but for a server that is not extremely busy with many connections
> coming in in parallel, there is no real difference between "10" or "500",
> as long as it's "more than 1".
> 
> Found and debugged by "mjo" in Trac.
> 
> Trac: #1208
> 
> Signed-off-by: Gert Doering <gert@greenie.muc.de>

Having 1 is definitely unreasonable, and we need something larger.
On top of that, mjo explained what kind of other side effects we can
have when setting the queue to such a small value.

32 is reasonable and may even be worth a define so that it can be easily
tweaked by whoever wants to. But this is another story..

Acked-by: Antonio Quartulli <a@unstable.cc>

> ---
>  src/openvpn/socket.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/openvpn/socket.c b/src/openvpn/socket.c
> index c472cf1b..983ed38a 100644
> --- a/src/openvpn/socket.c
> +++ b/src/openvpn/socket.c
> @@ -1175,7 +1175,7 @@ socket_do_listen(socket_descriptor_t sd,
>          ASSERT(local);
>          msg(M_INFO, "Listening for incoming TCP connection on %s",
>              print_sockaddr(local->ai_addr, &gc));
> -        if (listen(sd, 1))
> +        if (listen(sd, 32))
>          {
>              msg(M_ERR, "TCP: listen() failed");
>          }
>
David Sommerseth Aug. 16, 2019, 11:49 a.m. | #2
On 15/08/2019 17:53, Gert Doering wrote:
> For reasons historically unknown, OpenVPN sets the listen() backlog
> queue to "1", which signals the kernel "while there is one TCP connect
> waiting for OpenVPN to handle it, refuse all others" - which, on
> restarting a busy TCP server, will create connection issues.
> 
> The exact "best" value of the backlog queue is subject of discussion,
> but for a server that is not extremely busy with many connections
> coming in in parallel, there is no real difference between "10" or "500",
> as long as it's "more than 1".
> 
> Found and debugged by "mjo" in Trac.
> 
> Trac: #1208
> 
> Signed-off-by: Gert Doering <gert@greenie.muc.de>

Acked-By: David Sommerseth <davids@openvpn.net>

I agree with Antonio, and we should make it somewhat easier to modify.  I'm
not sure if there's value in having it as a runtime option, like
--socket-backlog (or something like that), or as a value you can pass to
./configure at compile time.
Antonio Quartulli Aug. 16, 2019, 3:13 p.m. | #3
Hi,

On 16/08/2019 13:49, David Sommerseth wrote:
> On 15/08/2019 17:53, Gert Doering wrote:
>> For reasons historically unknown, OpenVPN sets the listen() backlog
>> queue to "1", which signals the kernel "while there is one TCP connect
>> waiting for OpenVPN to handle it, refuse all others" - which, on
>> restarting a busy TCP server, will create connection issues.
>>
>> The exact "best" value of the backlog queue is subject of discussion,
>> but for a server that is not extremely busy with many connections
>> coming in in parallel, there is no real difference between "10" or "500",
>> as long as it's "more than 1".
>>
>> Found and debugged by "mjo" in Trac.
>>
>> Trac: #1208
>>
>> Signed-off-by: Gert Doering <gert@greenie.muc.de>
> 
> Acked-By: David Sommerseth <davids@openvpn.net>
> 
> I agree with Antonio, and we should make it somewhat easier to modify.

I disagree with you on this point :D This is not something we expect
people to play with. This is only a value that a developer with
networking knowledge is expected to find and tweak. Hence my suggestion
to make it a define in some header main header file.

>  I'm
> not sure if there's value in having it as a runtime option, like
> --socket-backlog (or something like that), or as a value you can pass to
> ./configure at compile time.
> 

Like above: yet another config option that the average joe can mess up
and come up with unknown problems nobody will understand? nonono ;)

Cheers,

> 
> 
> 
> _______________________________________________
> Openvpn-devel mailing list
> Openvpn-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/openvpn-devel
>
Gert Doering Aug. 16, 2019, 6:14 p.m. | #4
Wow, more ACKs than actual lines of codes affected .-) - I hear you
wrt "have a #define somewhere" (and yes, we could do that) or "make it
easier to configure for users" (nah... for the reasons Antonio gave) :-)

Patch has been applied to the master and release/2.4 branch.

2.4, because (see the discussion in the ticket) this effectively 
acknowledges the changing nature of the internet, with ubiquitous
background scanning on well-known ports like TCP/443 - thus getting
in the way of production openvpn setups with "listen(s,1)".

commit 6d8380c78bf77766454b93b49ab2ebf713b0be48 (master)
commit ec0ca68f4ed1e6aa6f08f470b18e0198b7e5a4da (release/2.4)
Author: Gert Doering
Date:   Thu Aug 15 17:53:19 2019 +0200

     Increase listen() backlog queue to 32

     Signed-off-by: Gert Doering <gert@greenie.muc.de>
     Acked-by: Antonio Quartulli <antonio@openvpn.net>
     Acked-by: David Sommerseth <davids@openvpn.net>
     Message-Id: <20190815155319.28249-1-gert@greenie.muc.de>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg18758.html
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering

Patch

diff --git a/src/openvpn/socket.c b/src/openvpn/socket.c
index c472cf1b..983ed38a 100644
--- a/src/openvpn/socket.c
+++ b/src/openvpn/socket.c
@@ -1175,7 +1175,7 @@  socket_do_listen(socket_descriptor_t sd,
         ASSERT(local);
         msg(M_INFO, "Listening for incoming TCP connection on %s",
             print_sockaddr(local->ai_addr, &gc));
-        if (listen(sd, 1))
+        if (listen(sd, 32))
         {
             msg(M_ERR, "TCP: listen() failed");
         }