From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail.lysator.liu.se (mail.lysator.liu.se [130.236.254.3]) by dpdk.org (Postfix) with ESMTP id D51304C91 for ; Tue, 30 Oct 2018 20:38:10 +0100 (CET) Received: from mail.lysator.liu.se (localhost [127.0.0.1]) by mail.lysator.liu.se (Postfix) with ESMTP id 27C7F4001D for ; Tue, 30 Oct 2018 20:38:10 +0100 (CET) Received: by mail.lysator.liu.se (Postfix, from userid 1004) id 12B2A4001C; Tue, 30 Oct 2018 20:38:10 +0100 (CET) X-Spam-Checker-Version: SpamAssassin 3.4.1 (2015-04-28) on bernadotte.lysator.liu.se X-Spam-Level: X-Spam-Status: No, score=-0.9 required=5.0 tests=ALL_TRUSTED,AWL autolearn=disabled version=3.4.1 X-Spam-Score: -0.9 Received: from [192.168.1.59] (host-90-232-127-56.mobileonline.telia.com [90.232.127.56]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.lysator.liu.se (Postfix) with ESMTPSA id D24054001D; Tue, 30 Oct 2018 20:38:07 +0100 (CET) To: Fan Zhang , dev@dpdk.org Cc: maxime.coquelin@redhat.com References: <20181030144852.43339-1-roy.fan.zhang@intel.com> From: =?UTF-8?Q?Mattias_R=c3=b6nnblom?= Message-ID: <4c3cdfb6-7b79-5b89-286d-8e8cae56f755@ericsson.com> Date: Tue, 30 Oct 2018 20:38:07 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.2.1 MIME-Version: 1.0 In-Reply-To: <20181030144852.43339-1-roy.fan.zhang@intel.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit X-Virus-Scanned: ClamAV using ClamSMTP Subject: Re: [dpdk-dev] [PATCH] examples/vhost_crypto: fix zero copy 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: , X-List-Received-Date: Tue, 30 Oct 2018 19:38:11 -0000 On 2018-10-30 15:48, Fan Zhang wrote: > This patch fixes the zero copy enable problem for vhost crypto > sample application. > > For some Crypto PMDs such as AESNI-MB and AESNI-GCM the data to > be processed will be made a copy in the same buffer but next to the > data. For example, to encrypt 64 bytes data the PMD will copy this > data from offset 64 to offset 123. This requires the application > provides the buffer with at least double of the data size. > > However there is no way for VMs to know this limitation. When > zero-copy is enabled in Vhost the PMD may overwrite the buffer > next to the VM data to be processed, and further cause problems > such as Segmentation Fault or even worse, crashes the VM. > > To fix the problem the user should avoid enabling the zero copy > for these Crypto PMDs. This patch adds the checking of the PMD > names to see if zero copy can be applied. > > Fixes: 709521f4c2cd ("examples/vhost_crypto: support multi-core") > > Signed-off-by: Fan Zhang > --- > examples/vhost_crypto/main.c | 23 +++++++++++++++++++++-- > 1 file changed, 21 insertions(+), 2 deletions(-) > > diff --git a/examples/vhost_crypto/main.c b/examples/vhost_crypto/main.c > index cbb5e49d2..887e3eb6f 100644 > --- a/examples/vhost_crypto/main.c > +++ b/examples/vhost_crypto/main.c > @@ -4,6 +4,7 @@ > > #include > #include > +#include > #include > #include > #include > @@ -442,8 +443,13 @@ free_resource(void) > struct lcore_option *lo = &options.los[i]; > struct vhost_crypto_info *info = options.infos[i]; > > - rte_mempool_free(info->cop_pool); > - rte_mempool_free(info->sess_pool); > + if (!info) > + continue; > + > + if (info->cop_pool) > + rte_mempool_free(info->cop_pool); > + if (info->sess_pool) > + rte_mempool_free(info->sess_pool); > rte_mempool_free() already does a NULL-check (as per libc free() convention), and if you are to do a NULL-check it should be an explicit one ("!= NULL"). > for (j = 0; j < lo->nb_sockets; j++) { > rte_vhost_driver_unregister(lo->socket_files[i]); > @@ -493,6 +499,19 @@ main(int argc, char *argv[]) > info->nb_vids = lo->nb_sockets; > > rte_cryptodev_info_get(info->cid, &dev_info); > + if (options.zero_copy == RTE_VHOST_CRYPTO_ZERO_COPY_ENABLE) { > +#define VHOST_CRYPTO_CDEV_NAME_AESNI_MB_PMD crypto_aesni_mb > +#define VHOST_CRYPTO_CDEV_NAME_AESNI_GCM_PMD crypto_aesni_gcm What's the purpose of these defines? > + if (strstr(dev_info.driver_name, > + RTE_STR(VHOST_CRYPTO_CDEV_NAME_AESNI_MB_PMD)) || > + strstr(dev_info.driver_name, > + RTE_STR(VHOST_CRYPTO_CDEV_NAME_AESNI_GCM_PMD))) > + RTE_LOG(ERR, USER1, "Cannot enable Zero Copy to %s\n", > + dev_info.driver_name); "Zero Copy to" should probably be "zero-copy in" or "Zero-copy in".