[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <1295039984.5386.19.camel@bwh-desktop>
Date: Fri, 14 Jan 2011 21:19:44 +0000
From: Ben Hutchings <bhutchings@...arflare.com>
To: Mahesh Bandewar <maheshb@...gle.com>
Cc: David Miller <davem@...emloft.net>,
Tom Herbert <therbert@...gle.com>,
Laurent Chavey <chavey@...gle.com>,
netdev <netdev@...r.kernel.org>
Subject: Re: [PATCH] ethtool : Add option -L | --set-common to set common
flags.
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@...gle.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.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists