Re: [dpdk-dev] [PATCH v10 0/5] kni: add IOVA=VA support

["Yigit, Ferruh" <ferruh.yigit@linux.intel.com>]

On 8/16/2019 7:12 AM, vattunuru@marvell.com wrote:
> From: Vamsi Attunuru <vattunuru@marvell.com>
> 
> ---
> V10 Changes:
> * Fixed function return code on failure when min_chunk_size > pg_sz.
> * Marked new mempool populate routine as EXPERIMENTAL.
> 
> V9 Changes:
> * Used rte_mempool_ops_calc_mem_size() instead of default handler in
> the new mempool populate routine.
> * Check min_chunk_size and return values.
> * Removed ethdev_info memset to '0' and moved pci dev_info populate into
> kni_dev_pci_addr_get() routine.
> * Addressed misc. review comments.
> 
> V8 Changes:
> * Remove default mempool populate() routine changes.
> * Add kni app specific mempool create & free routines.
> * Add new mempool populate routine to allocate page-aligned memzones
> with page size to make sure all mempool objects reside on a page.
> * Update release notes and map files.
> 
> V7 Changes:
> * Removed previously proposed mempool flag and made those page boundary
> checks default in mempool populate() except for the objects size bigger
> than the size of page.
> * Removed KNI example application related changes since pool related
> requirement is taken care in mempool lib.
> * All PCI dev related info is moved under rte_eal_iova_mode() == VA check.
> * Added wrapper functions in KNI module to hide IOVA checks and make
> address translation routines more readable.
> * Updated IOVA mode checks that enforcing IOVA=PA mode when IOVA=VA mode
> is enabled.
> 
> V6 Changes:
> * Added new mempool flag to ensure mbuf memory is not scattered
> across page boundaries.
> * Added KNI kernel module required PCI device information.
> * Modified KNI example application to create mempool with new
> mempool flag.
> 
> V5 changes:
> * Fixed build issue with 32b build
> 
> V4 changes:
> * Fixed build issues with older kernel versions
> * This approach will only work with kernel above 4.4.0
> 
> V3 Changes:
> * Add new approach to work kni with IOVA=VA mode using
> iommu_iova_to_phys API.
> 
> Kiran Kumar K (1):
>   kni: add IOVA=VA support in KNI module
> 
> Vamsi Attunuru (4):
>   mempool: populate mempool with the page sized chunks
>   kni: add IOVA=VA support in KNI lib
>   kni: add app specific mempool create and free routines
>   kni: modify IOVA mode checks to support VA

Hi Vamsi,

I am aware that this patchset is around for a long time, and I have seen your
request to merge in 19.11, but as you can understand the concern I have is to
break KNI or existing KNI applications while trying to add this new feature.

In high level, there are two issues,

1) kernel modules updates expect there will be a backed device of the KNI which
is not always true:

         if (dev_info.iova_mode) {
 #ifdef HAVE_IOVA_AS_VA_SUPPORT
                 pci = pci_get_device(dev_info.vendor_id,
                                      dev_info.device_id, NULL);
                 if (pci == NULL) {
                         pr_err("pci dev does not exist\n");
                         return -ENODEV;
                 }

For example this breaks:
./build/app/testpmd -w0:0.0 --vdev net_kni0 --vdev net_kni1  -- -i


2) Applications will have to change the API to allocate the mempool.
If the user upgraded to new version of DPDK, now it is possible to have iova=va
mode and application should use new KNI API 'rte_kni_pktmbuf_pool_create()' to
allocate mempool. And most probably application will have datapath and will use
the KNI only for exception path, will there be any affect using KNI version of
mempool alloc?


I would like to see KNI is enabled via iova=va mode, but can we have it limited
to a specific command line argument or config option? This increases the test
surface but at least old application can continue to work by default, what do
you think?

And I will put a few minor comments to the patches...