| Message ID | 20210516160129.791846-2-robimarko@gmail.com |
|---|---|
| State | Changes Requested |
| Delegated to: | David Bauer |
| Headers | show |
| Series | [1/2] ethtool: update to version 5.12 | expand |
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 >
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 --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
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(-)