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 B33C9A04DC; Tue, 20 Oct 2020 09:11:49 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id E2D86BAE6; Tue, 20 Oct 2020 09:11:46 +0200 (CEST) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [216.205.24.124]) by dpdk.org (Postfix) with ESMTP id 04309BADE for ; Tue, 20 Oct 2020 09:11:43 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1603177902; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=uZ46jH3rzVoX5/Grmq5Wer084OjNJtuy4AzwiWz12fA=; b=ZMfu6K6EGlZYd8C6+5n5FjxP4Crl4gOdW1USff1YpKFdiGIM2B3OllCctpC9fuCoE/6bmU 83r63mi/C5eGTLodjrpdFO+UzRW/gKw+9KmpXttpwpQM1Y1rco2ylm7tIf8YZSvW/rb7y8 me+KaNr+cjTZgvP102NTyZ5FY51qitA= 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-555-Bfhmx3rsPCmTKDfGLO60rQ-1; Tue, 20 Oct 2020 03:11:36 -0400 X-MC-Unique: Bfhmx3rsPCmTKDfGLO60rQ-1 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id E17EF1006C83; Tue, 20 Oct 2020 07:11:34 +0000 (UTC) Received: from [10.36.110.40] (unknown [10.36.110.40]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 0E6595C1C2; Tue, 20 Oct 2020 07:11:32 +0000 (UTC) To: "Wang, Yinan" , "dev@dpdk.org" , "Xia, Chenbo" , "Fu, Patrick" , "amorenoz@redhat.com" References: <20200929161404.124580-1-maxime.coquelin@redhat.com> <20200929161404.124580-4-maxime.coquelin@redhat.com> From: Maxime Coquelin Message-ID: Date: Tue, 20 Oct 2020 09:11:31 +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.16 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=windows-1252 Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH v3 3/8] net/virtio: move backend type selection to ethdev X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Hi Yinan, On 10/20/20 3:52 AM, Wang, Yinan wrote: > Hi Adrian/Maxime, > > Could you help to take a look at this issue? Most of automated regression cases will be blocked due to this issue in 20.11 RC1 test. We are working on it. Adrian managed to reproduce it, and fixed one part of the issue. We''l continue the debugging today. BTW, you are mentionning automated regression tests, any chances these test land in the Intel functionnal CI at some point? I would be great to catch these issues before the series are merged. Thanks! Maxime > Thanks in advance! > > BR, > Yinan > >> -----Original Message----- >> From: Wang, Yinan >> Sent: 2020?10?16? 13:42 >> To: Maxime Coquelin ; dev@dpdk.org; Xia, >> Chenbo ; Fu, Patrick ; >> amorenoz@redhat.com >> Subject: RE: [dpdk-dev] [PATCH v3 3/8] net/virtio: move backend type selection >> to ethdev >> >> Hi Adrian, >> >> This patch introduce a regression issue that vhost-user can't launch with server >> mode, details in https://bugs.dpdk.org/show_bug.cgi?id=559. >> This issue blocked amount of regression cases, could you help to take a look? >> Thanks in advance! >> >> >> BR, >> Yinan >> >> >>> -----Original Message----- >>> From: dev On Behalf Of Maxime Coquelin >>> Sent: 2020?9?30? 0:14 >>> To: dev@dpdk.org; Xia, Chenbo ; Fu, Patrick >>> ; amorenoz@redhat.com >>> Cc: Maxime Coquelin >>> Subject: [dpdk-dev] [PATCH v3 3/8] net/virtio: move backend type selection to >>> ethdev >>> >>> From: Adrian Moreno >>> >>> This is a preparation patch with no functional change. >>> >>> Use an enum instead of a boolean for the backend type. >>> Move the detection logic to the ethdev layer (where it is needed for the >>> first time). >>> The virtio_user_dev stores the backend type in the virtio_user_dev >>> struct so the type is only determined once >>> >>> Signed-off-by: Maxime Coquelin >>> Signed-off-by: Adrian Moreno >>> --- >>> .../net/virtio/virtio_user/virtio_user_dev.c | 37 ++++++++----------- >>> .../net/virtio/virtio_user/virtio_user_dev.h | 11 +++++- >>> drivers/net/virtio/virtio_user_ethdev.c | 28 +++++++++++++- >>> 3 files changed, 50 insertions(+), 26 deletions(-) >>> >>> diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c >>> b/drivers/net/virtio/virtio_user/virtio_user_dev.c >>> index 2a0c861085..b79a9f84aa 100644 >>> --- a/drivers/net/virtio/virtio_user/virtio_user_dev.c >>> +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c >>> @@ -111,17 +111,6 @@ virtio_user_queue_setup(struct virtio_user_dev *dev, >>> return 0; >>> } >>> >>> -int >>> -is_vhost_user_by_type(const char *path) >>> -{ >>> - struct stat sb; >>> - >>> - if (stat(path, &sb) == -1) >>> - return 0; >>> - >>> - return S_ISSOCK(sb.st_mode); >>> -} >>> - >>> int >>> virtio_user_start_device(struct virtio_user_dev *dev) >>> { >>> @@ -144,7 +133,8 @@ virtio_user_start_device(struct virtio_user_dev *dev) >>> rte_mcfg_mem_read_lock(); >>> pthread_mutex_lock(&dev->mutex); >>> >>> - if (is_vhost_user_by_type(dev->path) && dev->vhostfd < 0) >>> + if (dev->backend_type == VIRTIO_USER_BACKEND_VHOST_USER && >>> + dev->vhostfd < 0) >>> goto error; >>> >>> /* Step 0: tell vhost to create queues */ >>> @@ -360,16 +350,16 @@ virtio_user_dev_setup(struct virtio_user_dev *dev) >>> dev->tapfds = NULL; >>> >>> if (dev->is_server) { >>> - if (access(dev->path, F_OK) == 0 && >>> - !is_vhost_user_by_type(dev->path)) { >>> - PMD_DRV_LOG(ERR, "Server mode doesn't support >>> vhost-kernel!"); >>> + if (dev->backend_type != >>> VIRTIO_USER_BACKEND_VHOST_USER) { >>> + PMD_DRV_LOG(ERR, "Server mode only supports >>> vhost-user!"); >>> return -1; >>> } >>> dev->ops = &virtio_ops_user; >>> } else { >>> - if (is_vhost_user_by_type(dev->path)) { >>> + if (dev->backend_type == >>> VIRTIO_USER_BACKEND_VHOST_USER) { >>> dev->ops = &virtio_ops_user; >>> - } else { >>> + } else if (dev->backend_type == >>> + >>> VIRTIO_USER_BACKEND_VHOST_KERNEL) { >>> dev->ops = &virtio_ops_kernel; >>> >>> dev->vhostfds = malloc(dev->max_queue_pairs * >>> @@ -430,7 +420,8 @@ virtio_user_dev_setup(struct virtio_user_dev *dev) >>> int >>> virtio_user_dev_init(struct virtio_user_dev *dev, char *path, int queues, >>> int cq, int queue_size, const char *mac, char **ifname, >>> - int server, int mrg_rxbuf, int in_order, int packed_vq) >>> + int server, int mrg_rxbuf, int in_order, int packed_vq, >>> + enum virtio_user_backend_type backend_type) >>> { >>> uint64_t protocol_features = 0; >>> >>> @@ -445,6 +436,8 @@ virtio_user_dev_init(struct virtio_user_dev *dev, char >>> *path, int queues, >>> dev->frontend_features = 0; >>> dev->unsupported_features = ~VIRTIO_USER_SUPPORTED_FEATURES; >>> dev->protocol_features = >>> VIRTIO_USER_SUPPORTED_PROTOCOL_FEATURES; >>> + dev->backend_type = backend_type; >>> + >>> parse_mac(dev, mac); >>> >>> if (*ifname) { >>> @@ -457,7 +450,7 @@ virtio_user_dev_init(struct virtio_user_dev *dev, char >>> *path, int queues, >>> return -1; >>> } >>> >>> - if (!is_vhost_user_by_type(dev->path)) >>> + if (dev->backend_type != VIRTIO_USER_BACKEND_VHOST_USER) >>> dev->unsupported_features |= >>> (1ULL << VHOST_USER_F_PROTOCOL_FEATURES); >>> >>> @@ -539,7 +532,7 @@ virtio_user_dev_init(struct virtio_user_dev *dev, char >>> *path, int queues, >>> } >>> >>> /* The backend will not report this feature, we add it explicitly */ >>> - if (is_vhost_user_by_type(dev->path)) >>> + if (dev->backend_type == VIRTIO_USER_BACKEND_VHOST_USER) >>> dev->frontend_features |= (1ull << VIRTIO_NET_F_STATUS); >>> >>> /* >>> @@ -792,7 +785,7 @@ virtio_user_send_status_update(struct virtio_user_dev >>> *dev, uint8_t status) >>> uint64_t arg = status; >>> >>> /* Vhost-user only for now */ >>> - if (!is_vhost_user_by_type(dev->path)) >>> + if (dev->backend_type != VIRTIO_USER_BACKEND_VHOST_USER) >>> return 0; >>> >>> if (!(dev->protocol_features & (1ULL << >>> VHOST_USER_PROTOCOL_F_STATUS))) >>> @@ -815,7 +808,7 @@ virtio_user_update_status(struct virtio_user_dev *dev) >>> int err; >>> >>> /* Vhost-user only for now */ >>> - if (!is_vhost_user_by_type(dev->path)) >>> + if (dev->backend_type != VIRTIO_USER_BACKEND_VHOST_USER) >>> return 0; >>> >>> if (!(dev->protocol_features & (1UL << >>> VHOST_USER_PROTOCOL_F_STATUS))) >>> diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.h >>> b/drivers/net/virtio/virtio_user/virtio_user_dev.h >>> index 9377d5ba66..575bf430c0 100644 >>> --- a/drivers/net/virtio/virtio_user/virtio_user_dev.h >>> +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.h >>> @@ -10,6 +10,12 @@ >>> #include "../virtio_pci.h" >>> #include "../virtio_ring.h" >>> >>> +enum virtio_user_backend_type { >>> + VIRTIO_USER_BACKEND_UNKNOWN, >>> + VIRTIO_USER_BACKEND_VHOST_USER, >>> + VIRTIO_USER_BACKEND_VHOST_KERNEL, >>> +}; >>> + >>> struct virtio_user_queue { >>> uint16_t used_idx; >>> bool avail_wrap_counter; >>> @@ -17,6 +23,7 @@ struct virtio_user_queue { >>> }; >>> >>> struct virtio_user_dev { >>> + enum virtio_user_backend_type backend_type; >>> /* for vhost_user backend */ >>> int vhostfd; >>> int listenfd; /* listening fd */ >>> @@ -60,13 +67,13 @@ struct virtio_user_dev { >>> bool started; >>> }; >>> >>> -int is_vhost_user_by_type(const char *path); >>> int virtio_user_start_device(struct virtio_user_dev *dev); >>> int virtio_user_stop_device(struct virtio_user_dev *dev); >>> int virtio_user_dev_init(struct virtio_user_dev *dev, char *path, int queues, >>> int cq, int queue_size, const char *mac, char **ifname, >>> int server, int mrg_rxbuf, int in_order, >>> - int packed_vq); >>> + int packed_vq, >>> + enum virtio_user_backend_type backend_type); >>> void virtio_user_dev_uninit(struct virtio_user_dev *dev); >>> void virtio_user_handle_cq(struct virtio_user_dev *dev, uint16_t queue_idx); >>> void virtio_user_handle_cq_packed(struct virtio_user_dev *dev, >>> diff --git a/drivers/net/virtio/virtio_user_ethdev.c >>> b/drivers/net/virtio/virtio_user_ethdev.c >>> index 60d17af888..38b49bad5f 100644 >>> --- a/drivers/net/virtio/virtio_user_ethdev.c >>> +++ b/drivers/net/virtio/virtio_user_ethdev.c >>> @@ -6,6 +6,7 @@ >>> #include >>> #include >>> #include >>> +#include >>> #include >>> >>> #include >>> @@ -518,6 +519,19 @@ get_integer_arg(const char *key __rte_unused, >>> return -errno; >>> } >>> >>> +static enum virtio_user_backend_type >>> +virtio_user_backend_type(const char *path) >>> +{ >>> + struct stat sb; >>> + >>> + if (stat(path, &sb) == -1) >>> + return VIRTIO_USER_BACKEND_UNKNOWN; >>> + >>> + return S_ISSOCK(sb.st_mode) ? >>> + VIRTIO_USER_BACKEND_VHOST_USER : >>> + VIRTIO_USER_BACKEND_VHOST_KERNEL; >>> +} >>> + >>> static struct rte_eth_dev * >>> virtio_user_eth_dev_alloc(struct rte_vdev_device *vdev) >>> { >>> @@ -579,6 +593,7 @@ virtio_user_pmd_probe(struct rte_vdev_device *dev) >>> struct rte_kvargs *kvlist = NULL; >>> struct rte_eth_dev *eth_dev; >>> struct virtio_hw *hw; >>> + enum virtio_user_backend_type backend_type = >>> VIRTIO_USER_BACKEND_UNKNOWN; >>> uint64_t queues = VIRTIO_USER_DEF_Q_NUM; >>> uint64_t cq = VIRTIO_USER_DEF_CQ_EN; >>> uint64_t queue_size = VIRTIO_USER_DEF_Q_SZ; >>> @@ -631,8 +646,17 @@ virtio_user_pmd_probe(struct rte_vdev_device *dev) >>> goto end; >>> } >>> >>> + backend_type = virtio_user_backend_type(path); >>> + if (backend_type == VIRTIO_USER_BACKEND_UNKNOWN) { >>> + PMD_INIT_LOG(ERR, >>> + "unable to determine backend type for path %s", >>> + path); >>> + goto end; >>> + } >>> + >>> + >>> if (rte_kvargs_count(kvlist, VIRTIO_USER_ARG_INTERFACE_NAME) == 1) >>> { >>> - if (is_vhost_user_by_type(path)) { >>> + if (backend_type != VIRTIO_USER_BACKEND_VHOST_KERNEL) { >>> PMD_INIT_LOG(ERR, >>> "arg %s applies only to vhost-kernel backend", >>> VIRTIO_USER_ARG_INTERFACE_NAME); >>> @@ -751,7 +775,7 @@ virtio_user_pmd_probe(struct rte_vdev_device *dev) >>> hw = eth_dev->data->dev_private; >>> if (virtio_user_dev_init(hw->virtio_user_dev, path, queues, cq, >>> queue_size, mac_addr, &ifname, server_mode, >>> - mrg_rxbuf, in_order, packed_vq) < 0) { >>> + mrg_rxbuf, in_order, packed_vq, backend_type) < 0) { >>> PMD_INIT_LOG(ERR, "virtio_user_dev_init fails"); >>> virtio_user_eth_dev_free(eth_dev); >>> goto end; >>> -- >>> 2.26.2 >