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

[Jerin Jacob <jerinjacobk@gmail.com>]

On Fri, Nov 15, 2019 at 6:29 PM David Marchand
<david.marchand@redhat.com> wrote:
>
> 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:

We need achive the following.

IOVA as PA  has performance implication on KNI case. So we need to
make IOVA as PA when KNI module is loaded.
Your suggestion makes IOVA as PA when bus return IOVA as VA due the
fact the KNI check is latter. We need to move that up.

How about the following

[master]dell[dpdk.org] $ git diff
diff --git a/lib/librte_eal/linux/eal/eal.c b/lib/librte_eal/linux/eal/eal.c
index 9e2d50cfb..085fde767 100644
--- a/lib/librte_eal/linux/eal/eal.c
+++ b/lib/librte_eal/linux/eal/eal.c
@@ -1064,6 +1064,21 @@ rte_eal_init(int argc, char **argv)
                /* autodetect the IOVA mapping mode */
                enum rte_iova_mode iova_mode = rte_bus_get_iommu_class();

+#if defined(RTE_LIBRTE_KNI) && KERNEL_VERSION(4, 6, 0) > LINUX_VERSION_CODE
+               /* IOVA as PA gives better performance for KNI. Choose IOVA as
+                 * PA when bus returns RTE_IOVA_DC and KNI module is present.
+                 */
+               if (iova_mode == RTE_IOVA_DC &&
+                               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");
+                       }
+               }
+#endif
+
                if (iova_mode == RTE_IOVA_DC) {
                        RTE_LOG(DEBUG, EAL, "Buses did not request a
specific IOVA mode.\n");

@@ -1085,18 +1100,6 @@ 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
-               /* 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");
-                       }
-               }
-#endif
                rte_eal_get_configuration()->iova_mode = iova_mode;
        } else {
                rte_eal_get_configuration()->iova_mode =
[master]dell[dpdk.org] $
>
> @@ -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
>