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 48E7EA04DC; Tue, 20 Oct 2020 09:21:05 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 76868A953; Tue, 20 Oct 2020 09:21:03 +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 CD0152C16 for ; Tue, 20 Oct 2020 09:21:00 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1603178459; 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=K9Luk2InhdJ/f8jiFv8i59mgYBPiXhZA13dBbWNaPWk=; b=RhiLtyK1vDA+Wa8crSMSEyveR7jCShSN3xlT171va+hmpGWwRA9HVhCq23g9kqlqaYC0HN dr4lKplOI97sqOGNt40hvmGiEszqiPvVx/8BVLYlw5jEAbm/GjthpCNRKuKDb+FtoaL40R MemB+7BnS9vT3PVVmG3szsltlBgccHE= Received: from mail-wr1-f70.google.com (mail-wr1-f70.google.com [209.85.221.70]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-550-72pcz8OfNMeU2qE92jZ90g-1; Tue, 20 Oct 2020 03:20:56 -0400 X-MC-Unique: 72pcz8OfNMeU2qE92jZ90g-1 Received: by mail-wr1-f70.google.com with SMTP id u15so438115wrn.4 for ; Tue, 20 Oct 2020 00:20:56 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=xkQdi3ZY25auHh2qnkCQtXGgIB5GFarzaitzTYGJlLg=; b=GkhBgnq8zaUZd2Cd2jhgY4Co3BxBUust3xy3UBOM/gvukDswBq3n1wsnBwLpfyziMi KJXBhwnmmyPXVCbiAKDAPduWMJtXeaCqOZBy7NwKTF+9sqAI+g+bTvGhBfBgiOSfH+LA lzBPzwGmNlp+oa8zlkYiDK1m0IBnpn+HSnoCE74dYbbgCbAzTM+hgIJ8xAbwCGHXuEEG InDe9HpUyiqya5nVPa+rw+fiGPg9N2MVBiW8oJIYZwxeKKoYdcKKhVb+Y98OE31ZwQJN bBFoMEB78X9jbLLBWPYXz+opnCY57eihrhOdz9jOZ4YjKZgbOMT572VgOd1Z29y+xzlr INJQ== X-Gm-Message-State: AOAM531uT0Ua+jbi/lPXMtMJoT0ZxcwIA0Cf9pDmEfnFJZbvr4z4SWS2 NuUysEtSZrCTbGntl6tWLXm+OfEn2u6jrpTHeugqJ67qryJhPnv1gzb4SYLksQjUn490ReilkZ2 J2mE= X-Received: by 2002:a7b:c345:: with SMTP id l5mr1336235wmj.123.1603178454815; Tue, 20 Oct 2020 00:20:54 -0700 (PDT) X-Google-Smtp-Source: ABdhPJwg70zrRXHW0Kn7re3EFWb3sko0o/SRjgWFx6PfGLSBu28Rxz+F3bT1vi8WTOZ7Smr1AIIhYQ== X-Received: by 2002:a7b:c345:: with SMTP id l5mr1336220wmj.123.1603178454584; Tue, 20 Oct 2020 00:20:54 -0700 (PDT) Received: from amorenoz.users.ipa.redhat.com ([94.73.56.18]) by smtp.gmail.com with ESMTPSA id e11sm1719560wrj.75.2020.10.20.00.20.53 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 20 Oct 2020 00:20:53 -0700 (PDT) To: Maxime Coquelin , "Wang, Yinan" , "dev@dpdk.org" , "Xia, Chenbo" , "Fu, Patrick" References: <20200929161404.124580-1-maxime.coquelin@redhat.com> <20200929161404.124580-4-maxime.coquelin@redhat.com> From: Adrian Moreno Message-ID: <3b386ca6-e045-29a9-6522-c3bba2a77bc4@redhat.com> Date: Tue, 20 Oct 2020 09:20:52 +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: Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=amorenoz@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: 8bit 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, Still testing, but submitting for early review: https://patches.dpdk.org/patch/81431/ Thanks, On 10/20/20 9:11 AM, Maxime Coquelin wrote: > 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 >> > -- Adrián Moreno