From mboxrd@z Thu Jan 1 00:00:00 1970 From: Marc Dionne Subject: Re: [PATCH] net: do not allow changing SO_REUSEADDR/SO_REUSEPORT on bound sockets Date: Mon, 11 Jun 2018 18:25:02 -0300 Message-ID: References: <20180603174705.51802-1-zenczykowski@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Cc: =?UTF-8?Q?Maciej_=C5=BBenczykowski?= , "David S . Miller" , Eric Dumazet , netdev , Jeffrey Altman , Marc Dionne To: =?UTF-8?Q?Maciej_=C5=BBenczykowski?= Return-path: Received: from mail-wm0-f67.google.com ([74.125.82.67]:53061 "EHLO mail-wm0-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932474AbeFKVZE (ORCPT ); Mon, 11 Jun 2018 17:25:04 -0400 Received: by mail-wm0-f67.google.com with SMTP id p126-v6so17095935wmb.2 for ; Mon, 11 Jun 2018 14:25:03 -0700 (PDT) In-Reply-To: <20180603174705.51802-1-zenczykowski@gmail.com> Sender: netdev-owner@vger.kernel.org List-ID: On Sun, Jun 3, 2018 at 2:47 PM, Maciej =C5=BBenczykowski wrote: > From: Maciej =C5=BBenczykowski > > It is not safe to do so because such sockets are already in the > hash tables and changing these options can result in invalidating > the tb->fastreuse(port) caching. > > This can have later far reaching consequences wrt. bind conflict checks > which rely on these caches (for optimization purposes). > > Not to mention that you can currently end up with two identical > non-reuseport listening sockets bound to the same local ip:port > by clearing reuseport on them after they've already both been bound. > > There is unfortunately no EISBOUND error or anything similar, > and EISCONN seems to be misleading for a bound-but-not-connected > socket, so use EUCLEAN 'Structure needs cleaning' which AFAICT > is the closest you can get to meaning 'socket in bad state'. > (although perhaps EINVAL wouldn't be a bad choice either?) > > This does unfortunately run the risk of breaking buggy > userspace programs... This change is a potential performance regression for us. Our networking code sets up multiple sockets used by multiple threads to listen on the same udp port. For the first socket, the bind is done before setting SO_REUSEPORT so that we can get an error and detect that the port is currently in use by a different process, possibly a previous incarnation of the same application. With this change, the servers end up with a single listener socket and thread, which can have a large impact on performance for a busy server. While we can adjust for the new behaviour going forward, this could be an unpleasant surprise for our customers who get this update when moving to a new kernel version or through a backport to stable kernels, if this is targeted for stable. Marc