[v2,0/7] kni: cleanups and improvements
mbox series

Message ID 20190610175155.21374-1-stephen@networkplumber.org
Headers show
Series
  • kni: cleanups and improvements
Related show

Message

Stephen Hemminger June 10, 2019, 5:51 p.m. UTC
While testing KNI with netvsc, saw lots of places more code
could be safely removed from KNI kernel driver.

This is still mostly "putting lipstick on a pig" all users
would be better off using virtio_user rather than KNI.

v2 - get rid of unnecessary padding, combine the unused field patches

Stephen Hemminger (7):
  kni: don't need stubs for rx_mode or ioctl
  kni: use netdev_alloc_skb
  kni: don't keep stats in kni_net
  kni: drop unused fields
  kni: use proper type for kni fifo's
  kni: return -EFAULT if copy_from_user fails
  doc: update KNI documentation

 .../sample_app_ug/kernel_nic_interface.rst    | 18 ++---
 kernel/linux/kni/kni_dev.h                    | 21 ++---
 kernel/linux/kni/kni_misc.c                   | 17 ++--
 kernel/linux/kni/kni_net.c                    | 79 +++++--------------
 4 files changed, 38 insertions(+), 97 deletions(-)

Comments

Stephen Hemminger June 11, 2019, 8:54 p.m. UTC | #1
On Mon, 10 Jun 2019 10:51:48 -0700
Stephen Hemminger <stephen@networkplumber.org> wrote:

> While testing KNI with netvsc, saw lots of places more code
> could be safely removed from KNI kernel driver.
> 
> This is still mostly "putting lipstick on a pig" all users
> would be better off using virtio_user rather than KNI.
> 
> v2 - get rid of unnecessary padding, combine the unused field patches
> 
> Stephen Hemminger (7):
>   kni: don't need stubs for rx_mode or ioctl
>   kni: use netdev_alloc_skb
>   kni: don't keep stats in kni_net
>   kni: drop unused fields
>   kni: use proper type for kni fifo's
>   kni: return -EFAULT if copy_from_user fails
>   doc: update KNI documentation
> 
>  .../sample_app_ug/kernel_nic_interface.rst    | 18 ++---
>  kernel/linux/kni/kni_dev.h                    | 21 ++---
>  kernel/linux/kni/kni_misc.c                   | 17 ++--
>  kernel/linux/kni/kni_net.c                    | 79 +++++--------------
>  4 files changed, 38 insertions(+), 97 deletions(-)
> 

Don't believe patchwork the patch is fine, it is getting falsely blamed
for failures caused by other changes in ice, bnxt which fail
FreeBSD build.
Lance Richardson June 11, 2019, 9:18 p.m. UTC | #2
On Tue, Jun 11, 2019 at 4:54 PM Stephen Hemminger
<stephen@networkplumber.org> wrote:
>
> On Mon, 10 Jun 2019 10:51:48 -0700
> Stephen Hemminger <stephen@networkplumber.org> wrote:
>
> > While testing KNI with netvsc, saw lots of places more code
> > could be safely removed from KNI kernel driver.
> >
> > This is still mostly "putting lipstick on a pig" all users
> > would be better off using virtio_user rather than KNI.
> >
> > v2 - get rid of unnecessary padding, combine the unused field patches
> >
> > Stephen Hemminger (7):
> >   kni: don't need stubs for rx_mode or ioctl
> >   kni: use netdev_alloc_skb
> >   kni: don't keep stats in kni_net
> >   kni: drop unused fields
> >   kni: use proper type for kni fifo's
> >   kni: return -EFAULT if copy_from_user fails
> >   doc: update KNI documentation
> >
> >  .../sample_app_ug/kernel_nic_interface.rst    | 18 ++---
> >  kernel/linux/kni/kni_dev.h                    | 21 ++---
> >  kernel/linux/kni/kni_misc.c                   | 17 ++--
> >  kernel/linux/kni/kni_net.c                    | 79 +++++--------------
> >  4 files changed, 38 insertions(+), 97 deletions(-)
> >
>
> Don't believe patchwork the patch is fine, it is getting falsely blamed
> for failures caused by other changes in ice, bnxt which fail
> FreeBSD build.

Do you mean failures like the ones below? If so, I think think they
might be an unintended
consequence of commit a385972c3 "mk: disable warning for packed member pointer".

    Lance

OS: FreeBSD12-64
Target: x86_64-native-bsdapp-gcc
  CC ice_rxtx.o
  CC ice_ethdev.o
cc1: error: unrecognized command line option
'-Wno-address-of-packed-member' [-Werror]
cc1: all warnings being treated as errors
gmake[6]: *** [/tmp/FreeBSD12-64_K19.02_GCC7.3.0/x86_64-native-bsdapp-gcc/e6c3bc062b1045128114bdf0cfdd236b/dpdk/mk/internal/rte.compile-pre.mk:114:
bnxt_ethdev.o] Error 1
gmake[5]: *** [/tmp/FreeBSD12-64_K19.02_GCC7.3.0/x86_64-native-bsdapp-gcc/e6c3bc062b1045128114bdf0cfdd236b/dpdk/mk/rte.subdir.mk:37:
bnxt] Error 2
gmake[5]: *** Waiting for unfinished jobs....
Stephen Hemminger June 11, 2019, 9:30 p.m. UTC | #3
On Tue, 11 Jun 2019 17:18:42 -0400
Lance Richardson <lance.richardson@broadcom.com> wrote:

> On Tue, Jun 11, 2019 at 4:54 PM Stephen Hemminger
> <stephen@networkplumber.org> wrote:
> >
> > On Mon, 10 Jun 2019 10:51:48 -0700
> > Stephen Hemminger <stephen@networkplumber.org> wrote:
> >  
> > > While testing KNI with netvsc, saw lots of places more code
> > > could be safely removed from KNI kernel driver.
> > >
> > > This is still mostly "putting lipstick on a pig" all users
> > > would be better off using virtio_user rather than KNI.
> > >
> > > v2 - get rid of unnecessary padding, combine the unused field patches
> > >
> > > Stephen Hemminger (7):
> > >   kni: don't need stubs for rx_mode or ioctl
> > >   kni: use netdev_alloc_skb
> > >   kni: don't keep stats in kni_net
> > >   kni: drop unused fields
> > >   kni: use proper type for kni fifo's
> > >   kni: return -EFAULT if copy_from_user fails
> > >   doc: update KNI documentation
> > >
> > >  .../sample_app_ug/kernel_nic_interface.rst    | 18 ++---
> > >  kernel/linux/kni/kni_dev.h                    | 21 ++---
> > >  kernel/linux/kni/kni_misc.c                   | 17 ++--
> > >  kernel/linux/kni/kni_net.c                    | 79 +++++--------------
> > >  4 files changed, 38 insertions(+), 97 deletions(-)
> > >  
> >
> > Don't believe patchwork the patch is fine, it is getting falsely blamed
> > for failures caused by other changes in ice, bnxt which fail
> > FreeBSD build.  
> 
> Do you mean failures like the ones below? If so, I think think they
> might be an unintended
> consequence of commit a385972c3 "mk: disable warning for packed member pointer".
> 
>     Lance
> 
> OS: FreeBSD12-64
> Target: x86_64-native-bsdapp-gcc
>   CC ice_rxtx.o
>   CC ice_ethdev.o
> cc1: error: unrecognized command line option
> '-Wno-address-of-packed-member' [-Werror]
> cc1: all warnings being treated as errors
> gmake[6]: *** [/tmp/FreeBSD12-64_K19.02_GCC7.3.0/x86_64-native-bsdapp-gcc/e6c3bc062b1045128114bdf0cfdd236b/dpdk/mk/internal/rte.compile-pre.mk:114:
> bnxt_ethdev.o] Error 1
> gmake[5]: *** [/tmp/FreeBSD12-64_K19.02_GCC7.3.0/x86_64-native-bsdapp-gcc/e6c3bc062b1045128114bdf0cfdd236b/dpdk/mk/rte.subdir.mk:37:
> bnxt] Error 2
> gmake[5]: *** Waiting for unfinished jobs....


More than just that

*Build Failed Build #1:
OS: FreeBSD12-64
Target: x86_64-native-bsdapp-gcc+debug
  CC ipn3ke_flow.o
  CC lio_23xx_vf.o
cc1: error: unrecognized command line option '-Wno-address-of-packed-member' [-Werror]
cc1: all warnings being treated as errors
gmake[6]: *** [/tmp/FreeBSD12-64_K19.02_GCC7.3.0/x86_64-native-bsdapp-gcc+debug/025b54b548ed4ca894cbf5e754039b68/dpdk/mk/internal/rte.compile-pre.mk:116: bnxt_ethdev.o] Error 1
gmake[5]: *** [/tmp/FreeBSD12-64_K19.02_GCC7.3.0/x86_64-native-bsdapp-gcc+debug/025b54b548ed4ca894cbf5e754039b68/dpdk/mk/rte.subdir.mk:37: bnxt] Error 2
gmake[5]: *** Waiting for unfinished jobs....
  CC i40e_flow.o
  CC rte_eth_null.o
--
  LD ixgbe_ethdev.o
  AR librte_pmd_ixgbe.a
  INSTALL-LIB librte_pmd_ixgbe.a
gmake[4]: *** [/tmp/FreeBSD12-64_K19.02_GCC7.3.0/x86_64-native-bsdapp-gcc+debug/025b54b548ed4ca894cbf5e754039b68/dpdk/mk/rte.subdir.mk:35: net] Error 2
gmake[3]: *** [/tmp/FreeBSD12-64_K19.02_GCC7.3.0/x86_64-native-bsdapp-gcc+debug/025b54b548ed4ca894cbf5e754039b68/dpdk/mk/rte.sdkbuild.mk:46: drivers] Error 2
gmake[2]: *** [/tmp/FreeBSD12-64_K19.02_GCC7.3.0/x86_64-native-bsdapp-gcc+debug/025b54b548ed4ca894cbf5e754039b68/dpdk/mk/rte.sdkroot.mk:99: all] Error 2
gmake[1]: *** [/tmp/FreeBSD12-64_K19.02_GCC7.3.0/x86_64-native-bsdapp-gcc+debug/025b54b548ed4ca894cbf5e754039b68/dpdk/mk/rte.sdkinstall.mk:61: pre_install] Error 2
gmake: *** [/tmp/FreeBSD12-64_K19.02_GCC7.3.0/x86_64-native-bsdapp-gcc+debug/025b54b548ed4ca894cbf5e754039b68/dpdk/mk/rte.sdkroot.mk:77: install] Error 2

*Build Failed Build #2:
OS: FreeBSD12-64
Target: x86_64-native-bsdapp-gcc+shared
  CC rte_eth_null.o.pmd.o
  LD rte_eth_null.o
cc1: error: unrecognized command line option '-Wno-address-of-packed-member' [-Werror]
cc1: all warnings being treated as errors
gmake[6]: *** [/tmp/FreeBSD12-64_K19.02_GCC7.3.0/x86_64-native-bsdapp-gcc+shared/025b54b548ed4ca894cbf5e754039b68/dpdk/mk/internal/rte.compile-pre.mk:116: bnxt_ethdev.o] Error 1
gmake[6]: *** Waiting for unfinished jobs....
  CC ice_rxtx.o
== Build drivers/net/ring
--
  LD ice_ethdev.o
  CC ice_rxtx_vec_sse.o
  CC octeontx_pkovf.o
gmake[5]: *** [/tmp/FreeBSD12-64_K19.02_GCC7.3.0/x86_64-native-bsdapp-gcc+shared/025b54b548ed4ca894cbf5e754039b68/dpdk/mk/rte.subdir.mk:35: bnxt] Error 2
gmake[5]: *** Waiting for unfinished jobs....
  CC i40e_tm.o
  CC ice_rxtx_vec_avx2.o
--
  INSTALL-LIB librte_pmd_ixgbe.so.2.1
  LD librte_pmd_qede.so.1.1
  INSTALL-LIB librte_pmd_qede.so.1.1
gmake[4]: *** [/tmp/FreeBSD12-64_K19.02_GCC7.3.0/x86_64-native-bsdapp-gcc+shared/025b54b548ed4ca894cbf5e754039b68/dpdk/mk/rte.subdir.mk:35: net] Error 2
gmake[3]: *** [/tmp/FreeBSD12-64_K19.02_GCC7.3.0/x86_64-native-bsdapp-gcc+shared/025b54b548ed4ca894cbf5e754039b68/dpdk/mk/rte.sdkbuild.mk:46: drivers] Error 2
gmake[2]: *** [/tmp/FreeBSD12-64_K19.02_GCC7.3.0/x86_64-native-bsdapp-gcc+shared/025b54b548ed4ca894cbf5e754039b68/dpdk/mk/rte.sdkroot.mk:99: all] Error 2
gmake[1]: *** [/tmp/FreeBSD12-64_K19.02_GCC7.3.0/x86_64-native-bsdapp-gcc+shared/025b54b548ed4ca894cbf5e754039b68/dpdk/mk/rte.sdkinstall.mk:61: pre_install] Error 2
gmake: *** [/tmp/FreeBSD12-64_K19.02_GCC7.3.0/x86_64-native-bsdapp-gcc+shared/025b54b548ed4ca894cbf5e754039b68/dpdk/mk/rte.sdkroot.mk:77: install] Error 2

*Build Failed Build #3:
OS: FreeBSD12-64
Target: x86_64-native-bsdapp-gcc
  CC ice_dcb.o
  LD rte_eth_null.o
cc1: error: unrecognized command line option '-Wno-address-of-packed-member' [-Werror]
cc1: all warnings being treated as errors
gmake[6]: *** [/tmp/FreeBSD12-64_K19.02_GCC7.3.0/x86_64-native-bsdapp-gcc/025b54b548ed4ca894cbf5e754039b68/dpdk/mk/internal/rte.compile-pre.mk:114: bnxt_ethdev.o] Error 1
gmake[5]: *** [/tmp/FreeBSD12-64_K19.02_GCC7.3.0/x86_64-native-bsdapp-gcc/025b54b548ed4ca894cbf5e754039b68/dpdk/mk/rte.subdir.mk:37: bnxt] Error 2
gmake[5]: *** Waiting for unfinished jobs....
  CC ixgbe_phy.o
  CC octeontx_rxtx.o
--
  LD qede_ethdev.o
  AR librte_pmd_qede.a
  INSTALL-LIB librte_pmd_qede.a
gmake[4]: *** [/tmp/FreeBSD12-64_K19.02_GCC7.3.0/x86_64-native-bsdapp-gcc/025b54b548ed4ca894cbf5e754039b68/dpdk/mk/rte.subdir.mk:35: net] Error 2
gmake[3]: *** [/tmp/FreeBSD12-64_K19.02_GCC7.3.0/x86_64-native-bsdapp-gcc/025b54b548ed4ca894cbf5e754039b68/dpdk/mk/rte.sdkbuild.mk:46: drivers] Error 2
gmake[2]: *** [/tmp/FreeBSD12-64_K19.02_GCC7.3.0/x86_64-native-bsdapp-gcc/025b54b548ed4ca894cbf5e754039b68/dpdk/mk/rte.sdkroot.mk:99: all] Error 2
gmake[1]: *** [/tmp/FreeBSD12-64_K19.02_GCC7.3.0/x86_64-native-bsdapp-gcc/025b54b548ed4ca894cbf5e754039b68/dpdk/mk/rte.sdkinstall.mk:61: pre_install] Error 2
gmake: *** [/tmp/FreeBSD12-64_K19.02_GCC7.3.0/x86_64-native-bsdapp-gcc/025b54b548ed4ca894cbf5e754039b68/dpdk/mk/rte.sdkroot.mk:77: install] Error 2

*Build Failed Build #4:
OS: FreeBSD12-64
Target: x86_64-native-bsdapp-clang
#define roundup(x, y)   ((((x)+((y)-1))/(y))*(y))  /* to any y */
        ^
1 error generated.
gmake[6]: *** [/tmp/FreeBSD12-64_K19.02_Clang6.0.1/x86_64-native-bsdapp-clang/025b54b548ed4ca894cbf5e754039b68/dpdk/mk/internal/rte.compile-pre.mk:114: bnxt_ethdev.o] Error 1
  CC ice_flex_pipe.o
gmake[5]: *** [/tmp/FreeBSD12-64_K19.02_Clang6.0.1/x86_64-native-bsdapp-clang/025b54b548ed4ca894cbf5e754039b68/dpdk/mk/rte.subdir.mk:35: bnxt] Error 2
gmake[5]: *** Waiting for unfinished jobs....
  CC lio_mbox.o
  AR librte_pmd_null.a
--
  LD ixgbe_ethdev.o
  AR librte_pmd_ixgbe.a
  INSTALL-LIB librte_pmd_ixgbe.a
gmake[4]: *** [/tmp/FreeBSD12-64_K19.02_Clang6.0.1/x86_64-native-bsdapp-clang/025b54b548ed4ca894cbf5e754039b68/dpdk/mk/rte.subdir.mk:35: net] Error 2
gmake[3]: *** [/tmp/FreeBSD12-64_K19.02_Clang6.0.1/x86_64-native-bsdapp-clang/025b54b548ed4ca894cbf5e754039b68/dpdk/mk/rte.sdkbuild.mk:46: drivers] Error 2
gmake[2]: *** [/tmp/FreeBSD12-64_K19.02_Clang6.0.1/x86_64-native-bsdapp-clang/025b54b548ed4ca894cbf5e754039b68/dpdk/mk/rte.sdkroot.mk:99: all] Error 2
gmake[1]: *** [/tmp/FreeBSD12-64_K19.02_Clang6.0.1/x86_64-native-bsdapp-clang/025b54b548ed4ca894cbf5e754039b68/dpdk/mk/rte.sdkinstall.mk:61: pre_install] Error 2
gmake: *** [/tmp/FreeBSD12-64_K19.02_Clang6.0.1/x86_64-native-bsdapp-clang/025b54b548ed4ca894cbf5e754039b68/dpdk/mk/rte.sdkroot.mk:77: install] Error 2
*Meson Failed Build #1:
OS: UB1604-32
Target:build-gcc-static
FAILED: examples/c590b3c@@dpdk-rxtx_callbacks at exe/rxtx_callbacks_main.c.o 
gcc -Iexamples/c590b3c@@dpdk-rxtx_callbacks at exe -Iexamples -I../examples -Iexamples/rxtx_callbacks -I../examples/rxtx_callbacks -I. -I../ -Iconfig -I../config -Ilib/librte_eal/common/include -I../lib/librte_eal/common/include -I../lib/librte_eal/linux/eal/include -Ilib/librte_eal/common -I../lib/librte_eal/common -Ilib/librte_eal/common/include/arch/x86 -I../lib/librte_eal/common/include/arch/x86 -Ilib/librte_eal -I../lib/librte_eal -Ilib/librte_kvargs -I../lib/librte_kvargs -Ilib/librte_mempool -I../lib/librte_mempool -Ilib/librte_ring -I../lib/librte_ring -Ilib/librte_net -I../lib/librte_net -Ilib/librte_mbuf -I../lib/librte_mbuf -Ilib/librte_ethdev -I../lib/librte_ethdev -Ilib/librte_cmdline -I../lib/librte_cmdline -Ilib/librte_meter -I../lib/librte_meter -fdiagnostics-color=always -pipe -D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -Werror -O3 -include rte_config.h -Wunused-parameter -Wsign-compare -Wcast-qual -Wno-pointer-to-int-cast -march=native -D_GNU_SOURCE -DALLOW_EXPERIMENTAL_API -MD -MQ 'examples/c590b3c@@dpdk-rxtx_callbacks at exe/rxtx_callbacks_main.c.o' -MF 'examples/c590b3c@@dpdk-rxtx_callbacks at exe/rxtx_callbacks_main.c.o.d' -o 'examples/c590b3c@@dpdk-rxtx_callbacks at exe/rxtx_callbacks_main.c.o' -c ../examples/rxtx_callbacks/main.c
../examples/rxtx_callbacks/main.c: In function ‘port_init’:
../examples/rxtx_callbacks/main.c:172:10: error: format ‘%lu’ expects argument of type ‘long unsigned int’, but argument 2 has type ‘uint64_t {aka long long unsigned int}’ [-Werror=format=]
   printf("TSC Freq ~= %lu\nHW Freq ~= %lu\nRatio : %f\n",
          ^
../examples/rxtx_callbacks/main.c:172:10: error: format ‘%lu’ expects argument of type ‘long unsigned int’, but argument 3 has type ‘uint64_t {aka long long unsigned int}’ [-Werror=format=]
cc1: all warnings being treated as errors
[1531/1549] Linking target examples/dpdk-vdpa.
[1532/1549] Linking target examples/dpdk-timer.
[1533/1549] Linking target examples/dpdk-tep_termination.
[1534/1549] Compiling C object 'examples/c590b3c@@dpdk-vhost_crypto at exe/vhost_crypto_main.c.o'.
[1535/1549] Compiling C object 'examples/c590b3c@@dpdk-vhost at exe/vhost_main.c.o'.
ninja: build stopped: subcommand failed
Lance Richardson June 11, 2019, 9:49 p.m. UTC | #4
On Tue, Jun 11, 2019 at 5:30 PM Stephen Hemminger
<stephen@networkplumber.org> wrote:
>
> On Tue, 11 Jun 2019 17:18:42 -0400
> Lance Richardson <lance.richardson@broadcom.com> wrote:
>
> > On Tue, Jun 11, 2019 at 4:54 PM Stephen Hemminger
> > <stephen@networkplumber.org> wrote:
> > >
> > > On Mon, 10 Jun 2019 10:51:48 -0700
> > > Stephen Hemminger <stephen@networkplumber.org> wrote:
> > >
> > > > While testing KNI with netvsc, saw lots of places more code
> > > > could be safely removed from KNI kernel driver.
> > > >
> > > > This is still mostly "putting lipstick on a pig" all users
> > > > would be better off using virtio_user rather than KNI.
> > > >
> > > > v2 - get rid of unnecessary padding, combine the unused field patches
> > > >
> > > > Stephen Hemminger (7):
> > > >   kni: don't need stubs for rx_mode or ioctl
> > > >   kni: use netdev_alloc_skb
> > > >   kni: don't keep stats in kni_net
> > > >   kni: drop unused fields
> > > >   kni: use proper type for kni fifo's
> > > >   kni: return -EFAULT if copy_from_user fails
> > > >   doc: update KNI documentation
> > > >
> > > >  .../sample_app_ug/kernel_nic_interface.rst    | 18 ++---
> > > >  kernel/linux/kni/kni_dev.h                    | 21 ++---
> > > >  kernel/linux/kni/kni_misc.c                   | 17 ++--
> > > >  kernel/linux/kni/kni_net.c                    | 79 +++++--------------
> > > >  4 files changed, 38 insertions(+), 97 deletions(-)
> > > >
> > >
> > > Don't believe patchwork the patch is fine, it is getting falsely blamed
> > > for failures caused by other changes in ice, bnxt which fail
> > > FreeBSD build.
> >
> > Do you mean failures like the ones below? If so, I think think they
> > might be an unintended
> > consequence of commit a385972c3 "mk: disable warning for packed member pointer".
> >
> >     Lance
> >
> > OS: FreeBSD12-64
> > Target: x86_64-native-bsdapp-gcc
> >   CC ice_rxtx.o
> >   CC ice_ethdev.o
> > cc1: error: unrecognized command line option
> > '-Wno-address-of-packed-member' [-Werror]
> > cc1: all warnings being treated as errors
> > gmake[6]: *** [/tmp/FreeBSD12-64_K19.02_GCC7.3.0/x86_64-native-bsdapp-gcc/e6c3bc062b1045128114bdf0cfdd236b/dpdk/mk/internal/rte.compile-pre.mk:114:
> > bnxt_ethdev.o] Error 1
> > gmake[5]: *** [/tmp/FreeBSD12-64_K19.02_GCC7.3.0/x86_64-native-bsdapp-gcc/e6c3bc062b1045128114bdf0cfdd236b/dpdk/mk/rte.subdir.mk:37:
> > bnxt] Error 2
> > gmake[5]: *** Waiting for unfinished jobs....
>
>
> More than just that

OK, I see now... I'll spin a fix for this one:

> OS: FreeBSD12-64
> Target: x86_64-native-bsdapp-clang
> #define roundup(x, y)   ((((x)+((y)-1))/(y))*(y))  /* to any y */
>         ^
> 1 error generated.
> gmake[6]: *** [/tmp/FreeBSD12-64_K19.02_Clang6.0.1/x86_64-native-bsdapp-clang/025b54b548ed4ca894cbf5e754039b68/dpdk/mk/internal/rte.compile-pre.mk:114: bnxt_ethdev.o] Error 1

Thanks,

   Lance