[v12,0/2] add IOVA=VA mode support
mbox series

Message ID 20191105110416.8955-1-vattunuru@marvell.com
Headers show
Series
  • add IOVA=VA mode support
Related show

Message

Vamsi Krishna Attunuru Nov. 5, 2019, 11:04 a.m. UTC
From: Vamsi Attunuru <vattunuru@marvell.com>

---
V12 Changes:
* Removed previously added `--legacy-kni` eal option.
* Removed previously added kni specific mempool create routines
and mempool populate routines.

This patch set(V12) is dependent on following patch set, since the mempool
related support to enable KNI in IOVA=VA mode is taken care in below
patchset.

   https://patchwork.dpdk.org/cover/62376/

V11 Changes:
* Added iova to kva address translation routines in kernel module to
make it work in iova=va mode which enables DPDK to create kni devices
on any kind of backed device/memory.
* Added ``--legacy-kni`` eal option to make existing KNI applications
work with DPDK 19.11 and later versions.
* Removed previously added pci device info from kni device info struct.
 
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.

Vamsi Attunuru (2):
  kni: add IOVA=VA mode support
  kni: add IOVA=VA support in kernel module

 doc/guides/prog_guide/kernel_nic_interface.rst    |  9 ++++
 doc/guides/rel_notes/release_19_11.rst            |  5 ++
 kernel/linux/kni/compat.h                         | 15 ++++++
 kernel/linux/kni/kni_dev.h                        | 42 +++++++++++++++
 kernel/linux/kni/kni_misc.c                       | 39 ++++++++++----
 kernel/linux/kni/kni_net.c                        | 62 ++++++++++++++++++-----
 lib/librte_eal/linux/eal/eal.c                    | 29 ++++++-----
 lib/librte_eal/linux/eal/include/rte_kni_common.h |  1 +
 lib/librte_kni/rte_kni.c                          |  7 +--
 9 files changed, 170 insertions(+), 39 deletions(-)

Comments

Thomas Monjalon Nov. 6, 2019, 10:49 a.m. UTC | #1
For info, the mempool patches are merged now.

05/11/2019 12:04, vattunuru@marvell.com:
> Vamsi Attunuru (2):
>   kni: add IOVA=VA mode support
>   kni: add IOVA=VA support in kernel module

Should the kernel support be the first patch?

About the titles, can it be simply "support IOVA mode"?
Vamsi Krishna Attunuru Nov. 6, 2019, 11:09 a.m. UTC | #2
> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Wednesday, November 6, 2019 4:19 PM
> To: Vamsi Krishna Attunuru <vattunuru@marvell.com>
> Cc: dev@dpdk.org; Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Kiran
> Kumar Kokkilagadda <kirankumark@marvell.com>; olivier.matz@6wind.com;
> ferruh.yigit@intel.com; anatoly.burakov@intel.com;
> arybchenko@solarflare.com; stephen@networkplumber.org
> Subject: [EXT] Re: [dpdk-dev] [PATCH v12 0/2] add IOVA=VA mode support
> 
> External Email
> 
> ----------------------------------------------------------------------
> For info, the mempool patches are merged now.
> 
> 05/11/2019 12:04, vattunuru@marvell.com:
> > Vamsi Attunuru (2):
> >   kni: add IOVA=VA mode support
> >   kni: add IOVA=VA support in kernel module
> 
> Should the kernel support be the first patch?

But required variable `iova_mode` is defined in first patch currently. 
Either one is fine to me. Please let me know  if want to put kernel support patch at first, I will send next version accordingly.

> 
> About the titles, can it be simply "support IOVA mode"?

Yes, "support IOVA_VA mode" should be fine.
>
Thomas Monjalon Nov. 6, 2019, 11:53 a.m. UTC | #3
06/11/2019 12:09, Vamsi Krishna Attunuru:
> From: Thomas Monjalon <thomas@monjalon.net>
> > 05/11/2019 12:04, vattunuru@marvell.com:
> > > Vamsi Attunuru (2):
> > >   kni: add IOVA=VA mode support
> > >   kni: add IOVA=VA support in kernel module
> > 
> > Should the kernel support be the first patch?
> 
> But required variable `iova_mode` is defined in first patch currently. 
> Either one is fine to me. Please let me know  if want to put kernel support patch at first, I will send next version accordingly.
> 
> > 
> > About the titles, can it be simply "support IOVA mode"?
> 
> Yes, "support IOVA_VA mode" should be fine.

I really dislike the name IOVA_VA.

You mean support virtual IO addressing?
Vamsi Krishna Attunuru Nov. 6, 2019, 11:59 a.m. UTC | #4
> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Wednesday, November 6, 2019 5:23 PM
> To: Vamsi Krishna Attunuru <vattunuru@marvell.com>
> Cc: dev@dpdk.org; Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Kiran
> Kumar Kokkilagadda <kirankumark@marvell.com>; olivier.matz@6wind.com;
> ferruh.yigit@intel.com; anatoly.burakov@intel.com;
> arybchenko@solarflare.com; stephen@networkplumber.org
> Subject: Re: [EXT] Re: [dpdk-dev] [PATCH v12 0/2] add IOVA=VA mode support
> 
> 06/11/2019 12:09, Vamsi Krishna Attunuru:
> > From: Thomas Monjalon <thomas@monjalon.net>
> > > 05/11/2019 12:04, vattunuru@marvell.com:
> > > > Vamsi Attunuru (2):
> > > >   kni: add IOVA=VA mode support
> > > >   kni: add IOVA=VA support in kernel module
> > >
> > > Should the kernel support be the first patch?
> >
> > But required variable `iova_mode` is defined in first patch currently.
> > Either one is fine to me. Please let me know  if want to put kernel support
> patch at first, I will send next version accordingly.
> >
> > >
> > > About the titles, can it be simply "support IOVA mode"?
> >
> > Yes, "support IOVA_VA mode" should be fine.
> 
> I really dislike the name IOVA_VA.
> 
> You mean support virtual IO addressing?
> 
Yes.
Vamsi Krishna Attunuru Nov. 7, 2019, 10:34 a.m. UTC | #5
Hi Ferruh,

Could  you please review v12.  

 Regards,
A Vamsi

> -----Original Message-----
> From: Vamsi Krishna Attunuru
> Sent: Wednesday, November 6, 2019 4:39 PM
> To: Thomas Monjalon <thomas@monjalon.net>
> Cc: dev@dpdk.org; Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Kiran
> Kumar Kokkilagadda <kirankumark@marvell.com>; olivier.matz@6wind.com;
> ferruh.yigit@intel.com; anatoly.burakov@intel.com;
> arybchenko@solarflare.com; stephen@networkplumber.org
> Subject: RE: [EXT] Re: [dpdk-dev] [PATCH v12 0/2] add IOVA=VA mode support
> 
> 
> 
> > -----Original Message-----
> > From: Thomas Monjalon <thomas@monjalon.net>
> > Sent: Wednesday, November 6, 2019 4:19 PM
> > To: Vamsi Krishna Attunuru <vattunuru@marvell.com>
> > Cc: dev@dpdk.org; Jerin Jacob Kollanukkaran <jerinj@marvell.com>;
> > Kiran Kumar Kokkilagadda <kirankumark@marvell.com>;
> > olivier.matz@6wind.com; ferruh.yigit@intel.com;
> > anatoly.burakov@intel.com; arybchenko@solarflare.com;
> > stephen@networkplumber.org
> > Subject: [EXT] Re: [dpdk-dev] [PATCH v12 0/2] add IOVA=VA mode support
> >
> > External Email
> >
> > ----------------------------------------------------------------------
> > For info, the mempool patches are merged now.
> >
> > 05/11/2019 12:04, vattunuru@marvell.com:
> > > Vamsi Attunuru (2):
> > >   kni: add IOVA=VA mode support
> > >   kni: add IOVA=VA support in kernel module
> >
> > Should the kernel support be the first patch?
> 
> But required variable `iova_mode` is defined in first patch currently.
> Either one is fine to me. Please let me know  if want to put kernel support patch
> at first, I will send next version accordingly.
> 
> >
> > About the titles, can it be simply "support IOVA mode"?
> 
> Yes, "support IOVA_VA mode" should be fine.
> >
Ferruh Yigit Nov. 7, 2019, 7:53 p.m. UTC | #6
On 11/5/2019 11:04 AM, vattunuru@marvell.com wrote:
> From: Vamsi Attunuru <vattunuru@marvell.com>
> 
> ---
> V12 Changes:
> * Removed previously added `--legacy-kni` eal option.
> * Removed previously added kni specific mempool create routines
> and mempool populate routines.
> 
> This patch set(V12) is dependent on following patch set, since the mempool
> related support to enable KNI in IOVA=VA mode is taken care in below
> patchset.
> 
>    https://patchwork.dpdk.org/cover/62376/

Hi Vasim, Jerin,

Overall looks good and I not getting any functional error but I am observing a
huge performance drop with this update, 3.8Mpps to 0.7Mpps [1].

I don't know really what to do, I think we need to give a decision as community,
and even we go with the patch we should document this performance drop clearly
and document how to mitigate it.



[1]
This is with kni sample application,
a) IOVA=VA mode selected
./examples/kni/build/kni -l0,40-47 --log-level=*:debug -- -p 0x3 -P --config
"(0,44,45,40),(1,46,47,41)"

forwarding performance is around 0.7Mpps and 'kni_single' kernel thread consumes
all cpu.

b) IOVA=PA mode forced
./examples/kni/build/kni -l0,40-47 --log-level=*:debug --iova=pa -- -p 0x3 -P
--config "(0,44,45,40),(1,46,47,41)"

forwarding performance is around 3.8Mpps and 'kni_single' core utilization is ~80%.

I am on 5.1.20-300.fc30.x86_64 kernel.
kni module inserted as: "insmod ./build/kmod/rte_kni.ko lo_mode=lo_mode_fifo"

> 
> V11 Changes:
> * Added iova to kva address translation routines in kernel module to
> make it work in iova=va mode which enables DPDK to create kni devices
> on any kind of backed device/memory.
> * Added ``--legacy-kni`` eal option to make existing KNI applications
> work with DPDK 19.11 and later versions.
> * Removed previously added pci device info from kni device info struct.
>  
> 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.
> 
> Vamsi Attunuru (2):
>   kni: add IOVA=VA mode support
>   kni: add IOVA=VA support in kernel module
> 
>  doc/guides/prog_guide/kernel_nic_interface.rst    |  9 ++++
>  doc/guides/rel_notes/release_19_11.rst            |  5 ++
>  kernel/linux/kni/compat.h                         | 15 ++++++
>  kernel/linux/kni/kni_dev.h                        | 42 +++++++++++++++
>  kernel/linux/kni/kni_misc.c                       | 39 ++++++++++----
>  kernel/linux/kni/kni_net.c                        | 62 ++++++++++++++++++-----
>  lib/librte_eal/linux/eal/eal.c                    | 29 ++++++-----
>  lib/librte_eal/linux/eal/include/rte_kni_common.h |  1 +
>  lib/librte_kni/rte_kni.c                          |  7 +--
>  9 files changed, 170 insertions(+), 39 deletions(-)
>
Vamsi Krishna Attunuru Nov. 8, 2019, 4:16 a.m. UTC | #7
> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@intel.com>
> Sent: Friday, November 8, 2019 1:23 AM
> To: Vamsi Krishna Attunuru <vattunuru@marvell.com>; dev@dpdk.org
> Cc: thomas@monjalon.net; Jerin Jacob Kollanukkaran <jerinj@marvell.com>;
> Kiran Kumar Kokkilagadda <kirankumark@marvell.com>;
> olivier.matz@6wind.com; anatoly.burakov@intel.com;
> arybchenko@solarflare.com; stephen@networkplumber.org
> Subject: [EXT] Re: [dpdk-dev] [PATCH v12 0/2] add IOVA=VA mode support
> 
> External Email
> 
> ----------------------------------------------------------------------
> On 11/5/2019 11:04 AM, vattunuru@marvell.com wrote:
> 
> > From: Vamsi Attunuru <vattunuru@marvell.com>
> 
> >
> 
> > ---
> 
> > V12 Changes:
> 
> > * Removed previously added `--legacy-kni` eal option.
> 
> > * Removed previously added kni specific mempool create routines
> 
> > and mempool populate routines.
> 
> >
> 
> > This patch set(V12) is dependent on following patch set, since the mempool
> 
> > related support to enable KNI in IOVA=VA mode is taken care in below
> 
> > patchset.
> 
> >
> 
> >    https://urldefense.proofpoint.com/v2/url?u=https-
> 3A__patchwork.dpdk.org_cover_62376_&d=DwIDaQ&c=nKjWec2b6R0mOyPaz7
> xtfQ&r=WllrYaumVkxaWjgKto6E_rtDQshhIhik2jkvzFyRhW8&m=sEREtIZWQwtHJ
> CxXRRv8euBGdr5q6K4L8Kz7PI25QcY&s=5Rz1dbSeIWt56cCtiwR6pfGprEFUumtcd
> 34TmF3sjs4&e=
> 
> 
> 
> Hi Vasim, Jerin,
> 
> 
> 
> Overall looks good and I not getting any functional error but I am observing a
> 
> huge performance drop with this update, 3.8Mpps to 0.7Mpps [1].	

Hi Ferruh,

When it comes to actual kernel netdev test cases  like iperf or any other use cases, there would not be any impact on performance. I think synthetic test case like loopback mode might not be the actual test case alone to depend on when the kernel module is featured to work with kind of devices(pdev or vdev). Users can always fallback to pa mode with cmd line option.

Please suggest your thoughts on considering what test case to use & evaluate the performance difference.

> 
> 
> 
> I don't know really what to do, I think we need to give a decision as community,
> 
> and even we go with the patch we should document this performance drop
> clearly
> 
> and document how to mitigate it.
> 
> 
> 
> 
> 
> 
> 
> [1]
> 
> This is with kni sample application,
> 
> a) IOVA=VA mode selected
> 
> ./examples/kni/build/kni -l0,40-47 --log-level=*:debug -- -p 0x3 -P --config
> 
> "(0,44,45,40),(1,46,47,41)"
> 
> 
> 
> forwarding performance is around 0.7Mpps and 'kni_single' kernel thread
> consumes
> 
> all cpu.
> 
> 
> 
> b) IOVA=PA mode forced
> 
> ./examples/kni/build/kni -l0,40-47 --log-level=*:debug --iova=pa -- -p 0x3 -P
> 
> --config "(0,44,45,40),(1,46,47,41)"
> 
> 
> 
> forwarding performance is around 3.8Mpps and 'kni_single' core utilization is
> ~80%.
> 
> 
> 
> I am on 5.1.20-300.fc30.x86_64 kernel.
> 
> kni module inserted as: "insmod ./build/kmod/rte_kni.ko
> lo_mode=lo_mode_fifo"
> 
> 
> 
> >
> 
> > V11 Changes:
> 
> > * Added iova to kva address translation routines in kernel module to
> 
> > make it work in iova=va mode which enables DPDK to create kni devices
> 
> > on any kind of backed device/memory.
> 
> > * Added ``--legacy-kni`` eal option to make existing KNI applications
> 
> > work with DPDK 19.11 and later versions.
> 
> > * Removed previously added pci device info from kni device info struct.
> 
> >
> 
> > 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.
> 
> >
> 
> > Vamsi Attunuru (2):
> 
> >   kni: add IOVA=VA mode support
> 
> >   kni: add IOVA=VA support in kernel module
> 
> >
> 
> >  doc/guides/prog_guide/kernel_nic_interface.rst    |  9 ++++
> 
> >  doc/guides/rel_notes/release_19_11.rst            |  5 ++
> 
> >  kernel/linux/kni/compat.h                         | 15 ++++++
> 
> >  kernel/linux/kni/kni_dev.h                        | 42 +++++++++++++++
> 
> >  kernel/linux/kni/kni_misc.c                       | 39 ++++++++++----
> 
> >  kernel/linux/kni/kni_net.c                        | 62 ++++++++++++++++++-----
> 
> >  lib/librte_eal/linux/eal/eal.c                    | 29 ++++++-----
> 
> >  lib/librte_eal/linux/eal/include/rte_kni_common.h |  1 +
> 
> >  lib/librte_kni/rte_kni.c                          |  7 +--
> 
> >  9 files changed, 170 insertions(+), 39 deletions(-)
> 
> >
> 
>
Ferruh Yigit Nov. 8, 2019, 2:26 p.m. UTC | #8
>> Hi Vasim, Jerin,
>>
>> Overall looks good and I not getting any functional error but I am observing a
>> huge performance drop with this update, 3.8Mpps to 0.7Mpps [1].	
> 
> Hi Ferruh,
> When it comes to actual kernel netdev test cases  like iperf or any other use cases, there would not be any impact on performance. I think synthetic test case like loopback mode might not be the actual test case alone to depend on when the kernel module is featured to work with kind of devices(pdev or vdev). Users can always fallback to pa mode with cmd line option.
> 
> Please suggest your thoughts on considering what test case to use & evaluate the performance difference.

Hi Vasim,

I also assume the real life test cases will be affected less, but the loopback
performance testing is good to show performance impact of the change.

(Stephen's predictions that KNI is not as fast as tun/tap are getting more real
by time J)

At least I think the possible performance drop and how to mitigate it should be
documented both in release notes and kni documentation.

For the final decision, I am not objecting it but I would like to see more ack
from community to confirm that we trade off iova=va functionality against
performance.

@Jerin, @Thomas, should we conclude this in techboard? Perhaps we can get it for
rc2 and drop it back if rejected in techboard?


Regards,
ferruh
Jerin Jacob Nov. 8, 2019, 2:54 p.m. UTC | #9
On Fri, Nov 8, 2019 at 7:56 PM Ferruh Yigit <ferruh.yigit@intel.com> wrote:
>
>
> >> Hi Vasim, Jerin,
> >>
> >> Overall looks good and I not getting any functional error but I am observing a
> >> huge performance drop with this update, 3.8Mpps to 0.7Mpps [1].
> >
> > Hi Ferruh,
> > When it comes to actual kernel netdev test cases  like iperf or any other use cases, there would not be any impact on performance. I think synthetic test case like loopback mode might not be the actual test case alone to depend on when the kernel module is featured to work with kind of devices(pdev or vdev). Users can always fallback to pa mode with cmd line option.
> >
> > Please suggest your thoughts on considering what test case to use & evaluate the performance difference.
>
> Hi Vasim,
>
> I also assume the real life test cases will be affected less, but the loopback
> performance testing is good to show performance impact of the change.

Yes. real-world case Linux kernel stack will be the bottleneck.

>
> (Stephen's predictions that KNI is not as fast as tun/tap are getting more real
> by time J)
>
> At least I think the possible performance drop and how to mitigate it should be
> documented both in release notes and kni documentation.

+1 for adding documentation. Setting iova-mode=pa will be mitigation
if the application does
not care about iova-mode.

>
> For the final decision, I am not objecting it but I would like to see more ack
> from community to confirm that we trade off iova=va functionality against
> performance.

In my view, IOVA as VA mode case, translation cannot be avoided and we
have the requirement
where it needs to work with vdev(where is not backed by any IOMMU context) so
I am not sure how to avoid the translation cost. Since we have support
for both modes,i.e
existing IOVA as PA path still exists, I don't think, we are losing anything.

> @Jerin, @Thomas, should we conclude this in techboard? Perhaps we can get it for
> rc2 and drop it back if rejected in techboard?
>
> Regards,
> ferruh
Vamsi Krishna Attunuru Nov. 13, 2019, 6:33 a.m. UTC | #10
> -----Original Message-----
> From: Jerin Jacob <jerinjacobk@gmail.com>
> Sent: Friday, November 8, 2019 8:24 PM
> To: Ferruh Yigit <ferruh.yigit@intel.com>
> Cc: Vamsi Krishna Attunuru <vattunuru@marvell.com>; dev@dpdk.org;
> thomas@monjalon.net; Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Kiran
> Kumar Kokkilagadda <kirankumark@marvell.com>; olivier.matz@6wind.com;
> anatoly.burakov@intel.com; arybchenko@solarflare.com;
> stephen@networkplumber.org
> Subject: Re: [dpdk-dev] [EXT] Re: [PATCH v12 0/2] add IOVA=VA mode support
> 
> On Fri, Nov 8, 2019 at 7:56 PM Ferruh Yigit <ferruh.yigit@intel.com> wrote:
> >
> >
> > >> Hi Vasim, Jerin,
> > >>
> > >> Overall looks good and I not getting any functional error but I am
> > >> observing a huge performance drop with this update, 3.8Mpps to 0.7Mpps
> [1].
> > >
> > > Hi Ferruh,
> > > When it comes to actual kernel netdev test cases  like iperf or any other use
> cases, there would not be any impact on performance. I think synthetic test case
> like loopback mode might not be the actual test case alone to depend on when
> the kernel module is featured to work with kind of devices(pdev or vdev). Users
> can always fallback to pa mode with cmd line option.
> > >
> > > Please suggest your thoughts on considering what test case to use &
> evaluate the performance difference.
> >
> > Hi Vasim,
> >
> > I also assume the real life test cases will be affected less, but the
> > loopback performance testing is good to show performance impact of the
> change.
> 
> Yes. real-world case Linux kernel stack will be the bottleneck.
> 
> >
> > (Stephen's predictions that KNI is not as fast as tun/tap are getting
> > more real by time J)
> >
> > At least I think the possible performance drop and how to mitigate it
> > should be documented both in release notes and kni documentation.
> 
> +1 for adding documentation. Setting iova-mode=pa will be mitigation
> if the application does
> not care about iova-mode.
> 
> >
> > For the final decision, I am not objecting it but I would like to see
> > more ack from community to confirm that we trade off iova=va
> > functionality against performance.
> 
> In my view, IOVA as VA mode case, translation cannot be avoided and we have
> the requirement where it needs to work with vdev(where is not backed by any
> IOMMU context) so I am not sure how to avoid the translation cost. Since we
> have support for both modes,i.e existing IOVA as PA path still exists, I don't
> think, we are losing anything.
> 
> > @Jerin, @Thomas, should we conclude this in techboard? Perhaps we can
> > get it for
> > rc2 and drop it back if rejected in techboard?

Hi Ferruh,

Any update on the conclusion about the acceptance of patch set.  I will send the next version of patch set with updated documentation part on performance impact if there are no other concerns. 

Regards
A Vamsi

> >
> > Regards,
> > ferruh
Ferruh Yigit Nov. 13, 2019, 12:32 p.m. UTC | #11
On 11/13/2019 6:33 AM, Vamsi Krishna Attunuru wrote:
> 
>> -----Original Message-----
>> From: Jerin Jacob <jerinjacobk@gmail.com>
>> Sent: Friday, November 8, 2019 8:24 PM
>> To: Ferruh Yigit <ferruh.yigit@intel.com>
>> Cc: Vamsi Krishna Attunuru <vattunuru@marvell.com>; dev@dpdk.org;
>> thomas@monjalon.net; Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Kiran
>> Kumar Kokkilagadda <kirankumark@marvell.com>; olivier.matz@6wind.com;
>> anatoly.burakov@intel.com; arybchenko@solarflare.com;
>> stephen@networkplumber.org
>> Subject: Re: [dpdk-dev] [EXT] Re: [PATCH v12 0/2] add IOVA=VA mode support
>>
>> On Fri, Nov 8, 2019 at 7:56 PM Ferruh Yigit <ferruh.yigit@intel.com> wrote:
>>>
>>>
>>>>> Hi Vasim, Jerin,
>>>>>
>>>>> Overall looks good and I not getting any functional error but I am
>>>>> observing a huge performance drop with this update, 3.8Mpps to 0.7Mpps
>> [1].
>>>>
>>>> Hi Ferruh,
>>>> When it comes to actual kernel netdev test cases  like iperf or any other use
>> cases, there would not be any impact on performance. I think synthetic test case
>> like loopback mode might not be the actual test case alone to depend on when
>> the kernel module is featured to work with kind of devices(pdev or vdev). Users
>> can always fallback to pa mode with cmd line option.
>>>>
>>>> Please suggest your thoughts on considering what test case to use &
>> evaluate the performance difference.
>>>
>>> Hi Vasim,
>>>
>>> I also assume the real life test cases will be affected less, but the
>>> loopback performance testing is good to show performance impact of the
>> change.
>>
>> Yes. real-world case Linux kernel stack will be the bottleneck.
>>
>>>
>>> (Stephen's predictions that KNI is not as fast as tun/tap are getting
>>> more real by time J)
>>>
>>> At least I think the possible performance drop and how to mitigate it
>>> should be documented both in release notes and kni documentation.
>>
>> +1 for adding documentation. Setting iova-mode=pa will be mitigation
>> if the application does
>> not care about iova-mode.
>>
>>>
>>> For the final decision, I am not objecting it but I would like to see
>>> more ack from community to confirm that we trade off iova=va
>>> functionality against performance.
>>
>> In my view, IOVA as VA mode case, translation cannot be avoided and we have
>> the requirement where it needs to work with vdev(where is not backed by any
>> IOMMU context) so I am not sure how to avoid the translation cost. Since we
>> have support for both modes,i.e existing IOVA as PA path still exists, I don't
>> think, we are losing anything.
>>
>>> @Jerin, @Thomas, should we conclude this in techboard? Perhaps we can
>>> get it for
>>> rc2 and drop it back if rejected in techboard?
> 
> Hi Ferruh,
> 
> Any update on the conclusion about the acceptance of patch set.  I will send the next version of patch set with updated documentation part on performance impact if there are no other concerns. 
> 

Hi Vamsi,

From my perspective there is no other change request than the documentation
update, I suggest sending new version.

For the final decision, it can be in technical board but there is no meeting
this week, so I will start an offline discussion.

Thanks,
ferruh