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 8A6BFA0C47; Tue, 26 Oct 2021 09:27:39 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 26FCE40A4B; Tue, 26 Oct 2021 09:27:39 +0200 (CEST) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by mails.dpdk.org (Postfix) with ESMTP id 2C5CE4003E for ; Tue, 26 Oct 2021 09:27:38 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1635233257; 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=1K241lltq+O43kN9Xzz2pzm8l/0JbBhFbHdQ98eiW4U=; b=VZvwl3pYlIK1spnw/178cdiiDpil0D86HLnG0yb+0wMg2tnirmdwWzrEYrTZdbjK4Lz8Dx QRqNf/uXHegG0oUmIE0zwzbbGSkslme9ZD1mpTEbGvLXiNp3WdKDEntP5hfOq3htdYTXss YOHILjmkpk6pgcgAaTRB+rHa0c4TWSs= 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-68-2ut3ih2mMIqfC_aVPcAjtQ-1; Tue, 26 Oct 2021 03:27:34 -0400 X-MC-Unique: 2ut3ih2mMIqfC_aVPcAjtQ-1 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id CA40A10A8E00; Tue, 26 Oct 2021 07:27:32 +0000 (UTC) Received: from [10.39.208.37] (unknown [10.39.208.37]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 27D805DD68; Tue, 26 Oct 2021 07:27:16 +0000 (UTC) Message-ID: Date: Tue, 26 Oct 2021 09:27:15 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.2.0 To: "Hu, Jiayu" , "dev@dpdk.org" , "Xia, Chenbo" , "Wang, YuanX" , "Ma, WenwuX" , "Richardson, Bruce" , "Mcnamara, John" , "david.marchand@redhat.com" References: <20211018130229.308694-1-maxime.coquelin@redhat.com> <20211018130229.308694-9-maxime.coquelin@redhat.com> <9239770ed6c74f67975524bd081af39a@intel.com> <285afc18-6ef6-543e-d4d4-5968132c0cd5@redhat.com> <2ad367cda39b442c9c5b01d80c7c050a@intel.com> From: Maxime Coquelin In-Reply-To: <2ad367cda39b442c9c5b01d80c7c050a@intel.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 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-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH v1 08/14] vhost: improve IO vector logic 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 Sender: "dev" On 10/26/21 09:07, Hu, Jiayu wrote: > Hi Maxime, > >> -----Original Message----- >> From: Maxime Coquelin >> Sent: Monday, October 25, 2021 6:03 PM >> To: Hu, Jiayu ; dev@dpdk.org; Xia, Chenbo >> ; Wang, YuanX ; Ma, >> WenwuX ; Richardson, Bruce >> ; Mcnamara, John >> ; david.marchand@redhat.com >> Subject: Re: [PATCH v1 08/14] vhost: improve IO vector logic >> >> Hi Jiayu, >> >> On 10/25/21 09:22, Hu, Jiayu wrote: >>> Hi Maxime, >>> >>>> -----Original Message----- >>>> From: Maxime Coquelin >>>> Sent: Monday, October 18, 2021 9:02 PM >>>> To: dev@dpdk.org; Xia, Chenbo ; Hu, Jiayu >>>> ; Wang, YuanX ; Ma, >> WenwuX >>>> ; Richardson, Bruce >>>> ; Mcnamara, John >>>> ; david.marchand@redhat.com >>>> Cc: Maxime Coquelin >>>> Subject: [PATCH v1 08/14] vhost: improve IO vector logic >>>> >>>> IO vectors and their iterators arrays were part of the async metadata >>>> but not their indexes. >>>> >>>> In order to makes this more consistent, the patch adds the indexes to >>>> the async metadata. Doing that, we can avoid triggering DMA transfer >>>> within the loop as it IO vector index overflow is now prevented in >>>> the >>>> async_mbuf_to_desc() function. >>>> >>>> Note that previous detection mechanism was broken since the overflow >>>> already happened when detected, so OOB memory access would already >>>> have happened. >>>> >>>> With this changes done, virtio_dev_rx_async_submit_split() >>>> and virtio_dev_rx_async_submit_packed() can be further simplified. >>>> >>>> Signed-off-by: Maxime Coquelin >>>> --- >>>> lib/vhost/vhost.h | 2 + >>>> lib/vhost/virtio_net.c | 291 ++++++++++++++++++----------------------- >>>> 2 files changed, 131 insertions(+), 162 deletions(-) >>>> >>>> diff --git a/lib/vhost/vhost.h b/lib/vhost/vhost.h index >>>> dae9a1ac2d..812d4c55a5 100644 >>>> --- a/lib/vhost/vhost.h >>>> +++ b/lib/vhost/vhost.h >>>> @@ -134,6 +134,8 @@ struct vhost_async { >>>> >>>> struct rte_vhost_iov_iter iov_iter[VHOST_MAX_ASYNC_IT]; >>>> struct rte_vhost_iovec iovec[VHOST_MAX_ASYNC_VEC]; >>>> + uint16_t iter_idx; >>>> + uint16_t iovec_idx; >>>> >>>> /* data transfer status */ >>>> struct async_inflight_info *pkts_info; diff --git >>>> a/lib/vhost/virtio_net.c b/lib/vhost/virtio_net.c index >>>> ae7dded979..c80823a8de 100644 >>>> --- a/lib/vhost/virtio_net.c >>>> +++ b/lib/vhost/virtio_net.c >>>> @@ -924,33 +924,86 @@ copy_mbuf_to_desc(struct virtio_net *dev, >>>> struct vhost_virtqueue *vq, >>>> return error; >>>> } >>>> >>>> +static __rte_always_inline int >>>> +async_iter_initialize(struct vhost_async *async) { >>>> + struct rte_vhost_iov_iter *iter; >>>> + >>>> + if (unlikely(async->iovec_idx >= VHOST_MAX_ASYNC_VEC)) { >>>> + VHOST_LOG_DATA(ERR, "no more async iovec available\n"); >>>> + return -1; >>>> + } >>>> + >>>> + iter = async->iov_iter + async->iter_idx; >>>> + iter->iov = async->iovec + async->iovec_idx; >>>> + iter->nr_segs = 0; >>>> + >>>> + return 0; >>>> +} >>>> + >>>> +static __rte_always_inline int >>>> +async_iter_add_iovec(struct vhost_async *async, void *src, void >>>> +*dst, size_t len) { >>>> + struct rte_vhost_iov_iter *iter; >>>> + struct rte_vhost_iovec *iovec; >>>> + >>>> + if (unlikely(async->iovec_idx >= VHOST_MAX_ASYNC_VEC)) { >>>> + VHOST_LOG_DATA(ERR, "no more async iovec available\n"); >>>> + return -1; >>>> + } >>> >>> For large packets, like 64KB in iperf test, async_iter_add_iovec() >>> frequently reports the log above, as we run out of iovecs. I think >>> it's better to change the log from ERR to DEBUG. >> >> I think it is better to keep it as an error, we want to see it if it happens >> without having the user to enable debug. >> >> But maybe we can only print it once, not to flood the logs. > > OK. > >> >>> In addition, the size of iovec is too small. For burst 32 and 64KB >>> pkts, it's easy to run out of iovecs and we will drop the pkts to >>> enqueue if it happens, which hurts performance. Enlarging the array is >>> a choice to mitigate the issue, but another solution is to reallocate >>> iovec once we run out of it. How do you think? >> >> I would prefer we enlarge the array, reallocating the array when the issue >> happens sounds like over-engineering to me. >> >> Any idea what size it should be based on your experiments? > > 2048 is enough for iperf and 64KB pkts. Thanks for the insight, I will change to 2048 in next revision. Maxime > > Thanks, > Jiayu >> >> Thanks, >> Maxime >> >>> Thanks, >>> Jiayu >>>> + >>>> + iter = async->iov_iter + async->iter_idx; >>>> + iovec = async->iovec + async->iovec_idx; >>>> + >>>> + iovec->src_addr = src; >>>> + iovec->dst_addr = dst; >>>> + iovec->len = len; >>>> + >>>> + iter->nr_segs++; >>>> + async->iovec_idx++; >>>> + >>>> + return 0; >>>> +} >>> >