From mboxrd@z Thu Jan  1 00:00:00 1970
From: Laszlo Ersek <lersek@redhat.com>
Subject: Re: [PATCH] xennet_get_drvinfo()
Date: Wed, 24 Nov 2010 21:47:32 +0100
Message-ID: <4CED79E4.4030608@redhat.com>
References: <4CDC3003.5040306@redhat.com>
	<1289501456.20667.7.camel@localhost.localdomain>
	<4CED7792.1090600@goop.org>
Mime-Version: 1.0
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xensource.com>
In-Reply-To: <4CED7792.1090600@goop.org>
List-Unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xensource.com>
List-Help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-Subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
Sender: xen-devel-bounces@lists.xensource.com
Errors-To: xen-devel-bounces@lists.xensource.com
To: Jeremy Fitzhardinge <jeremy@goop.org>
Cc: "xen-devel@lists.xensource.com" <xen-devel@lists.xensource.com>, Ian Campbell <Ian.Campbell@citrix.com>
List-Id: xen-devel@lists.xenproject.org

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@solarflare.com>
>      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.

Thanks for considering!
lacos