netdev - Re: [PATCH v2 3/3] ethtool: Add ETHTOOL_RESET support via --reset command

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20171205213012.i2me64tpnump362c@unicorn.suse.cz>
Date:   Tue, 5 Dec 2017 22:30:12 +0100
From:   Michal Kubecek <mkubecek@...e.cz>
To:     Scott Branden <scott.branden@...adcom.com>
Cc:     "John W. Linville" <linville@...driver.com>,
        BCM Kernel Feedback <bcm-kernel-feedback-list@...adcom.com>,
        Steve Lin <steven.lin1@...adcom.com>,
        Michael Chan <michael.chan@...adcom.com>,
        netdev@...r.kernel.org, Paul Greenwalt <paul.greenwalt@...el.com>,
        Stephen Hemminger <shemminger@...tta.com>
Subject: Re: [PATCH v2 3/3] ethtool: Add ETHTOOL_RESET support via --reset
 command

On Tue, Dec 05, 2017 at 12:53:23PM -0800, 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

IMHO it would be more consistent with e.g. msglvl without the keyword
"flags". It would be also nice to provide a symbolic way to specify the
shared flags.

> +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], "ap")) {
> +			resetinfo.data |= ETH_RESET_AP;
> +		} 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;
> +	}
> +	fprintf(stdout, "RESET 0x%x issued\n", resetinfo.data);

According to documentation, driver is supposed to clear the flags
corresponding to components which were reset so that what is left are
those which were _not_ reset.

Michal Kubecek