Re: [PATCHv1] e1000e: Allow ethtool to enable/disable loopback.

[Mahesh Bandewar <maheshb@google.com>]

On Wed, May 11, 2011 at 10:50 PM, Michał Mirosław <mirqus@gmail.com> wrote:
> W dniu 12 maja 2011 01:11 użytkownik Mahesh Bandewar
> <maheshb@google.com> napisał:
>> On Wed, May 11, 2011 at 12:15 PM, Michał Mirosław <mirqus@gmail.com> wrote:
>>> 2011/5/11 Mahesh Bandewar <maheshb@google.com>:
>>>> This patch adds e1000_set_features() to handle loopback mode. When loopback
>>>> is enabled, it enables internal-MAC loopback.
>>> Please wait for this driver's conversion to hw_features. One comment
>>> below, though.
>> This is not intrusive so should not create problems when that happens.
>
> Fine by me then.
>
>>> [...]
>>>> --- a/drivers/net/e1000e/netdev.c
>>>> +++ b/drivers/net/e1000e/netdev.c
>>> [...]
>>>> +static int e1000_set_features(struct net_device *dev, u32 features)
>>>> +{
>>>> +       u32 changed = dev->features ^ features;
>>>> +
>>>> +       if ((changed & NETIF_F_LOOPBACK) && netif_running(dev))
>>>> +               e1000_set_loopback(dev, features);
>>>> +
>>>> +       return 0;
>>>> +}
>>> [...]
>>>
>>> If e1000_set_loopback() fails, this should set dev->features to passed
>>> features (but keeping NETIF_F_LOOPBACK unchanged in dev->features) to
>>> keep the state consistent.
>> set_features() can return the return code of set_loopback() instead of
>> 0; this way the consistency will be maintained.
>
> Only as long as NETIF_F_LOOPBACK is the only bit set in hw_features.
> netdev_update_features() can't really know which features were changed
> and which failed when ndo_set_features callback returns non-zero.
>
This is more of an API shortcoming. Callback will have to revert
changes made (rollback) before returning non-zero value to keep it
consistent.

Thanks,
--mahesh..

> Best Regards,
> Michał Mirosław
>

------------------------------------------------------------------------------
Achieve unprecedented app performance and reliability
What every C/C++ and Fortran developer should know.
Learn how Intel has extended the reach of its next-generation tools
to help boost performance applications - inlcuding clusters.
http://p.sf.net/sfu/intel-dev2devmay
_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel&#174; Ethernet, visit http://communities.intel.com/community/wired