mbox series

[v10,0/5] kni: add IOVA=VA support

Message ID 20190816061252.17214-1-vattunuru@marvell.com (mailing list archive)
Headers
Series kni: add IOVA=VA support |

Message

Vamsi Krishna Attunuru Aug. 16, 2019, 6:12 a.m. UTC
  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

 doc/guides/prog_guide/kernel_nic_interface.rst    |  8 ++
 doc/guides/rel_notes/release_19_11.rst            |  5 ++
 examples/kni/main.c                               |  5 +-
 kernel/linux/kni/compat.h                         |  4 +
 kernel/linux/kni/kni_dev.h                        |  4 +
 kernel/linux/kni/kni_misc.c                       | 71 ++++++++++++++---
 kernel/linux/kni/kni_net.c                        | 59 ++++++++++----
 lib/librte_eal/linux/eal/eal.c                    |  4 +-
 lib/librte_eal/linux/eal/include/rte_kni_common.h |  8 ++
 lib/librte_kni/Makefile                           |  2 +
 lib/librte_kni/meson.build                        |  2 +
 lib/librte_kni/rte_kni.c                          | 95 +++++++++++++++++++++--
 lib/librte_kni/rte_kni.h                          | 48 ++++++++++++
 lib/librte_kni/rte_kni_version.map                |  2 +
 lib/librte_mempool/rte_mempool.c                  | 69 ++++++++++++++++
 lib/librte_mempool/rte_mempool.h                  | 20 +++++
 lib/librte_mempool/rte_mempool_version.map        |  1 +
 17 files changed, 378 insertions(+), 29 deletions(-)
  

Comments

Vamsi Krishna Attunuru Sept. 25, 2019, 4 a.m. UTC | #1
PING.

-----Original Message-----
From: vattunuru@marvell.com <vattunuru@marvell.com> 
Sent: Friday, August 16, 2019 11:43 AM
To: dev@dpdk.org
Cc: thomas@monjalon.net; Jerin Jacob Kollanukkaran <jerinj@marvell.com>; olivier.matz@6wind.com; ferruh.yigit@intel.com; anatoly.burakov@intel.com; arybchenko@solarflare.com; Kiran Kumar Kokkilagadda <kirankumark@marvell.com>; Vamsi Krishna Attunuru <vattunuru@marvell.com>
Subject: [dpdk-dev] [PATCH v10 0/5] kni: add IOVA=VA support 

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

 doc/guides/prog_guide/kernel_nic_interface.rst    |  8 ++
 doc/guides/rel_notes/release_19_11.rst            |  5 ++
 examples/kni/main.c                               |  5 +-
 kernel/linux/kni/compat.h                         |  4 +
 kernel/linux/kni/kni_dev.h                        |  4 +
 kernel/linux/kni/kni_misc.c                       | 71 ++++++++++++++---
 kernel/linux/kni/kni_net.c                        | 59 ++++++++++----
 lib/librte_eal/linux/eal/eal.c                    |  4 +-
 lib/librte_eal/linux/eal/include/rte_kni_common.h |  8 ++
 lib/librte_kni/Makefile                           |  2 +
 lib/librte_kni/meson.build                        |  2 +
 lib/librte_kni/rte_kni.c                          | 95 +++++++++++++++++++++--
 lib/librte_kni/rte_kni.h                          | 48 ++++++++++++
 lib/librte_kni/rte_kni_version.map                |  2 +
 lib/librte_mempool/rte_mempool.c                  | 69 ++++++++++++++++
 lib/librte_mempool/rte_mempool.h                  | 20 +++++
 lib/librte_mempool/rte_mempool_version.map        |  1 +
 17 files changed, 378 insertions(+), 29 deletions(-)

--
2.8.4
  
Vamsi Krishna Attunuru Oct. 8, 2019, 5:08 a.m. UTC | #2
@All, we are expecting to merge this in 19.11 release and if any one have comments please respond.

> -----Original Message-----
> From: Vamsi Krishna Attunuru
> Sent: Wednesday, September 25, 2019 9:30 AM
> To: vattunuru@marvell.com; dev@dpdk.org
> Cc: thomas@monjalon.net; Jerin Jacob Kollanukkaran <jerinj@marvell.com>;
> olivier.matz@6wind.com; ferruh.yigit@intel.com;
> anatoly.burakov@intel.com; arybchenko@solarflare.com; Kiran Kumar
> Kokkilagadda <kirankumark@marvell.com>
> Subject: RE: [dpdk-dev] [PATCH v10 0/5] kni: add IOVA=VA support
> 
> PING.
> 
> -----Original Message-----
> From: vattunuru@marvell.com <vattunuru@marvell.com>
> Sent: Friday, August 16, 2019 11:43 AM
> To: dev@dpdk.org
> Cc: thomas@monjalon.net; Jerin Jacob Kollanukkaran <jerinj@marvell.com>;
> olivier.matz@6wind.com; ferruh.yigit@intel.com;
> anatoly.burakov@intel.com; arybchenko@solarflare.com; Kiran Kumar
> Kokkilagadda <kirankumark@marvell.com>; Vamsi Krishna Attunuru
> <vattunuru@marvell.com>
> Subject: [dpdk-dev] [PATCH v10 0/5] kni: add IOVA=VA support
> 
> 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
> 
>  doc/guides/prog_guide/kernel_nic_interface.rst    |  8 ++
>  doc/guides/rel_notes/release_19_11.rst            |  5 ++
>  examples/kni/main.c                               |  5 +-
>  kernel/linux/kni/compat.h                         |  4 +
>  kernel/linux/kni/kni_dev.h                        |  4 +
>  kernel/linux/kni/kni_misc.c                       | 71 ++++++++++++++---
>  kernel/linux/kni/kni_net.c                        | 59 ++++++++++----
>  lib/librte_eal/linux/eal/eal.c                    |  4 +-
>  lib/librte_eal/linux/eal/include/rte_kni_common.h |  8 ++
>  lib/librte_kni/Makefile                           |  2 +
>  lib/librte_kni/meson.build                        |  2 +
>  lib/librte_kni/rte_kni.c                          | 95 +++++++++++++++++++++--
>  lib/librte_kni/rte_kni.h                          | 48 ++++++++++++
>  lib/librte_kni/rte_kni_version.map                |  2 +
>  lib/librte_mempool/rte_mempool.c                  | 69 ++++++++++++++++
>  lib/librte_mempool/rte_mempool.h                  | 20 +++++
>  lib/librte_mempool/rte_mempool_version.map        |  1 +
>  17 files changed, 378 insertions(+), 29 deletions(-)
> 
> --
> 2.8.4
  
Vamsi Krishna Attunuru Oct. 14, 2019, 4:05 a.m. UTC | #3
Hi Ferruh,

Mempool related patch has been acked by Olivier. Could you please review the kni related patches and make sure that it will be merged in 19.11 release.

Regards,
Vamsi

> -----Original Message-----
> From: Vamsi Krishna Attunuru <vattunuru@marvell.com>
> Sent: Tuesday, October 8, 2019 10:39 AM
> To: Vamsi Krishna Attunuru <vattunuru@marvell.com>; dev@dpdk.org
> Cc: thomas@monjalon.net; Jerin Jacob Kollanukkaran <jerinj@marvell.com>;
> olivier.matz@6wind.com; ferruh.yigit@intel.com; anatoly.burakov@intel.com;
> arybchenko@solarflare.com; Kiran Kumar Kokkilagadda
> <kirankumark@marvell.com>
> Subject: RE: [dpdk-dev] [PATCH v10 0/5] kni: add IOVA=VA support
> 
> @All, we are expecting to merge this in 19.11 release and if any one have
> comments please respond.
> 
> > -----Original Message-----
> > From: Vamsi Krishna Attunuru
> > Sent: Wednesday, September 25, 2019 9:30 AM
> > To: vattunuru@marvell.com; dev@dpdk.org
> > Cc: thomas@monjalon.net; Jerin Jacob Kollanukkaran
> > <jerinj@marvell.com>; olivier.matz@6wind.com; ferruh.yigit@intel.com;
> > anatoly.burakov@intel.com; arybchenko@solarflare.com; Kiran Kumar
> > Kokkilagadda <kirankumark@marvell.com>
> > Subject: RE: [dpdk-dev] [PATCH v10 0/5] kni: add IOVA=VA support
> >
> > PING.
> >
> > -----Original Message-----
> > From: vattunuru@marvell.com <vattunuru@marvell.com>
> > Sent: Friday, August 16, 2019 11:43 AM
> > To: dev@dpdk.org
> > Cc: thomas@monjalon.net; Jerin Jacob Kollanukkaran
> > <jerinj@marvell.com>; olivier.matz@6wind.com; ferruh.yigit@intel.com;
> > anatoly.burakov@intel.com; arybchenko@solarflare.com; Kiran Kumar
> > Kokkilagadda <kirankumark@marvell.com>; Vamsi Krishna Attunuru
> > <vattunuru@marvell.com>
> > Subject: [dpdk-dev] [PATCH v10 0/5] kni: add IOVA=VA support
> >
> > 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
> >
> >  doc/guides/prog_guide/kernel_nic_interface.rst    |  8 ++
> >  doc/guides/rel_notes/release_19_11.rst            |  5 ++
> >  examples/kni/main.c                               |  5 +-
> >  kernel/linux/kni/compat.h                         |  4 +
> >  kernel/linux/kni/kni_dev.h                        |  4 +
> >  kernel/linux/kni/kni_misc.c                       | 71 ++++++++++++++---
> >  kernel/linux/kni/kni_net.c                        | 59 ++++++++++----
> >  lib/librte_eal/linux/eal/eal.c                    |  4 +-
> >  lib/librte_eal/linux/eal/include/rte_kni_common.h |  8 ++
> >  lib/librte_kni/Makefile                           |  2 +
> >  lib/librte_kni/meson.build                        |  2 +
> >  lib/librte_kni/rte_kni.c                          | 95 +++++++++++++++++++++--
> >  lib/librte_kni/rte_kni.h                          | 48 ++++++++++++
> >  lib/librte_kni/rte_kni_version.map                |  2 +
> >  lib/librte_mempool/rte_mempool.c                  | 69 ++++++++++++++++
> >  lib/librte_mempool/rte_mempool.h                  | 20 +++++
> >  lib/librte_mempool/rte_mempool_version.map        |  1 +
> >  17 files changed, 378 insertions(+), 29 deletions(-)
> >
> > --
> > 2.8.4
  
Ferruh Yigit Oct. 15, 2019, 3:34 p.m. UTC | #4
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...
  
Vamsi Krishna Attunuru Oct. 16, 2019, 12:17 p.m. UTC | #5
> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Yigit, Ferruh
> Sent: Tuesday, October 15, 2019 9:05 PM
> To: Vamsi Krishna Attunuru <vattunuru@marvell.com>; dev@dpdk.org
> Cc: thomas@monjalon.net; Jerin Jacob Kollanukkaran <jerinj@marvell.com>;
> olivier.matz@6wind.com; ferruh.yigit@intel.com; anatoly.burakov@intel.com;
> arybchenko@solarflare.com; Kiran Kumar Kokkilagadda
> <kirankumark@marvell.com>
> Subject: Re: [dpdk-dev] [PATCH v10 0/5] kni: add IOVA=VA support
> 
> 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

Vamsi> Yes, these can be fixed by forcing iommu_mode to PA for
vdev or vdev&pdev based KNI usecases. 

> 
> 
> 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?

Vamsi> There would not be any affect in using KNI version of mempool.

> 
> 
> 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?

Vamsi> Yes, it's appropriate to control the mode to ensure old apps work by default.
We are fine with having a command line arg or config option to enable KNI in iova=va mode.
Earlier we thought of having similar approach that also controls mempool allocation using
a newer mempool flag. After multiple reviews, flag has been discard and added a separate
mempool populate routine for these usecase. 

When command line arg/config option is introduced, functionality will be as below. 
Please correct me if any cases are missed or not considered.
Without command:
Existing KNI is intact, iommu mode will be PA.
With  command:
Pdev/vdev's iommu mode is considered and accordingly iova=va/pa is enabled. Application is
supposed to use KNI version of mempool alloc. I think these mempool quirk will go away when
Olivier's mempool patchset(RFC) is merged.

> 
> And I will put a few minor comments to the patches...
>
  
Ferruh Yigit Oct. 16, 2019, 4:21 p.m. UTC | #6
On 10/16/2019 1:17 PM, Vamsi Krishna Attunuru wrote:
> 
> 
>> -----Original Message-----
>> From: dev <dev-bounces@dpdk.org> On Behalf Of Yigit, Ferruh
>> Sent: Tuesday, October 15, 2019 9:05 PM
>> To: Vamsi Krishna Attunuru <vattunuru@marvell.com>; dev@dpdk.org
>> Cc: thomas@monjalon.net; Jerin Jacob Kollanukkaran <jerinj@marvell.com>;
>> olivier.matz@6wind.com; ferruh.yigit@intel.com; anatoly.burakov@intel.com;
>> arybchenko@solarflare.com; Kiran Kumar Kokkilagadda
>> <kirankumark@marvell.com>
>> Subject: Re: [dpdk-dev] [PATCH v10 0/5] kni: add IOVA=VA support
>>
>> 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
> 
> Vamsi> Yes, these can be fixed by forcing iommu_mode to PA for
> vdev or vdev&pdev based KNI usecases. 
> 
>>
>>
>> 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?
> 
> Vamsi> There would not be any affect in using KNI version of mempool.

If you were developing a product, would you rely on a KNI API for pkt_mbuf
allocation? What if there is a problem, will it be as easy as mempool/mbuf API
to figure out and fix? I am not sure about pushing users to this direction?

> 
>>
>>
>> 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?
> 
> Vamsi> Yes, it's appropriate to control the mode to ensure old apps work by default.
> We are fine with having a command line arg or config option to enable KNI in iova=va mode.
> Earlier we thought of having similar approach that also controls mempool allocation using
> a newer mempool flag. After multiple reviews, flag has been discard and added a separate
> mempool populate routine for these usecase. 

I didn't like the idea of having a flag in mempool library, but perhaps we can
have it in KNI scope.

> 
> When command line arg/config option is introduced, functionality will be as below. 
> Please correct me if any cases are missed or not considered.
> Without command:
> Existing KNI is intact, iommu mode will be PA.

+1

> With  command:
> Pdev/vdev's iommu mode is considered and accordingly iova=va/pa is enabled. Application is
> supposed to use KNI version of mempool alloc. 

+1

> I think these mempool quirk will go away when
> Olivier's mempool patchset(RFC) is merged.
> 
>>
>> And I will put a few minor comments to the patches...
>>
>
  
Vamsi Krishna Attunuru Oct. 18, 2019, 4:42 p.m. UTC | #7
> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@intel.com>
> Sent: Wednesday, October 16, 2019 9:52 PM
> To: Vamsi Krishna Attunuru <vattunuru@marvell.com>; Yigit, Ferruh
> <ferruh.yigit@linux.intel.com>
> Cc: thomas@monjalon.net; Jerin Jacob Kollanukkaran <jerinj@marvell.com>;
> olivier.matz@6wind.com; anatoly.burakov@intel.com;
> arybchenko@solarflare.com; Kiran Kumar Kokkilagadda
> <kirankumark@marvell.com>; dev@dpdk.org
> Subject: [EXT] Re: [dpdk-dev] [PATCH v10 0/5] kni: add IOVA=VA support
> 
> External Email
> 
> ----------------------------------------------------------------------
> On 10/16/2019 1:17 PM, Vamsi Krishna Attunuru wrote:
> >
> >
> >> -----Original Message-----
> >> From: dev <dev-bounces@dpdk.org> On Behalf Of Yigit, Ferruh
> >> Sent: Tuesday, October 15, 2019 9:05 PM
> >> To: Vamsi Krishna Attunuru <vattunuru@marvell.com>; dev@dpdk.org
> >> Cc: thomas@monjalon.net; Jerin Jacob Kollanukkaran
> >> <jerinj@marvell.com>; olivier.matz@6wind.com; ferruh.yigit@intel.com;
> >> anatoly.burakov@intel.com; arybchenko@solarflare.com; Kiran Kumar
> >> Kokkilagadda <kirankumark@marvell.com>
> >> Subject: Re: [dpdk-dev] [PATCH v10 0/5] kni: add IOVA=VA support
> >>
> >> 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
> >
> > Vamsi> Yes, these can be fixed by forcing iommu_mode to PA for
> > vdev or vdev&pdev based KNI usecases.
> >
> >>
> >>
> >> 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?
> >
> > Vamsi> There would not be any affect in using KNI version of mempool.
> 
> If you were developing a product, would you rely on a KNI API for pkt_mbuf
> allocation? What if there is a problem, will it be as easy as mempool/mbuf
> API to figure out and fix? I am not sure about pushing users to this direction?

Vamsi >  If user wants to run KNI app in iova=va mode, mempool populate needs to ensure mbuf would not be allocated from a page boundary. IMO after having config option/cmd line parameter to enable KNI iova=va mode, existing KNI will be intact and these mempool API only be called upon request. I will document these details in KNI document. Based on earlier comments and discussions with Olivier and Andrew on the mempool populate patch, we arrived at these solution.

Hi Olivier, 
Please let us know your thoughts on these patch. I am open to any of your suggestions/solution to fix mempool populate issue for KNI iova=va use case.
 
> 
> >
> >>
> >>
> >> 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?
> >
> > Vamsi> Yes, it's appropriate to control the mode to ensure old apps work
> by default.
> > We are fine with having a command line arg or config option to enable KNI
> in iova=va mode.
> > Earlier we thought of having similar approach that also controls
> > mempool allocation using a newer mempool flag. After multiple reviews,
> > flag has been discard and added a separate mempool populate routine for
> these usecase.
> 
> I didn't like the idea of having a flag in mempool library, but perhaps we can
> have it in KNI scope.

Vamsi> I failed to understand KNI scope here, IMO, though we have flag in KNI scope, it needs to be affected in mempool lib right finally. 

> 
> >
> > When command line arg/config option is introduced, functionality will be
> as below.
> > Please correct me if any cases are missed or not considered.
> > Without command:
> > Existing KNI is intact, iommu mode will be PA.
> 
> +1
> 
> > With  command:
> > Pdev/vdev's iommu mode is considered and accordingly iova=va/pa is
> > enabled. Application is supposed to use KNI version of mempool alloc.
> 
> +1
> 
> > I think these mempool quirk will go away when Olivier's mempool
> > patchset(RFC) is merged.
> >
> >>
> >> And I will put a few minor comments to the patches...
> >>
> >