From mboxrd@z Thu Jan  1 00:00:00 1970
From: Jay Zhou <jianjay.zhou@huawei.com>
Subject: Re: [PATCH] virtio: add new driver for crypto devices
Date: Tue, 28 Nov 2017 09:27:14 +0800
Message-ID: <5A1CBB72.4050104@huawei.com>
References: <1510938620-15268-1-git-send-email-jianjay.zhou@huawei.com>
 <9F7182E3F746AB4EA17801C148F3C6043305A340@IRSMSX101.ger.corp.intel.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="windows-1252"; format=flowed
Content-Transfer-Encoding: 7bit
Cc: "yliu@fridaylinux.org" <yliu@fridaylinux.org>,
 "maxime.coquelin@redhat.com" <maxime.coquelin@redhat.com>,
 "arei.gonglei@huawei.com" <arei.gonglei@huawei.com>, "Zeng, Xin"
 <xin.zeng@intel.com>, "weidong.huang@huawei.com" <weidong.huang@huawei.com>,
 "wangxinxin.wang@huawei.com" <wangxinxin.wang@huawei.com>,
 "longpeng2@huawei.com" <longpeng2@huawei.com>
To: "Zhang, Roy Fan" <roy.fan.zhang@intel.com>, "dev@dpdk.org" <dev@dpdk.org>
Return-path: <dev-bounces@dpdk.org>
Received: from huawei.com (unknown [45.249.212.32])
 by dpdk.org (Postfix) with ESMTP id DA78E271
 for <dev@dpdk.org>; Tue, 28 Nov 2017 02:28:28 +0100 (CET)
In-Reply-To: <9F7182E3F746AB4EA17801C148F3C6043305A340@IRSMSX101.ger.corp.intel.com>
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <http://dpdk.org/ml/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <http://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>

Hi Fan,

On 2017/11/28 0:47, Zhang, Roy Fan wrote:
> Hi Jay,
>
> Thanks for contributing to DPDK.
>
> The code has been tested and works fine.
>
> A few comments:
>
> 1. Could you split the patch into a patchset, as suggested in contribution guide in http://dpdk.org/doc/guides/contributing/patches.html, section 5.4?
> 2. Please update doc/guides/cryptodevs for describing your virtio crypto PMD.
> 3. Please update doc/guides/rel_notes/release_18.02.
> 4. One more comment inline
>

For DPDK, I'm a newbie. Thanks for testing and pointing these steps
out, will fix them in V2.

>
>> -----Original Message-----
>> From: Jay Zhou [mailto:jianjay.zhou@huawei.com]
>> Sent: Friday, November 17, 2017 5:10 PM
>> To: dev@dpdk.org
>> Cc: yliu@fridaylinux.org; maxime.coquelin@redhat.com;
>> arei.gonglei@huawei.com; Zhang, Roy Fan <roy.fan.zhang@intel.com>; Zeng,
>> Xin <xin.zeng@intel.com>; weidong.huang@huawei.com;
>> wangxinxin.wang@huawei.com; longpeng2@huawei.com;
>> jianjay.zhou@huawei.com
>> Subject: [PATCH] virtio: add new driver for crypto devices
>> +	/*
>> +	 * malloc memory to store indirect vring_desc entries, including
>> +	 * ctrl request, cipher key, auth key, session input and desc vring
>> +	 */
>> +	desc_offset = ctrl_req_length + cipher_keylen + auth_keylen
>> +		+ input_length;
>
> Instead of using rte_malloc() as below, you could pre-allocate a mempool and use
> rte_mempool_get() or rte_mempool_get_bulk() to get these memory to store descriptors.
> You can use rte_mempool_virt2iova() to obtain the physical address of this memory. This shall
> have better performance.

I will have a try.

Regards,
Jay

>
>> +	virt_addr_started = rte_malloc(NULL,
>> +		desc_offset +
>> NUM_ENTRY_VIRTIO_CRYPTO_SYM_CREATE_SESSION
>> +			* sizeof(struct vring_desc), RTE_CACHE_LINE_SIZE);
>> +	if (virt_addr_started == NULL) {
>> +		PMD_SESSION_LOG(ERR, "not enough heap memory");
>> +		return -ENOSPC;
>> +	}
>> +	phys_addr_started = rte_malloc_virt2phy(virt_addr_started);
>
> .
>