[PATCH 0/4] ethtool: add support for new PHY tunable Fast Link Down

[PATCH 0/4] ethtool: add support for new PHY tunable Fast Link Down

[Heiner Kallweit]

This series adds support for Fast Link Down as new PHY tunable.
See [0] for the kernel part incl. the Marvell PHY driver as
first user.

[0] https://marc.info/?t=155353900800001&r=1&w=2

Heiner Kallweit (4):
  ethtool: sync ethtool-copy.h with linux-next from 03/28/2019
  ethtool: simplify handling of PHY tunable downshift
  ethtool: add support for PHY tunable Fast Link Down
  ethtool: add PHY Fast Link Down tunable to man page

 ethtool-copy.h | 44 +++++++++++------------
 ethtool.8.in   | 28 +++++++++++++--
 ethtool.c      | 98 +++++++++++++++++++++++++++++++++-----------------
 3 files changed, 114 insertions(+), 56 deletions(-)

-- 
2.21.0

[PATCH 1/4] ethtool: sync ethtool-copy.h with linux-next from 03/28/2019

[Heiner Kallweit]

Sync ethtool-copy.h with linux-next from 03/28/2019. This provides
access to the new PHY tunable for Fast Link Down support.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 ethtool-copy.h | 44 ++++++++++++++++++++++----------------------
 1 file changed, 22 insertions(+), 22 deletions(-)

diff --git a/ethtool-copy.h b/ethtool-copy.h
index 1204076..92ab10d 100644
--- a/ethtool-copy.h
+++ b/ethtool-copy.h
@@ -89,10 +89,6 @@
  * %ETHTOOL_GSET to get the current values before making specific
  * changes and then applying them with %ETHTOOL_SSET.
  *
- * Drivers that implement set_settings() should validate all fields
- * other than @cmd that are not described as read-only or deprecated,
- * and must ignore all fields described as read-only.
- *
  * Deprecated fields should be ignored by both users and drivers.
  */
 struct ethtool_cmd {
@@ -254,9 +250,17 @@ struct ethtool_tunable {
 #define DOWNSHIFT_DEV_DEFAULT_COUNT	0xff
 #define DOWNSHIFT_DEV_DISABLE		0
 
+/* Time in msecs after which link is reported as down
+ * 0 = lowest time supported by the PHY
+ * 0xff = off, link down detection according to standard
+ */
+#define ETHTOOL_PHY_FAST_LINK_DOWN_ON	0
+#define ETHTOOL_PHY_FAST_LINK_DOWN_OFF	0xff
+
 enum phy_tunable_id {
 	ETHTOOL_PHY_ID_UNSPEC,
 	ETHTOOL_PHY_DOWNSHIFT,
+	ETHTOOL_PHY_FAST_LINK_DOWN,
 	/*
 	 * Add your fresh new phy tunable attribute above and remember to update
 	 * phy_tunable_strings[] in net/core/ethtool.c
@@ -884,7 +888,7 @@ struct ethtool_rx_flow_spec {
 	__u32		location;
 };
 
-/* How rings are layed out when accessing virtual functions or
+/* How rings are laid out when accessing virtual functions or
  * offloaded queues is device specific. To allow users to do flow
  * steering and specify these queues the ring cookie is partitioned
  * into a 32bit queue index with an 8 bit virtual function id.
@@ -893,7 +897,7 @@ struct ethtool_rx_flow_spec {
  * devices start supporting PCIe w/ARI. However at the moment I
  * do not know of any devices that support this so I do not reserve
  * space for this at this time. If a future patch consumes the next
- * byte it should be aware of this possiblity.
+ * byte it should be aware of this possibility.
  */
 #define ETHTOOL_RX_FLOW_SPEC_RING	0x00000000FFFFFFFFLL
 #define ETHTOOL_RX_FLOW_SPEC_RING_VF	0x000000FF00000000LL
@@ -1434,6 +1438,13 @@ enum ethtool_link_mode_bit_indices {
 	ETHTOOL_LINK_MODE_56000baseSR4_Full_BIT	= 29,
 	ETHTOOL_LINK_MODE_56000baseLR4_Full_BIT	= 30,
 	ETHTOOL_LINK_MODE_25000baseCR_Full_BIT	= 31,
+
+	/* Last allowed bit for __ETHTOOL_LINK_MODE_LEGACY_MASK is bit
+	 * 31. Please do NOT define any SUPPORTED_* or ADVERTISED_*
+	 * macro for bits > 31. The only way to use indices > 31 is to
+	 * use the new ETHTOOL_GLINKSETTINGS/ETHTOOL_SLINKSETTINGS API.
+	 */
+
 	ETHTOOL_LINK_MODE_25000baseKR_Full_BIT	= 32,
 	ETHTOOL_LINK_MODE_25000baseSR_Full_BIT	= 33,
 	ETHTOOL_LINK_MODE_50000baseCR2_Full_BIT	= 34,
@@ -1471,14 +1482,8 @@ enum ethtool_link_mode_bit_indices {
 	ETHTOOL_LINK_MODE_200000baseDR4_Full_BIT	 = 65,
 	ETHTOOL_LINK_MODE_200000baseCR4_Full_BIT	 = 66,
 
-	/* Last allowed bit for __ETHTOOL_LINK_MODE_LEGACY_MASK is bit
-	 * 31. Please do NOT define any SUPPORTED_* or ADVERTISED_*
-	 * macro for bits > 31. The only way to use indices > 31 is to
-	 * use the new ETHTOOL_GLINKSETTINGS/ETHTOOL_SLINKSETTINGS API.
-	 */
-
-	__ETHTOOL_LINK_MODE_LAST
-	  = ETHTOOL_LINK_MODE_200000baseCR4_Full_BIT,
+	/* must be last entry */
+	__ETHTOOL_LINK_MODE_MASK_NBITS
 };
 
 #define __ETHTOOL_LINK_MODE_LEGACY_MASK(base_name)	\
@@ -1814,14 +1819,9 @@ enum ethtool_reset_flags {
  * rejected.
  *
  * Deprecated %ethtool_cmd fields transceiver, maxtxpkt and maxrxpkt
- * are not available in %ethtool_link_settings. Until all drivers are
- * converted to ignore them or to the new %ethtool_link_settings API,
- * for both queries and changes, users should always try
- * %ETHTOOL_GLINKSETTINGS first, and if it fails with -ENOTSUPP stick
- * only to %ETHTOOL_GSET and %ETHTOOL_SSET consistently. If it
- * succeeds, then users should stick to %ETHTOOL_GLINKSETTINGS and
- * %ETHTOOL_SLINKSETTINGS (which would support drivers implementing
- * either %ethtool_cmd or %ethtool_link_settings).
+ * are not available in %ethtool_link_settings. These fields will be
+ * always set to zero in %ETHTOOL_GSET reply and %ETHTOOL_SSET will
+ * fail if any of them is set to non-zero value.
  *
  * Users should assume that all fields not marked read-only are
  * writable and subject to validation by the driver.  They should use
-- 
2.21.0

[PATCH 2/4] ethtool: simplify handling of PHY tunable downshift

[Heiner Kallweit]

In preparation of adding support for Fast Link Down as PHY tunable
let's simplify the handling of PHY tunable downshift a little.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 ethtool.c | 43 +++++++++++++------------------------------
 1 file changed, 13 insertions(+), 30 deletions(-)

diff --git a/ethtool.c b/ethtool.c
index 59131e8..f67cd1b 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -4818,28 +4818,15 @@ static int do_get_phy_tunable(struct cmd_context *ctx)
 {
 	int argc = ctx->argc;
 	char **argp = ctx->argp;
-	u8 downshift_changed = 0;
-	int i;
 
 	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) {
+	if (!strcmp(argp[0], "downshift")) {
 		struct {
 			struct ethtool_tunable ds;
-			u8 __count;
+			u8 count;
 		} cont;
-		u8 count = 0;
 
 		cont.ds.cmd = ETHTOOL_PHY_GTUNABLE;
 		cont.ds.id = ETHTOOL_PHY_DOWNSHIFT;
@@ -4849,11 +4836,12 @@ static int do_get_phy_tunable(struct cmd_context *ctx)
 			perror("Cannot Get PHY downshift count");
 			return 87;
 		}
-		count = *((u8 *)&cont.ds.data[0]);
-		if (count)
-			fprintf(stdout, "Downshift count: %d\n", count);
+		if (cont.count)
+			fprintf(stdout, "Downshift count: %d\n", cont.count);
 		else
 			fprintf(stdout, "Downshift disabled\n");
+	} else {
+		exit_bad_args();
 	}
 
 	return 0;
@@ -4995,17 +4983,12 @@ static int do_set_phy_tunable(struct cmd_context *ctx)
 	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();
-		}
+	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 */
@@ -5029,14 +5012,14 @@ static int do_set_phy_tunable(struct cmd_context *ctx)
 	if (ds_changed) {
 		struct {
 			struct ethtool_tunable ds;
-			u8 __count;
+			u8 count;
 		} cont;
 
 		cont.ds.cmd = ETHTOOL_PHY_STUNABLE;
 		cont.ds.id = ETHTOOL_PHY_DOWNSHIFT;
 		cont.ds.type_id = ETHTOOL_TUNABLE_U8;
 		cont.ds.len = 1;
-		*((u8 *)&cont.ds.data[0]) = ds_cnt;
+		cont.count = ds_cnt;
 		err = send_ioctl(ctx, &cont.ds);
 		if (err < 0) {
 			perror("Cannot Set PHY downshift count");
-- 
2.21.0

[PATCH 3/4] ethtool: add support for PHY tunable Fast Link Down

[Heiner Kallweit]

This patch adds support for PHY tunable Fast Link Down. Like downshift
it uses an u8 parameter.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 ethtool.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 53 insertions(+), 2 deletions(-)

diff --git a/ethtool.c b/ethtool.c
index f67cd1b..f56a604 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -4840,6 +4840,28 @@ static int do_get_phy_tunable(struct cmd_context *ctx)
 			fprintf(stdout, "Downshift count: %d\n", cont.count);
 		else
 			fprintf(stdout, "Downshift disabled\n");
+	} else if (!strcmp(argp[0], "fast-link-down")) {
+		struct {
+			struct ethtool_tunable fld;
+			u8 msecs;
+		} cont;
+
+		cont.fld.cmd = ETHTOOL_PHY_GTUNABLE;
+		cont.fld.id = ETHTOOL_PHY_FAST_LINK_DOWN;
+		cont.fld.type_id = ETHTOOL_TUNABLE_U8;
+		cont.fld.len = 1;
+		if (send_ioctl(ctx, &cont.fld) < 0) {
+			perror("Cannot Get PHY Fast Link Down value");
+			return 87;
+		}
+
+		if (cont.msecs == ETHTOOL_PHY_FAST_LINK_DOWN_ON)
+			fprintf(stdout, "Fast Link Down enabled\n");
+		else if (cont.msecs == ETHTOOL_PHY_FAST_LINK_DOWN_OFF)
+			fprintf(stdout, "Fast Link Down disabled\n");
+		else
+			fprintf(stdout, "Fast Link Down enabled, %d msecs\n",
+				cont.msecs);
 	} else {
 		exit_bad_args();
 	}
@@ -4982,11 +5004,17 @@ 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;
+	u8 fld_changed = 0, fld_enable = 0;
+	u8 fld_msecs = ETHTOOL_PHY_FAST_LINK_DOWN_ON;
 
 	/* Parse arguments */
 	if (parse_named_bool(ctx, "downshift", &ds_enable)) {
 		ds_changed = 1;
 		ds_has_cnt = parse_named_u8(ctx, "count", &ds_cnt);
+	} else if (parse_named_bool(ctx, "fast-link-down", &fld_enable)) {
+		fld_changed = 1;
+		if (fld_enable)
+			parse_named_u8(ctx, "msecs", &fld_msecs);
 	} else {
 		exit_bad_args();
 	}
@@ -5006,6 +5034,11 @@ static int do_set_phy_tunable(struct cmd_context *ctx)
 
 		if (!ds_enable)
 			ds_cnt = DOWNSHIFT_DEV_DISABLE;
+	} else if (fld_changed) {
+		if (!fld_enable)
+			fld_msecs = ETHTOOL_PHY_FAST_LINK_DOWN_OFF;
+		else if (fld_msecs == ETHTOOL_PHY_FAST_LINK_DOWN_OFF)
+			exit_bad_args();
 	}
 
 	/* Do it */
@@ -5025,6 +5058,22 @@ static int do_set_phy_tunable(struct cmd_context *ctx)
 			perror("Cannot Set PHY downshift count");
 			err = 87;
 		}
+	} else if (fld_changed) {
+		struct {
+			struct ethtool_tunable fld;
+			u8 msecs;
+		} cont;
+
+		cont.fld.cmd = ETHTOOL_PHY_STUNABLE;
+		cont.fld.id = ETHTOOL_PHY_FAST_LINK_DOWN;
+		cont.fld.type_id = ETHTOOL_TUNABLE_U8;
+		cont.fld.len = 1;
+		cont.msecs = fld_msecs;
+		err = send_ioctl(ctx, &cont.fld);
+		if (err < 0) {
+			perror("Cannot Set PHY Fast Link Down value");
+			err = 87;
+		}
 	}
 
 	return err;
@@ -5276,9 +5325,11 @@ static const struct option {
 	  "		[ 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"},
+	  "		[ downshift on|off [count N] ]\n"
+	  "		[ fast-link-down on|off [msecs N] ]\n"},
 	{ "--get-phy-tunable", 1, do_get_phy_tunable, "Get PHY tunable",
-	  "		[ downshift ]\n"},
+	  "		[ downshift ]\n"
+	  "		[ fast-link-down ]\n"},
 	{ "--reset", 1, do_reset, "Reset components",
 	  "		[ flags %x ]\n"
 	  "		[ mgmt ]\n"
-- 
2.21.0

[PATCH 4/4] ethtool: add PHY Fast Link Down tunable to man page

[Heiner Kallweit]

Add description of new PHY tunable Fast Link Down to ethtool man page.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 ethtool.8.in | 28 ++++++++++++++++++++++++++--
 1 file changed, 26 insertions(+), 2 deletions(-)

diff --git a/ethtool.8.in b/ethtool.8.in
index b878521..c2a97f2 100644
--- a/ethtool.8.in
+++ b/ethtool.8.in
@@ -355,10 +355,16 @@ ethtool \- query or control network driver and hardware settings
 .A1 on off
 .BN count
 .RB ]
+.RB [
+.B fast\-link\-down
+.A1 on off
+.BN msecs
+.RB ]
 .HP
 .B ethtool \-\-get\-phy\-tunable
 .I devname
 .RB [ downshift ]
+.RB [ fast-link-down ]
 .HP
 .B ethtool \-\-reset
 .I devname
@@ -1069,12 +1075,22 @@ Sets the PHY tunable parameters.
 .RS 4
 .TP
 .A2 downshift on off
-Specifies whether downshift should be enabled
+Specifies whether downshift should be enabled.
 .TS
 nokeep;
 lB	l.
 .BI count \ N
-Sets the PHY downshift re-tries count.
+	Sets the PHY downshift re-tries count.
+.TE
+.TP
+.A2 fast-link-down on off
+Specifies whether Fast Link Down should be enabled and time until link down (if supported).
+.TS
+nokeep;
+lB	l.
+.BI msecs \ N
+	Sets the period after which the link is reported as down. Note that the PHY may choose
+	the closest supported value. Only on reading back you get the actual value.
 .TE
 .PD
 .RE
@@ -1090,6 +1106,14 @@ 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.
+.TP
+.B fast\-link\-down
+Depending on the mode it may take 0.5s - 1s until a broken link is reported as down.
+In certain use cases a link-down event needs to be reported as soon as possible.
+Some PHYs support a Fast Link Down feature, and partially even allow to configure
+the period until until a broken link is reported as being down.
+
+Gets the PHY Fast Link Down status / period.
 .RE
 .TP
 .B \-\-reset
-- 
2.21.0

Re: [PATCH 4/4] ethtool: add PHY Fast Link Down tunable to man page

[Andrew Lunn]

> +	Sets the period after which the link is reported as down. Note that the PHY may choose
> +	the closest supported value. Only on reading back you get the actual value.

Hi Heiner

'Only on reading back the tunable do you get the actual value.'

would be better.

>  .TE
>  .PD
>  .RE
> @@ -1090,6 +1106,14 @@ 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.
> +.TP
> +.B fast\-link\-down
> +Depending on the mode it may take 0.5s - 1s until a broken link is reported as down.
> +In certain use cases a link-down event needs to be reported as soon as possible.
> +Some PHYs support a Fast Link Down feature, and partially even allow to configure
> +the period until until a broken link is reported as being down.

Some PHYs support a Fast Link Down Feature and may allow configuration
of the delay before a broken link is reported as being down.

   Andrew

Re: [PATCH 4/4] ethtool: add PHY Fast Link Down tunable to man page

[Heiner Kallweit]

On 28.03.2019 21:01, Andrew Lunn wrote:
>> +	Sets the period after which the link is reported as down. Note that the PHY may choose
>> +	the closest supported value. Only on reading back you get the actual value.
> 
> Hi Heiner
> 
> 'Only on reading back the tunable do you get the actual value.'
> 
> would be better.
> 
OK. I'll wait for more review feedback and will consider this in a v2.
Currently all mails to John are bounced due to some spam filter rejecting mails from
Google servers, so let's see when he reads this.

>>  .TE
>>  .PD
>>  .RE
>> @@ -1090,6 +1106,14 @@ 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.
>> +.TP
>> +.B fast\-link\-down
>> +Depending on the mode it may take 0.5s - 1s until a broken link is reported as down.
>> +In certain use cases a link-down event needs to be reported as soon as possible.
>> +Some PHYs support a Fast Link Down feature, and partially even allow to configure
>> +the period until until a broken link is reported as being down.
> 
> Some PHYs support a Fast Link Down Feature and may allow configuration
> of the delay before a broken link is reported as being down.
> 
OK

>    Andrew
> 
Heiner

Re: [PATCH 4/4] ethtool: add PHY Fast Link Down tunable to man page

[John W. Linville]

On Thu, Mar 28, 2019 at 09:12:45PM +0100, Heiner Kallweit wrote:
> On 28.03.2019 21:01, Andrew Lunn wrote:
> >> +	Sets the period after which the link is reported as down. Note that the PHY may choose
> >> +	the closest supported value. Only on reading back you get the actual value.
> > 
> > Hi Heiner
> > 
> > 'Only on reading back the tunable do you get the actual value.'
> > 
> > would be better.
> > 
> OK. I'll wait for more review feedback and will consider this in a v2.
> Currently all mails to John are bounced due to some spam filter rejecting mails from
> Google servers, so let's see when he reads this.

I'm having some mail server problems right now, but I got this message... ;-)

I like Andrew's wording suggestions, FWIW. Should I expect a repost?

John
 
> >>  .TE
> >>  .PD
> >>  .RE
> >> @@ -1090,6 +1106,14 @@ 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.
> >> +.TP
> >> +.B fast\-link\-down
> >> +Depending on the mode it may take 0.5s - 1s until a broken link is reported as down.
> >> +In certain use cases a link-down event needs to be reported as soon as possible.
> >> +Some PHYs support a Fast Link Down feature, and partially even allow to configure
> >> +the period until until a broken link is reported as being down.
> > 
> > Some PHYs support a Fast Link Down Feature and may allow configuration
> > of the delay before a broken link is reported as being down.
> > 
> OK
> 
> >    Andrew
> > 
> Heiner
> 
> 

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

Re: [PATCH 4/4] ethtool: add PHY Fast Link Down tunable to man page

[Heiner Kallweit]

On 29.03.2019 15:19, John W. Linville wrote:
> On Thu, Mar 28, 2019 at 09:12:45PM +0100, Heiner Kallweit wrote:
>> On 28.03.2019 21:01, Andrew Lunn wrote:
>>>> +	Sets the period after which the link is reported as down. Note that the PHY may choose
>>>> +	the closest supported value. Only on reading back you get the actual value.
>>>
>>> Hi Heiner
>>>
>>> 'Only on reading back the tunable do you get the actual value.'
>>>
>>> would be better.
>>>
>> OK. I'll wait for more review feedback and will consider this in a v2.
>> Currently all mails to John are bounced due to some spam filter rejecting mails from
>> Google servers, so let's see when he reads this.
> 
> I'm having some mail server problems right now, but I got this message... ;-)
> 
> I like Andrew's wording suggestions, FWIW. Should I expect a repost?
> 
You're alive, great ;) Yes, I'll send a v2.

> John
>  
>>>>  .TE
>>>>  .PD
>>>>  .RE
>>>> @@ -1090,6 +1106,14 @@ 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.
>>>> +.TP
>>>> +.B fast\-link\-down
>>>> +Depending on the mode it may take 0.5s - 1s until a broken link is reported as down.
>>>> +In certain use cases a link-down event needs to be reported as soon as possible.
>>>> +Some PHYs support a Fast Link Down feature, and partially even allow to configure
>>>> +the period until until a broken link is reported as being down.
>>>
>>> Some PHYs support a Fast Link Down Feature and may allow configuration
>>> of the delay before a broken link is reported as being down.
>>>
>> OK
>>
>>>    Andrew
>>>
>> Heiner
>>
>>
>