Re: [dpdk-dev] [EXT] Re: [PATCH v13 2/2] kni: support IOVA mode

[Vamsi Krishna Attunuru <vattunuru@marvell.com>]

> -----Original Message-----
> From: David Marchand <david.marchand@redhat.com>
> Sent: Friday, November 15, 2019 6:29 PM
> To: Vamsi Krishna Attunuru <vattunuru@marvell.com>; Yigit, Ferruh
> <ferruh.yigit@intel.com>
> Cc: dev <dev@dpdk.org>; Thomas Monjalon <thomas@monjalon.net>; Jerin
> Jacob Kollanukkaran <jerinj@marvell.com>; Kiran Kumar Kokkilagadda
> <kirankumark@marvell.com>; Olivier Matz <olivier.matz@6wind.com>;
> Burakov, Anatoly <anatoly.burakov@intel.com>; Andrew Rybchenko
> <arybchenko@solarflare.com>; Stephen Hemminger
> <stephen@networkplumber.org>
> Subject: [EXT] Re: [dpdk-dev] [PATCH v13 2/2] kni: support IOVA mode
> 
> External Email
> 
> ----------------------------------------------------------------------
> I can't see an interest in splitting this patch from the kmod update.
> Ferruh, what do you think?
> 
> 
> On Fri, Nov 15, 2019 at 12:19 PM <vattunuru@marvell.com> wrote:
> >
> > From: Vamsi Attunuru <vattunuru@marvell.com>
> >
> > Current KNI implementation only operates in IOVA_PA mode patch adds
> > required functionality to enable KNI in IOVA_VA mode.
> >
> > KNI loopback mode tests will have performance impact in this mode due
> > to IOVA to KVA address translations.
> > However, In KNI real world use cases, the performace
> 
> performance
> 
> > impact will be based on Linux kernel stack and scheduler latencies.
> > Performance varies based on the KNI use case.
> > If bus iommu scheme is IOVA_DC and KNI module is loaded, DPDK chooses
> > IOVA as PA as existing behaviour.
> >
> > During KNI creation, app's iova_mode details are passed to the KNI
> > kernel module, accordingly kernel module translates PA/IOVA addresses
> > to KVA and vice-versa.
> >
> > Signed-off-by: Vamsi Attunuru <vattunuru@marvell.com>
> > Signed-off-by: Kiran Kumar K <kirankumark@marvell.com>
> > Suggested-by: Ferruh Yigit <ferruh.yigit@intel.com>
> > ---
> >  doc/guides/prog_guide/kernel_nic_interface.rst | 15 +++++++++++++++
> >  doc/guides/rel_notes/release_19_11.rst         | 15 ++++++++++++++-
> >  lib/librte_eal/linux/eal/eal.c                 | 23 ++++++++++++++++-------
> >  lib/librte_kni/rte_kni.c                       |  6 ++++++
> >  4 files changed, 51 insertions(+), 8 deletions(-)
> >
> 
> [snip]
> 
> > diff --git a/lib/librte_eal/linux/eal/eal.c
> > b/lib/librte_eal/linux/eal/eal.c index 9e2d50c..53ca84b 100644
> > --- a/lib/librte_eal/linux/eal/eal.c
> > +++ b/lib/librte_eal/linux/eal/eal.c
> > @@ -1086,14 +1086,23 @@ rte_eal_init(int argc, char **argv)
> >                         }
> >                 }
> >  #ifdef RTE_LIBRTE_KNI
> > -               /* Workaround for KNI which requires physical address to work */
> > -               if (iova_mode == RTE_IOVA_VA &&
> > -                               rte_eal_check_module("rte_kni") == 1) {
> > -                       if (phys_addrs) {
> > +               if (rte_eal_check_module("rte_kni") == 1) { #if
> > +KERNEL_VERSION(4, 6, 0) > LINUX_VERSION_CODE
> > +                       if (iova_mode == RTE_IOVA_VA) {
> >                                 iova_mode = RTE_IOVA_PA;
> > -                               RTE_LOG(WARNING, EAL, "Forcing IOVA as 'PA' because KNI
> module is loaded\n");
> > -                       } else {
> > -                               RTE_LOG(DEBUG, EAL, "KNI can not work since physical
> addresses are unavailable\n");
> > +                               RTE_LOG(WARNING, EAL, "Forcing IOVA as 'PA' because "
> > +                                               "Kernel version supports only 'PA' mode for KNI
> module\n");
> > +                       }
> > +#endif
> > +                       if (rte_bus_get_iommu_class() == RTE_IOVA_DC)
> > +                               iova_mode = RTE_IOVA_PA;
> 
> If physical addresses are unavailable, this code forces PA anyway.
> 
> 
> > +
> > +                       if (iova_mode == RTE_IOVA_PA) {
> > +                               if (phys_addrs && is_iommu_enabled())
> > +                                       RTE_LOG(WARNING, EAL, "Forced
> > + IOVA as 'PA' because KNI module is loaded\n");
> > +
> > +                               if (!phys_addrs)
> > +                                       RTE_LOG(DEBUG, EAL, "KNI can
> > + not work since physical addresses are unavailable\n");
> >                         }
> 
> Checking physical addresses availability after, and having a log is not enough.
> 
> 
> So far, KNI could not work with IOVA as VA.
> Your patchset adds support for IOVA as VA if kernel is >= 4.6.
> Repeating my proposal (as far as eal.c is concerned) of just changing:

To keep the existing behavior intact when bus iommu returns IOVA_DC, had to handle those case also here.

How about below scheme:

#ifdef RTE_LIBRTE_KNI
-               /* Workaround for KNI which requires physical address to work */
-               if (iova_mode == RTE_IOVA_VA &&
-                               rte_eal_check_module("rte_kni") == 1) {
-                       if (phys_addrs) {
-                               iova_mode = RTE_IOVA_PA;
-                               RTE_LOG(WARNING, EAL, "Forcing IOVA as 'PA' because KNI module is loaded\n");
-                       } else {
-                               RTE_LOG(DEBUG, EAL, "KNI can not work since physical addresses are unavailable\n");
+               if (rte_eal_check_module("rte_kni") == 1) {
+                       bool force_iova_as_pa = false;
+#if KERNEL_VERSION(4, 6, 0) > LINUX_VERSION_CODE
+                       if (iova_mode == RTE_IOVA_VA) {
+                               force_iova_as_pa = true;
+                               RTE_LOG(WARNING, EAL, "Forcing IOVA as 'PA' because "
+                                               "Kernel version supports only 'PA' mode for KNI module\n");
+                       }
+#endif
+                       if (rte_bus_get_iommu_class() == RTE_IOVA_DC)
+                               force_iova_as_pa = true;
+
+                       if (force_iova_as_pa) {
+                               if (phys_addrs) {
+                                       iova_mode = RTE_IOVA_PA;
+                                       RTE_LOG(WARNING, EAL, "Forced IOVA as 'PA' because KNI module is loaded\n");
+                               } else
+                                       RTE_LOG(DEBUG, EAL, "KNI can not work since physical addresses are unavailable\n");
                        }
                }
 #endif



> 
> @@ -1085,7 +1085,7 @@ rte_eal_init(int argc, char **argv)
>                                 RTE_LOG(DEBUG, EAL, "IOMMU is not available, selecting
> IOVA as PA mode.\n");
>                         }
>                 }
> -#ifdef RTE_LIBRTE_KNI
> +#if defined(RTE_LIBRTE_KNI) && KERNEL_VERSION(4, 6, 0) >
> +LINUX_VERSION_CODE
>                 /* Workaround for KNI which requires physical address to work */
>                 if (iova_mode == RTE_IOVA_VA &&
> 
> 
> 
> --
> David Marchand