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

[Laurent Chavey <chavey@google.com>]

On Thu, Apr 8, 2010 at 12:51 PM, Jeff Garzik <jeff@garzik.org> wrote:
> On 04/08/2010 02:29 PM, Ben Hutchings wrote:
>>
>> On Thu, 2010-04-08 at 10:35 -0700, chavey@google.com wrote:
>>>
>>> From: Ranjit Manomohan<ranjitm@google.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@google.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?

we have some autotests that make use of this feature for e1000(e), forcedeth and
tg3 that we would like to share with the community to do perf / functional tests
regression.
>
>        Jeff
>
>
>