[PATCH] util: don't set SO_REUSEADDR on client sockets

[PATCH] util: don't set SO_REUSEADDR on client sockets

[Daniel P. Berrangé]

Setting the SO_REUSEADDR property on a socket allows binding to a port
number that is in the TIMED_WAIT state. This is usually done on listener
sockets, to enable a server to restart itself without having to wait for
the completion of TIMED_WAIT on the port.

It is also possible, but highly unusual, to set it on client sockets. It
is rare to explicitly bind() a client socket, since it is almost always
fine to allow the kernel to auto-bind a client socket to a random free
port. Most systems will have many 10's of 1000's of free ports that
client sockets will be bound to.

eg on Linux

  $ sysctl -a | grep local_port
  net.ipv4.ip_local_port_range = 32768	60999

eg on OpenBSD

  $ sysctl -a | grep net.inet.ip.port
  net.inet.ip.portfirst=1024
  net.inet.ip.portlast=49151
  net.inet.ip.porthifirst=49152
  net.inet.ip.porthilast=65535

A connected socket must have a unique set of value for

 (protocol, localip, localport, remoteip, remoteport)

otherwise it is liable to get EADDRINUSE.

A client connection should trivially avoid EADDRINUSE if letting the
kernel auto-assign the 'localport' value, which QEMU always does.

When QEMU sets SO_REUSEADDR on a client socket on OpenBSD, however, it
upsets this situation.

The OpenBSD kernel appears to happily pick a 'localport' that is in the
TIMED_WAIT state, even if there are many other available local ports
available for use that are not in the TIMED_WAIT state.

A test program that just loops opening client sockets will start seeing
EADDRINUSE on OpenBSD when as few as 2000 ports are in TIMED_WAIT,
despite 10's of 1000's ports still being unused. This contrasts with
Linux which appears to avoid picking local ports in TIMED_WAIT state.

This problem on OpenBSD exhibits itself periodically with the migration
test failing with a message like[1]:

  qemu-system-ppc64: Failed to connect to '127.0.0.1:24109': Address already in use

While I have not been able to reproduce the OpenBSD failure in my own
testing, given the scope of what QEMU tests do, it is entirely possible
that there could be a lot of ports in TIMED_WAIT state when the
migration test runs.

Removing SO_REUSEADDR from the client sockets should not affect normal
QEMU usage, and should improve reliability on OpenBSD.

This use of SO_REUSEADDR on client sockets is highly unusual, and
appears to have been present since the very start of the QEMU socket
helpers in 2008. The orignal commit has no comment about the use of
SO_REUSEADDR on the client, so is most likely just an 16 year old
copy+paste bug.

[1] https://lists.nongnu.org/archive/html/qemu-devel/2024-10/msg03427.html
    https://lists.nongnu.org/archive/html/qemu-devel/2024-02/msg01572.html

Fixes: d247d25f18764402899b37c381bb696a79000b4e
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 util/qemu-sockets.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
index 60c44b2b56..80594ecad5 100644
--- a/util/qemu-sockets.c
+++ b/util/qemu-sockets.c
@@ -367,7 +367,6 @@ static int inet_connect_addr(const InetSocketAddress *saddr,
                          addr->ai_family);
         return -1;
     }
-    socket_set_fast_reuse(sock);
 
     /* connect to peer */
     do {
-- 
2.46.0

Re: [PATCH] util: don't set SO_REUSEADDR on client sockets

[Peter Xu]

On Mon, Oct 21, 2024 at 03:54:10PM +0100, Daniel P. Berrangé wrote:
> Setting the SO_REUSEADDR property on a socket allows binding to a port
> number that is in the TIMED_WAIT state. This is usually done on listener
> sockets, to enable a server to restart itself without having to wait for
> the completion of TIMED_WAIT on the port.
> 
> It is also possible, but highly unusual, to set it on client sockets. It
> is rare to explicitly bind() a client socket, since it is almost always
> fine to allow the kernel to auto-bind a client socket to a random free
> port. Most systems will have many 10's of 1000's of free ports that
> client sockets will be bound to.
> 
> eg on Linux
> 
>   $ sysctl -a | grep local_port
>   net.ipv4.ip_local_port_range = 32768	60999
> 
> eg on OpenBSD
> 
>   $ sysctl -a | grep net.inet.ip.port
>   net.inet.ip.portfirst=1024
>   net.inet.ip.portlast=49151
>   net.inet.ip.porthifirst=49152
>   net.inet.ip.porthilast=65535
> 
> A connected socket must have a unique set of value for
> 
>  (protocol, localip, localport, remoteip, remoteport)
> 
> otherwise it is liable to get EADDRINUSE.
> 
> A client connection should trivially avoid EADDRINUSE if letting the
> kernel auto-assign the 'localport' value, which QEMU always does.
> 
> When QEMU sets SO_REUSEADDR on a client socket on OpenBSD, however, it
> upsets this situation.
> 
> The OpenBSD kernel appears to happily pick a 'localport' that is in the
> TIMED_WAIT state, even if there are many other available local ports
> available for use that are not in the TIMED_WAIT state.
> 
> A test program that just loops opening client sockets will start seeing
> EADDRINUSE on OpenBSD when as few as 2000 ports are in TIMED_WAIT,
> despite 10's of 1000's ports still being unused. This contrasts with
> Linux which appears to avoid picking local ports in TIMED_WAIT state.
> 
> This problem on OpenBSD exhibits itself periodically with the migration
> test failing with a message like[1]:
> 
>   qemu-system-ppc64: Failed to connect to '127.0.0.1:24109': Address already in use
> 
> While I have not been able to reproduce the OpenBSD failure in my own
> testing, given the scope of what QEMU tests do, it is entirely possible
> that there could be a lot of ports in TIMED_WAIT state when the
> migration test runs.
> 
> Removing SO_REUSEADDR from the client sockets should not affect normal
> QEMU usage, and should improve reliability on OpenBSD.
> 
> This use of SO_REUSEADDR on client sockets is highly unusual, and
> appears to have been present since the very start of the QEMU socket
> helpers in 2008. The orignal commit has no comment about the use of
> SO_REUSEADDR on the client, so is most likely just an 16 year old
> copy+paste bug.
> 
> [1] https://lists.nongnu.org/archive/html/qemu-devel/2024-10/msg03427.html
>     https://lists.nongnu.org/archive/html/qemu-devel/2024-02/msg01572.html
> 
> Fixes: d247d25f18764402899b37c381bb696a79000b4e
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>

Thanks for digging this.  It's good already to see some line removed on the
client side where it doesn't seem to clear why REUSE is needed at all..

Reviewed-by: Peter Xu <peterx@redhat.com>

-- 
Peter Xu

Re: [PATCH] util: don't set SO_REUSEADDR on client sockets

[Fabiano Rosas]

Daniel P. Berrangé <berrange@redhat.com> writes:

> Setting the SO_REUSEADDR property on a socket allows binding to a port
> number that is in the TIMED_WAIT state. This is usually done on listener
> sockets, to enable a server to restart itself without having to wait for
> the completion of TIMED_WAIT on the port.
>
> It is also possible, but highly unusual, to set it on client sockets. It
> is rare to explicitly bind() a client socket, since it is almost always
> fine to allow the kernel to auto-bind a client socket to a random free
> port. Most systems will have many 10's of 1000's of free ports that
> client sockets will be bound to.
>
> eg on Linux
>
>   $ sysctl -a | grep local_port
>   net.ipv4.ip_local_port_range = 32768	60999
>
> eg on OpenBSD
>
>   $ sysctl -a | grep net.inet.ip.port
>   net.inet.ip.portfirst=1024
>   net.inet.ip.portlast=49151
>   net.inet.ip.porthifirst=49152
>   net.inet.ip.porthilast=65535
>
> A connected socket must have a unique set of value for
>
>  (protocol, localip, localport, remoteip, remoteport)
>
> otherwise it is liable to get EADDRINUSE.
>
> A client connection should trivially avoid EADDRINUSE if letting the
> kernel auto-assign the 'localport' value, which QEMU always does.
>
> When QEMU sets SO_REUSEADDR on a client socket on OpenBSD, however, it
> upsets this situation.
>
> The OpenBSD kernel appears to happily pick a 'localport' that is in the
> TIMED_WAIT state, even if there are many other available local ports
> available for use that are not in the TIMED_WAIT state.
>
> A test program that just loops opening client sockets will start seeing
> EADDRINUSE on OpenBSD when as few as 2000 ports are in TIMED_WAIT,
> despite 10's of 1000's ports still being unused. This contrasts with
> Linux which appears to avoid picking local ports in TIMED_WAIT state.
>
> This problem on OpenBSD exhibits itself periodically with the migration
> test failing with a message like[1]:
>
>   qemu-system-ppc64: Failed to connect to '127.0.0.1:24109': Address already in use
>
> While I have not been able to reproduce the OpenBSD failure in my own
> testing, given the scope of what QEMU tests do, it is entirely possible
> that there could be a lot of ports in TIMED_WAIT state when the
> migration test runs.
>
> Removing SO_REUSEADDR from the client sockets should not affect normal
> QEMU usage, and should improve reliability on OpenBSD.
>
> This use of SO_REUSEADDR on client sockets is highly unusual, and
> appears to have been present since the very start of the QEMU socket
> helpers in 2008. The orignal commit has no comment about the use of
> SO_REUSEADDR on the client, so is most likely just an 16 year old
> copy+paste bug.
>
> [1] https://lists.nongnu.org/archive/html/qemu-devel/2024-10/msg03427.html
>     https://lists.nongnu.org/archive/html/qemu-devel/2024-02/msg01572.html
>
> Fixes: d247d25f18764402899b37c381bb696a79000b4e
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>

Reviewed-by: Fabiano Rosas <farosas@suse.de>

Re: [PATCH] util: don't set SO_REUSEADDR on client sockets

[Peter Maydell]

On Mon, 21 Oct 2024 at 15:54, Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> Setting the SO_REUSEADDR property on a socket allows binding to a port
> number that is in the TIMED_WAIT state. This is usually done on listener
> sockets, to enable a server to restart itself without having to wait for
> the completion of TIMED_WAIT on the port.

[...]

> diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
> index 60c44b2b56..80594ecad5 100644
> --- a/util/qemu-sockets.c
> +++ b/util/qemu-sockets.c
> @@ -367,7 +367,6 @@ static int inet_connect_addr(const InetSocketAddress *saddr,
>                           addr->ai_family);
>          return -1;
>      }
> -    socket_set_fast_reuse(sock);
>
>      /* connect to peer */
>      do {

We definitely want to keep the socket_set_fast_reuse()
call in create_fast_reuse_socket() as that function is
used (only) in the "create socket, listen, bind" server
socket code path. (Arguably create_fast_reuse_socket()
is a bit unnecessary as it has only one callsite.)

The one in inet_connect_addr() is clearly wrong as that's
the client end (fixed in this patch).

How about the call in inet_dgram_saddr() ? I'm not sure how
SO_REUSEADDR interacts with UDP sockets... (I'm assuming
the answer is "we need it there" so I'm kind of asking for
the code-review record really.)

In net/socket.c we already set SO_REUSEADDR for dgram
and for listening sockets but not for client ones, so
we're now consistent there.

thanks
-- PMM

Re: [PATCH] util: don't set SO_REUSEADDR on client sockets

[Daniel P. Berrangé]

On Mon, Oct 21, 2024 at 05:53:17PM +0100, Peter Maydell wrote:
> On Mon, 21 Oct 2024 at 15:54, Daniel P. Berrangé <berrange@redhat.com> wrote:
> >
> > Setting the SO_REUSEADDR property on a socket allows binding to a port
> > number that is in the TIMED_WAIT state. This is usually done on listener
> > sockets, to enable a server to restart itself without having to wait for
> > the completion of TIMED_WAIT on the port.
> 
> [...]
> 
> > diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
> > index 60c44b2b56..80594ecad5 100644
> > --- a/util/qemu-sockets.c
> > +++ b/util/qemu-sockets.c
> > @@ -367,7 +367,6 @@ static int inet_connect_addr(const InetSocketAddress *saddr,
> >                           addr->ai_family);
> >          return -1;
> >      }
> > -    socket_set_fast_reuse(sock);
> >
> >      /* connect to peer */
> >      do {
> 
> We definitely want to keep the socket_set_fast_reuse()
> call in create_fast_reuse_socket() as that function is
> used (only) in the "create socket, listen, bind" server
> socket code path. (Arguably create_fast_reuse_socket()
> is a bit unnecessary as it has only one callsite.)
> 
> The one in inet_connect_addr() is clearly wrong as that's
> the client end (fixed in this patch).
> 
> How about the call in inet_dgram_saddr() ? I'm not sure how
> SO_REUSEADDR interacts with UDP sockets... (I'm assuming
> the answer is "we need it there" so I'm kind of asking for
> the code-review record really.)

We need the one in inet_dgram_saddr, because there is an
explicit bind() call there, for the situation where the
local UDP address is set.

> In net/socket.c we already set SO_REUSEADDR for dgram
> and for listening sockets but not for client ones, so
> we're now consistent there.
> 
> thanks
> -- PMM
> 

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|