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 Regards, Fan > -----Original Message----- > From: Jay Zhou [mailto:jianjay.z...@huawei.com] > Sent: Friday, November 17, 2017 5:10 PM > To: dev@dpdk.org > Cc: y...@fridaylinux.org; maxime.coque...@redhat.com; > arei.gong...@huawei.com; Zhang, Roy Fan <roy.fan.zh...@intel.com>; Zeng, > Xin <xin.z...@intel.com>; weidong.hu...@huawei.com; > wangxinxin.w...@huawei.com; longpe...@huawei.com; > jianjay.z...@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. > + 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);
