diff mbox series

[2/2] ethtool: add optional netlink support

Message ID 20210516160129.791846-2-robimarko@gmail.com
State New
Headers show
Series [1/2] ethtool: update to version 5.12 | expand

Commit Message

Robert Marko May 16, 2021, 4:01 p.m. UTC
Netlink support is required for stuff like cable testing,
so offer it as an option.

Signed-off-by: Robert Marko <robimarko@gmail.com>
---
 package/network/utils/ethtool/Makefile | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

Comments

David Bauer May 26, 2021, 6:01 p.m. UTC | #1
Hi Robert,

On 5/16/21 6:01 PM, Robert Marko wrote:
> Netlink support is required for stuff like cable testing,
> so offer it as an option.
> 
> Signed-off-by: Robert Marko <robimarko@gmail.com>
> ---
>  package/network/utils/ethtool/Makefile | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/package/network/utils/ethtool/Makefile b/package/network/utils/ethtool/Makefile
> index d645cf9bbb..8cccf838a0 100644
> --- a/package/network/utils/ethtool/Makefile
> +++ b/package/network/utils/ethtool/Makefile
> @@ -23,7 +23,7 @@ PKG_FIXUP:=autoreconf
>  PKG_INSTALL:=1
>  PKG_BUILD_PARALLEL:=1
>  
> -PKG_CONFIG_DEPENDS:=CONFIG_ETHTOOL_PRETTY_DUMP
> +PKG_CONFIG_DEPENDS:=CONFIG_ETHTOOL_PRETTY_DUMP CONFIG_ETHTOOL_NETLINK
>  
>  include $(INCLUDE_DIR)/package.mk
>  
> @@ -32,6 +32,7 @@ define Package/ethtool
>    CATEGORY:=Network
>    TITLE:=Display or change ethernet card settings
>    URL:=http://www.kernel.org/pub/software/network/ethtool/
> +  DEPENDS:=+ETHTOOL_NETLINK:libmnl
>  endef
>  
>  define Package/ethtool/description
> @@ -43,9 +44,17 @@ define Package/ethtool/config
>  	config ETHTOOL_PRETTY_DUMP
>  		depends on PACKAGE_ethtool
>  		bool "Enable pretty printing"
> +
> +	config ETHTOOL_NETLINK
> +		depends on PACKAGE_ethtool
> +		bool "Enable netlink interface"

I did submit a similar patch a while ago which i didn't yet came across to apply to master.

IMHO, adding netlink support as an additional build variant is superior, as it allows users
to obtain the virtual cable tester via OPKG, which they can not with this approach.

I'm aware this complicates the configuration in menuconfig. FWIW, I'd like to redo the config
options and provide a ethtool-full package in the future, dropping the pretty print build option.

Apart from that, ETHTOOL_NETLINK needs to be enabled for kernel 5.10 in order to actually work.

What do you think?

Best
David

>  endef
>  
> +ifeq ($(CONFIG_ETHTOOL_NETLINK),y)
> +CONFIGURE_ARGS += --enable-netlink
> +else
>  CONFIGURE_ARGS += --disable-netlink
> +endif
>  
>  ifeq ($(CONFIG_ETHTOOL_PRETTY_DUMP),y)
>  CONFIGURE_ARGS += --enable-pretty-dump
>
Robert Marko May 26, 2021, 6:14 p.m. UTC | #2
On Wed, 26 May 2021 at 20:01, David Bauer <mail@david-bauer.net> wrote:
>
> Hi Robert,
>
> On 5/16/21 6:01 PM, Robert Marko wrote:
> > Netlink support is required for stuff like cable testing,
> > so offer it as an option.
> >
> > Signed-off-by: Robert Marko <robimarko@gmail.com>
> > ---
> >  package/network/utils/ethtool/Makefile | 11 ++++++++++-
> >  1 file changed, 10 insertions(+), 1 deletion(-)
> >
> > diff --git a/package/network/utils/ethtool/Makefile b/package/network/utils/ethtool/Makefile
> > index d645cf9bbb..8cccf838a0 100644
> > --- a/package/network/utils/ethtool/Makefile
> > +++ b/package/network/utils/ethtool/Makefile
> > @@ -23,7 +23,7 @@ PKG_FIXUP:=autoreconf
> >  PKG_INSTALL:=1
> >  PKG_BUILD_PARALLEL:=1
> >
> > -PKG_CONFIG_DEPENDS:=CONFIG_ETHTOOL_PRETTY_DUMP
> > +PKG_CONFIG_DEPENDS:=CONFIG_ETHTOOL_PRETTY_DUMP CONFIG_ETHTOOL_NETLINK
> >
> >  include $(INCLUDE_DIR)/package.mk
> >
> > @@ -32,6 +32,7 @@ define Package/ethtool
> >    CATEGORY:=Network
> >    TITLE:=Display or change ethernet card settings
> >    URL:=http://www.kernel.org/pub/software/network/ethtool/
> > +  DEPENDS:=+ETHTOOL_NETLINK:libmnl
> >  endef
> >
> >  define Package/ethtool/description
> > @@ -43,9 +44,17 @@ define Package/ethtool/config
> >       config ETHTOOL_PRETTY_DUMP
> >               depends on PACKAGE_ethtool
> >               bool "Enable pretty printing"
> > +
> > +     config ETHTOOL_NETLINK
> > +             depends on PACKAGE_ethtool
> > +             bool "Enable netlink interface"
>
> I did submit a similar patch a while ago which i didn't yet came across to apply to master.
>
> IMHO, adding netlink support as an additional build variant is superior, as it allows users
> to obtain the virtual cable tester via OPKG, which they can not with this approach.
>
> I'm aware this complicates the configuration in menuconfig. FWIW, I'd like to redo the config
> options and provide a ethtool-full package in the future, dropping the pretty print build option.
>
> Apart from that, ETHTOOL_NETLINK needs to be enabled for kernel 5.10 in order to actually work.
>
> What do you think?

Having a Netlink build variant is fine with me, yeah ETHTOOL_NETLINK
is required.
I don't think we can check that from menuconfig unfortunately.

Regards,
Robert
>
> Best
> David
>
> >  endef
> >
> > +ifeq ($(CONFIG_ETHTOOL_NETLINK),y)
> > +CONFIGURE_ARGS += --enable-netlink
> > +else
> >  CONFIGURE_ARGS += --disable-netlink
> > +endif
> >
> >  ifeq ($(CONFIG_ETHTOOL_PRETTY_DUMP),y)
> >  CONFIGURE_ARGS += --enable-pretty-dump
> >
diff mbox series

Patch

diff --git a/package/network/utils/ethtool/Makefile b/package/network/utils/ethtool/Makefile
index d645cf9bbb..8cccf838a0 100644
--- a/package/network/utils/ethtool/Makefile
+++ b/package/network/utils/ethtool/Makefile
@@ -23,7 +23,7 @@  PKG_FIXUP:=autoreconf
 PKG_INSTALL:=1
 PKG_BUILD_PARALLEL:=1
 
-PKG_CONFIG_DEPENDS:=CONFIG_ETHTOOL_PRETTY_DUMP
+PKG_CONFIG_DEPENDS:=CONFIG_ETHTOOL_PRETTY_DUMP CONFIG_ETHTOOL_NETLINK
 
 include $(INCLUDE_DIR)/package.mk
 
@@ -32,6 +32,7 @@  define Package/ethtool
   CATEGORY:=Network
   TITLE:=Display or change ethernet card settings
   URL:=http://www.kernel.org/pub/software/network/ethtool/
+  DEPENDS:=+ETHTOOL_NETLINK:libmnl
 endef
 
 define Package/ethtool/description
@@ -43,9 +44,17 @@  define Package/ethtool/config
 	config ETHTOOL_PRETTY_DUMP
 		depends on PACKAGE_ethtool
 		bool "Enable pretty printing"
+
+	config ETHTOOL_NETLINK
+		depends on PACKAGE_ethtool
+		bool "Enable netlink interface"
 endef
 
+ifeq ($(CONFIG_ETHTOOL_NETLINK),y)
+CONFIGURE_ARGS += --enable-netlink
+else
 CONFIGURE_ARGS += --disable-netlink
+endif
 
 ifeq ($(CONFIG_ETHTOOL_PRETTY_DUMP),y)
 CONFIGURE_ARGS += --enable-pretty-dump