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] [day] [month] [year] [list]
Message-ID: <1466953532.2551.5.camel@decadent.org.uk>
Date:	Sun, 26 Jun 2016 17:05:32 +0200
From:	Ben Hutchings <ben@...adent.org.uk>
To:	kan.liang@...el.com, bwh@...nel.org
Cc:	netdev@...r.kernel.org, davem@...emloft.net,
	nicolas.dichtel@...nd.com, jesse.brandeburg@...el.com,
	andi@...stfloor.org
Subject: Re: [PATCH 2/5] ethtool: move cmdline_coalesce out of do_scoalesce

On Sun, 2016-05-15 at 00:14 -0700, kan.liang@...el.com wrote:
> From: Kan Liang <kan.liang@...el.com>
> 
> Moving cmdline_coalesce out of do_scoalesce, so it can be shared with
> other functions.
> No behavior change.

I spent some time getting rid of static variables, and I think the only
ones left are (at least logically) constant.  I won't accept a change
that reverses this.  There's really no need to pass information around
this way.

Ben.

> Signed-off-by: Kan Liang <kan.liang@...el.com>
> ---
>  ethtool.c | 147 +++++++++++++++++++++++++++++++-------------------------------
>  1 file changed, 74 insertions(+), 73 deletions(-)
> 
> diff --git a/ethtool.c b/ethtool.c
> index bd0583c..86724a2 100644
> --- a/ethtool.c
> +++ b/ethtool.c
> @@ -1883,85 +1883,86 @@ static int do_gcoalesce(struct cmd_context *ctx)
>  	return 0;
>  }
>  
> +static struct ethtool_coalesce s_ecoal;
> +static s32 coal_stats_wanted = -1;
> +static int coal_adaptive_rx_wanted = -1;
> +static int coal_adaptive_tx_wanted = -1;
> +static s32 coal_sample_rate_wanted = -1;
> +static s32 coal_pkt_rate_low_wanted = -1;
> +static s32 coal_pkt_rate_high_wanted = -1;
> +static s32 coal_rx_usec_wanted = -1;
> +static s32 coal_rx_frames_wanted = -1;
> +static s32 coal_rx_usec_irq_wanted = -1;
> +static s32 coal_rx_frames_irq_wanted = -1;
> +static s32 coal_tx_usec_wanted = -1;
> +static s32 coal_tx_frames_wanted = -1;
> +static s32 coal_tx_usec_irq_wanted = -1;
> +static s32 coal_tx_frames_irq_wanted = -1;
> +static s32 coal_rx_usec_low_wanted = -1;
> +static s32 coal_rx_frames_low_wanted = -1;
> +static s32 coal_tx_usec_low_wanted = -1;
> +static s32 coal_tx_frames_low_wanted = -1;
> +static s32 coal_rx_usec_high_wanted = -1;
> +static s32 coal_rx_frames_high_wanted = -1;
> +static s32 coal_tx_usec_high_wanted = -1;
> +static s32 coal_tx_frames_high_wanted = -1;
> +
> +static struct cmdline_info cmdline_coalesce[] = {
> +	{ "adaptive-rx", CMDL_BOOL, &coal_adaptive_rx_wanted,
> +	  &s_ecoal.use_adaptive_rx_coalesce },
> +	{ "adaptive-tx", CMDL_BOOL, &coal_adaptive_tx_wanted,
> +	  &s_ecoal.use_adaptive_tx_coalesce },
> +	{ "sample-interval", CMDL_S32, &coal_sample_rate_wanted,
> +	  &s_ecoal.rate_sample_interval },
> +	{ "stats-block-usecs", CMDL_S32, &coal_stats_wanted,
> +	  &s_ecoal.stats_block_coalesce_usecs },
> +	{ "pkt-rate-low", CMDL_S32, &coal_pkt_rate_low_wanted,
> +	  &s_ecoal.pkt_rate_low },
> +	{ "pkt-rate-high", CMDL_S32, &coal_pkt_rate_high_wanted,
> +	  &s_ecoal.pkt_rate_high },
> +	{ "rx-usecs", CMDL_S32, &coal_rx_usec_wanted,
> +	  &s_ecoal.rx_coalesce_usecs },
> +	{ "rx-frames", CMDL_S32, &coal_rx_frames_wanted,
> +	  &s_ecoal.rx_max_coalesced_frames },
> +	{ "rx-usecs-irq", CMDL_S32, &coal_rx_usec_irq_wanted,
> +	  &s_ecoal.rx_coalesce_usecs_irq },
> +	{ "rx-frames-irq", CMDL_S32, &coal_rx_frames_irq_wanted,
> +	  &s_ecoal.rx_max_coalesced_frames_irq },
> +	{ "tx-usecs", CMDL_S32, &coal_tx_usec_wanted,
> +	  &s_ecoal.tx_coalesce_usecs },
> +	{ "tx-frames", CMDL_S32, &coal_tx_frames_wanted,
> +	  &s_ecoal.tx_max_coalesced_frames },
> +	{ "tx-usecs-irq", CMDL_S32, &coal_tx_usec_irq_wanted,
> +	  &s_ecoal.tx_coalesce_usecs_irq },
> +	{ "tx-frames-irq", CMDL_S32, &coal_tx_frames_irq_wanted,
> +	  &s_ecoal.tx_max_coalesced_frames_irq },
> +	{ "rx-usecs-low", CMDL_S32, &coal_rx_usec_low_wanted,
> +	  &s_ecoal.rx_coalesce_usecs_low },
> +	{ "rx-frames-low", CMDL_S32, &coal_rx_frames_low_wanted,
> +	  &s_ecoal.rx_max_coalesced_frames_low },
> +	{ "tx-usecs-low", CMDL_S32, &coal_tx_usec_low_wanted,
> +	  &s_ecoal.tx_coalesce_usecs_low },
> +	{ "tx-frames-low", CMDL_S32, &coal_tx_frames_low_wanted,
> +	  &s_ecoal.tx_max_coalesced_frames_low },
> +	{ "rx-usecs-high", CMDL_S32, &coal_rx_usec_high_wanted,
> +	  &s_ecoal.rx_coalesce_usecs_high },
> +	{ "rx-frames-high", CMDL_S32, &coal_rx_frames_high_wanted,
> +	  &s_ecoal.rx_max_coalesced_frames_high },
> +	{ "tx-usecs-high", CMDL_S32, &coal_tx_usec_high_wanted,
> +	  &s_ecoal.tx_coalesce_usecs_high },
> +	{ "tx-frames-high", CMDL_S32, &coal_tx_frames_high_wanted,
> +	  &s_ecoal.tx_max_coalesced_frames_high },
> +};
>  static int do_scoalesce(struct cmd_context *ctx)
>  {
> -	struct ethtool_coalesce ecoal;
>  	int gcoalesce_changed = 0;
> -	s32 coal_stats_wanted = -1;
> -	int coal_adaptive_rx_wanted = -1;
> -	int coal_adaptive_tx_wanted = -1;
> -	s32 coal_sample_rate_wanted = -1;
> -	s32 coal_pkt_rate_low_wanted = -1;
> -	s32 coal_pkt_rate_high_wanted = -1;
> -	s32 coal_rx_usec_wanted = -1;
> -	s32 coal_rx_frames_wanted = -1;
> -	s32 coal_rx_usec_irq_wanted = -1;
> -	s32 coal_rx_frames_irq_wanted = -1;
> -	s32 coal_tx_usec_wanted = -1;
> -	s32 coal_tx_frames_wanted = -1;
> -	s32 coal_tx_usec_irq_wanted = -1;
> -	s32 coal_tx_frames_irq_wanted = -1;
> -	s32 coal_rx_usec_low_wanted = -1;
> -	s32 coal_rx_frames_low_wanted = -1;
> -	s32 coal_tx_usec_low_wanted = -1;
> -	s32 coal_tx_frames_low_wanted = -1;
> -	s32 coal_rx_usec_high_wanted = -1;
> -	s32 coal_rx_frames_high_wanted = -1;
> -	s32 coal_tx_usec_high_wanted = -1;
> -	s32 coal_tx_frames_high_wanted = -1;
> -	struct cmdline_info cmdline_coalesce[] = {
> -		{ "adaptive-rx", CMDL_BOOL, &coal_adaptive_rx_wanted,
> -		  &ecoal.use_adaptive_rx_coalesce },
> -		{ "adaptive-tx", CMDL_BOOL, &coal_adaptive_tx_wanted,
> -		  &ecoal.use_adaptive_tx_coalesce },
> -		{ "sample-interval", CMDL_S32, &coal_sample_rate_wanted,
> -		  &ecoal.rate_sample_interval },
> -		{ "stats-block-usecs", CMDL_S32, &coal_stats_wanted,
> -		  &ecoal.stats_block_coalesce_usecs },
> -		{ "pkt-rate-low", CMDL_S32, &coal_pkt_rate_low_wanted,
> -		  &ecoal.pkt_rate_low },
> -		{ "pkt-rate-high", CMDL_S32, &coal_pkt_rate_high_wanted,
> -		  &ecoal.pkt_rate_high },
> -		{ "rx-usecs", CMDL_S32, &coal_rx_usec_wanted,
> -		  &ecoal.rx_coalesce_usecs },
> -		{ "rx-frames", CMDL_S32, &coal_rx_frames_wanted,
> -		  &ecoal.rx_max_coalesced_frames },
> -		{ "rx-usecs-irq", CMDL_S32, &coal_rx_usec_irq_wanted,
> -		  &ecoal.rx_coalesce_usecs_irq },
> -		{ "rx-frames-irq", CMDL_S32, &coal_rx_frames_irq_wanted,
> -		  &ecoal.rx_max_coalesced_frames_irq },
> -		{ "tx-usecs", CMDL_S32, &coal_tx_usec_wanted,
> -		  &ecoal.tx_coalesce_usecs },
> -		{ "tx-frames", CMDL_S32, &coal_tx_frames_wanted,
> -		  &ecoal.tx_max_coalesced_frames },
> -		{ "tx-usecs-irq", CMDL_S32, &coal_tx_usec_irq_wanted,
> -		  &ecoal.tx_coalesce_usecs_irq },
> -		{ "tx-frames-irq", CMDL_S32, &coal_tx_frames_irq_wanted,
> -		  &ecoal.tx_max_coalesced_frames_irq },
> -		{ "rx-usecs-low", CMDL_S32, &coal_rx_usec_low_wanted,
> -		  &ecoal.rx_coalesce_usecs_low },
> -		{ "rx-frames-low", CMDL_S32, &coal_rx_frames_low_wanted,
> -		  &ecoal.rx_max_coalesced_frames_low },
> -		{ "tx-usecs-low", CMDL_S32, &coal_tx_usec_low_wanted,
> -		  &ecoal.tx_coalesce_usecs_low },
> -		{ "tx-frames-low", CMDL_S32, &coal_tx_frames_low_wanted,
> -		  &ecoal.tx_max_coalesced_frames_low },
> -		{ "rx-usecs-high", CMDL_S32, &coal_rx_usec_high_wanted,
> -		  &ecoal.rx_coalesce_usecs_high },
> -		{ "rx-frames-high", CMDL_S32, &coal_rx_frames_high_wanted,
> -		  &ecoal.rx_max_coalesced_frames_high },
> -		{ "tx-usecs-high", CMDL_S32, &coal_tx_usec_high_wanted,
> -		  &ecoal.tx_coalesce_usecs_high },
> -		{ "tx-frames-high", CMDL_S32, &coal_tx_frames_high_wanted,
> -		  &ecoal.tx_max_coalesced_frames_high },
> -	};
>  	int err, changed = 0;
>  
>  	parse_generic_cmdline(ctx, &gcoalesce_changed,
>  			      cmdline_coalesce, ARRAY_SIZE(cmdline_coalesce));
>  
> -	ecoal.cmd = ETHTOOL_GCOALESCE;
> -	err = send_ioctl(ctx, &ecoal);
> +	s_ecoal.cmd = ETHTOOL_GCOALESCE;
> +	err = send_ioctl(ctx, &s_ecoal);
>  	if (err) {
>  		perror("Cannot get device coalesce settings");
>  		return 76;
> @@ -1975,8 +1976,8 @@ static int do_scoalesce(struct cmd_context *ctx)
>  		return 80;
>  	}
>  
> -	ecoal.cmd = ETHTOOL_SCOALESCE;
> -	err = send_ioctl(ctx, &ecoal);
> +	s_ecoal.cmd = ETHTOOL_SCOALESCE;
> +	err = send_ioctl(ctx, &s_ecoal);
>  	if (err) {
>  		perror("Cannot set device coalesce parameters");
>  		return 81;
-- 

Ben Hutchings
compatible: Gracefully accepts erroneous data from any source

Download attachment "signature.asc" of type "application/pgp-signature" (820 bytes)

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ