Re: [dpdk-dev] [PATCH] virtio: add new driver for crypto devices

["Zhang, Roy Fan" <roy.fan.zhang@intel.com>]

Hi Jay,

No worries. Welcome to DPDK :-D

Regards,
Fan

> -----Original Message-----
> From: Jay Zhou [mailto:jianjay.zhou@huawei.com]
> Sent: Tuesday, November 28, 2017 1:27 AM
> To: Zhang, Roy Fan <roy.fan.zhang@intel.com>; dev@dpdk.org
> Cc: yliu@fridaylinux.org; maxime.coquelin@redhat.com;
> arei.gonglei@huawei.com; Zeng, Xin <xin.zeng@intel.com>;
> weidong.huang@huawei.com; wangxinxin.wang@huawei.com;
> longpeng2@huawei.com
> Subject: Re: [PATCH] virtio: add new driver for crypto devices
> 
> 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);
> >
> > .
> >