netdev - Re: [PATCH 1/1] add ethtool loopback support

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: <4BBE33C9.5000507@garzik.org>
Date:	Thu, 08 Apr 2010 15:51:37 -0400
From:	Jeff Garzik <jeff@...zik.org>
To:	Ben Hutchings <bhutchings@...arflare.com>, chavey@...gle.com
CC:	davem@...emloft.net, netdev@...r.kernel.org, therbert@...gle.com
Subject: Re: [PATCH 1/1] add ethtool loopback support

On 04/08/2010 02:29 PM, Ben Hutchings wrote:
> On Thu, 2010-04-08 at 10:35 -0700, chavey@...gle.com wrote:
>> From: Ranjit Manomohan<ranjitm@...gle.com>
>> Date: Wed, 7 Apr 2010 15:19:48 -0700
>>
>> Add an ethtool option to use internal loopback mode for testing.
>> This feature is used for component and driver test coverage by putting
>> the device in hardware loopback mode and sending / receiving network
>> traffic from a user application to test the hardware and driver
>> transmit / receive paths.
>>
>> Signed-off-by: laurent chavey<chavey@...gle.com>
>> ---
>>   include/linux/ethtool.h |   16 ++++++++++++++++
>>   net/core/ethtool.c      |   40 ++++++++++++++++++++++++++++++++++++++++
>>   2 files changed, 56 insertions(+), 0 deletions(-)
>>
>> diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
>> index b33f316..df1dcc7 100644
>> --- a/include/linux/ethtool.h
>> +++ b/include/linux/ethtool.h
>> @@ -292,6 +292,18 @@ struct ethtool_stats {
>>   	__u64	data[0];
>>   };
>>
>> +/* for setting the NIC into loopback mode */
>> +struct ethtool_loopback {
>> +	u32 cmd;		/* ETHTOOL_SLOOPBACK */
>> +	u32 type;		/* ethtool_loopback_type */
>> +};
>> +
>> +enum ethtool_loopback_type {
>> +	ETH_MAC			= 0x00000001,
>> +	ETH_PHY_INT		= 0x00000002,
>> +	ETH_PHY_EXT		= 0x00000004
>> +};
> [...]
>
> There are many different places you can loop back within a MAC or PHY,
> not to mention bypassing the MAC altogether.  See
> drivers/net/sfc/mcdi_pcol.h, starting from the line
> '#define MC_CMD_LOOPBACK_NONE 0'.  I believe we implement all of those
> loopback modes on at least one board.
>
> Also are these supposed to be an enumeration or flags?  In theory you
> could use wire-side and host-side loopback at the same time if they
> don't overlap, but it's probably too much trouble to bother with.  Any
> other combination is meaningless.


Additionally, ethtool is intended as an interface that exports 
capabilities generally useful to users.  Is this feature really 
something that users will make use of?  It seems more in the realm of 
design validation

The existing ETHTOOL_TEST ioctl was intended as a coarse-grained method 
of verifying that the NIC is working, because fine-grained tests are 
inevitably both NIC-specific, and too involved for all but the most 
sophisticated users (read: the designers who built the NIC).

So, consider this a _weak_ objection.  Sure we could implement this, but 
will it really be useful to most users?

	Jeff


--
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