| 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
| ||
|
Message-ID: <AANLkTiko=12CLgG_MMPshfwSRt7mZA+3DW+GRtxhaxh=@mail.gmail.com>
Date: Mon, 17 Jan 2011 18:17:41 -0800
From: Mahesh Bandewar <maheshb@...gle.com>
To: Ben Hutchings <bhutchings@...arflare.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 Fri, Jan 14, 2011 at 1:19 PM, Ben Hutchings
<bhutchings@...arflare.com> wrote:
> 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.
>
OK. I'll make those changes.
>> .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.
>
That is right! Actually the get (ETHTOOL_GFLAGS) operation confused
me. I thought the values are fetched and *preserved* while setting the
new value. But when looked at it carefully, that is not the case.
Actually why that ioctl() with ETHTOOL_GFLAGS required?
>> + 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?
For the same (wrong) reason mentioned above. I'll correct it and post
the new patch.
Thanks,
--mahesh..
>
> 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