[PATCH 0/2] Ethtool: EEE support

[PATCH 0/2] Ethtool: EEE support

[Yuval Mintz]

Hi Ben,

This patch series follows another series sent to net-next
which adds EEE support in the Ethtool API.

This series extends the ethtool application and allows it
to control the eee feature via the new '--get-eee' and
and '--set-eee' commands.

Thanks,
Yuval Mintz

[PATCH 1/2] Ethtool: Add EEE support

[Yuval Mintz]

This patch adds 2 new ethtool commands which can be
used to manipulate network interfaces' support in
EEE.

Output of 'get' has the following form:

	EEE Settings for p2p1:
		EEE status: enabled - active
		Tx LPI: 1000 (u)
		Supported EEE link modes:  10000baseT/Full
		Advertised EEE link modes:  10000baseT/Full
		Link partner advertised EEE link modes:  10000baseT/Full

Thanks goes to Giuseppe Cavallaro for his original patch.

Signed-off-by: Yuval Mintz <yuvalmin@broadcom.com>
Signed-off-by: Eilon Greenstein <eilong@broadcom.com>
---
 ethtool-copy.h |   15 ++++++
 ethtool.c      |  139 ++++++++++++++++++++++++++++++++++++++++++++++++--------
 2 files changed, 134 insertions(+), 20 deletions(-)

diff --git a/ethtool-copy.h b/ethtool-copy.h
index 9e26a76..1fcc799 100644
--- a/ethtool-copy.h
+++ b/ethtool-copy.h
@@ -133,6 +133,19 @@ struct ethtool_eeprom {
 	__u8	data[0];
 };
 
+/* EEE settings */
+struct ethtool_eee {
+	__u32	cmd;
+	__u32	supported;
+	__u32	advertised;
+	__u32	lp_advertised;
+	__u32	eee_active;
+	__u32	eee_enabled;
+	__u32	tx_lpi_enabled;
+	__u32	tx_lpi_timer;
+	__u32	reserved[2];
+};
+
 /**
  * struct ethtool_modinfo - plugin module eeprom information
  * @cmd: %ETHTOOL_GMODULEINFO
@@ -848,6 +861,8 @@ enum ethtool_sfeatures_retval_bits {
 #define ETHTOOL_GET_TS_INFO	0x00000041 /* Get time stamping and PHC info */
 #define ETHTOOL_GMODULEINFO	0x00000042 /* Get plug-in module information */
 #define ETHTOOL_GMODULEEEPROM	0x00000043 /* Get plug-in module eeprom */
+#define ETHTOOL_GEEE		0x00000044 /* Get EEE settings */
+#define ETHTOOL_SEEE		0x00000045 /* Set EEE settings */
 
 /* compatibility with older code */
 #define SPARC_ETH_GSET		ETHTOOL_GSET
diff --git a/ethtool.c b/ethtool.c
index f18f611..063e72b 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -359,7 +359,8 @@ static int do_version(struct cmd_context *ctx)
 	return 0;
 }
 
-static void dump_link_caps(const char *prefix, const char *an_prefix, u32 mask);
+static void dump_link_caps(const char *prefix, const char *an_prefix, u32 mask,
+			    u8 all);
 
 static void dump_supported(struct ethtool_cmd *ep)
 {
@@ -378,14 +379,14 @@ static void dump_supported(struct ethtool_cmd *ep)
 		fprintf(stdout, "FIBRE ");
 	fprintf(stdout, "]\n");
 
-	dump_link_caps("Supported", "Supports", mask);
+	dump_link_caps("Supported", "Supports", mask, 1);
 }
 
 /* Print link capability flags (supported, advertised or lp_advertised).
  * Assumes that the corresponding SUPPORTED and ADVERTISED flags are equal.
  */
 static void
-dump_link_caps(const char *prefix, const char *an_prefix, u32 mask)
+dump_link_caps(const char *prefix, const char *an_prefix, u32 mask, u8 all)
 {
 	int indent;
 	int did1;
@@ -456,24 +457,26 @@ dump_link_caps(const char *prefix, const char *an_prefix, u32 mask)
 		 fprintf(stdout, "Not reported");
 	fprintf(stdout, "\n");
 
-	fprintf(stdout, "	%s pause frame use: ", prefix);
-	if (mask & ADVERTISED_Pause) {
-		fprintf(stdout, "Symmetric");
-		if (mask & ADVERTISED_Asym_Pause)
-			fprintf(stdout, " Receive-only");
-		fprintf(stdout, "\n");
-	} else {
-		if (mask & ADVERTISED_Asym_Pause)
-			fprintf(stdout, "Transmit-only\n");
+	if (all) {
+		fprintf(stdout, "	%s pause frame use: ", prefix);
+		if (mask & ADVERTISED_Pause) {
+			fprintf(stdout, "Symmetric");
+			if (mask & ADVERTISED_Asym_Pause)
+				fprintf(stdout, " Receive-only");
+			fprintf(stdout, "\n");
+		} else {
+			if (mask & ADVERTISED_Asym_Pause)
+				fprintf(stdout, "Transmit-only\n");
+			else
+				fprintf(stdout, "No\n");
+		}
+
+		fprintf(stdout, "	%s auto-negotiation: ", an_prefix);
+		if (mask & ADVERTISED_Autoneg)
+			fprintf(stdout, "Yes\n");
 		else
 			fprintf(stdout, "No\n");
 	}
-
-	fprintf(stdout, "	%s auto-negotiation: ", an_prefix);
-	if (mask & ADVERTISED_Autoneg)
-		fprintf(stdout, "Yes\n");
-	else
-		fprintf(stdout, "No\n");
 }
 
 static int dump_ecmd(struct ethtool_cmd *ep)
@@ -481,10 +484,11 @@ static int dump_ecmd(struct ethtool_cmd *ep)
 	u32 speed;
 
 	dump_supported(ep);
-	dump_link_caps("Advertised", "Advertised", ep->advertising);
+	dump_link_caps("Advertised", "Advertised", ep->advertising, 1);
 	if (ep->lp_advertising)
 		dump_link_caps("Link partner advertised",
-			       "Link partner advertised", ep->lp_advertising);
+			       "Link partner advertised", ep->lp_advertising,
+			       1);
 
 	fprintf(stdout, "	Speed: ");
 	speed = ethtool_cmd_speed(ep);
@@ -1116,6 +1120,36 @@ static int dump_rxfhash(int fhash, u64 val)
 	return 0;
 }
 
+static int dump_eeecmd(struct ethtool_eee *ep)
+{
+
+	fprintf(stdout, "	EEE status: ");
+	if (!ep->supported) {
+		fprintf(stdout, "not supported\n");
+		return 0;
+	} else if (!ep->eee_enabled) {
+		fprintf(stdout, "disabled\n");
+	} else {
+		fprintf(stdout, "enabled - ");
+		if (ep->eee_active)
+			fprintf(stdout, "active\n");
+		else
+			fprintf(stdout, "inactive\n");
+	}
+
+	fprintf(stdout, "	Tx LPI:");
+	if (ep->tx_lpi_enabled)
+		fprintf(stdout, " %d (u)\n", ep->tx_lpi_timer);
+	else
+		fprintf(stdout, " disabled\n");
+
+	dump_link_caps("Supported EEE", "", ep->supported, 0);
+	dump_link_caps("Advertised EEE", "", ep->advertised, 0);
+	dump_link_caps("Link partner advertised EEE", "", ep->lp_advertised, 0);
+
+	return 0;
+}
+
 #define N_SOTS 7
 
 static char *so_timestamping_labels[N_SOTS] = {
@@ -3261,6 +3295,65 @@ static int do_getmodule(struct cmd_context *ctx)
 	return 0;
 }
 
+static int do_geee(struct cmd_context *ctx)
+{
+	struct ethtool_eee eeecmd;
+
+	if (ctx->argc != 0)
+		exit_bad_args();
+
+	fprintf(stdout, "EEE Settings for %s:\n", ctx->devname);
+
+	eeecmd.cmd = ETHTOOL_GEEE;
+	if (send_ioctl(ctx, &eeecmd)) {
+		perror("Cannot get EEE settings");
+		return 1;
+	}
+
+	if (dump_eeecmd(&eeecmd))
+		return 1;
+
+	return 0;
+}
+
+static int do_seee(struct cmd_context *ctx)
+{
+	int adv_c = -1, lpi_c = -1, lpi_time_c = -1, eee_c = -1;
+	int change = -1, change2 = -1;
+	struct ethtool_eee eeecmd;
+	struct cmdline_info cmdline_eee[] = {
+		{ "advertise",    CMDL_U32,  &adv_c,       &eeecmd.advertised },
+		{ "tx-lpi",       CMDL_BOOL, &lpi_c,   &eeecmd.tx_lpi_enabled },
+		{ "tx-timer",	  CMDL_U32,  &lpi_time_c, &eeecmd.tx_lpi_timer},
+		{ "eee",	  CMDL_BOOL, &eee_c,	   &eeecmd.eee_enabled},
+	};
+
+	if (ctx->argc == 0)
+		exit_bad_args();
+
+	parse_generic_cmdline(ctx, &change, cmdline_eee,
+			      ARRAY_SIZE(cmdline_eee));
+
+	eeecmd.cmd = ETHTOOL_GEEE;
+	if (send_ioctl(ctx, &eeecmd)) {
+		perror("Cannot get EEE settings");
+		return 1;
+	}
+
+	do_generic_set(cmdline_eee, ARRAY_SIZE(cmdline_eee), &change2);
+
+	if (change2) {
+
+		eeecmd.cmd = ETHTOOL_SEEE;
+		if (send_ioctl(ctx, &eeecmd)) {
+			perror("Cannot set EEE settings");
+			return 1;
+		}
+	}
+
+	return 1;
+}
+
 int send_ioctl(struct cmd_context *ctx, void *cmd)
 {
 #ifndef TEST_ETHTOOL
@@ -3423,6 +3516,12 @@ static const struct option {
 	  "		[ hex on|off ]\n"
 	  "		[ offset N ]\n"
 	  "		[ length N ]\n" },
+	{ "--get-eee", 1, do_geee, "Get EEE settings"},
+	{ "--set-eee", 1, do_seee, "Set EEE settings",
+	  "		[ eee on|off ]\n"
+	  "		[ advertise %x ]\n"
+	  "		[ tx-lpi on|off ]\n"
+	  "		[ tx-timer %x ]\n"},
 	{ "-h|--help", 0, show_usage, "Show this help" },
 	{ "--version", 0, do_version, "Show version number" },
 	{}
-- 
1.7.9.rc2

Re: [PATCH 1/2] Ethtool: Add EEE support

[Ben Hutchings]

On Tue, 2012-06-05 at 09:41 +0300, Yuval Mintz wrote:
> This patch adds 2 new ethtool commands which can be
> used to manipulate network interfaces' support in
> EEE.
> 
> Output of 'get' has the following form:
> 
> 	EEE Settings for p2p1:
> 		EEE status: enabled - active
> 		Tx LPI: 1000 (u)
> 		Supported EEE link modes:  10000baseT/Full
> 		Advertised EEE link modes:  10000baseT/Full
> 		Link partner advertised EEE link modes:  10000baseT/Full
> 
> Thanks goes to Giuseppe Cavallaro for his original patch.
[...]
> diff --git a/ethtool.c b/ethtool.c
> index f18f611..063e72b 100644
> --- a/ethtool.c
> +++ b/ethtool.c
> @@ -359,7 +359,8 @@ static int do_version(struct cmd_context *ctx)
>  	return 0;
>  }
>  
> -static void dump_link_caps(const char *prefix, const char *an_prefix, u32 mask);
> +static void dump_link_caps(const char *prefix, const char *an_prefix, u32 mask,
> +			    u8 all);

For now, use int for booleans.  At some point I would like to see a
thorough cleanup of ethtool to use bool where appropriate - but that's
independent of this.

[...]
>  /* Print link capability flags (supported, advertised or lp_advertised).
>   * Assumes that the corresponding SUPPORTED and ADVERTISED flags are equal.
>   */
>  static void
> -dump_link_caps(const char *prefix, const char *an_prefix, u32 mask)
> +dump_link_caps(const char *prefix, const char *an_prefix, u32 mask, u8 all)
>  {
>  	int indent;
>  	int did1;
> @@ -456,24 +457,26 @@ dump_link_caps(const char *prefix, const char *an_prefix, u32 mask)
>  		 fprintf(stdout, "Not reported");
>  	fprintf(stdout, "\n");
>  
> -	fprintf(stdout, "	%s pause frame use: ", prefix);
> -	if (mask & ADVERTISED_Pause) {
> -		fprintf(stdout, "Symmetric");
> -		if (mask & ADVERTISED_Asym_Pause)
> -			fprintf(stdout, " Receive-only");
> -		fprintf(stdout, "\n");
> -	} else {
> -		if (mask & ADVERTISED_Asym_Pause)
> -			fprintf(stdout, "Transmit-only\n");
> +	if (all) {

It might be clearer to invert this flag and name it something like
'link_mode_only'.

> +		fprintf(stdout, "	%s pause frame use: ", prefix);
> +		if (mask & ADVERTISED_Pause) {
> +			fprintf(stdout, "Symmetric");
> +			if (mask & ADVERTISED_Asym_Pause)
> +				fprintf(stdout, " Receive-only");
> +			fprintf(stdout, "\n");
> +		} else {
> +			if (mask & ADVERTISED_Asym_Pause)
> +				fprintf(stdout, "Transmit-only\n");
> +			else
> +				fprintf(stdout, "No\n");
> +		}
> +
> +		fprintf(stdout, "	%s auto-negotiation: ", an_prefix);
> +		if (mask & ADVERTISED_Autoneg)
> +			fprintf(stdout, "Yes\n");
>  		else
>  			fprintf(stdout, "No\n");
>  	}
> -
> -	fprintf(stdout, "	%s auto-negotiation: ", an_prefix);
> -	if (mask & ADVERTISED_Autoneg)
> -		fprintf(stdout, "Yes\n");
> -	else
> -		fprintf(stdout, "No\n");
>  }
>  
>  static int dump_ecmd(struct ethtool_cmd *ep)
[...]
> @@ -1116,6 +1120,36 @@ static int dump_rxfhash(int fhash, u64 val)
>  	return 0;
>  }
>  
> +static int dump_eeecmd(struct ethtool_eee *ep)

Is there any reason for this not to return void?

> +{
> +
> +	fprintf(stdout, "	EEE status: ");
> +	if (!ep->supported) {
> +		fprintf(stdout, "not supported\n");
> +		return 0;
> +	} else if (!ep->eee_enabled) {
> +		fprintf(stdout, "disabled\n");
> +	} else {
> +		fprintf(stdout, "enabled - ");
> +		if (ep->eee_active)
> +			fprintf(stdout, "active\n");
> +		else
> +			fprintf(stdout, "inactive\n");
> +	}
> +
> +	fprintf(stdout, "	Tx LPI:");
> +	if (ep->tx_lpi_enabled)
> +		fprintf(stdout, " %d (u)\n", ep->tx_lpi_timer);

"us" not "(u)"

> +	else
> +		fprintf(stdout, " disabled\n");
> +
> +	dump_link_caps("Supported EEE", "", ep->supported, 0);
> +	dump_link_caps("Advertised EEE", "", ep->advertised, 0);
> +	dump_link_caps("Link partner advertised EEE", "", ep->lp_advertised, 0);
> +
> +	return 0;
> +}
> +
[...]
> +static int do_seee(struct cmd_context *ctx)
> +{
> +	int adv_c = -1, lpi_c = -1, lpi_time_c = -1, eee_c = -1;
> +	int change = -1, change2 = -1;
> +	struct ethtool_eee eeecmd;
> +	struct cmdline_info cmdline_eee[] = {
> +		{ "advertise",    CMDL_U32,  &adv_c,       &eeecmd.advertised },
> +		{ "tx-lpi",       CMDL_BOOL, &lpi_c,   &eeecmd.tx_lpi_enabled },
> +		{ "tx-timer",	  CMDL_U32,  &lpi_time_c, &eeecmd.tx_lpi_timer},
> +		{ "eee",	  CMDL_BOOL, &eee_c,	   &eeecmd.eee_enabled},
> +	};
> +
> +	if (ctx->argc == 0)
> +		exit_bad_args();
> +
> +	parse_generic_cmdline(ctx, &change, cmdline_eee,
> +			      ARRAY_SIZE(cmdline_eee));
> +
> +	eeecmd.cmd = ETHTOOL_GEEE;
> +	if (send_ioctl(ctx, &eeecmd)) {
> +		perror("Cannot get EEE settings");
> +		return 1;
> +	}
> +
> +	do_generic_set(cmdline_eee, ARRAY_SIZE(cmdline_eee), &change2);
> +
> +	if (change2) {
> +
> +		eeecmd.cmd = ETHTOOL_SEEE;
> +		if (send_ioctl(ctx, &eeecmd)) {
> +			perror("Cannot set EEE settings");
> +			return 1;
> +		}
> +	}
> +
> +	return 1;

return 0, I think!

> +}
> +
>  int send_ioctl(struct cmd_context *ctx, void *cmd)
>  {
>  #ifndef TEST_ETHTOOL
> @@ -3423,6 +3516,12 @@ static const struct option {
>  	  "		[ hex on|off ]\n"
>  	  "		[ offset N ]\n"
>  	  "		[ length N ]\n" },
> +	{ "--get-eee", 1, do_geee, "Get EEE settings"},
> +	{ "--set-eee", 1, do_seee, "Set EEE settings",
> +	  "		[ eee on|off ]\n"
> +	  "		[ advertise %x ]\n"
> +	  "		[ tx-lpi on|off ]\n"
> +	  "		[ tx-timer %x ]\n"},

The tx-timer value would normally be specified in decimal, so put "%d"
here.

>  	{ "-h|--help", 0, show_usage, "Show this help" },
>  	{ "--version", 0, do_version, "Show version number" },
>  	{}

You also need to add some test cases for the command line parsing in
test-cmdline.c.

Ben.

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

RE: [PATCH 1/2] Ethtool: Add EEE support

[Yuval Mintz]

Hi Ben,

I stand corrected; I'll supply a new version shortly. 

I've got 2 questions  though:

> For now, use int for booleans.  At some point I would like to see a
> thorough cleanup of ethtool to use bool where appropriate - but that's
> independent of this

I've noticed that the 'do_generic_set' function assumes all fields are ints.
Is this a convention we should stick to (using  __u32 in the ethtool structs)?

I'm asking because I'm "wasting" fields in the ethtool_eee struct as I use
__u32 for boolean fields, simply because what seems to be the conventional
method won't work with smaller fields (corrupts the following fields).

The seconds question - is there a dependency between your acceptance of
this patch series and Dave's acceptance of the kernel's ethtool modification?
I'm asking because changes in the ethtool header there should be applied in
this patch series as well (in ethtool-copy.h).

Thanks,
Yuval 

RE: [PATCH 1/2] Ethtool: Add EEE support

[Ben Hutchings]

On Wed, 2012-06-06 at 16:27 +0000, Yuval Mintz wrote:
> Hi Ben,
> 
> I stand corrected; I'll supply a new version shortly. 
> 
> I've got 2 questions  though:
> 
> > For now, use int for booleans.  At some point I would like to see a
> > thorough cleanup of ethtool to use bool where appropriate - but that's
> > independent of this
> 
> I've noticed that the 'do_generic_set' function assumes all fields are ints.

Yes, but that can and should be fixed.

> Is this a convention we should stick to (using  __u32 in the ethtool structs)?

Yes.  It is generally preferable to use explicitly fixed-size types in
kernel-userland ABI, even though I wouldn't expect the size of int to
change for many decades to come.

> I'm asking because I'm "wasting" fields in the ethtool_eee struct as I use
> __u32 for boolean fields, simply because what seems to be the conventional
> method won't work with smaller fields (corrupts the following fields).

It's slightly wasteful but not worth worrying about.

> The seconds question - is there a dependency between your acceptance of
> this patch series and Dave's acceptance of the kernel's ethtool modification?
> I'm asking because changes in the ethtool header there should be applied in
> this patch series as well (in ethtool-copy.h).

There is a dependency, yes.  However I generally update ethtool-copy.h
as a separate commit myself (as you can see in the git commit log for
it) so I won't demand that you refresh the version in your patch.

Ben.

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

[PATCH 2/2] Ethtool: add EEE to ethtool's documentation

[Yuval Mintz]

under Synopsis:
	ethtool --get-eee devname
	ethtool --set-eee devname [eee on|off] [tx-lpi on|off] [tx-timer N] [advertise N]

under Options:
       --get-eee
              Queries  the  specified network device for its support in Efficient Energy Ethernet (ac-
	      cording to the IEEE 802.3az specifications)
       --set-eee
	      Sets the device EEE behaviour.
       eee on|off
	      Enables/Disables the device support in EEE.
       tx-lpi on|off
	      Determines whether the device should assert its tx lpi.
       advertise N
              Sets the speeds for which the device would advertise EEE capabliities.   Values  are  as
	      for --change advertise
       tx-timer N
              Sets  the  amount  of time the device should stay in idle mode prior to asserting its tx
              lpi (in microseconds). This has meaning only when tx lpi is on.

Signed-off-by: Yuval Mintz <yuvalmin@broadcom.com>
Signed-off-by: Eilon Greenstein <eilong@broadcom.com>
---
 ethtool.8.in |   32 ++++++++++++++++++++++++++++++++
 1 files changed, 32 insertions(+), 0 deletions(-)

diff --git a/ethtool.8.in b/ethtool.8.in
index 523b737..b906d8e 100644
--- a/ethtool.8.in
+++ b/ethtool.8.in
@@ -335,6 +335,16 @@ ethtool \- query or control network driver and hardware settings
 .I devname flag
 .A1 on off
 .RB ...
+.HP
+.B ethtool \-\-get\-eee
+.I devname
+.HP
+.B ethtool \-\-set\-eee
+.I devname
+.B2 eee on off
+.B2 tx-lpi on off
+.BN tx-timer
+.BN advertise
 .
 .\" Adjust lines (i.e. full justification) and hyphenate.
 .ad
@@ -817,6 +827,28 @@ Sets the device's private flags as specified.
 .I flag
 .A1 on off
 Sets the state of the named private flag.
+.TP
+.B \-\-get\-eee
+Queries the specified network device for its support in Efficient Energy
+Ethernet (according to the IEEE 802.3az specifications)
+.TP
+.B \-\-set\-eee
+Sets the device EEE behaviour.
+.TP
+.A2 eee on off
+Enables/Disables the device support in EEE.
+.TP
+.A2 tx-lpi on off
+Determines whether the device should assert its tx lpi.
+.TP
+.BI advertise \ N
+Sets the speeds for which the device would advertise EEE capabliities.
+Values are as for
+.B \-\-change advertise
+.TP
+.BI tx-timer \ N
+Sets the amount of time the device should stay in idle mode prior to asserting
+its tx lpi (in microseconds). This has meaning only when tx lpi is on.
 .SH BUGS
 Not supported (in part or whole) on all network drivers.
 .SH AUTHOR
-- 
1.7.9.rc2

Re: [PATCH 2/2] Ethtool: add EEE to ethtool's documentation

[Ben Hutchings]

On Tue, 2012-06-05 at 09:41 +0300, Yuval Mintz wrote:
> under Synopsis:
> 	ethtool --get-eee devname
> 	ethtool --set-eee devname [eee on|off] [tx-lpi on|off] [tx-timer N] [advertise N]
> 
> under Options:
>        --get-eee
>               Queries  the  specified network device for its support in Efficient Energy Ethernet (ac-
> 	      cording to the IEEE 802.3az specifications)
>        --set-eee
> 	      Sets the device EEE behaviour.
>        eee on|off
> 	      Enables/Disables the device support in EEE.
>        tx-lpi on|off
> 	      Determines whether the device should assert its tx lpi.
>        advertise N
>               Sets the speeds for which the device would advertise EEE capabliities.   Values  are  as
> 	      for --change advertise
>        tx-timer N
>               Sets  the  amount  of time the device should stay in idle mode prior to asserting its tx
>               lpi (in microseconds). This has meaning only when tx lpi is on.

Please just fold this change into the patch that adds the options.  Also
there is no need to repeat the content in the commit message.

> Signed-off-by: Yuval Mintz <yuvalmin@broadcom.com>
> Signed-off-by: Eilon Greenstein <eilong@broadcom.com>
> ---
>  ethtool.8.in |   32 ++++++++++++++++++++++++++++++++
>  1 files changed, 32 insertions(+), 0 deletions(-)
> 
> diff --git a/ethtool.8.in b/ethtool.8.in
> index 523b737..b906d8e 100644
> --- a/ethtool.8.in
> +++ b/ethtool.8.in
> @@ -335,6 +335,16 @@ ethtool \- query or control network driver and hardware settings
>  .I devname flag
>  .A1 on off
>  .RB ...
> +.HP
> +.B ethtool \-\-get\-eee
> +.I devname
> +.HP
> +.B ethtool \-\-set\-eee
> +.I devname
> +.B2 eee on off
> +.B2 tx-lpi on off
> +.BN tx-timer
> +.BN advertise
>  .
>  .\" Adjust lines (i.e. full justification) and hyphenate.
>  .ad
> @@ -817,6 +827,28 @@ Sets the device's private flags as specified.
>  .I flag
>  .A1 on off
>  Sets the state of the named private flag.
> +.TP
> +.B \-\-get\-eee
> +Queries the specified network device for its support in Efficient Energy
> +Ethernet (according to the IEEE 802.3az specifications)

'of', not 'in'

> +.TP
> +.B \-\-set\-eee
> +Sets the device EEE behaviour.
> +.TP
> +.A2 eee on off
> +Enables/Disables the device support in EEE.

Lower-case 'd' for 'disables'.
'of', not 'in'

> +.TP
> +.A2 tx-lpi on off
> +Determines whether the device should assert its tx lpi.

'TX LPI' should be in upper-case.

> +.TP
> +.BI advertise \ N
> +Sets the speeds for which the device would advertise EEE capabliities.

'would', not 'should'
'capabilities', not 'capabliities'

> +Values are as for
> +.B \-\-change advertise
> +.TP
> +.BI tx-timer \ N
> +Sets the amount of time the device should stay in idle mode prior to asserting
> +its tx lpi (in microseconds). This has meaning only when tx lpi is on.

Same here.

Ben.

>  .SH BUGS
>  Not supported (in part or whole) on all network drivers.
>  .SH AUTHOR

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.