From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 00BBFA0552; Thu, 9 Jun 2022 09:06:33 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id E44D240689; Thu, 9 Jun 2022 09:06:33 +0200 (CEST) Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by mails.dpdk.org (Postfix) with ESMTP id 016C740220 for ; Thu, 9 Jun 2022 09:06:30 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1654758391; x=1686294391; h=from:to:cc:subject:date:message-id:references: in-reply-to:content-transfer-encoding:mime-version; bh=iQ01pKDUps1ttRS9Q6UeqvbiUz3NZsKeptFJcvYbbQE=; b=FRw2IiHgofwYl45JD868hCc6NY/RCgfjX7XFMYxcBXYYBHDi7zKwOVUo /0m9vbFP51QqmQXxs3nPmRKdf9aDe27G+SRPNacMSx9MEz3f8HgTXZ6kC V4CGp2PwtQcU1ShKoDmmfsTN0+NoZjpEra1m9TxWEGHiPcV6tJVHBZZfH Vj4kI3pxqDNMQAGpP1Nb4cv85h5oW+KOsPogstY5lF/IIc0kf07bOCSje cOvFumVQuXcxPqv27y+o14wIBe16Kci1G9C6PYpMYsKhqt/tYLC3L8Zot k5bt/InJDbBI8bpfdE591A3JDQUEr0ec2JpuaVQ/fU5FHAadNoCcy7s9q g==; X-IronPort-AV: E=McAfee;i="6400,9594,10372"; a="302544172" X-IronPort-AV: E=Sophos;i="5.91,287,1647327600"; d="scan'208";a="302544172" Received: from orsmga006.jf.intel.com ([10.7.209.51]) by fmsmga101.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 09 Jun 2022 00:06:29 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.91,287,1647327600"; d="scan'208";a="555770702" Received: from fmsmsx605.amr.corp.intel.com ([10.18.126.85]) by orsmga006.jf.intel.com with ESMTP; 09 Jun 2022 00:06:27 -0700 Received: from fmsmsx611.amr.corp.intel.com (10.18.126.91) by fmsmsx605.amr.corp.intel.com (10.18.126.85) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2308.27; Thu, 9 Jun 2022 00:06:26 -0700 Received: from fmsmsx612.amr.corp.intel.com (10.18.126.92) by fmsmsx611.amr.corp.intel.com (10.18.126.91) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2308.27; Thu, 9 Jun 2022 00:06:26 -0700 Received: from fmsmsx612.amr.corp.intel.com ([10.18.126.92]) by fmsmsx612.amr.corp.intel.com ([10.18.126.92]) with mapi id 15.01.2308.027; Thu, 9 Jun 2022 00:06:26 -0700 From: "Hu, Jiayu" To: "Wang, YuanX" , "maxime.coquelin@redhat.com" , "Xia, Chenbo" , "dev@dpdk.org" CC: "Ding, Xuan" , "Pai G, Sunil" Subject: RE: [PATCH v4 1/2] vhost: support clear in-flight packets for async dequeue Thread-Topic: [PATCH v4 1/2] vhost: support clear in-flight packets for async dequeue Thread-Index: AQHYeYvU5p2kVmUG5Eyod5oC/XnXFa1Gpv/g Date: Thu, 9 Jun 2022 07:06:26 +0000 Message-ID: <799af9123f6d40c3ad3ddc5c889278be@intel.com> References: <20220413182742.860659-1-yuanx.wang@intel.com> <20220606174507.1272360-1-yuanx.wang@intel.com> <20220606174507.1272360-2-yuanx.wang@intel.com> In-Reply-To: <20220606174507.1272360-2-yuanx.wang@intel.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: dlp-reaction: no-action dlp-version: 11.6.500.17 dlp-product: dlpe-windows x-originating-ip: [10.239.127.36] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Hi Yuan, > -----Original Message----- > From: Wang, YuanX > Sent: Tuesday, June 7, 2022 1:45 AM > To: maxime.coquelin@redhat.com; Xia, Chenbo ; > dev@dpdk.org > Cc: Hu, Jiayu ; Ding, Xuan ; Pai > G, Sunil ; Wang, YuanX > Subject: [PATCH v4 1/2] vhost: support clear in-flight packets for async > dequeue >=20 > rte_vhost_clear_queue_thread_unsafe() supports to clear in-flight packets > for async enqueue only. But after supporting async dequeue, this API shou= ld > support async dequeue too. >=20 > This patch also adds the thread-safe version of this API, the difference > between the two API is that thread safety uses lock. >=20 > These APIs maybe used to clean up packets in the async channel to prevent > packet loss when the device state changes or when the device is destroyed= . >=20 > Signed-off-by: Yuan Wang > --- > doc/guides/prog_guide/vhost_lib.rst | 8 ++- > doc/guides/rel_notes/release_22_07.rst | 5 ++ > lib/vhost/rte_vhost_async.h | 25 ++++++++ > lib/vhost/version.map | 1 + > lib/vhost/virtio_net.c | 82 +++++++++++++++++++++++++- > 5 files changed, 118 insertions(+), 3 deletions(-) >=20 > diff --git a/doc/guides/prog_guide/vhost_lib.rst > b/doc/guides/prog_guide/vhost_lib.rst > index cd3f6caa9a..b9545770d0 100644 > --- a/doc/guides/prog_guide/vhost_lib.rst > +++ b/doc/guides/prog_guide/vhost_lib.rst > @@ -288,7 +288,13 @@ The following is an overview of some key Vhost API > functions: >=20 > * ``rte_vhost_clear_queue_thread_unsafe(vid, queue_id, **pkts, count, > dma_id, vchan_id)`` >=20 > - Clear inflight packets which are submitted to DMA engine in vhost asyn= c > data > + Clear in-flight packets which are submitted to async channel in vhost > + async data path without performing any locking. Completed packets are Better specify "without performing locking on virtqueue". > + returned to applications through ``pkts``. > + > +* ``rte_vhost_clear_queue(vid, queue_id, **pkts, count, dma_id, > +vchan_id)`` > + > + Clear in-flight packets which are submitted to async channel in vhost > + async data > path. Completed packets are returned to applications through ``pkts``. >=20 > * ``rte_vhost_vring_stats_get_names(int vid, uint16_t queue_id, struct > rte_vhost_stat_name *names, unsigned int size)`` diff --git > a/doc/guides/rel_notes/release_22_07.rst > b/doc/guides/rel_notes/release_22_07.rst > index c81383f4a3..2ca06b543c 100644 > --- a/doc/guides/rel_notes/release_22_07.rst > +++ b/doc/guides/rel_notes/release_22_07.rst > @@ -147,6 +147,11 @@ New Features > Added vhost async dequeue API which can leverage DMA devices to > accelerate receiving pkts from guest. >=20 > +* **Added thread-safe version of inflight packet clear API in vhost > +library.** > + > + Added an API which can clear the inflight packets submitted to the > + async channel in a thread-safe manner in the vhost async data path. > + > Removed Items > ------------- >=20 > diff --git a/lib/vhost/rte_vhost_async.h b/lib/vhost/rte_vhost_async.h in= dex > a1e7f674ed..1db2a10124 100644 > --- a/lib/vhost/rte_vhost_async.h > +++ b/lib/vhost/rte_vhost_async.h > @@ -183,6 +183,31 @@ uint16_t rte_vhost_clear_queue_thread_unsafe(int > vid, uint16_t queue_id, > struct rte_mbuf **pkts, uint16_t count, int16_t dma_id, > uint16_t vchan_id); >=20 > +/** > + * This function checks async completion status and clear packets for > + * a specific vhost device queue. Packets which are inflight will be > + * returned in an array. > + * > + * @param vid > + * ID of vhost device to clear data > + * @param queue_id > + * Queue id to clear data > + * @param pkts > + * Blank array to get return packet pointer > + * @param count > + * Size of the packet array > + * @param dma_id > + * The identifier of the DMA device > + * @param vchan_id > + * The identifier of virtual DMA channel > + * @return > + * Number of packets returned > + */ > +__rte_experimental > +uint16_t rte_vhost_clear_queue(int vid, uint16_t queue_id, > + struct rte_mbuf **pkts, uint16_t count, int16_t dma_id, > + uint16_t vchan_id); > + > /** > * The DMA vChannels used in asynchronous data path must be configured > * first. So this function needs to be called before enabling DMA diff -= -git > a/lib/vhost/version.map b/lib/vhost/version.map index > 4880b9a422..9329f88e79 100644 > --- a/lib/vhost/version.map > +++ b/lib/vhost/version.map > @@ -95,6 +95,7 @@ EXPERIMENTAL { > rte_vhost_vring_stats_reset; > rte_vhost_async_try_dequeue_burst; > rte_vhost_driver_get_vdpa_dev_type; > + rte_vhost_clear_queue; > }; >=20 > INTERNAL { > diff --git a/lib/vhost/virtio_net.c b/lib/vhost/virtio_net.c index > 68a26eb17d..a90ae3cb96 100644 > --- a/lib/vhost/virtio_net.c > +++ b/lib/vhost/virtio_net.c > @@ -26,6 +26,11 @@ >=20 > #define MAX_BATCH_LEN 256 >=20 > +static __rte_always_inline uint16_t > +async_poll_dequeue_completed_split(struct virtio_net *dev, struct > vhost_virtqueue *vq, > + struct rte_mbuf **pkts, uint16_t count, int16_t dma_id, > + uint16_t vchan_id, bool legacy_ol_flags); > + > /* DMA device copy operation tracking array. */ struct async_dma_info > dma_copy_track[RTE_DMADEV_DEFAULT_MAX]; >=20 > @@ -2155,7 +2160,7 @@ rte_vhost_clear_queue_thread_unsafe(int vid, > uint16_t queue_id, > return 0; >=20 > VHOST_LOG_DATA(DEBUG, "(%s) %s\n", dev->ifname, __func__); > - if (unlikely(!is_valid_virt_queue_idx(queue_id, 0, dev->nr_vring))) { > + if (unlikely(queue_id >=3D dev->nr_vring)) { > VHOST_LOG_DATA(ERR, "(%s) %s: invalid virtqueue > idx %d.\n", > dev->ifname, __func__, queue_id); > return 0; > @@ -2182,7 +2187,18 @@ rte_vhost_clear_queue_thread_unsafe(int vid, > uint16_t queue_id, > return 0; > } >=20 > - n_pkts_cpl =3D vhost_poll_enqueue_completed(dev, queue_id, pkts, > count, dma_id, vchan_id); > + if (queue_id % 2 =3D=3D 0) Replace "%" with "&" should help the performance a bit. > + n_pkts_cpl =3D vhost_poll_enqueue_completed(dev, queue_id, > + pkts, count, dma_id, vchan_id); > + else { > + if (unlikely(vq_is_packed(dev))) > + VHOST_LOG_DATA(ERR, > + "(%d) %s: async dequeue does not > support packed ring.\n", > + dev->vid, __func__); > + else > + n_pkts_cpl =3D > async_poll_dequeue_completed_split(dev, vq, pkts, count, > + dma_id, vchan_id, dev->flags & > VIRTIO_DEV_LEGACY_OL_FLAGS); > + } >=20 > vhost_queue_stats_update(dev, vq, pkts, n_pkts_cpl); > vq->stats.inflight_completed +=3D n_pkts_cpl; @@ -2190,6 +2206,68 > @@ rte_vhost_clear_queue_thread_unsafe(int vid, uint16_t queue_id, > return n_pkts_cpl; > } >=20 > +uint16_t > +rte_vhost_clear_queue(int vid, uint16_t queue_id, struct rte_mbuf **pkts= , > + uint16_t count, int16_t dma_id, uint16_t vchan_id) { > + struct virtio_net *dev =3D get_device(vid); > + struct vhost_virtqueue *vq; > + uint16_t n_pkts_cpl =3D 0; > + > + if (!dev) > + return 0; > + > + VHOST_LOG_DATA(DEBUG, "(%s) %s\n", dev->ifname, __func__); > + if (unlikely(queue_id >=3D dev->nr_vring)) { > + VHOST_LOG_DATA(ERR, "(%s) %s: invalid virtqueue > idx %d.\n", > + dev->ifname, __func__, queue_id); > + return 0; > + } > + > + vq =3D dev->virtqueue[queue_id]; > + > + if (!rte_spinlock_trylock(&vq->access_lock)) { > + VHOST_LOG_DATA(ERR, > + "(%d) %s: failed to clear async queue id %d, > virtqueue busy.\n", > + dev->vid, __func__, queue_id); > + return 0; > + } Failing to acquire the lock shouldn't be treated as an error. > + > + if (unlikely(!vq->async)) { > + VHOST_LOG_DATA(ERR, "(%s) %s: async not registered for > queue id %d.\n", > + dev->ifname, __func__, queue_id); > + goto out_access_unlock; > + } > + > + if (unlikely(!dma_copy_track[dma_id].vchans || > + > !dma_copy_track[dma_id].vchans[vchan_id].pkts_cmpl_flag_addr)) { > + VHOST_LOG_DATA(ERR, "(%s) %s: invalid channel %d:%u.\n", > dev->ifname, __func__, > + dma_id, vchan_id); > + goto out_access_unlock; > + } Also need to check if dma_id is valid. > + > + if (queue_id % 2 =3D=3D 0) Ditto. Thanks, Jiayu