[PATCH] xennet_get_drvinfo()

[PATCH] xennet_get_drvinfo()

[Laszlo Ersek]

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

Thanks for considering,
Signed-off-by: Laszlo Ersek <lersek@redhat.com>

[0] http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=01414802054c382072b6cb9a1bdc6e243c74b2d5#patch17

---
 drivers/net/xen-netfront.c |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
index 3f71199..64ee838 100644
--- a/drivers/net/xen-netfront.c
+++ b/drivers/net/xen-netfront.c
@@ -1768,8 +1768,17 @@ static int xennet_set_coalesce(struct net_device *netdev,
 	return 0;
 }
 
+static void xennet_get_drvinfo(struct net_device *dev,
+			       struct ethtool_drvinfo *info)
+{
+	strcpy(info->driver, "xen-netfront");
+	strlcpy(info->bus_info, dev_name(dev->dev.parent),
+		sizeof info->bus_info);
+}
+
 static const struct ethtool_ops xennet_ethtool_ops =
 {
+	.get_drvinfo = xennet_get_drvinfo,
 	.set_tx_csum = ethtool_op_set_tx_csum,
 	.set_sg = xennet_set_sg,
 	.set_tso = xennet_set_tso,

Re: [PATCH] xennet_get_drvinfo()

[Ian Campbell]

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:

Acked-by: Ian Campbell <ian.campbell@citrix.com>

My only quibble would be that Linux generally prefers "sizeof(x)" to
"sizeof x".

Ian.

[0]
http://xenbits.xen.org/gitweb?p=people/ianc/linux-2.6.git;a=commitdiff;h=9a0429ca56f56d415a8b34a92dfa4e8737baf348

> 
> Thanks for considering,
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> 
> [0] http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=01414802054c382072b6cb9a1bdc6e243c74b2d5#patch17
> 
> ---
>  drivers/net/xen-netfront.c |    9 +++++++++
>  1 files changed, 9 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
> index 3f71199..64ee838 100644
> --- a/drivers/net/xen-netfront.c
> +++ b/drivers/net/xen-netfront.c
> @@ -1768,8 +1768,17 @@ static int xennet_set_coalesce(struct net_device *netdev,
>  	return 0;
>  }
>  
> +static void xennet_get_drvinfo(struct net_device *dev,
> +			       struct ethtool_drvinfo *info)
> +{
> +	strcpy(info->driver, "xen-netfront");
> +	strlcpy(info->bus_info, dev_name(dev->dev.parent),
> +		sizeof info->bus_info);
> +}
> +
>  static const struct ethtool_ops xennet_ethtool_ops =
>  {
> +	.get_drvinfo = xennet_get_drvinfo,
>  	.set_tx_csum = ethtool_op_set_tx_csum,
>  	.set_sg = xennet_set_sg,
>  	.set_tso = xennet_set_tso,
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel

Re: [PATCH] xennet_get_drvinfo()

[Jeremy Fitzhardinge]

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?

	J


> Acked-by: Ian Campbell <ian.campbell@citrix.com>
>
> My only quibble would be that Linux generally prefers "sizeof(x)" to
> "sizeof x".
>
> Ian.
>
> [0]
> http://xenbits.xen.org/gitweb?p=people/ianc/linux-2.6.git;a=commitdiff;h=9a0429ca56f56d415a8b34a92dfa4e8737baf348
>
>> Thanks for considering,
>> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
>>
>> [0] http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=01414802054c382072b6cb9a1bdc6e243c74b2d5#patch17
>>
>> ---
>>  drivers/net/xen-netfront.c |    9 +++++++++
>>  1 files changed, 9 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
>> index 3f71199..64ee838 100644
>> --- a/drivers/net/xen-netfront.c
>> +++ b/drivers/net/xen-netfront.c
>> @@ -1768,8 +1768,17 @@ static int xennet_set_coalesce(struct net_device *netdev,
>>  	return 0;
>>  }
>>  
>> +static void xennet_get_drvinfo(struct net_device *dev,
>> +			       struct ethtool_drvinfo *info)
>> +{
>> +	strcpy(info->driver, "xen-netfront");
>> +	strlcpy(info->bus_info, dev_name(dev->dev.parent),
>> +		sizeof info->bus_info);
>> +}
>> +
>>  static const struct ethtool_ops xennet_ethtool_ops =
>>  {
>> +	.get_drvinfo = xennet_get_drvinfo,
>>  	.set_tx_csum = ethtool_op_set_tx_csum,
>>  	.set_sg = xennet_set_sg,
>>  	.set_tso = xennet_set_tso,
>>
>> _______________________________________________
>> Xen-devel mailing list
>> Xen-devel@lists.xensource.com
>> http://lists.xensource.com/xen-devel
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel
>

Re: [PATCH] xennet_get_drvinfo()

[Laszlo Ersek]

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

Re: [PATCH] xennet_get_drvinfo()

[Jeremy Fitzhardinge]

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

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

    J

VGA console low power mode in Xen

[Carsten Schiers]

Hi,

I am a bit lost, because after exchanging my PCI console VGA (oldtimer) 
with aprox 7-9W with a
new Radeon 5450, which will draw less than 7W theoretically in 2D mode, 
I have 15W more on the 
meter.

In "normal" kernels, I guess I have to enable radeonfb in old versions 
and KMS in newer. 

I currently use Xen 4.0.1 with 2.6.18 Kernel. 2.6.34.7 forward patched 
is theoretically available.

Is my assumptions right that the card is not in low power mode and what 
would you recommend to
set on Xen and/or Kernel in order to get the promised 7W. I need the PCI 
slot for something else.

I only need console and prefer VGA over serial, because of BIOS settings 
etc.

Thanks,
Carsten.

Re: VGA console low power mode in Xen

[Stefano Stabellini]

On Wed, 24 Nov 2010, Carsten Schiers wrote:
> Hi,
> 
> I am a bit lost, because after exchanging my PCI console VGA (oldtimer) 
> with aprox 7-9W with a
> new Radeon 5450, which will draw less than 7W theoretically in 2D mode, 
> I have 15W more on the 
> meter.
> 
> In "normal" kernels, I guess I have to enable radeonfb in old versions 
> and KMS in newer. 
> 
> I currently use Xen 4.0.1 with 2.6.18 Kernel. 2.6.34.7 forward patched 
> is theoretically available.
> 
> Is my assumptions right that the card is not in low power mode and what 
> would you recommend to
> set on Xen and/or Kernel in order to get the promised 7W. I need the PCI 
> slot for something else.
> 
> I only need console and prefer VGA over serial, because of BIOS settings 
> etc.
> 

It seems like you need to upgrade your dom0 kernel, give a look at this
wiki page:

http://wiki.xen.org/xenwiki/XenDom0Kernels

Re: [PATCH] xennet_get_drvinfo()

[Ian Campbell]

On Wed, 2010-11-24 at 20:56 +0000, Jeremy Fitzhardinge wrote:
> 
> > 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. 

Whatever we do we shouldn't deliberately cause the xen.git 2.6.32 branch
to deviate from what upstream does, if we think this field needs
renaming lets do it on linux-netdev and backport the result.

Ian.

Re: [PATCH] xennet_get_drvinfo()

[Jeremy Fitzhardinge]

On 11/25/2010 02:17 AM, Ian Campbell wrote:
> On Wed, 2010-11-24 at 20:56 +0000, Jeremy Fitzhardinge wrote:
>>> 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. 
> Whatever we do we shouldn't deliberately cause the xen.git 2.6.32 branch
> to deviate from what upstream does, if we think this field needs
> renaming lets do it on linux-netdev and backport the result.

Yes, exactly.  I backported Ben's change so that .32 would be more like
later kernels; if the device name is useless (which it is), then we
should change it everywhere.

    J