[1/2] kni: flag deprecated status at build time

Message ID 20221005143451.157613-1-bruce.richardson@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Andrew Rybchenko
Headers
Series [1/2] kni: flag deprecated status at build time |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Bruce Richardson Oct. 5, 2022, 2:34 p.m. UTC
  To ensure all users are aware of KNI's deprecated status at build time
we can take the following actions:

1. disable the library by default. It can be re-enabled by setting
   disabled_libs to the empty string (or other string not including
   'kni')

2. add support for a list of deprecated libs to the lib/meson.build file
   so the error message for KNI being disabled includes the fact that it
   is a deprecated library.

3. for the dependent NIC driver, drivers/net/kni, modify its disabled
   message to also reference the fact that KNI is disabled.

NOTE: This is part of the deprecation process for KNI agreed by the DPDK
technical board.[1]

[1] http://mails.dpdk.org/archives/dev/2022-June/243596.html

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 doc/guides/prog_guide/kernel_nic_interface.rst | 4 ++++
 drivers/net/kni/meson.build                    | 5 +++++
 lib/meson.build                                | 7 +++++++
 meson_options.txt                              | 2 +-
 4 files changed, 17 insertions(+), 1 deletion(-)
  

Comments

Morten Brørup Oct. 5, 2022, 2:45 p.m. UTC | #1
> From: Bruce Richardson [mailto:bruce.richardson@intel.com]
> Sent: Wednesday, 5 October 2022 16.35
> 
> To ensure all users are aware of KNI's deprecated status at build time
> we can take the following actions:
> 
> 1. disable the library by default. It can be re-enabled by setting
>    disabled_libs to the empty string (or other string not including
>    'kni')
> 
> 2. add support for a list of deprecated libs to the lib/meson.build
> file
>    so the error message for KNI being disabled includes the fact that
> it
>    is a deprecated library.
> 
> 3. for the dependent NIC driver, drivers/net/kni, modify its disabled
>    message to also reference the fact that KNI is disabled.
> 
> NOTE: This is part of the deprecation process for KNI agreed by the
> DPDK
> technical board.[1]
> 
> [1] http://mails.dpdk.org/archives/dev/2022-June/243596.html
> 
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> ---

Probably not required, but cannot hurt either:

Series-acked-by: Morten Brørup <mb@smartsharesystems.com>
  
David Marchand Oct. 5, 2022, 8:10 p.m. UTC | #2
On Wed, Oct 5, 2022 at 4:35 PM Bruce Richardson
<bruce.richardson@intel.com> wrote:
>
> To ensure all users are aware of KNI's deprecated status at build time
> we can take the following actions:
>
> 1. disable the library by default. It can be re-enabled by setting
>    disabled_libs to the empty string (or other string not including
>    'kni')
>
> 2. add support for a list of deprecated libs to the lib/meson.build file
>    so the error message for KNI being disabled includes the fact that it
>    is a deprecated library.
>
> 3. for the dependent NIC driver, drivers/net/kni, modify its disabled
>    message to also reference the fact that KNI is disabled.

Let's say I want to re-enable the net/kni driver.
How should I do this?

Passing -Denable_drivers=net/kni results in only getting net/kni.
Passing -Denable_drivers=*/* gives the same result as passing none:
net/kni is disabled because relying on a deprecated lib.

>
> NOTE: This is part of the deprecation process for KNI agreed by the DPDK
> technical board.[1]
>
> [1] http://mails.dpdk.org/archives/dev/2022-June/243596.html
>
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
  
Bruce Richardson Oct. 7, 2022, 10:34 a.m. UTC | #3
On Wed, Oct 05, 2022 at 10:10:10PM +0200, David Marchand wrote:
> On Wed, Oct 5, 2022 at 4:35 PM Bruce Richardson
> <bruce.richardson@intel.com> wrote:
> >
> > To ensure all users are aware of KNI's deprecated status at build time
> > we can take the following actions:
> >
> > 1. disable the library by default. It can be re-enabled by setting
> >    disabled_libs to the empty string (or other string not including
> >    'kni')
> >
> > 2. add support for a list of deprecated libs to the lib/meson.build file
> >    so the error message for KNI being disabled includes the fact that it
> >    is a deprecated library.
> >
> > 3. for the dependent NIC driver, drivers/net/kni, modify its disabled
> >    message to also reference the fact that KNI is disabled.
> 
> Let's say I want to re-enable the net/kni driver.
> How should I do this?
> 
> Passing -Denable_drivers=net/kni results in only getting net/kni.
> Passing -Denable_drivers=*/* gives the same result as passing none:
> net/kni is disabled because relying on a deprecated lib.
> 

Well, the error message still says that the library part is missing and
that is why is has been disabled. In terms of enabling the library itself,
from the doc part of the patch:

+.. note::
+
+   KNI is disabled by default in the DPDK build.
+   To re-enable the library, remove 'kni' from the "disable_libs" meson option when configuring a build.

I can similarly add a note to the net/kni documentation to say that it will
be disabled by default because KNI is, and to re-enable it just enable KNI
lib as described there. Is something like that needed, you think?

/Bruce
  
David Marchand Oct. 7, 2022, 11:08 a.m. UTC | #4
Hello Bruce,

On Wed, Oct 5, 2022 at 4:35 PM Bruce Richardson
<bruce.richardson@intel.com> wrote:
> diff --git a/drivers/net/kni/meson.build b/drivers/net/kni/meson.build
> index 2acc989694..2deb2c4166 100644
> --- a/drivers/net/kni/meson.build
> +++ b/drivers/net/kni/meson.build
> @@ -6,6 +6,11 @@ if is_windows
>      reason = 'not supported on Windows'
>      subdir_done()
>  endif
> +if not dpdk_conf.has('kni')

I think it should be:

if not dpdk_conf.has('RTE_LIB_KNI')

And to reply to your recent comment, I don't think we need to update
this driver documentation.
The meson log is enough to figure out why net/kni is disabled.

> +    build = false
> +    reason = 'needs deprecated lib, "kni"'
> +    subdir_done()
> +endif
>
>  deps += 'kni'
>  sources = files('rte_eth_kni.c')
  
Bruce Richardson Oct. 7, 2022, 1:05 p.m. UTC | #5
On Fri, Oct 07, 2022 at 01:08:38PM +0200, David Marchand wrote:
> Hello Bruce,
> 
> On Wed, Oct 5, 2022 at 4:35 PM Bruce Richardson
> <bruce.richardson@intel.com> wrote:
> > diff --git a/drivers/net/kni/meson.build b/drivers/net/kni/meson.build
> > index 2acc989694..2deb2c4166 100644
> > --- a/drivers/net/kni/meson.build
> > +++ b/drivers/net/kni/meson.build
> > @@ -6,6 +6,11 @@ if is_windows
> >      reason = 'not supported on Windows'
> >      subdir_done()
> >  endif
> > +if not dpdk_conf.has('kni')
> 
> I think it should be:
> 
> if not dpdk_conf.has('RTE_LIB_KNI')
> 

Ah, yes it should, sorry. Thanks for catching that. Will do V2 with this fixed.

/Bruce
  

Patch

diff --git a/doc/guides/prog_guide/kernel_nic_interface.rst b/doc/guides/prog_guide/kernel_nic_interface.rst
index 03b5bca958..6a564f61ca 100644
--- a/doc/guides/prog_guide/kernel_nic_interface.rst
+++ b/doc/guides/prog_guide/kernel_nic_interface.rst
@@ -14,6 +14,10 @@  Kernel NIC Interface
    For an alternative to KNI, that does not require any out-of-tree Linux kernel modules,
    or a custom library, see :ref:`virtio_user_as_exception_path`.
 
+.. note::
+
+   KNI is disabled by default in the DPDK build.
+   To re-enable the library, remove 'kni' from the "disable_libs" meson option when configuring a build.
 
 The DPDK Kernel NIC Interface (KNI) allows userspace applications access to the Linux* control plane.
 
diff --git a/drivers/net/kni/meson.build b/drivers/net/kni/meson.build
index 2acc989694..2deb2c4166 100644
--- a/drivers/net/kni/meson.build
+++ b/drivers/net/kni/meson.build
@@ -6,6 +6,11 @@  if is_windows
     reason = 'not supported on Windows'
     subdir_done()
 endif
+if not dpdk_conf.has('kni')
+    build = false
+    reason = 'needs deprecated lib, "kni"'
+    subdir_done()
+endif
 
 deps += 'kni'
 sources = files('rte_eth_kni.c')
diff --git a/lib/meson.build b/lib/meson.build
index c648f7d800..264a1671df 100644
--- a/lib/meson.build
+++ b/lib/meson.build
@@ -85,6 +85,7 @@  optional_libs = [
         'vhost',
 ]
 
+deprecated_libs = ['kni']
 disabled_libs = []
 opt_disabled_libs = run_command(list_dir_globs, get_option('disable_libs'),
         check: true).stdout().split()
@@ -133,7 +134,13 @@  foreach l:libraries
     if disabled_libs.contains(l)
         build = false
         reason = 'explicitly disabled via build config'
+        if deprecated_libs.contains(l)
+            reason += ' (deprecated lib)'
+        endif
     else
+        if deprecated_libs.contains(l)
+            warning('Enabling deprecated library, "@0@"'.format(l))
+        endif
         subdir(l)
     endif
     if name != l
diff --git a/meson_options.txt b/meson_options.txt
index 8640f599ae..39af78787c 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -8,7 +8,7 @@  option('developer_mode', type: 'feature', description:
        'turn on additional build checks relevant for DPDK developers')
 option('disable_drivers', type: 'string', value: '', description:
        'Comma-separated list of drivers to explicitly disable.')
-option('disable_libs', type: 'string', value: '', description:
+option('disable_libs', type: 'string', value: 'kni', description:
        'Comma-separated list of libraries to explicitly disable. [NOTE: not all libs can be disabled]')
 option('drivers_install_subdir', type: 'string', value: 'dpdk/pmds-<VERSION>', description:
        'Subdirectory of libdir where to install PMDs. Defaults to using a versioned subdirectory.')