Re: [PATCH] ethtool : Add option -L | --set-common to set common flags.

[Ben Hutchings <bhutchings@solarflare.com>]

On Thu, 2011-01-13 at 16:11 -0800, Mahesh Bandewar wrote:
> This patch adds -L | --set-common option to add / remove common flags which
> includes loopback flag. The -l | --show-common displays the current values
> for these common flags.
> 
> Signed-off-by: Mahesh Bandewar <maheshb@google.com>
> ---
>  ethtool-copy.h |    1 +
>  ethtool.8      |   16 ++++++++++
>  ethtool.c      |   90 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  3 files changed, 105 insertions(+), 2 deletions(-)
> 
> diff --git a/ethtool-copy.h b/ethtool-copy.h
> index 75c3ae7..5fd18c7 100644
> --- a/ethtool-copy.h
> +++ b/ethtool-copy.h
> @@ -309,6 +309,7 @@ struct ethtool_perm_addr {
>   * flag differs from the read-only value.
>   */
>  enum ethtool_flags {
> +	ETH_FLAG_LOOPBACK	= (1 << 2),	/* Loopback enable / disable */
>  	ETH_FLAG_TXVLAN		= (1 << 7),	/* TX VLAN offload enabled */
>  	ETH_FLAG_RXVLAN		= (1 << 8),	/* RX VLAN offload enabled */
>  	ETH_FLAG_LRO		= (1 << 15),	/* LRO is enabled */
> diff --git a/ethtool.8 b/ethtool.8
> index 1760924..cf7128f 100644
> --- a/ethtool.8
> +++ b/ethtool.8
> @@ -174,6 +174,13 @@ ethtool \- Display or change ethernet card settings
>  .B2 txvlan on off
>  .B2 rxhash on off
>  
> +.B ethtool \-l|\-\-show\-common
> +.I ethX
> +
> +.B ethtool \-L|\-\-set\-common
> +.I ethX
> +.B2 loopback on off
> +
>  .B ethtool \-p|\-\-identify
>  .I ethX
>  .RI [ N ]
> @@ -406,6 +413,15 @@ Specifies whether TX VLAN acceleration should be enabled
>  .A2 rxhash on off
>  Specifies whether receive hashing offload should be enabled
>  .TP
> +.B \-l \-\-show\-common
> +Queries the specified ethernet device for common flag settings.
> +.TP
> +.B \-L \-\-set\-common
> +Changes the common parameters of the specified ethernet device.
> +.TP
> +.A2 loopback on off
> +Specifies whether loopback should be enabled.
> +.TP

I've just gone through the manual page and changed 'ethernet device' to
'network device' for all generic operations; please follow that.  The
source for the manual page was also renamed to ethtool.8.in as it now
goes through autoconf substitution.

>  .B \-p \-\-identify
>  Initiates adapter-specific action intended to enable an operator to
>  easily identify the adapter by sight.  Typically this involves
> diff --git a/ethtool.c b/ethtool.c
> index 63e0ead..1a0c10c 100644
> --- a/ethtool.c
> +++ b/ethtool.c
[...]
> @@ -1905,6 +1932,13 @@ static int dump_offload(int rx, int tx, int sg, int tso, int ufo, int gso,
>  	return 0;
>  }
>  
> +static int dump_common_flags(int loopback)
> +{
> +	fprintf(stdout, "loopback: %s\n", loopback ? "on" : "off");
> +
> +	return 0;
> +}
> +
>  static int dump_rxfhash(int fhash, u64 val)
>  {
>  	switch (fhash) {
[...]
> @@ -2219,6 +2257,53 @@ static int do_scoalesce(int fd, struct ifreq *ifr)
>  	return 0;
>  }
>  
> +static int do_gcommon(int fd, struct ifreq *ifr)
> +{
> +	struct ethtool_value eval;
> +	int loopback = 0;
> +
> +	fprintf(stdout, "Common flags for %s:\n", devname);
> +
> +	eval.cmd = ETHTOOL_GFLAGS;
> +	ifr->ifr_data = (caddr_t)&eval;
> +	if (ioctl(fd, SIOCETHTOOL, ifr)) {
> +		perror("Cannot get device flags");
> +	} else {
> +		loopback = (eval.data & ETH_FLAG_LOOPBACK) != 0;
> +	}
> +
> +	return dump_common_flags(loopback);

Breaking up a bitmask into a list of flag parameters is fairly
pointless.  I realise do_goffload() and dump_offload() do that but I am
just waiting for Michał Mirosław's changes to offload flags to be
settled before I fix them.

> +}
> +
> +static int do_scommon(int fd, struct ifreq *ifr)
> +{
> +	struct ethtool_value eval;
> +
> +	if (common_flags_mask) {
> +		eval.cmd = ETHTOOL_GFLAGS;
> +		eval.data = 0;
> +		ifr->ifr_data = (caddr_t)&eval;
> +		if (ioctl(fd, SIOCETHTOOL, ifr)) {
> +			perror("Cannot get device common flags");
> +			return 1;
> +		}
> +
> +		eval.cmd = ETHTOOL_SFLAGS;
> +		eval.data =
> +		    ((eval.data & ~(common_flags_mask | off_flags_mask)) |
> +		     (common_flags_wanted | off_flags_wanted));

Why should this use off_flags_mask and off_flags_wanted?  They should
both be 0 if this function is called.

> +		if (ioctl(fd, SIOCETHTOOL, ifr)) {
> +			perror("Cannot set device common flags");
> +			return 1;
> +		}
> +	} else {
> +		fprintf(stdout, "No common settings changed\n");
> +	}
> +
> +	return 0;
> +}
> +
>  static int do_goffload(int fd, struct ifreq *ifr)
>  {
>  	struct ethtool_value eval;
> @@ -2407,8 +2492,9 @@ static int do_soffload(int fd, struct ifreq *ifr)
>  		}
>  
>  		eval.cmd = ETHTOOL_SFLAGS;
> -		eval.data = ((eval.data & ~off_flags_mask) |
> -			     off_flags_wanted);
> +		eval.data =
> +		    ((eval.data & ~(off_flags_mask | common_flags_mask)) |
> +		     (off_flags_wanted | common_flags_wanted));

Similarly, why should this use common_flags_mask and
common_flags_wanted?

Ben.

>  
>  		err = ioctl(fd, SIOCETHTOOL, ifr);
>  		if (err) {

-- 
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.