From patchwork Fri Feb 14 11:48:01 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Boeuf, Sebastien" X-Patchwork-Id: 1237998 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming-netdev@ozlabs.org Delivered-To: patchwork-incoming-netdev@ozlabs.org Authentication-Results: ozlabs.org; spf=none (no SPF record) smtp.mailfrom=vger.kernel.org (client-ip=209.132.180.67; helo=vger.kernel.org; envelope-from=netdev-owner@vger.kernel.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=intel.com Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 48Js8v0cr1z9s29 for ; Fri, 14 Feb 2020 22:48:47 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728427AbgBNLsq (ORCPT ); Fri, 14 Feb 2020 06:48:46 -0500 Received: from mga07.intel.com ([134.134.136.100]:41106 "EHLO mga07.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727835AbgBNLsq (ORCPT ); Fri, 14 Feb 2020 06:48:46 -0500 X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga008.fm.intel.com ([10.253.24.58]) by orsmga105.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 14 Feb 2020 03:48:44 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.70,440,1574150400"; d="scan'208";a="228468524" Received: from dsitkin-mobl.ccr.corp.intel.com (HELO localhost.localdomain) ([10.252.24.179]) by fmsmga008.fm.intel.com with ESMTP; 14 Feb 2020 03:48:42 -0800 From: Sebastien Boeuf To: netdev@vger.kernel.org Cc: sgarzare@redhat.com, stefanha@redhat.com, davem@davemloft.net, Sebastien Boeuf Subject: [PATCH v3 1/2] net: virtio_vsock: Enhance connection semantics Date: Fri, 14 Feb 2020 12:48:01 +0100 Message-Id: <20200214114802.23638-2-sebastien.boeuf@intel.com> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20200214114802.23638-1-sebastien.boeuf@intel.com> References: <20200214114802.23638-1-sebastien.boeuf@intel.com> MIME-Version: 1.0 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org Whenever the vsock backend on the host sends a packet through the RX queue, it expects an answer on the TX queue. Unfortunately, there is one case where the host side will hang waiting for the answer and might effectively never recover if no timeout mechanism was implemented. This issue happens when the guest side starts binding to the socket, which insert a new bound socket into the list of already bound sockets. At this time, we expect the guest to also start listening, which will trigger the sk_state to move from TCP_CLOSE to TCP_LISTEN. The problem occurs if the host side queued a RX packet and triggered an interrupt right between the end of the binding process and the beginning of the listening process. In this specific case, the function processing the packet virtio_transport_recv_pkt() will find a bound socket, which means it will hit the switch statement checking for the sk_state, but the state won't be changed into TCP_LISTEN yet, which leads the code to pick the default statement. This default statement will only free the buffer, while it should also respond to the host side, by sending a packet on its TX queue. In order to simply fix this unfortunate chain of events, it is important that in case the default statement is entered, and because at this stage we know the host side is waiting for an answer, we must send back a packet containing the operation VIRTIO_VSOCK_OP_RST. One could say that a proper timeout mechanism on the host side will be enough to avoid the backend to hang. But the point of this patch is to ensure the normal use case will be provided with proper responsiveness when it comes to establishing the connection. Signed-off-by: Sebastien Boeuf --- net/vmw_vsock/virtio_transport_common.c | 1 + 1 file changed, 1 insertion(+) diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c index d9f0c9c5425a..2f696124bab6 100644 --- a/net/vmw_vsock/virtio_transport_common.c +++ b/net/vmw_vsock/virtio_transport_common.c @@ -1153,6 +1153,7 @@ void virtio_transport_recv_pkt(struct virtio_transport *t, virtio_transport_free_pkt(pkt); break; default: + (void)virtio_transport_reset_no_sock(t, pkt); virtio_transport_free_pkt(pkt); break; } From patchwork Fri Feb 14 11:48:02 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Boeuf, Sebastien" X-Patchwork-Id: 1237999 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming-netdev@ozlabs.org Delivered-To: patchwork-incoming-netdev@ozlabs.org Authentication-Results: ozlabs.org; spf=none (no SPF record) smtp.mailfrom=vger.kernel.org (client-ip=209.132.180.67; helo=vger.kernel.org; envelope-from=netdev-owner@vger.kernel.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=intel.com Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 48Js8z3V9Nz9s29 for ; Fri, 14 Feb 2020 22:48:51 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728915AbgBNLsu (ORCPT ); Fri, 14 Feb 2020 06:48:50 -0500 Received: from mga04.intel.com ([192.55.52.120]:59363 "EHLO mga04.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727835AbgBNLsu (ORCPT ); Fri, 14 Feb 2020 06:48:50 -0500 X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga008.fm.intel.com ([10.253.24.58]) by fmsmga104.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 14 Feb 2020 03:48:50 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.70,440,1574150400"; d="scan'208";a="228468552" Received: from dsitkin-mobl.ccr.corp.intel.com (HELO localhost.localdomain) ([10.252.24.179]) by fmsmga008.fm.intel.com with ESMTP; 14 Feb 2020 03:48:48 -0800 From: Sebastien Boeuf To: netdev@vger.kernel.org Cc: sgarzare@redhat.com, stefanha@redhat.com, davem@davemloft.net, Sebastien Boeuf Subject: [PATCH v3 2/2] tools: testing: vsock: Test when server is bound but not listening Date: Fri, 14 Feb 2020 12:48:02 +0100 Message-Id: <20200214114802.23638-3-sebastien.boeuf@intel.com> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20200214114802.23638-1-sebastien.boeuf@intel.com> References: <20200214114802.23638-1-sebastien.boeuf@intel.com> MIME-Version: 1.0 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org Whenever the server side of vsock is binding to the socket, but not listening yet, we expect the behavior from the client to be identical to what happens when the server is not even started. This new test runs the server side so that it binds to the socket without ever listening to it. The client side will try to connect and should receive an ECONNRESET error. This new test provides a way to validate the previously introduced patch for making sure the server side will always answer with a RST packet in case the client requested a new connection. Signed-off-by: Sebastien Boeuf --- tools/testing/vsock/vsock_test.c | 77 ++++++++++++++++++++++++++++++++ 1 file changed, 77 insertions(+) diff --git a/tools/testing/vsock/vsock_test.c b/tools/testing/vsock/vsock_test.c index 1d8b93f1af31..5a4fb80fa832 100644 --- a/tools/testing/vsock/vsock_test.c +++ b/tools/testing/vsock/vsock_test.c @@ -55,6 +55,78 @@ static void test_stream_connection_reset(const struct test_opts *opts) close(fd); } +static void test_stream_bind_only_client(const struct test_opts *opts) +{ + union { + struct sockaddr sa; + struct sockaddr_vm svm; + } addr = { + .svm = { + .svm_family = AF_VSOCK, + .svm_port = 1234, + .svm_cid = opts->peer_cid, + }, + }; + int ret; + int fd; + + /* Wait for the server to be ready */ + control_expectln("BIND"); + + fd = socket(AF_VSOCK, SOCK_STREAM, 0); + + timeout_begin(TIMEOUT); + do { + ret = connect(fd, &addr.sa, sizeof(addr.svm)); + timeout_check("connect"); + } while (ret < 0 && errno == EINTR); + timeout_end(); + + if (ret != -1) { + fprintf(stderr, "expected connect(2) failure, got %d\n", ret); + exit(EXIT_FAILURE); + } + if (errno != ECONNRESET) { + fprintf(stderr, "unexpected connect(2) errno %d\n", errno); + exit(EXIT_FAILURE); + } + + /* Notify the server that the client has finished */ + control_writeln("DONE"); + + close(fd); +} + +static void test_stream_bind_only_server(const struct test_opts *opts) +{ + union { + struct sockaddr sa; + struct sockaddr_vm svm; + } addr = { + .svm = { + .svm_family = AF_VSOCK, + .svm_port = 1234, + .svm_cid = VMADDR_CID_ANY, + }, + }; + int fd; + + fd = socket(AF_VSOCK, SOCK_STREAM, 0); + + if (bind(fd, &addr.sa, sizeof(addr.svm)) < 0) { + perror("bind"); + exit(EXIT_FAILURE); + } + + /* Notify the client that the server is ready */ + control_writeln("BIND"); + + /* Wait for the client to finish */ + control_expectln("DONE"); + + close(fd); +} + static void test_stream_client_close_client(const struct test_opts *opts) { int fd; @@ -212,6 +284,11 @@ static struct test_case test_cases[] = { .name = "SOCK_STREAM connection reset", .run_client = test_stream_connection_reset, }, + { + .name = "SOCK_STREAM bind only", + .run_client = test_stream_bind_only_client, + .run_server = test_stream_bind_only_server, + }, { .name = "SOCK_STREAM client close", .run_client = test_stream_client_close_client,