what's in a bus_info

what's in a bus_info

[Rick Jones]

...or would an interface name smell as sweet? (as PCI bus addressing)

Is there a "standard" for what is returned in bus_info of 
ethtool_drvinfo?  I have been very used to seeing PCI bus addressing 
information in that field (at least as displayed by ethtool -i) and when 
I went to "leverage how to" from other drivers, to add "native" ethtool 
-i support to virtio_net, I ended-up with "eth0" rather than the PCI 
information I see in lspci output and in ethtool -i against other 
devices.  Including an emulated e1000 interface in the same kernel.

What I'm doing is calling pci_name(), feeding it with to_pci_dev() from 
the address of the struct device in the struct net_device.  The perhaps 
overly paranoid work-in-progress:

static void virtnet_get_drvinfo(struct net_device *dev,
                                 struct ethtool_drvinfo *info)
{

         struct device *dev_dev;
         struct pci_dev *pci_dev = NULL;


         dev_dev = &(dev->dev);
         if (dev_dev != NULL)
                 pci_dev = to_pci_dev(dev_dev);
         strlcpy(info->driver, KBUILD_MODNAME, sizeof(info->driver));
         strlcpy(info->version,"I am versionless", sizeof(info->version));
         strlcpy(info->fw_version,"I have no firmware", 
sizeof(info->fw_version));
         strlcpy(info->bus_info,
                 (pci_dev != NULL) ? pci_name(pci_dev) : "",
                 sizeof(info->bus_info));

}

So, with the emulated e1000 I get:

raj@raj-ubuntu-guest:~$ ethtool -i eth0
driver: e1000
version: 7.3.21-k8-NAPI
firmware-version: N/A
bus-info: 0000:00:11.0
raj@raj-ubuntu-guest:~$

and I see that the e1000 driver calls pci_name().  However, the code 
above, when I boot the guest with the virtio device gives me:

raj@raj-ubuntu-guest:~$ ethtool -i eth0
driver: virtio_net
version: I am versionless
firmware-version: I have no firmware
bus-info: eth0

00:03.0 Ethernet controller: Red Hat, Inc Virtio network device

Am I chasing the wrong pointers?  Is it  a function of virtio?

rick jones

BTW, I notice some drivers call strlcpy and some strncpy, and some even 
call strcpy.  Is there one that is meant to be preferred over the others?

Re: what's in a bus_info

[Ben Hutchings]

On Fri, 2011-11-04 at 15:27 -0700, Rick Jones wrote:
> ...or would an interface name smell as sweet? (as PCI bus addressing)
> 
> Is there a "standard" for what is returned in bus_info of 
> ethtool_drvinfo?  I have been very used to seeing PCI bus addressing 
> information in that field (at least as displayed by ethtool -i) and when 
> I went to "leverage how to" from other drivers, to add "native" ethtool 
> -i support to virtio_net, I ended-up with "eth0" rather than the PCI 
> information I see in lspci output and in ethtool -i against other 
> devices.  Including an emulated e1000 interface in the same kernel.
>
> What I'm doing is calling pci_name(), feeding it with to_pci_dev() from 
> the address of the struct device in the struct net_device.

to_pci_dev() just uses container_of() to find a pci_dev when you have a
pointer to the generic device structure embedded in it.  However, you're
passing a pointer to the device embedded in a net_device.  The net
device is a child of the PCI device, so you need to do:

	dev_dev = dev->dev.parent;

And you don't even have to assume that the parent is a PCI device,
because you can use the generic dev_name().

But you don't even need to this, since the ethtool core has a default
implementation that does this...

[...]
> BTW, I notice some drivers call strlcpy and some strncpy, and some even 
> call strcpy.  Is there one that is meant to be preferred over the others?

strlcpy() is preferred - if it has to truncate, it will at least leave a
null terminator, as clients may expect.  Back when drivers handled
SIOCETHTOOL directly strncpy() may have been preferable since they were
responsible for initialising the entire structure returned to
user-space.

Ben.

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

Re: what's in a bus_info

[Rick Jones]

On 11/04/2011 04:02 PM, Ben Hutchings wrote:
> On Fri, 2011-11-04 at 15:27 -0700, Rick Jones wrote:
>> ...or would an interface name smell as sweet? (as PCI bus addressing)
>>
>> Is there a "standard" for what is returned in bus_info of
>> ethtool_drvinfo?  I have been very used to seeing PCI bus addressing
>> information in that field (at least as displayed by ethtool -i) and when
>> I went to "leverage how to" from other drivers, to add "native" ethtool
>> -i support to virtio_net, I ended-up with "eth0" rather than the PCI
>> information I see in lspci output and in ethtool -i against other
>> devices.  Including an emulated e1000 interface in the same kernel.
>>
>> What I'm doing is calling pci_name(), feeding it with to_pci_dev() from
>> the address of the struct device in the struct net_device.
>
> to_pci_dev() just uses container_of() to find a pci_dev when you have a
> pointer to the generic device structure embedded in it.  However, you're
> passing a pointer to the device embedded in a net_device.  The net
> device is a child of the PCI device, so you need to do:
>
> 	dev_dev = dev->dev.parent;
>
> And you don't even have to assume that the parent is a PCI device,
> because you can use the generic dev_name().
>
> But you don't even need to this, since the ethtool core has a default
> implementation that does this...

Yes, I noticed that.  For a little while I was confused because ethtool 
-i was emitting something even before I added a ".get_drvinfo" - though 
what it ends-up returning in my case is "virtio0." Which is also what I 
get if I take the path through the virtio_device to the struct device 
therein instead of the path through the struct net_device alone.

I guess that wraps back around to the question of whether there is a 
"standard" for what should be in bus_info.  And if it is impractical to 
get the PCI bus information, whether it is better to return virtioN or 
ethN.  Or perhaps something else entirely.

> [...]
>> BTW, I notice some drivers call strlcpy and some strncpy, and some even
>> call strcpy.  Is there one that is meant to be preferred over the others?
>
> strlcpy() is preferred - if it has to truncate, it will at least leave a
> null terminator, as clients may expect.  Back when drivers handled
> SIOCETHTOOL directly strncpy() may have been preferable since they were
> responsible for initialising the entire structure returned to
> user-space.

Thanks.  Perhaps that is another "floor sweeping" opportunity.

rick jones

Re: what's in a bus_info

[Ben Hutchings]

On Fri, 2011-11-04 at 16:31 -0700, Rick Jones wrote:
> On 11/04/2011 04:02 PM, Ben Hutchings wrote:
> > On Fri, 2011-11-04 at 15:27 -0700, Rick Jones wrote:
> >> ...or would an interface name smell as sweet? (as PCI bus addressing)
> >>
> >> Is there a "standard" for what is returned in bus_info of
> >> ethtool_drvinfo?  I have been very used to seeing PCI bus addressing
> >> information in that field (at least as displayed by ethtool -i) and when
> >> I went to "leverage how to" from other drivers, to add "native" ethtool
> >> -i support to virtio_net, I ended-up with "eth0" rather than the PCI
> >> information I see in lspci output and in ethtool -i against other
> >> devices.  Including an emulated e1000 interface in the same kernel.
> >>
> >> What I'm doing is calling pci_name(), feeding it with to_pci_dev() from
> >> the address of the struct device in the struct net_device.
> >
> > to_pci_dev() just uses container_of() to find a pci_dev when you have a
> > pointer to the generic device structure embedded in it.  However, you're
> > passing a pointer to the device embedded in a net_device.  The net
> > device is a child of the PCI device, so you need to do:
> >
> > 	dev_dev = dev->dev.parent;
> >
> > And you don't even have to assume that the parent is a PCI device,
> > because you can use the generic dev_name().
> >
> > But you don't even need to this, since the ethtool core has a default
> > implementation that does this...
> 
> Yes, I noticed that.  For a little while I was confused because ethtool 
> -i was emitting something even before I added a ".get_drvinfo" - though 
> what it ends-up returning in my case is "virtio0." Which is also what I 
> get if I take the path through the virtio_device to the struct device 
> therein instead of the path through the struct net_device alone.
> 
> I guess that wraps back around to the question of whether there is a 
> "standard" for what should be in bus_info.  And if it is impractical to 
> get the PCI bus information,

I'm not that familiar with virtio, but would I be right in thinking that
the virtio 'bus' device is likely to be the child of a PCI device?  So
then you mgiht want to get bus_name() for the grandparent of the net
device:
	dev_dev = dev->dev.parent->parent;
(possibly checking for nulls).

If there's some reasonable way to distinguish a 'real' from a virtual
bus then we could have the generic implementation try to follow parents
until it finds a bus device.  However I think the device model
maintainers have been gradually moving away from the bus/class
distinction and so we may not be able to do that.

> whether it is better to return virtioN or 
> ethN.  Or perhaps something else entirely.
[...]

Returning the device name seems entirely unhelpful since the user
already has that.  'virtioN' is perhaps not much better though.

Ben.

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

Re: what's in a bus_info

[Rick Jones]

On 11/04/2011 04:42 PM, Ben Hutchings wrote:
> On Fri, 2011-11-04 at 16:31 -0700, Rick Jones wrote:

>> I guess that wraps back around to the question of whether there is a
>> "standard" for what should be in bus_info.  And if it is impractical to
>> get the PCI bus information,
>
> I'm not that familiar with virtio, but would I be right in thinking that
> the virtio 'bus' device is likely to be the child of a PCI device?  So
> then you mgiht want to get bus_name() for the grandparent of the net
> device:
> 	dev_dev = dev->dev.parent->parent;
> (possibly checking for nulls).

I'll take a look.

> If there's some reasonable way to distinguish a 'real' from a virtual
> bus then we could have the generic implementation try to follow parents
> until it finds a bus device.  However I think the device model
> maintainers have been gradually moving away from the bus/class
> distinction and so we may not be able to do that.
>
>> whether it is better to return virtioN or
>> ethN.  Or perhaps something else entirely.
> [...]
>
> Returning the device name seems entirely unhelpful since the user
> already has that.  'virtioN' is perhaps not much better though.

Agreed, but thought I should ask :)

rick

Re: what's in a bus_info

[Stephen Hemminger]

On Fri, 04 Nov 2011 17:05:05 -0700
Rick Jones <rick.jones2@hp.com> wrote:

> On 11/04/2011 04:42 PM, Ben Hutchings wrote:
> > On Fri, 2011-11-04 at 16:31 -0700, Rick Jones wrote:
> 
> >> I guess that wraps back around to the question of whether there is a
> >> "standard" for what should be in bus_info.  And if it is impractical to
> >> get the PCI bus information,
> >
> > I'm not that familiar with virtio, but would I be right in thinking that
> > the virtio 'bus' device is likely to be the child of a PCI device?  So
> > then you mgiht want to get bus_name() for the grandparent of the net
> > device:
> > 	dev_dev = dev->dev.parent->parent;
> > (possibly checking for nulls).
> 
> I'll take a look.
> 
> > If there's some reasonable way to distinguish a 'real' from a virtual
> > bus then we could have the generic implementation try to follow parents
> > until it finds a bus device.  However I think the device model
> > maintainers have been gradually moving away from the bus/class
> > distinction and so we may not be able to do that.
> >
> >> whether it is better to return virtioN or
> >> ethN.  Or perhaps something else entirely.
> > [...]
> >
> > Returning the device name seems entirely unhelpful since the user
> > already has that.  'virtioN' is perhaps not much better though.
> 
> Agreed, but thought I should ask :)

My view of bus_info, is that it is an informational string for administrators.
Tools shouldn't depend on the value.  If a tool wants to find out about
the physical device, then it should readlink the value of /sys/class/net/ethX/device

Re: what's in a bus_info

[Rick Jones]

On 11/04/2011 05:07 PM, Stephen Hemminger wrote:
> My view of bus_info, is that it is an informational string for administrators.
> Tools shouldn't depend on the value.  If a tool wants to find out about
> the physical device, then it should readlink the value of /sys/class/net/ethX/device

Just a quick look under device for the virtio_net interface shows it 
isn't quite as "complete" as a bare iron tg3 interface (as an example)

The bare iron tg3 example:
raj@tardy:~$ ls /sys/class/net/eth0/device
broken_parity_status      enable         net        resource0
class                     firmware_node  numa_node  subsystem
config                    irq            power      subsystem_device
consistent_dma_mask_bits  local_cpulist  remove     subsystem_vendor
device                    local_cpus     rescan     uevent
dma_mask_bits             modalias       reset      vendor
driver                    msi_bus        resource   vpd

The virtio_net interface:

raj@raj-ubuntu-guest:~$ ls /sys/class/net/eth0/device
device  features  net    status     uevent
driver  modalias  power  subsystem  vendor


I have found that if I add a function to return a struct pci_dev * given 
a struct virtio_device, to virtio/virtio_pci.c and do a pci_name() 
against that, I do get a familiar looking bus address:

raj@raj-ubuntu-guest:~$ ethtool -i eth0
driver: virtio_net
version: I am versionless
firmware-version: I have no firmware
bus-info: 0000:00:03.0

and it even matches the lspci output for the guest:

00:03.0 Ethernet controller: Red Hat, Inc Virtio network device

(still my "playing around" version, hence the silly strings in version 
and firmware-version)

I suppose the question becomes whether it is worthwhile to clean it up 
and submit.    I'm also thinking it would be goodness to start 
displaying some sort of version number, and if there is anything the 
driver might query to fill-in as a firmware-version - perhaps something 
about the version of the driver on the other side?

rick jones