Re: [Xen-devel] [PATCH] xennet_get_drvinfo()

[Jeremy Fitzhardinge <jeremy@xxxxxxxx>]

On 11/24/2010 12:47 PM, Laszlo Ersek wrote:
> On 11/24/10 21:37, Jeremy Fitzhardinge wrote:
>> On 11/11/2010 10:50 AM, Ian Campbell wrote:
>>> On Thu, 2010-11-11 at 18:03 +0000, Laszlo Ersek wrote:
>>>> The following patch, written for xen/next-2.6.32, adds
>>>> get_drvinfo() ("ethtool -i") support to netfront. If there is no
>>>> default get_drvinfo() in effect [0], then the patch is intended to
>>>> remedy the following situation:
>>>>
>>>>    # ethtool -i eth0
>>>>    Cannot get driver information: Operation not supported
>>>>
>>>> like this:
>>>>
>>>>    # ethtool -i eth0
>>>>    driver: xen-netfront
>>>>    version:
>>>>    firmware-version:
>>>>    bus-info: vif-0
>>>>
>>>> If a default get_drvinfo() is in effect (see [0] again), then the
>>>> fallback works like this:
>>>>
>>>>    # ethtool -i eth0
>>>>    driver: vif
>>>>    version:
>>>>    firmware-version:
>>>>    bus-info: vif-0
>>>>
>>>> and it's more fortunate to return the module name ("driver:
>>>> xen-netfront") than "driver: vif".
>>> This is pretty much the same as a patch which I clearly forgot to
>>> send a
>>> pull request for ages ago[0] so:
>>
>> I remember a review comment which suggested it was unnecessary?  Ah,
>> here:
>>
>>      This should already be covered by:
>>
>>      commit 01414802054c382072b6cb9a1bdc6e243c74b2d5
>>      Author: Ben Hutchings<bhutchings@xxxxxxxxxxxxxx>
>>      Date: Tue Aug 17 02:31:15 2010 -0700
>>
>>      ethtool: Provide a default implementation of
>> ethtool_ops::get_drvinfo
>>
>>      Ben.
>>
>> Maybe I should just cherry-pick that back?
>
> I don't think so, that patch is super-useful.
>
> I knew about that, though. I wrote
>
> >>> The following patch, written for xen/next-2.6.32, adds
> >>> get_drvinfo() ("ethtool -i") support to netfront. If there is no
> >>> default get_drvinfo() in effect [0], then the patch is intended to
> >>> remedy the following situation:
>
> and
>
> >>> If a default get_drvinfo() is in effect (see [0] again), then the
> >>> fallback works like this:
>
> and
>
> >>> [0]
> http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=01414802054c382072b6cb9a1bdc6e243c74b2d5#patch17
>
> The fallback does cover the functionality ("driver: vif"), but
> "driver: xen-netfront" seems to be nicer, because that matches the
> module (driver) name.

Hm.  Well, perhaps we should change the driver name?  We can keep "vif"
as an alias.

    J

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel