From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id BD31FA04DD for ; Thu, 22 Oct 2020 09:14:03 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id ADF645AB9; Thu, 22 Oct 2020 09:14:02 +0200 (CEST) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [63.128.21.124]) by dpdk.org (Postfix) with ESMTP id 7C6334C94 for ; Thu, 22 Oct 2020 09:14:00 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1603350839; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=T7LAGhdfIl9XSog3ssLVVPlHZmwx3EPmWZPgKWg8jpg=; b=jGVbf4C49wIk+WmpBgUlCOQtdxEPra7pkSCQ0KJZWmw3bqAM/g9iBS1u4P1Ik9ULsexPK/ HB3zVukS0njNDqKaofEuhUJ61zAU+gSF+SFl5402DU30l0B2BeYTWU0dK369H1eVEb/Ce+ CIwxovefWAu9ViwbQ06mqeDJHKHyShM= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-386-F4Y-qsUWNYWlJZ6vLAEsfA-1; Thu, 22 Oct 2020 03:13:50 -0400 X-MC-Unique: F4Y-qsUWNYWlJZ6vLAEsfA-1 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id CC86381CBE3; Thu, 22 Oct 2020 07:13:48 +0000 (UTC) Received: from [10.36.110.30] (unknown [10.36.110.30]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 9D3966EF67; Thu, 22 Oct 2020 07:13:46 +0000 (UTC) To: "Wang, Yinan" , Adrian Moreno , "dev@dpdk.org" , "Xia, Chenbo" Cc: "Fu, Patrick" , "stable@dpdk.org" , "Wang, Zhihong" , "Xu, Qian Q" References: <20201020152052.389446-1-amorenoz@redhat.com> <20201020152052.389446-4-amorenoz@redhat.com> <282c116e-0f8a-754e-f3fd-8eb256d68246@redhat.com> From: Maxime Coquelin Message-ID: <59a07add-84e4-b111-4fc0-f8a491fe86b0@redhat.com> Date: Thu, 22 Oct 2020 09:13:43 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.3.1 MIME-Version: 1.0 In-Reply-To: X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=maxime.coquelin@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit Subject: Re: [dpdk-stable] [PATCH v2 3/3] virtio-user: set status on virtio-user reconnect X-BeenThere: stable@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches for DPDK stable branches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: stable-bounces@dpdk.org Sender: "stable" Hi Yinan, On 10/22/20 6:01 AM, Wang, Yinan wrote: > Hi Maxime/ Adrian, > > Thanks for the patch. we can launch vhost-user with client mode with this fix patch. > But still fail to get throughput with basic vhost/virtio-user server mode loopback test. This is another problem which introduced by 57912824615fd7787a48a7b18e40661466. > Bugzilla: https://bugs.dpdk.org/show_bug.cgi?id=541 Thanks for reporting the issue, I will look at it this morning. BTW, this virtio-user server mode is broken by design, we should really fix it. For example, it assumes features to be supported by the backend before the negotiation took place: https://git.dpdk.org/dpdk/tree/drivers/net/virtio/virtio_user/virtio_user_dev.c#n513 As part of my rework, I suggest we implement the same behaviour as QEMU does, which will be reliable and consistent with QEMU. It means that if server mode is enabled in device command-line, the driver waits until the socket is ready. Chenbo, Adrian, what do you think? Thanks, Maxime > BR, > Yinan >> -----Original Message----- >> From: Maxime Coquelin >> Sent: 2020年10月21日 0:43 >> To: Adrian Moreno ; dev@dpdk.org >> Cc: Wang, Yinan ; Fu, Patrick ; >> stable@dpdk.org; Xia, Chenbo ; Wang, Zhihong >> >> Subject: Re: [PATCH v2 3/3] virtio-user: set status on virtio-user reconnect >> >> >> >> On 10/20/20 5:20 PM, Adrian Moreno wrote: >>> Newer vhost-user backends will rely on SET_STATUS to start the device >>> so this required to support them. >>> >>> Fixes: 57912824615f ("net/virtio-user: support vhost status setting") >>> Cc: maxime.coquelin@redhat.com >>> Cc: stable@dpdk.org >>> >>> Signed-off-by: Adrian Moreno >>> --- >>> drivers/net/virtio/virtio_user_ethdev.c | 14 ++++++++++++-- >>> 1 file changed, 12 insertions(+), 2 deletions(-) >>> >>> diff --git a/drivers/net/virtio/virtio_user_ethdev.c >> b/drivers/net/virtio/virtio_user_ethdev.c >>> index e870fb2ff..d8bea4537 100644 >>> --- a/drivers/net/virtio/virtio_user_ethdev.c >>> +++ b/drivers/net/virtio/virtio_user_ethdev.c >>> @@ -78,6 +78,13 @@ virtio_user_server_reconnect(struct virtio_user_dev >> *dev) >>> return -1; >>> >>> dev->vhostfd = connectfd; >>> + >>> + vtpci_reset(hw); >>> + >>> + vtpci_set_status(hw, VIRTIO_CONFIG_STATUS_ACK); >>> + >>> + vtpci_set_status(hw, VIRTIO_CONFIG_STATUS_DRIVER); >>> + >>> if (dev->ops->send_request(dev, VHOST_USER_GET_FEATURES, >>> &dev->device_features) < 0) { >>> PMD_INIT_LOG(ERR, "get_features failed: %s", >>> @@ -111,6 +118,8 @@ virtio_user_server_reconnect(struct virtio_user_dev >> *dev) >>> >>> dev->features &= dev->device_features; >>> >>> + vtpci_set_status(hw, VIRTIO_CONFIG_STATUS_FEATURES_OK); >>> + >>> /* For packed ring, resetting queues is required in reconnection. */ >>> if (vtpci_packed_queue(hw) && >>> (vtpci_get_status(hw) & VIRTIO_CONFIG_STATUS_DRIVER_OK)) { >>> @@ -119,8 +128,9 @@ virtio_user_server_reconnect(struct virtio_user_dev >> *dev) >>> virtio_user_reset_queues_packed(eth_dev); >>> } >>> >>> - ret = virtio_user_start_device(dev); >>> - if (ret < 0) >>> + /* Start the device */ >>> + vtpci_set_status(hw, VIRTIO_CONFIG_STATUS_DRIVER_OK); >>> + if (!dev->started) >>> return -1; >>> >>> if (dev->queue_pairs > 1) { >>> >> >> Reviewed-by: Maxime Coquelin >> >> Thanks, >> Maxime >