[PATCH 0/2] ethtool: enable a smaller build

[PATCH 0/2] ethtool: enable a smaller build

[Florian Fainelli]

Hi Ben,

This patchset fixes a missing entry in .gitignore and adds a new
option to the configure script to produced a smaller ethtool executable
(saves about 65% of its size) by disabling register and EEPROM dumps
pretty printing.

Florian Fainelli (2):
  gitignore: ignore test-driver
  ethtool: allow for a smaller build

 .gitignore   | 1 +
 Makefile.am  | 6 +++++-
 configure.ac | 8 ++++++++
 ethtool.c    | 8 ++++++--
 4 files changed, 20 insertions(+), 3 deletions(-)

-- 
1.8.3.2

[PATCH 1/2] gitignore: ignore test-driver

[Florian Fainelli]

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 .gitignore | 1 +
 1 file changed, 1 insertion(+)

diff --git a/.gitignore b/.gitignore
index 45c95015df1b..b03b56f88132 100644
--- a/.gitignore
+++ b/.gitignore
@@ -13,6 +13,7 @@ ethtool.spec
 ethtool.8
 ethtool
 test-cmdline
+test-driver
 test-features
 stamp-h1
 config.*
-- 
1.8.3.2

Re: [PATCH 1/2] gitignore: ignore test-driver

[Ben Hutchings]

[-- Attachment #1: Type: text/plain, Size: 573 bytes --]

On Wed, 2014-02-26 at 17:35 -0800, Florian Fainelli wrote:
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
>  .gitignore | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/.gitignore b/.gitignore
> index 45c95015df1b..b03b56f88132 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -13,6 +13,7 @@ ethtool.spec
>  ethtool.8
>  ethtool
>  test-cmdline
> +test-driver
>  test-features
>  stamp-h1
>  config.*

Applied, thanks.

Ben.

-- 
Ben Hutchings
It is easier to change the specification to fit the program than vice versa.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 811 bytes --]

[PATCH 2/2] ethtool: allow for a smaller build

[Florian Fainelli]

Introduce a new configure script option: --enable-small which will
disable building the driver specific register dump objects, the savings
are:

~500 Kbytes on a x86_64 build
~337 Kbytes on a arm build

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 Makefile.am  | 6 +++++-
 configure.ac | 8 ++++++++
 ethtool.c    | 8 ++++++--
 3 files changed, 19 insertions(+), 3 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index db89d94b168e..05d140008a9c 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -6,11 +6,15 @@ EXTRA_DIST = LICENSE ethtool.8 ethtool.spec.in aclocal.m4 ChangeLog autogen.sh
 
 sbin_PROGRAMS = ethtool
 ethtool_SOURCES = ethtool.c ethtool-copy.h internal.h net_tstamp-copy.h \
+		  rxclass.c
+if !SMALL
+ethtool_SOURCES += \
 		  amd8111e.c de2104x.c e100.c e1000.c et131x.c igb.c	\
 		  fec_8xx.c ibm_emac.c ixgb.c ixgbe.c natsemi.c	\
 		  pcnet32.c realtek.c tg3.c marvell.c vioc.c	\
 		  smsc911x.c at76c50x-usb.c sfc.c stmmac.c	\
-		  rxclass.c sfpid.c sfpdiag.c ixgbevf.c
+		  sfpid.c sfpdiag.c ixgbevf.c
+endif
 
 TESTS = test-cmdline test-features
 check_PROGRAMS = test-cmdline test-features
diff --git a/configure.ac b/configure.ac
index ac7977a777f0..995d937db4c9 100644
--- a/configure.ac
+++ b/configure.ac
@@ -30,5 +30,13 @@ dnl Checks for library functions.
 AC_HEADER_STDC
 AC_CHECK_FUNCS(socket strtol)
 
+dnl Check for options
+AC_ARG_ENABLE(small,
+	       [  --enable-small	  disable registers and EEPROM pretty dumps (disabled by default)],
+	       [if test x$enableval = xyes; then
+			AC_DEFINE(SMALL, 1, [Define this to disable register and EEPROM pretty dumps.]) enable_small=yes
+		fi])
+AM_CONDITIONAL([SMALL], [test x$enable_small = xyes])
+
 AC_CONFIG_FILES([Makefile ethtool.spec ethtool.8])
 AC_OUTPUT
diff --git a/ethtool.c b/ethtool.c
index 315d00f5102f..ff4822389b67 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -877,6 +877,7 @@ static const struct {
 	int (*func)(struct ethtool_drvinfo *info, struct ethtool_regs *regs);
 
 } driver_list[] = {
+#ifndef SMALL
 	{ "8139cp", realtek_dump_regs },
 	{ "8139too", realtek_dump_regs },
 	{ "r8169", realtek_dump_regs },
@@ -903,6 +904,7 @@ static const struct {
 	{ "st_mac100", st_mac100_dump_regs },
 	{ "st_gmac", st_gmac_dump_regs },
 	{ "et131x", et131x_dump_regs },
+#endif
 };
 
 void dump_hex(FILE *file, const u8 *data, int len, int offset)
@@ -971,13 +973,13 @@ static int dump_eeprom(int geeprom_dump_raw, struct ethtool_drvinfo *info,
 		fwrite(ee->data, 1, ee->len, stdout);
 		return 0;
 	}
-
+#ifndef SMALL
 	if (!strncmp("natsemi", info->driver, ETHTOOL_BUSINFO_LEN)) {
 		return natsemi_dump_eeprom(info, ee);
 	} else if (!strncmp("tg3", info->driver, ETHTOOL_BUSINFO_LEN)) {
 		return tg3_dump_eeprom(info, ee);
 	}
-
+#endif
 	dump_hex(stdout, ee->data, ee->len, ee->offset);
 
 	return 0;
@@ -3633,6 +3635,7 @@ static int do_getmodule(struct cmd_context *ctx)
 			geeprom_dump_hex = 1;
 		} else if (!geeprom_dump_hex) {
 			switch (modinfo.type) {
+#ifndef SMALL
 			case ETH_MODULE_SFF_8079:
 				sff8079_show_all(eeprom->data);
 				break;
@@ -3640,6 +3643,7 @@ static int do_getmodule(struct cmd_context *ctx)
 				sff8079_show_all(eeprom->data);
 				sff8472_show_all(eeprom->data);
 				break;
+#endif
 			default:
 				geeprom_dump_hex = 1;
 				break;
-- 
1.8.3.2

RE: [PATCH 2/2] ethtool: allow for a smaller build

[David Laight]

From: Florian Fainelli
> Introduce a new configure script option: --enable-small which will
> disable building the driver specific register dump objects, the savings
> are:
...
> +if !SMALL
> +ethtool_SOURCES += \
>  		  amd8111e.c de2104x.c e100.c e1000.c et131x.c igb.c	\
>  		  fec_8xx.c ibm_emac.c ixgb.c ixgbe.c natsemi.c	\
>  		  pcnet32.c realtek.c tg3.c marvell.c vioc.c	\
>  		  smsc911x.c at76c50x-usb.c sfc.c stmmac.c	\
> -		  rxclass.c sfpid.c sfpdiag.c ixgbevf.c
> +		  sfpid.c sfpdiag.c ixgbevf.c
> +endif

What about allowing a subset of the register dumps be enabled?

	David

Re: [PATCH 2/2] ethtool: allow for a smaller build

[Florian Fainelli]

2014-02-27 2:05 GMT-08:00 David Laight <David.Laight@aculab.com>:
> From: Florian Fainelli
>> Introduce a new configure script option: --enable-small which will
>> disable building the driver specific register dump objects, the savings
>> are:
> ...
>> +if !SMALL
>> +ethtool_SOURCES += \
>>                 amd8111e.c de2104x.c e100.c e1000.c et131x.c igb.c    \
>>                 fec_8xx.c ibm_emac.c ixgb.c ixgbe.c natsemi.c \
>>                 pcnet32.c realtek.c tg3.c marvell.c vioc.c    \
>>                 smsc911x.c at76c50x-usb.c sfc.c stmmac.c      \
>> -               rxclass.c sfpid.c sfpdiag.c ixgbevf.c
>> +               sfpid.c sfpdiag.c ixgbevf.c
>> +endif
>
> What about allowing a subset of the register dumps be enabled?

Sure, I can do that too, as a separate list of changes though. How
would you like to see that impelmented:

--with-drivers="amd8111e de2104x....ixgbevf"

or with individual per-driver switches:

--with-adm8111e
--with-de2104x
-- 
Florian

Re: [PATCH 2/2] ethtool: allow for a smaller build

[Ben Hutchings]

[-- Attachment #1: Type: text/plain, Size: 827 bytes --]

On Thu, 2014-02-27 at 10:05 +0000, David Laight wrote:
> From: Florian Fainelli
> > Introduce a new configure script option: --enable-small which will
> > disable building the driver specific register dump objects, the savings
> > are:
> ...
> > +if !SMALL
> > +ethtool_SOURCES += \
> >  		  amd8111e.c de2104x.c e100.c e1000.c et131x.c igb.c	\
> >  		  fec_8xx.c ibm_emac.c ixgb.c ixgbe.c natsemi.c	\
> >  		  pcnet32.c realtek.c tg3.c marvell.c vioc.c	\
> >  		  smsc911x.c at76c50x-usb.c sfc.c stmmac.c	\
> > -		  rxclass.c sfpid.c sfpdiag.c ixgbevf.c
> > +		  sfpid.c sfpdiag.c ixgbevf.c
> > +endif
> 
> What about allowing a subset of the register dumps be enabled?

I'm not interested in this.

Ben.

-- 
Ben Hutchings
It is easier to change the specification to fit the program than vice versa.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 811 bytes --]

Re: [PATCH 2/2] ethtool: allow for a smaller build

[Ben Hutchings]

[-- Attachment #1: Type: text/plain, Size: 1462 bytes --]

On Wed, 2014-02-26 at 17:35 -0800, Florian Fainelli wrote:
> Introduce a new configure script option: --enable-small which will
> disable building the driver specific register dump objects, the savings
> are:
> 
> ~500 Kbytes on a x86_64 build
> ~337 Kbytes on a arm build

OK, I can see that will be useful in some environments.

[...]
> --- a/configure.ac
> +++ b/configure.ac
> @@ -30,5 +30,13 @@ dnl Checks for library functions.
>  AC_HEADER_STDC
>  AC_CHECK_FUNCS(socket strtol)
>  
> +dnl Check for options
> +AC_ARG_ENABLE(small,
> +	       [  --enable-small	  disable registers and EEPROM pretty dumps (disabled by default)],

That's a slightly confusing description.  It could be read as meaning
that the pretty dumps are disabled by default.

Maybe change the sense of this and call it --enable-pretty-dump, enabled
by default?

> +	       [if test x$enableval = xyes; then
> +			AC_DEFINE(SMALL, 1, [Define this to disable register and EEPROM pretty dumps.]) enable_small=yes
> +		fi])
> +AM_CONDITIONAL([SMALL], [test x$enable_small = xyes])
[...]

As this is defining a macro which is effectively global, please rename
it to a more specific name like ETHTOOL_SMALL (or reversing the sense,
ETHTOOL_ENABLE_PRETTY_DUMP).

Please also make the ethtool --version option report whether it's a
small build.

Ben.

-- 
Ben Hutchings
It is easier to change the specification to fit the program than vice versa.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 811 bytes --]