ethtool: Add ETHTOOL_RESET support via --reset command

Message ID 1512069844-15664-1-git-send-email-scott.branden@broadcom.com
State Superseded
Delegated to: John Linville
Headers show
Series
  • ethtool: Add ETHTOOL_RESET support via --reset command
Related show

Commit Message

Scott Branden Nov. 30, 2017, 7:24 p.m.
Add ETHTOOL_RESET support via --reset command.

ie.  ethtool --reset DEVNAME <flagname(s)>

flagnames currently match the ETH_RESET_xxx names:
mgmt,irq,dma,filter,offload,mac,phy,ram,dedicated,all

Alternatively, you can specific component bitfield directly using
ethtool --reset DEVNAME flags %x

Signed-off-by: Scott Branden <scott.branden@broadcom.com>
---
 ethtool.8.in | 55 ++++++++++++++++++++++++++++++++++++++++++++-
 ethtool.c    | 73 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 127 insertions(+), 1 deletion(-)

Comments

Gal Pressman Dec. 3, 2017, 8:07 a.m. | #1
On 30-Nov-17 21:24, Scott Branden wrote:
> Add ETHTOOL_RESET support via --reset command.
>
> ie.  ethtool --reset DEVNAME <flagname(s)>
>
> flagnames currently match the ETH_RESET_xxx names:
> mgmt,irq,dma,filter,offload,mac,phy,ram,dedicated,all
>
> Alternatively, you can specific component bitfield directly using
> ethtool --reset DEVNAME flags %x
>
> Signed-off-by: Scott Branden <scott.branden@broadcom.com>
> ---
>  ethtool.8.in | 55 ++++++++++++++++++++++++++++++++++++++++++++-
>  ethtool.c    | 73 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 127 insertions(+), 1 deletion(-)
>
> diff --git a/ethtool.8.in b/ethtool.8.in
> index 6ad3065..925cfe3 100644
> --- a/ethtool.8.in
> +++ b/ethtool.8.in
> @@ -355,6 +355,20 @@ ethtool \- query or control network driver and hardware settings
>  .B ethtool \-\-get\-phy\-tunable
>  .I devname
>  .RB [ downshift ]
> +.HP
> +.B ethtool \-\-reset
> +.I devname
> +.BN flags
> +.B [mgmt]
> +.B [irq]
> +.B [dma]
> +.B [filter]
> +.B [offload]
> +.B [mac]
> +.B [phy]
> +.B [ram]
> +.B [dedicated]
> +.B [all]
>  .
Nit:
Usually, the brackets formatting is different than the keyword inside them.

Gal
Scott Branden Dec. 4, 2017, 8:16 p.m. | #2
Hi Gal,

I do not understand you're comment - questions inline


On 17-12-03 12:07 AM, Gal Pressman wrote:
> On 30-Nov-17 21:24, Scott Branden wrote:
>> Add ETHTOOL_RESET support via --reset command.
>>
>> ie.  ethtool --reset DEVNAME <flagname(s)>
>>
>> flagnames currently match the ETH_RESET_xxx names:
>> mgmt,irq,dma,filter,offload,mac,phy,ram,dedicated,all
>>
>> Alternatively, you can specific component bitfield directly using
>> ethtool --reset DEVNAME flags %x
>>
>> Signed-off-by: Scott Branden <scott.branden@broadcom.com>
>> ---
>>   ethtool.8.in | 55 ++++++++++++++++++++++++++++++++++++++++++++-
>>   ethtool.c    | 73 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>   2 files changed, 127 insertions(+), 1 deletion(-)
>>
>> diff --git a/ethtool.8.in b/ethtool.8.in
>> index 6ad3065..925cfe3 100644
>> --- a/ethtool.8.in
>> +++ b/ethtool.8.in
>> @@ -355,6 +355,20 @@ ethtool \- query or control network driver and hardware settings
>>   .B ethtool \-\-get\-phy\-tunable
>>   .I devname
>>   .RB [ downshift ]
>> +.HP
>> +.B ethtool \-\-reset
>> +.I devname
>> +.BN flags
>> +.B [mgmt]
>> +.B [irq]
>> +.B [dma]
>> +.B [filter]
>> +.B [offload]
>> +.B [mac]
>> +.B [phy]
>> +.B [ram]
>> +.B [dedicated]
>> +.B [all]
>>   .
> Nit:
> Usually, the brackets formatting is different than the keyword inside them.
Could you please explain what the formatting should be then?  Any of the 
options listed are possible components that can be selected.  At least 
one must be chosen.  Should the brackets be removed then?
>
> Gal
Gal Pressman Dec. 5, 2017, 8:21 a.m. | #3
On 04-Dec-17 22:16, Scott Branden wrote:
> Hi Gal,
>
> I do not understand you're comment - questions inline
>
>
> On 17-12-03 12:07 AM, Gal Pressman wrote:
>> On 30-Nov-17 21:24, Scott Branden wrote:
>>> Add ETHTOOL_RESET support via --reset command.
>>>
>>> ie.  ethtool --reset DEVNAME <flagname(s)>
>>>
>>> flagnames currently match the ETH_RESET_xxx names:
>>> mgmt,irq,dma,filter,offload,mac,phy,ram,dedicated,all
>>>
>>> Alternatively, you can specific component bitfield directly using
>>> ethtool --reset DEVNAME flags %x
>>>
>>> Signed-off-by: Scott Branden <scott.branden@broadcom.com>
>>> ---
>>>   ethtool.8.in | 55 ++++++++++++++++++++++++++++++++++++++++++++-
>>>   ethtool.c    | 73 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>   2 files changed, 127 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/ethtool.8.in b/ethtool.8.in
>>> index 6ad3065..925cfe3 100644
>>> --- a/ethtool.8.in
>>> +++ b/ethtool.8.in
>>> @@ -355,6 +355,20 @@ ethtool \- query or control network driver and hardware settings
>>>   .B ethtool \-\-get\-phy\-tunable
>>>   .I devname
>>>   .RB [ downshift ]
>>> +.HP
>>> +.B ethtool \-\-reset
>>> +.I devname
>>> +.BN flags
>>> +.B [mgmt]
>>> +.B [irq]
>>> +.B [dma]
>>> +.B [filter]
>>> +.B [offload]
>>> +.B [mac]
>>> +.B [phy]
>>> +.B [ram]
>>> +.B [dedicated]
>>> +.B [all]
>>>   .
>> Nit:
>> Usually, the brackets formatting is different than the keyword inside them.
> Could you please explain what the formatting should be then?  Any of the options listed are possible components that can be selected.  At least one must be chosen.  Should the brackets be removed then?
The brackets are fine, IMO .B [mgmt] should be replaced with .RB [ mgmt ].
Looks better, and keeps us consistent with other flags (--get-phy-tunable for example).
Scott Branden Dec. 5, 2017, 8:24 p.m. | #4
On 17-12-05 12:21 AM, Gal Pressman wrote:
> On 04-Dec-17 22:16, Scott Branden wrote:
>> Hi Gal,
>>
>> I do not understand you're comment - questions inline
>>
>>
>> On 17-12-03 12:07 AM, Gal Pressman wrote:
>>> On 30-Nov-17 21:24, Scott Branden wrote:
>>>> Add ETHTOOL_RESET support via --reset command.
>>>>
>>>> ie.  ethtool --reset DEVNAME <flagname(s)>
>>>>
>>>> flagnames currently match the ETH_RESET_xxx names:
>>>> mgmt,irq,dma,filter,offload,mac,phy,ram,dedicated,all
>>>>
>>>> Alternatively, you can specific component bitfield directly using
>>>> ethtool --reset DEVNAME flags %x
>>>>
>>>> Signed-off-by: Scott Branden <scott.branden@broadcom.com>
>>>> ---
>>>>    ethtool.8.in | 55 ++++++++++++++++++++++++++++++++++++++++++++-
>>>>    ethtool.c    | 73 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>>    2 files changed, 127 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/ethtool.8.in b/ethtool.8.in
>>>> index 6ad3065..925cfe3 100644
>>>> --- a/ethtool.8.in
>>>> +++ b/ethtool.8.in
>>>> @@ -355,6 +355,20 @@ ethtool \- query or control network driver and hardware settings
>>>>    .B ethtool \-\-get\-phy\-tunable
>>>>    .I devname
>>>>    .RB [ downshift ]
>>>> +.HP
>>>> +.B ethtool \-\-reset
>>>> +.I devname
>>>> +.BN flags
>>>> +.B [mgmt]
>>>> +.B [irq]
>>>> +.B [dma]
>>>> +.B [filter]
>>>> +.B [offload]
>>>> +.B [mac]
>>>> +.B [phy]
>>>> +.B [ram]
>>>> +.B [dedicated]
>>>> +.B [all]
>>>>    .
>>> Nit:
>>> Usually, the brackets formatting is different than the keyword inside them.
>> Could you please explain what the formatting should be then?  Any of the options listed are possible components that can be selected.  At least one must be chosen.  Should the brackets be removed then?
> The brackets are fine, IMO .B [mgmt] should be replaced with .RB [ mgmt ].
> Looks better, and keeps us consistent with other flags (--get-phy-tunable for example).
OK - I'll update.  Thanks.
>
>

Patch

diff --git a/ethtool.8.in b/ethtool.8.in
index 6ad3065..925cfe3 100644
--- a/ethtool.8.in
+++ b/ethtool.8.in
@@ -355,6 +355,20 @@  ethtool \- query or control network driver and hardware settings
 .B ethtool \-\-get\-phy\-tunable
 .I devname
 .RB [ downshift ]
+.HP
+.B ethtool \-\-reset
+.I devname
+.BN flags
+.B [mgmt]
+.B [irq]
+.B [dma]
+.B [filter]
+.B [offload]
+.B [mac]
+.B [phy]
+.B [ram]
+.B [dedicated]
+.B [all]
 .
 .\" Adjust lines (i.e. full justification) and hyphenate.
 .ad
@@ -1007,6 +1021,44 @@  Downshift is useful where cable does not have the 4 pairs instance.
 
 Gets the PHY downshift count/status.
 .RE
+.TP
+.B \-\-reset
+Reset hardware components specified by flags and components listed below
+.RS 4
+.TP
+.BI flags \ N
+Resets the components based on direct flags mask
+.TP
+.B mgmt
+Management processor
+.TP
+.B irq
+Interrupt requester
+.TP
+.B dma
+DMA engine
+.TP
+.B filter
+Filtering/flow direction
+.TP
+.B offload
+Protocol offload
+.TP
+.B mac
+Media access controller
+.TP
+.B phy
+Transceiver/PHY
+.TP
+.B ram
+RAM shared between multiple components
+.TP
+.B dedicated
+All components dedicated to this interface
+.TP
+.B all
+All components used by this interface, even if shared
+.RE
 .SH BUGS
 Not supported (in part or whole) on all network drivers.
 .SH AUTHOR
@@ -1024,7 +1076,8 @@  Andi Kleen,
 Alexander Duyck,
 Sucheta Chakraborty,
 Jesse Brandeburg,
-Ben Hutchings.
+Ben Hutchings,
+Scott Branden.
 .SH AVAILABILITY
 .B ethtool
 is available from
diff --git a/ethtool.c b/ethtool.c
index 1a2b7cc..8c9d169 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -4641,6 +4641,67 @@  static int do_get_phy_tunable(struct cmd_context *ctx)
 	return err;
 }
 
+static int do_reset(struct cmd_context *ctx)
+{
+	struct ethtool_value resetinfo;
+	int argc = ctx->argc;
+	char **argp = ctx->argp;
+	int i;
+
+	if (argc == 0)
+		exit_bad_args();
+
+	resetinfo.data = 0;
+
+	for (i = 0; i < argc; i++) {
+		if (!strcmp(argp[i], "flags")) {
+			__u32 flags;
+
+			i++;
+			if (i >= argc)
+				exit_bad_args();
+			flags = strtoul(argp[i], NULL, 0);
+			if (flags == 0)
+				exit_bad_args();
+			else
+				resetinfo.data |= flags;
+		} else if (!strcmp(argp[i], "mgmt")) {
+			resetinfo.data |= ETH_RESET_MGMT;
+		} else if (!strcmp(argp[i], "irq")) {
+			resetinfo.data |= ETH_RESET_IRQ;
+		} else if (!strcmp(argp[i], "dma")) {
+			resetinfo.data |= ETH_RESET_DMA;
+		} else if (!strcmp(argp[i], "filter")) {
+			resetinfo.data |= ETH_RESET_FILTER;
+		} else if (!strcmp(argp[i], "offload")) {
+			resetinfo.data |= ETH_RESET_OFFLOAD;
+		} else if (!strcmp(argp[i], "mac")) {
+			resetinfo.data |= ETH_RESET_MAC;
+		} else if (!strcmp(argp[i], "phy")) {
+			resetinfo.data |= ETH_RESET_PHY;
+		} else if (!strcmp(argp[i], "ram")) {
+			resetinfo.data |= ETH_RESET_RAM;
+		} else if (!strcmp(argp[i], "dedicated")) {
+			resetinfo.data |= ETH_RESET_DEDICATED;
+		} else if (!strcmp(argp[i], "all")) {
+			resetinfo.data |= ETH_RESET_ALL;
+		} else {
+			exit_bad_args();
+		}
+	}
+
+	resetinfo.cmd = ETHTOOL_RESET;
+
+	if (send_ioctl(ctx, &resetinfo)) {
+		perror("Cannot issue RESET");
+		return 1;
+	} else {
+		fprintf(stdout, "RESET 0x%x issued\n", resetinfo.data);
+	}
+
+	return 0;
+}
+
 static int parse_named_bool(struct cmd_context *ctx, const char *name, u8 *on)
 {
 	if (ctx->argc < 2)
@@ -4904,6 +4965,18 @@  static const struct option {
 	  "		[ downshift on|off [count N] ]\n"},
 	{ "--get-phy-tunable", 1, do_get_phy_tunable, "Get PHY tunable",
 	  "		[ downshift ]\n"},
+	{ "--reset", 1, do_reset, "Reset components",
+	  "		[ flags %x ]\n"
+	  "		[ mgmt ]\n"
+	  "		[ irq ]\n"
+	  "		[ dma ]\n"
+	  "		[ filter ]\n"
+	  "		[ offload ]\n"
+	  "		[ mac ]\n"
+	  "		[ phy ]\n"
+	  "		[ ram ]\n"
+	  "		[ dedicated ]\n"
+	  "		[ all ]\n"},
 	{ "-h|--help", 0, show_usage, "Show this help" },
 	{ "--version", 0, do_version, "Show version number" },
 	{}