From patchwork Tue Feb 12 18:48:35 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Alan Hayward X-Patchwork-Id: 31424 Received: (qmail 63367 invoked by alias); 12 Feb 2019 18:48:41 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 63356 invoked by uid 89); 12 Feb 2019 18:48:41 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=connect, Meanwhile, approximately, rapid X-HELO: EUR02-VE1-obe.outbound.protection.outlook.com Received: from mail-eopbgr20055.outbound.protection.outlook.com (HELO EUR02-VE1-obe.outbound.protection.outlook.com) (40.107.2.55) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 12 Feb 2019 18:48:39 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=armh.onmicrosoft.com; s=selector1-arm-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=SRXOwKCcC66vEN+YR23RMq/XCvcp5ELZPqn0DHHXoac=; b=GCN/JnynOlwPRgfhZHBqvoo4o2SrWQ9nDh5Lc9GMVX0sggDswBfV0q3XbaPqxqYfpuetVsyTHTub8knowZk9zlBvz62FUahTBSu3KRDc9VmiFTaK7/hwoAxos6RxVvTenhE1BDxvQGP/yJxZqoB8M7u+3rQnoTicG5iZNjzcdx0= Received: from DB6PR0802MB2133.eurprd08.prod.outlook.com (10.172.227.22) by DB6PR0802MB2421.eurprd08.prod.outlook.com (10.172.252.9) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.1601.22; Tue, 12 Feb 2019 18:48:35 +0000 Received: from DB6PR0802MB2133.eurprd08.prod.outlook.com ([fe80::acd7:a958:2aaa:562e]) by DB6PR0802MB2133.eurprd08.prod.outlook.com ([fe80::acd7:a958:2aaa:562e%5]) with mapi id 15.20.1601.023; Tue, 12 Feb 2019 18:48:35 +0000 From: Alan Hayward To: "gdb-patches@sourceware.org" CC: nd , Alan Hayward Subject: [PATCH/RFC] gdbserver: Add command line option to not use SO_REUSEADDR Date: Tue, 12 Feb 2019 18:48:35 +0000 Message-ID: <20190212184827.91720-1-alan.hayward@arm.com> authentication-results: spf=none (sender IP is ) smtp.mailfrom=Alan.Hayward@arm.com; received-spf: None (protection.outlook.com: arm.com does not designate permitted sender hosts) x-ms-exchange-senderadcheck: 1 MIME-Version: 1.0 X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-IsSubscribed: yes (Not sure if there is any additional documentation that will need updating alongside this patch, or any other additions that need to go alongside new command line flags). When running the testsuite using target_board=native-gdbserver across many processors, there are random failures generated by timeouts connecting to gdbserver. Each timeout will cause the whole of an .exp test script to fail (creating potentially dozens of FAILs). Per full make check run: On X86-64 Ubuntu 16.04, I see approximately 1 of these. On AArch64 Ubuntu 16.04 / Centos 7.4, I see approximately 3 of these. Forcing on gdb and gdbserver debug shows that for the failure cases gdbserver is launched, connects to gdb and sucessfully sends and receives packets. Meanwhile on the gdb side, no packets are sent or received and the connection simply times out. This indicates that the gdb from another running test has connected to this gdbserver. On the gdbserver side, the socket is set to SO_REUSEADDR. This allows the socket to be quickly reused on exit of gdbserver. Without it the socket has to fully timeout in the OS before it can be reused again (typically between 30 and 120 seconds). Removing the SO_REUSEADDR code solves the issue. One downside of this is an increase in the number of ports being used. When running with -j55 on HEAD, I see roughly 100 different ports used (ie ports 2346 to 2436). With the patch applied I see almost 300 ports used (ie ports 2346 to 2627). The other downside is that launching dozens of gdbservers at the same time is not the usual user workflow. The user is more likely to connect to gdbserver, do some work, kill it, then launch again - here the user will expect to be able to reuse the same port. This patch adds new command line flag to enable/disable SO_REUSEADDR. By default use of SO_REUSEADDR remains on and so the use will not see any change. In the testsuite, always start gdbserver with it disabled. Tested on AArch64 and X86_64 with default board, native gdbserver and native extended gdbserver. gdb/gdbserver/ChangeLog: 2019-02-12 Alan Hayward * remote-utils.c (remote_prepare): Check reuse flag. * server.c (gdbserver_usage): Add reuse help messages. (captured_main): Check for reuse flags * server.h (struct client_state): Add reuse flag. gdb/testsuite/ChangeLog: 2019-02-12 Alan Hayward * lib/gdbserver-support.exp: Start gdbserver without socket reuse. --- gdb/gdbserver/remote-utils.c | 12 +++++++----- gdb/gdbserver/server.c | 10 ++++++++++ gdb/gdbserver/server.h | 10 ++++++++++ gdb/testsuite/lib/gdbserver-support.exp | 3 +++ 4 files changed, 30 insertions(+), 5 deletions(-) diff --git a/gdb/gdbserver/remote-utils.c b/gdb/gdbserver/remote-utils.c index ad0228db99..b46c42da0b 100644 --- a/gdb/gdbserver/remote-utils.c +++ b/gdb/gdbserver/remote-utils.c @@ -233,7 +233,6 @@ remote_prepare (const char *name) #ifdef USE_WIN32API static int winsock_initialized; #endif - socklen_t tmp; remote_is_stdio = 0; if (strcmp (name, STDIO_CONNECTION_NAME) == 0) @@ -297,10 +296,13 @@ remote_prepare (const char *name) if (iter == NULL) perror_with_name ("Can't open socket"); - /* Allow rapid reuse of this port. */ - tmp = 1; - setsockopt (listen_desc, SOL_SOCKET, SO_REUSEADDR, (char *) &tmp, - sizeof (tmp)); + /* Allow rapid reuse of this port. */ + if (cs.socket_reuse) + { + socklen_t tmp = 1; + setsockopt (listen_desc, SOL_SOCKET, SO_REUSEADDR, (char *) &tmp, + sizeof (tmp)); + } switch (iter->ai_family) { diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c index e960c10d40..c6bcd26bee 100644 --- a/gdb/gdbserver/server.c +++ b/gdb/gdbserver/server.c @@ -3453,6 +3453,12 @@ gdbserver_usage (FILE *stream) " Exec PROG directly instead of using a shell.\n" " Disables argument globbing and variable substitution\n" " on UNIX-like systems.\n" + " --reuse-socket\n" + " Set the connection socket to allow reuse of local\n" + " addresses. (default)\n" + " --no-reuse-socket\n" + " Do not set the connection socket to allow reuse of\n" + " local addresses.\n" "\n" "Debug options:\n" "\n" @@ -3710,6 +3716,10 @@ captured_main (int argc, char *argv[]) startup_with_shell = false; else if (strcmp (*next_arg, "--once") == 0) run_once = 1; + else if (strcmp (*next_arg, "--reuse-socket") == 0) + cs.socket_reuse = true; + else if (strcmp (*next_arg, "--no-reuse-socket") == 0) + cs.socket_reuse = false; else if (strcmp (*next_arg, "--selftest") == 0) selftest = true; else if (startswith (*next_arg, "--selftest=")) diff --git a/gdb/gdbserver/server.h b/gdb/gdbserver/server.h index 400001addd..0ac90a93ce 100644 --- a/gdb/gdbserver/server.h +++ b/gdb/gdbserver/server.h @@ -198,6 +198,16 @@ struct client_state int current_traceframe = -1; + /* Should the connection socket be set to allow the use of local addresses via + the socket option SO_REUSEADDR. The default user desired behavior is for + the socket to become available immediately after gdbserver exits, so that + the same port can be used after restarting gdbserver. When running large + numbers of short lived gdbservers in quick succession across many + processors (such as during the testsuite) this can cause race conditions + in socket allocation. When disabled, after gdbserver exits, the socket has + to timeout before the OS will reuse it, causing the potential for + additional socket addresses being used. */ + bool socket_reuse = true; }; client_state &get_client_state (); diff --git a/gdb/testsuite/lib/gdbserver-support.exp b/gdb/testsuite/lib/gdbserver-support.exp index 05234c43bd..63f1a5343c 100644 --- a/gdb/testsuite/lib/gdbserver-support.exp +++ b/gdb/testsuite/lib/gdbserver-support.exp @@ -278,6 +278,9 @@ proc gdbserver_start { options arguments } { # Fire off the debug agent. set gdbserver_command "$gdbserver" + # Prevent the potential for port address clashes + append gdbserver_command " --no-reuse-socket" + # If gdbserver_reconnect will be called $gdbserver_reconnect_p must be # set to true already during gdbserver_start. global gdbserver_reconnect_p