[PATCH ethtool v4 0/2] Adding downshift support to ethtool

[PATCH ethtool v4 0/2] Adding downshift support to ethtool

[Allan W. Nielsen]

(downshift feature is applied in the net-next tree - d3c19c0a72)

This series adds support for downshift (using phy-tunables).

Downshifting can either be turned on/off, or it can be configured to a
specifc count.

"count" is optional.

Change set:
v1:
- Initial version of set/get phy tunable with downshift feature.
v2:
- (ethtool) Syntax is changed from "--set-phy-tunable downshift on|off|%d"
  to "--set-phy-tunable [downshift on|off [count N]]" - as requested by
  Andrew.
v3:
- Fixed Spelling in "ethtool-copy.h:sync with net" 
- Fixed "if send_ioctl() returns an error, print the error message and then
  still print th value of count".
v4:
- Fixing spelling in the example included in the commit message
- Improve the description in the man-page

Raju Lakkaraju (2):
  ethtool-copy.h:sync with net
  Ethtool: Implements ETHTOOL_PHY_GTUNABLE/ETHTOOL_PHY_STUNABLE and PHY
    downshift

 ethtool-copy.h |  18 +++++++-
 ethtool.8.in   |  40 ++++++++++++++++
 ethtool.c      | 144 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 201 insertions(+), 1 deletion(-)

-- 
2.7.3

[PATCH ethtool v4 1/2] ethtool-copy.h:sync with net

[Allan W. Nielsen]

From: Raju Lakkaraju <Raju.Lakkaraju@microsemi.com>

This covers kernel changes upto:

commit f5a4732f85613b3fb43f8bc33a017e3db3b3605a
Author: Raju Lakkaraju <Raju.Lakkaraju@microsemi.com>
Date:   Wed Nov 9 16:33:09 2016 +0530

    ethtool: (uapi) Add ETHTOOL_PHY_DOWNSHIFT to PHY tunables

    For operation in cabling environments that are incompatible with
    1000BASE-T, PHY device may provide an automatic link speed downshift
    operation. When enabled, the device automatically changes its 1000BASE-T
    auto-negotiation to the next slower speed after a configured number of
    failed attempts at 1000BASE-T.  This feature is useful in setting up in
    networks using older cable installations that include only pairs A and B,
    and not pairs C and D.

    Signed-off-by: Raju Lakkaraju <Raju.Lakkaraju@microsemi.com>
    Signed-off-by: Allan W. Nielsen <allan.nielsen@microsemi.com>

Signed-off-by: Allan W. Nielsen <allan.nielsen@microsemi.com>
Acked-by: Florian Fainelli <f.fainelli@gmail.com>
---
 ethtool-copy.h | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/ethtool-copy.h b/ethtool-copy.h
index 70748f5..2e2448f 100644
--- a/ethtool-copy.h
+++ b/ethtool-copy.h
@@ -247,6 +247,19 @@ struct ethtool_tunable {
 	void	*data[0];
 };
 
+#define DOWNSHIFT_DEV_DEFAULT_COUNT	0xff
+#define DOWNSHIFT_DEV_DISABLE		0
+
+enum phy_tunable_id {
+	ETHTOOL_PHY_ID_UNSPEC,
+	ETHTOOL_PHY_DOWNSHIFT,
+	/*
+	 * Add your fresh new phy tunable attribute above and remember to update
+	 * phy_tunable_strings[] in net/core/ethtool.c
+	 */
+	__ETHTOOL_PHY_TUNABLE_COUNT,
+};
+
 /**
  * struct ethtool_regs - hardware register dump
  * @cmd: Command number = %ETHTOOL_GREGS
@@ -547,6 +560,7 @@ struct ethtool_pauseparam {
  * @ETH_SS_FEATURES: Device feature names
  * @ETH_SS_RSS_HASH_FUNCS: RSS hush function names
  * @ETH_SS_PHY_STATS: Statistic names, for use with %ETHTOOL_GPHYSTATS
+ * @ETH_SS_PHY_TUNABLES: PHY tunable names
  */
 enum ethtool_stringset {
 	ETH_SS_TEST		= 0,
@@ -557,6 +571,7 @@ enum ethtool_stringset {
 	ETH_SS_RSS_HASH_FUNCS,
 	ETH_SS_TUNABLES,
 	ETH_SS_PHY_STATS,
+	ETH_SS_PHY_TUNABLES,
 };
 
 /**
@@ -1312,7 +1327,8 @@ struct ethtool_per_queue_op {
 
 #define ETHTOOL_GLINKSETTINGS	0x0000004c /* Get ethtool_link_settings */
 #define ETHTOOL_SLINKSETTINGS	0x0000004d /* Set ethtool_link_settings */
-
+#define ETHTOOL_PHY_GTUNABLE	0x0000004e /* Get PHY tunable configuration */
+#define ETHTOOL_PHY_STUNABLE	0x0000004f /* Set PHY tunable configuration */
 
 /* compatibility with older code */
 #define SPARC_ETH_GSET		ETHTOOL_GSET
-- 
2.7.3

[PATCH ethtool v4 2/2] Ethtool: Implements ETHTOOL_PHY_GTUNABLE/ETHTOOL_PHY_STUNABLE and PHY downshift

[Allan W. Nielsen]

From: Raju Lakkaraju <Raju.Lakkaraju@microsemi.com>

Add ethtool get and set tunable to access PHY drivers.

Ethtool Help: ethtool -h for PHY tunables
    ethtool --set-phy-tunable DEVNAME      Set PHY tunable
                [ downshift on|off [count N] ]
    ethtool --get-phy-tunable DEVNAME      Get PHY tunable
                [ downshift ]

Ethtool ex:
  ethtool --set-phy-tunable eth0 downshift on
  ethtool --set-phy-tunable eth0 downshift off
  ethtool --set-phy-tunable eth0 downshift on count 2

  ethtool --get-phy-tunable eth0 downshift

Signed-off-by: Raju Lakkaraju <Raju.Lakkaraju@microsemi.com>
Signed-off-by: Allan W. Nielsen <allan.nielsen@microsemi.com>
Acked-by: Florian Fainelli <f.fainelli@gmail.com>
Tested-by: Florian Fainelli <f.fainelli@gmail.com>
---
 ethtool.8.in |  40 +++++++++++++++++
 ethtool.c    | 144 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 184 insertions(+)

diff --git a/ethtool.8.in b/ethtool.8.in
index 9631847..5c36c06 100644
--- a/ethtool.8.in
+++ b/ethtool.8.in
@@ -340,6 +340,18 @@ ethtool \- query or control network driver and hardware settings
 .B2 tx-lpi on off
 .BN tx-timer
 .BN advertise
+.HP
+.B ethtool \-\-set\-phy\-tunable
+.I devname
+.RB [
+.B downshift
+.A1 on off
+.BN count
+.RB ]
+.HP
+.B ethtool \-\-get\-phy\-tunable
+.I devname
+.RB [ downshift ]
 .
 .\" Adjust lines (i.e. full justification) and hyphenate.
 .ad
@@ -947,6 +959,34 @@ Values are as for
 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 enabled.
 .RE
+.TP
+.B \-\-set\-phy\-tunable
+Sets the PHY tunable parameters.
+.RS 4
+.TP
+.A2 downshift on off
+Specifies whether downshift should be enabled
+.TS
+nokeep;
+lB	l.
+.BI count \ N
+Sets the PHY downshift re-tries count.
+.TE
+.PD
+.RE
+.TP
+.B \-\-get\-phy\-tunable
+Gets the PHY tunable parameters.
+.RS 4
+.TP
+.B downshift
+For operation in cabling environments that are incompatible with 1000BASE-T,
+PHY device provides an automatic link speed downshift operation.
+Link speed downshift after N failed 1000BASE-T auto-negotiation attempts.
+Downshift is useful where cable does not have the 4 pairs instance.
+
+Gets the PHY downshift count/status.
+.RE
 .SH BUGS
 Not supported (in part or whole) on all network drivers.
 .SH AUTHOR
diff --git a/ethtool.c b/ethtool.c
index 49ac94e..7dcd005 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -4520,6 +4520,146 @@ static int do_seee(struct cmd_context *ctx)
 	return 0;
 }
 
+static int do_get_phy_tunable(struct cmd_context *ctx)
+{
+	int argc = ctx->argc;
+	char **argp = ctx->argp;
+	int err, i;
+	u8 downshift_changed = 0;
+
+	if (argc < 1)
+		exit_bad_args();
+	for (i = 0; i < argc; i++) {
+		if (!strcmp(argp[i], "downshift")) {
+			downshift_changed = 1;
+			i += 1;
+			if (i < argc)
+				exit_bad_args();
+		} else  {
+			exit_bad_args();
+		}
+	}
+
+	if (downshift_changed) {
+		struct ethtool_tunable ds;
+		u8 count = 0;
+
+		ds.cmd = ETHTOOL_PHY_GTUNABLE;
+		ds.id = ETHTOOL_PHY_DOWNSHIFT;
+		ds.type_id = ETHTOOL_TUNABLE_U8;
+		ds.len = 1;
+		ds.data[0] = &count;
+		err = send_ioctl(ctx, &ds);
+		if (err < 0) {
+			perror("Cannot Get PHY downshift count");
+			return 87;
+		}
+		count = *((u8 *)&ds.data[0]);
+		if (count)
+			fprintf(stdout, "Downshift count: %d\n", count);
+		else
+			fprintf(stdout, "Downshift disabled\n");
+	}
+
+	return err;
+}
+
+static int parse_named_bool(struct cmd_context *ctx, const char *name, u8 *on)
+{
+	if (ctx->argc < 2)
+		return 0;
+
+	if (strcmp(*ctx->argp, name))
+		return 0;
+
+	if (!strcmp(*(ctx->argp + 1), "on")) {
+		*on = 1;
+	} else if (!strcmp(*(ctx->argp + 1), "off")) {
+		*on = 0;
+	} else {
+		fprintf(stderr, "Invalid boolean\n");
+		exit_bad_args();
+	}
+
+	ctx->argc -= 2;
+	ctx->argp += 2;
+
+	return 1;
+}
+
+static int parse_named_u8(struct cmd_context *ctx, const char *name, u8 *val)
+{
+	if (ctx->argc < 2)
+		return 0;
+
+	if (strcmp(*ctx->argp, name))
+		return 0;
+
+	*val = get_uint_range(*(ctx->argp + 1), 0, 0xff);
+
+	ctx->argc -= 2;
+	ctx->argp += 2;
+
+	return 1;
+}
+
+static int do_set_phy_tunable(struct cmd_context *ctx)
+{
+	int err = 0;
+	u8 ds_cnt = DOWNSHIFT_DEV_DEFAULT_COUNT;
+	u8 ds_changed = 0, ds_has_cnt = 0, ds_enable = 0;
+
+	if (ctx->argc == 0)
+		exit_bad_args();
+
+	/* Parse arguments */
+	while (ctx->argc) {
+		if (parse_named_bool(ctx, "downshift", &ds_enable)) {
+			ds_changed = 1;
+			ds_has_cnt = parse_named_u8(ctx, "count", &ds_cnt);
+		} else {
+			exit_bad_args();
+		}
+	}
+
+	/* Validate parameters */
+	if (ds_changed) {
+		if (!ds_enable && ds_has_cnt) {
+			fprintf(stderr, "'count' may not be set when downshift "
+				        "is off.\n");
+			exit_bad_args();
+		}
+
+		if (ds_enable && ds_has_cnt && ds_cnt == 0) {
+			fprintf(stderr, "'count' may not be zero.\n");
+			exit_bad_args();
+		}
+
+		if (!ds_enable)
+			ds_cnt = DOWNSHIFT_DEV_DISABLE;
+	}
+
+	/* Do it */
+	if (ds_changed) {
+		struct ethtool_tunable ds;
+		u8 count;
+
+		ds.cmd = ETHTOOL_PHY_STUNABLE;
+		ds.id = ETHTOOL_PHY_DOWNSHIFT;
+		ds.type_id = ETHTOOL_TUNABLE_U8;
+		ds.len = 1;
+		ds.data[0] = &count;
+		*((u8 *)&ds.data[0]) = ds_cnt;
+		err = send_ioctl(ctx, &ds);
+		if (err < 0) {
+			perror("Cannot Set PHY downshift count");
+			err = 87;
+		}
+	}
+
+	return err;
+}
+
 #ifndef TEST_ETHTOOL
 int send_ioctl(struct cmd_context *ctx, void *cmd)
 {
@@ -4681,6 +4821,10 @@ static const struct option {
 	  "		[ advertise %x ]\n"
 	  "		[ tx-lpi on|off ]\n"
 	  "		[ tx-timer %d ]\n"},
+	{ "--set-phy-tunable", 1, do_set_phy_tunable, "Set PHY tunable",
+	  "		[ downshift on|off [count N] ]\n"},
+	{ "--get-phy-tunable", 1, do_get_phy_tunable, "Get PHY tunable",
+	  "		[ downshift ]\n"},
 	{ "-h|--help", 0, show_usage, "Show this help" },
 	{ "--version", 0, do_version, "Show version number" },
 	{}
-- 
2.7.3

Re: [PATCH ethtool v4 1/2] ethtool-copy.h:sync with net

[John W. Linville]

It looks like this patch somehow misses the following commit:

commit 85a624403c77c3f074931aefdfced59f61b668cb
Author: Jesse Brandeburg <jesse.brandeburg@intel.com>
Date:   Thu Oct 13 16:13:55 2016 -0700

    ethtool: silence warning on bit loss
    
    Sparse was complaining when we went to prototype some code
    using ethtool_cmd_speed_set and SPEED_100000, which uses
    the upper 16 bits of __u32 speed for the first time.
    
    CHECK
    ...
    .../uapi/linux/ethtool.h:123:28: warning:
      cast truncates bits from constant value (186a0 becomes 86a0)
    
    The warning is actually bogus, as no bits are really lost, but
    we can get rid of the sparse warning with this one small change.
    
    Reported-by: Preethi Banala <preethi.banala@intel.com>
    Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
    Signed-off-by: David S. Miller <davem@davemloft.net>

Plus, the referenced commit does not seem to exist in the net-next tree.

Please resubmit with the complete history and correct commit IDs -- thanks!

John

On Tue, Nov 22, 2016 at 09:32:16PM +0100, Allan W. Nielsen wrote:
> From: Raju Lakkaraju <Raju.Lakkaraju@microsemi.com>
> 
> This covers kernel changes upto:
> 
> commit f5a4732f85613b3fb43f8bc33a017e3db3b3605a
> Author: Raju Lakkaraju <Raju.Lakkaraju@microsemi.com>
> Date:   Wed Nov 9 16:33:09 2016 +0530
> 
>     ethtool: (uapi) Add ETHTOOL_PHY_DOWNSHIFT to PHY tunables
> 
>     For operation in cabling environments that are incompatible with
>     1000BASE-T, PHY device may provide an automatic link speed downshift
>     operation. When enabled, the device automatically changes its 1000BASE-T
>     auto-negotiation to the next slower speed after a configured number of
>     failed attempts at 1000BASE-T.  This feature is useful in setting up in
>     networks using older cable installations that include only pairs A and B,
>     and not pairs C and D.
> 
>     Signed-off-by: Raju Lakkaraju <Raju.Lakkaraju@microsemi.com>
>     Signed-off-by: Allan W. Nielsen <allan.nielsen@microsemi.com>
> 
> Signed-off-by: Allan W. Nielsen <allan.nielsen@microsemi.com>
> Acked-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
>  ethtool-copy.h | 18 +++++++++++++++++-
>  1 file changed, 17 insertions(+), 1 deletion(-)
> 
> diff --git a/ethtool-copy.h b/ethtool-copy.h
> index 70748f5..2e2448f 100644
> --- a/ethtool-copy.h
> +++ b/ethtool-copy.h
> @@ -247,6 +247,19 @@ struct ethtool_tunable {
>  	void	*data[0];
>  };
>  
> +#define DOWNSHIFT_DEV_DEFAULT_COUNT	0xff
> +#define DOWNSHIFT_DEV_DISABLE		0
> +
> +enum phy_tunable_id {
> +	ETHTOOL_PHY_ID_UNSPEC,
> +	ETHTOOL_PHY_DOWNSHIFT,
> +	/*
> +	 * Add your fresh new phy tunable attribute above and remember to update
> +	 * phy_tunable_strings[] in net/core/ethtool.c
> +	 */
> +	__ETHTOOL_PHY_TUNABLE_COUNT,
> +};
> +
>  /**
>   * struct ethtool_regs - hardware register dump
>   * @cmd: Command number = %ETHTOOL_GREGS
> @@ -547,6 +560,7 @@ struct ethtool_pauseparam {
>   * @ETH_SS_FEATURES: Device feature names
>   * @ETH_SS_RSS_HASH_FUNCS: RSS hush function names
>   * @ETH_SS_PHY_STATS: Statistic names, for use with %ETHTOOL_GPHYSTATS
> + * @ETH_SS_PHY_TUNABLES: PHY tunable names
>   */
>  enum ethtool_stringset {
>  	ETH_SS_TEST		= 0,
> @@ -557,6 +571,7 @@ enum ethtool_stringset {
>  	ETH_SS_RSS_HASH_FUNCS,
>  	ETH_SS_TUNABLES,
>  	ETH_SS_PHY_STATS,
> +	ETH_SS_PHY_TUNABLES,
>  };
>  
>  /**
> @@ -1312,7 +1327,8 @@ struct ethtool_per_queue_op {
>  
>  #define ETHTOOL_GLINKSETTINGS	0x0000004c /* Get ethtool_link_settings */
>  #define ETHTOOL_SLINKSETTINGS	0x0000004d /* Set ethtool_link_settings */
> -
> +#define ETHTOOL_PHY_GTUNABLE	0x0000004e /* Get PHY tunable configuration */
> +#define ETHTOOL_PHY_STUNABLE	0x0000004f /* Set PHY tunable configuration */
>  
>  /* compatibility with older code */
>  #define SPARC_ETH_GSET		ETHTOOL_GSET
> -- 
> 2.7.3
> 
> 

-- 
John W. Linville		Someday the world will need a hero, and you
linville@tuxdriver.com			might be all we have.  Be ready.

Re: [PATCH ethtool v4 1/2] ethtool-copy.h:sync with net

[John W. Linville]

On Wed, Nov 23, 2016 at 10:24:47AM -0500, John W. Linville wrote:
> It looks like this patch somehow misses the following commit:
> 
> commit 85a624403c77c3f074931aefdfced59f61b668cb
> Author: Jesse Brandeburg <jesse.brandeburg@intel.com>
> Date:   Thu Oct 13 16:13:55 2016 -0700
> 
>     ethtool: silence warning on bit loss
>     
>     Sparse was complaining when we went to prototype some code
>     using ethtool_cmd_speed_set and SPEED_100000, which uses
>     the upper 16 bits of __u32 speed for the first time.
>     
>     CHECK
>     ...
>     .../uapi/linux/ethtool.h:123:28: warning:
>       cast truncates bits from constant value (186a0 becomes 86a0)
>     
>     The warning is actually bogus, as no bits are really lost, but
>     we can get rid of the sparse warning with this one small change.
>     
>     Reported-by: Preethi Banala <preethi.banala@intel.com>
>     Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
>     Signed-off-by: David S. Miller <davem@davemloft.net>
> 
> Plus, the referenced commit does not seem to exist in the net-next tree.
> 
> Please resubmit with the complete history and correct commit IDs -- thanks!

I mean, of course, all the preceding changes accounted for -- not
every commit explicitly listed. The top commit does, of course,
need to reflect the correct commit ID from Dave's/Linus's tree.
 
John
-- 
John W. Linville		Someday the world will need a hero, and you
linville@tuxdriver.com			might be all we have.  Be ready.