All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Daniel P. Berrangé" <berrange@redhat.com>
To: Peter Maydell <peter.maydell@linaro.org>
Cc: qemu-devel@nongnu.org, Peter Xu <peterx@redhat.com>,
	Brad Smith <brad@comstyle.com>, Fabiano Rosas <farosas@suse.de>
Subject: Re: [PATCH] util: don't set SO_REUSEADDR on client sockets
Date: Mon, 21 Oct 2024 18:04:29 +0100	[thread overview]
Message-ID: <ZxaJnRIqZl9ock_x@redhat.com> (raw)
In-Reply-To: <CAFEAcA9_9jYHHspAbR=3uXLHD=7AcMN3dQubYxsAQgyCyMOFUw@mail.gmail.com>

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 :|



      reply	other threads:[~2024-10-21 17:05 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-10-21 14:54 [PATCH] util: don't set SO_REUSEADDR on client sockets Daniel P. Berrangé
2024-10-21 15:45 ` Peter Xu
2024-10-21 16:41 ` Fabiano Rosas
2024-10-21 16:53 ` Peter Maydell
2024-10-21 17:04   ` Daniel P. Berrangé [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ZxaJnRIqZl9ock_x@redhat.com \
    --to=berrange@redhat.com \
    --cc=brad@comstyle.com \
    --cc=farosas@suse.de \
    --cc=peter.maydell@linaro.org \
    --cc=peterx@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.