[v12,1/2] kni: add IOVA=VA mode support

  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 can operate in IOVA_VA mode when this mode is requested
using eal option (`iova-mode=va`) or when bus iommu scheme is
selected as RTE_IOVA_VA.

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    |  9 +++++++
 doc/guides/rel_notes/release_19_11.rst            |  5 ++++
 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 ++----
 5 files changed, 34 insertions(+), 17 deletions(-)
  

On Tue, Nov 5, 2019 at 12:05 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 can operate in IOVA_VA mode when this mode is requested
> using eal option (`iova-mode=va`) or when bus iommu scheme is
> selected as RTE_IOVA_VA.
>
> 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    |  9 +++++++
>  doc/guides/rel_notes/release_19_11.rst            |  5 ++++
>  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 ++----
>  5 files changed, 34 insertions(+), 17 deletions(-)
>
> diff --git a/doc/guides/prog_guide/kernel_nic_interface.rst b/doc/guides/prog_guide/kernel_nic_interface.rst
> index 2fd58e1..f869493 100644
> --- a/doc/guides/prog_guide/kernel_nic_interface.rst
> +++ b/doc/guides/prog_guide/kernel_nic_interface.rst
> @@ -300,6 +300,15 @@ The sk_buff is then freed and the mbuf sent in the tx_q FIFO.
>  The DPDK TX thread dequeues the mbuf and sends it to the PMD via ``rte_eth_tx_burst()``.
>  It then puts the mbuf back in the cache.
>
> +IOVA = VA: Support
> +------------------
> +
> +KNI operates in IOVA_VA scheme when
> +
> +- LINUX_VERSION_CODE >= KERNEL_VERSION(4, 6, 0) and
> +- eal option `iova-mode=va` is passed or bus IOVA scheme in the DPDK is selected
> +  as RTE_IOVA_VA.
> +
>  Ethtool
>  -------
>
> diff --git a/doc/guides/rel_notes/release_19_11.rst b/doc/guides/rel_notes/release_19_11.rst
> index ae8e7b2..e5d2996 100644
> --- a/doc/guides/rel_notes/release_19_11.rst
> +++ b/doc/guides/rel_notes/release_19_11.rst
> @@ -231,6 +231,11 @@ New Features
>    * Added a console command to testpmd app, ``show port (port_id) ptypes`` which
>      gives ability to print port supported ptypes in different protocol layers.
>
> +* **Added IOVA as VA support for KNI.**
> +
> +  Added IOVA = VA support for KNI, KNI can operate in IOVA = VA mode when
> +  `iova-mode=va` eal option is passed to the application or when bus IOVA
> +  scheme is selected as RTE_IOVA_VA.

Add a mention about the kernel requirement here too please.

Missing newline.

>
>  Removed Items
>  -------------
> diff --git a/lib/librte_eal/linux/eal/eal.c b/lib/librte_eal/linux/eal/eal.c
> index 9e2d50c..a1c5bf6 100644
> --- a/lib/librte_eal/linux/eal/eal.c
> +++ b/lib/librte_eal/linux/eal/eal.c
> @@ -922,6 +922,19 @@ static int rte_eal_vfio_setup(void)
>  }
>  #endif
>
> +static enum rte_iova_mode
> +rte_eal_kni_get_iova_mode(enum rte_iova_mode iova_mode)
> +{
> +       if (iova_mode == RTE_IOVA_VA) {
> +#if KERNEL_VERSION(4, 6, 0) > LINUX_VERSION_CODE

I understood that IOVA as VA is possible with kernel >= 4.6.
Should it be < ?


> +               iova_mode = RTE_IOVA_PA;
> +               RTE_LOG(WARNING, EAL, "Forcing IOVA as 'PA' because KNI module does not support VA\n");
> +#endif
> +       }
> +
> +       return iova_mode;
> +}
> +
>  static void rte_eal_init_alert(const char *msg)
>  {
>         fprintf(stderr, "EAL: FATAL: %s\n", msg);
> @@ -1085,24 +1098,16 @@ 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

You removed this part, but we don't have to consider kni things if
RTE_LIBRTE_KNI is not compiled in.


> -               /* 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) {

The check on physical address availability has been lost.


> -                               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 =
>                         internal_config.iova_mode;
>         }
>
> +       if (rte_eal_check_module("rte_kni") == 1)
> +               rte_eal_get_configuration()->iova_mode =
> +                               rte_eal_kni_get_iova_mode(rte_eal_iova_mode());
> +

You are forcing this even if the user specified a iova mode.


In the end, can't we just change the check on the workaround?
Something like:

@@ -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 &&
                                rte_eal_check_module("rte_kni") == 1) {


It would be the only change in the whole file lib/librte_eal/linux/eal/eal.c


>         if (rte_eal_iova_mode() == RTE_IOVA_PA && !phys_addrs) {
>                 rte_eal_init_alert("Cannot use IOVA as 'PA' since physical addresses are not available");
>                 rte_errno = EINVAL;
> diff --git a/lib/librte_eal/linux/eal/include/rte_kni_common.h b/lib/librte_eal/linux/eal/include/rte_kni_common.h
> index 46f75a7..2427a96 100644
> --- a/lib/librte_eal/linux/eal/include/rte_kni_common.h
> +++ b/lib/librte_eal/linux/eal/include/rte_kni_common.h
> @@ -125,6 +125,7 @@ struct rte_kni_device_info {
>         unsigned int min_mtu;
>         unsigned int max_mtu;
>         uint8_t mac_addr[6];
> +       uint8_t iova_mode;
>  };
>
>  #define KNI_DEVICE "kni"
> diff --git a/lib/librte_kni/rte_kni.c b/lib/librte_kni/rte_kni.c
> index 7fbcf22..7221280 100644
> --- a/lib/librte_kni/rte_kni.c
> +++ b/lib/librte_kni/rte_kni.c
> @@ -97,11 +97,6 @@ static volatile int kni_fd = -1;
>  int
>  rte_kni_init(unsigned int max_kni_ifaces __rte_unused)
>  {

Should this check still apply with kernel < 4.6?
Like:
+ #if KERNEL_VERSION(4, 6, 0) < LINUX_VERSION_CODE
> -       if (rte_eal_iova_mode() != RTE_IOVA_PA) {
> -               RTE_LOG(ERR, KNI, "KNI requires IOVA as PA\n");
> -               return -1;
> -       }
> -
+ #endif


>         /* Check FD and open */
>         if (kni_fd < 0) {
>                 kni_fd = open("/dev/" KNI_DEVICE, O_RDWR);
> @@ -302,6 +297,8 @@ rte_kni_alloc(struct rte_mempool *pktmbuf_pool,
>         kni->group_id = conf->group_id;
>         kni->mbuf_size = conf->mbuf_size;
>
> +       dev_info.iova_mode = (rte_eal_iova_mode() == RTE_IOVA_VA) ? 1 : 0;
> +
>         ret = ioctl(kni_fd, RTE_KNI_IOCTL_CREATE, &dev_info);
>         if (ret < 0)
>                 goto ioctl_fail;
> --
> 2.8.4
>
  

On Thu, Nov 14, 2019 at 11:57 AM David Marchand
<david.marchand@redhat.com> wrote:
> On Tue, Nov 5, 2019 at 12:05 PM <vattunuru@marvell.com> wrote:
> > diff --git a/lib/librte_eal/linux/eal/eal.c b/lib/librte_eal/linux/eal/eal.c
> > index 9e2d50c..a1c5bf6 100644
> > --- a/lib/librte_eal/linux/eal/eal.c
> > +++ b/lib/librte_eal/linux/eal/eal.c
> > @@ -922,6 +922,19 @@ static int rte_eal_vfio_setup(void)
> >  }
> >  #endif
> >
> > +static enum rte_iova_mode
> > +rte_eal_kni_get_iova_mode(enum rte_iova_mode iova_mode)
> > +{
> > +       if (iova_mode == RTE_IOVA_VA) {
> > +#if KERNEL_VERSION(4, 6, 0) > LINUX_VERSION_CODE
>
> I understood that IOVA as VA is possible with kernel >= 4.6.
> Should it be < ?

I meant "Should the #if check be < ?".

I did not mention this first, but thinking again, don't we have an issue here?
This won't work for distributions that ship dpdk with kni built with dkms.

Cc: Luca.
  

> -----Original Message-----
> From: David Marchand <david.marchand@redhat.com>
> Sent: Thursday, November 14, 2019 4:43 PM
> To: Vamsi Krishna Attunuru <vattunuru@marvell.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>; Yigit,
> Ferruh <ferruh.yigit@intel.com>; Burakov, Anatoly
> <anatoly.burakov@intel.com>; Andrew Rybchenko
> <arybchenko@solarflare.com>; Stephen Hemminger
> <stephen@networkplumber.org>; Luca Boccassi <bluca@debian.org>
> Subject: [EXT] Re: [dpdk-dev] [PATCH v12 1/2] kni: add IOVA=VA mode support
> 
> External Email
> 
> ----------------------------------------------------------------------
> On Thu, Nov 14, 2019 at 11:57 AM David Marchand
> <david.marchand@redhat.com> wrote:
> > On Tue, Nov 5, 2019 at 12:05 PM <vattunuru@marvell.com> wrote:
> > > diff --git a/lib/librte_eal/linux/eal/eal.c
> > > b/lib/librte_eal/linux/eal/eal.c index 9e2d50c..a1c5bf6 100644
> > > --- a/lib/librte_eal/linux/eal/eal.c
> > > +++ b/lib/librte_eal/linux/eal/eal.c
> > > @@ -922,6 +922,19 @@ static int rte_eal_vfio_setup(void)  }  #endif
> > >
> > > +static enum rte_iova_mode
> > > +rte_eal_kni_get_iova_mode(enum rte_iova_mode iova_mode) {
> > > +       if (iova_mode == RTE_IOVA_VA) { #if KERNEL_VERSION(4, 6, 0)
> > > +> LINUX_VERSION_CODE
> >
> > I understood that IOVA as VA is possible with kernel >= 4.6.
> > Should it be < ?
> 
> I meant "Should the #if check be < ?".
> 
> I did not mention this first, but thinking again, don't we have an issue here?
> This won't work for distributions that ship dpdk with kni built with dkms.
> 

Hi David,

Are you referring the following problem, like if dpdk was build with some older kernel (<4.6) version and shipped, later kernel was updated to 4.8 and kni module got build with newer kernel.?, In this case, kni application will continue work in PA mode without any issues. KNI module decides it's iova mode that application has communicated during kni allocation. If the user expectation is to make KNI application work in VA mode, the check has to be dynamic than. Please correct me if the issue that is anticipated is something different.

BTW, eal has few of these kernel version checks in header files too(rte_vfio.h & eal_vfio.h), not sure if it would cause similar issues.

Regards
A Vamsi

> Cc: Luca.
> 
> 
> --
> David Marchand
  

On Thu, Nov 14, 2019 at 1:10 PM Vamsi Krishna Attunuru
<vattunuru@marvell.com> wrote:
>
>
>
> > -----Original Message-----
> > From: David Marchand <david.marchand@redhat.com>
> > Sent: Thursday, November 14, 2019 4:43 PM
> > To: Vamsi Krishna Attunuru <vattunuru@marvell.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>; Yigit,
> > Ferruh <ferruh.yigit@intel.com>; Burakov, Anatoly
> > <anatoly.burakov@intel.com>; Andrew Rybchenko
> > <arybchenko@solarflare.com>; Stephen Hemminger
> > <stephen@networkplumber.org>; Luca Boccassi <bluca@debian.org>
> > Subject: [EXT] Re: [dpdk-dev] [PATCH v12 1/2] kni: add IOVA=VA mode support
> >
> > External Email
> >
> > ----------------------------------------------------------------------
> > On Thu, Nov 14, 2019 at 11:57 AM David Marchand
> > <david.marchand@redhat.com> wrote:
> > > On Tue, Nov 5, 2019 at 12:05 PM <vattunuru@marvell.com> wrote:
> > > > diff --git a/lib/librte_eal/linux/eal/eal.c
> > > > b/lib/librte_eal/linux/eal/eal.c index 9e2d50c..a1c5bf6 100644
> > > > --- a/lib/librte_eal/linux/eal/eal.c
> > > > +++ b/lib/librte_eal/linux/eal/eal.c
> > > > @@ -922,6 +922,19 @@ static int rte_eal_vfio_setup(void)  }  #endif
> > > >
> > > > +static enum rte_iova_mode
> > > > +rte_eal_kni_get_iova_mode(enum rte_iova_mode iova_mode) {
> > > > +       if (iova_mode == RTE_IOVA_VA) { #if KERNEL_VERSION(4, 6, 0)
> > > > +> LINUX_VERSION_CODE
> > >
> > > I understood that IOVA as VA is possible with kernel >= 4.6.
> > > Should it be < ?
> >
> > I meant "Should the #if check be < ?".
> >
> > I did not mention this first, but thinking again, don't we have an issue here?
> > This won't work for distributions that ship dpdk with kni built with dkms.
> >
>
> Hi David,
>
> Are you referring the following problem, like if dpdk was build with some older kernel (<4.6) version and shipped, later kernel was updated to 4.8 and kni module got build with newer kernel.?, In this case, kni application will continue work in PA mode without any issues. KNI module decides it's iova mode that application has communicated during kni allocation. If the user expectation is to make KNI application work in VA mode, the check has to be dynamic than. Please correct me if the issue that is anticipated is something different.

The dpdk application will request PA (since compiled in with no regard
to the actual kni capabilities) and will refuse to run as non root
user.
Which is already the case before this change.
Ok, maybe something to improve later if people wants to use kni as non root.


My other comments still apply though.

--
David Marchand
  

On Thu, Nov 14, 2019 at 11:57 AM David Marchand
<david.marchand@redhat.com> wrote:
> On Tue, Nov 5, 2019 at 12:05 PM <vattunuru@marvell.com> wrote:
> > diff --git a/lib/librte_eal/linux/eal/eal.c b/lib/librte_eal/linux/eal/eal.c
> > index 9e2d50c..a1c5bf6 100644
> > --- a/lib/librte_eal/linux/eal/eal.c
> > +++ b/lib/librte_eal/linux/eal/eal.c
> > @@ -922,6 +922,19 @@ static int rte_eal_vfio_setup(void)
> >  }
> >  #endif
> >
> > +static enum rte_iova_mode
> > +rte_eal_kni_get_iova_mode(enum rte_iova_mode iova_mode)
> > +{
> > +       if (iova_mode == RTE_IOVA_VA) {
> > +#if KERNEL_VERSION(4, 6, 0) > LINUX_VERSION_CODE
>
> I understood that IOVA as VA is possible with kernel >= 4.6.
> Should it be < ?

I had read it backwards.
So, the check is ok.

[snip]

> In the end, can't we just change the check on the workaround?
> Something like:
>
> @@ -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

Then it becomes:

+#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 &&
>                                 rte_eal_check_module("rte_kni") == 1) {
>
>
> It would be the only change in the whole file lib/librte_eal/linux/eal/eal.c