| Message ID | 20111114215241.5B8BF2900307@tardy |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
On Mon, 2011-11-14 at 13:52 -0800, Rick Jones wrote: > From: Rick Jones <rick.jones2@hp.com> > > Add a new .bus_name to virtio_config_ops then modify virtio_net to > call through to it in an ethtool .get_drvinfo routine to report > bus_info in ethtool -i output which is consistent with other > emulated NICs and the output of lspci. [...] > diff --git a/drivers/lguest/lguest_device.c b/drivers/lguest/lguest_device.c > index 0dc30ff..3724d45 100644 > --- a/drivers/lguest/lguest_device.c > +++ b/drivers/lguest/lguest_device.c > @@ -381,6 +381,11 @@ error: > return PTR_ERR(vqs[i]); > } > > +static const char *lg_bus_name(struct virtio_device *vdev) > +{ > + return "Not Implemented"; > +} [...] > +static const char *kvm_bus_name(struct virtio_device *vdev) > +{ > + return "Not Implemented"; > +} [...] Please use the existing 'not implemented' value, which is the empty string. If you think ethtool should print some helpful message instead of an empty string, please submit a patch for ethtool. Ben.
On 11/14/2011 02:30 PM, Ben Hutchings wrote: > On Mon, 2011-11-14 at 13:52 -0800, Rick Jones wrote: >> From: Rick Jones<rick.jones2@hp.com> >> >> Add a new .bus_name to virtio_config_ops then modify virtio_net to >> call through to it in an ethtool .get_drvinfo routine to report >> bus_info in ethtool -i output which is consistent with other >> emulated NICs and the output of lspci. > [...] >> diff --git a/drivers/lguest/lguest_device.c b/drivers/lguest/lguest_device.c >> index 0dc30ff..3724d45 100644 >> --- a/drivers/lguest/lguest_device.c >> +++ b/drivers/lguest/lguest_device.c >> @@ -381,6 +381,11 @@ error: >> return PTR_ERR(vqs[i]); >> } >> >> +static const char *lg_bus_name(struct virtio_device *vdev) >> +{ >> + return "Not Implemented"; >> +} > [...] >> +static const char *kvm_bus_name(struct virtio_device *vdev) >> +{ >> + return "Not Implemented"; >> +} > [...] > > Please use the existing 'not implemented' value, which is the empty > string. Will do. thanks, rick -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 11/14/2011 02:30 PM, Ben Hutchings wrote: > On Mon, 2011-11-14 at 13:52 -0800, Rick Jones wrote: >> From: Rick Jones<rick.jones2@hp.com> >> >> Add a new .bus_name to virtio_config_ops then modify virtio_net to >> call through to it in an ethtool .get_drvinfo routine to report >> bus_info in ethtool -i output which is consistent with other >> emulated NICs and the output of lspci. > [...] >> diff --git a/drivers/lguest/lguest_device.c b/drivers/lguest/lguest_device.c >> index 0dc30ff..3724d45 100644 >> --- a/drivers/lguest/lguest_device.c >> +++ b/drivers/lguest/lguest_device.c >> @@ -381,6 +381,11 @@ error: >> return PTR_ERR(vqs[i]); >> } >> >> +static const char *lg_bus_name(struct virtio_device *vdev) >> +{ >> + return "Not Implemented"; >> +} > [...] >> +static const char *kvm_bus_name(struct virtio_device *vdev) >> +{ >> + return "Not Implemented"; >> +} > [...] > > Please use the existing 'not implemented' value, which is the empty > string. If you think ethtool should print some helpful message instead > of an empty string, please submit a patch for ethtool. One question - will those actually be called via an ethtool path? In my poking about through the virtio code, I got the impression those modules were for "other than networking" sorts of things. rick -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, 2011-11-14 at 16:06 -0800, Rick Jones wrote: > On 11/14/2011 02:30 PM, Ben Hutchings wrote: > > On Mon, 2011-11-14 at 13:52 -0800, Rick Jones wrote: > >> From: Rick Jones<rick.jones2@hp.com> > >> > >> Add a new .bus_name to virtio_config_ops then modify virtio_net to > >> call through to it in an ethtool .get_drvinfo routine to report > >> bus_info in ethtool -i output which is consistent with other > >> emulated NICs and the output of lspci. > > [...] > >> diff --git a/drivers/lguest/lguest_device.c b/drivers/lguest/lguest_device.c > >> index 0dc30ff..3724d45 100644 > >> --- a/drivers/lguest/lguest_device.c > >> +++ b/drivers/lguest/lguest_device.c > >> @@ -381,6 +381,11 @@ error: > >> return PTR_ERR(vqs[i]); > >> } > >> > >> +static const char *lg_bus_name(struct virtio_device *vdev) > >> +{ > >> + return "Not Implemented"; > >> +} > > [...] > >> +static const char *kvm_bus_name(struct virtio_device *vdev) > >> +{ > >> + return "Not Implemented"; > >> +} > > [...] > > > > Please use the existing 'not implemented' value, which is the empty > > string. If you think ethtool should print some helpful message instead > > of an empty string, please submit a patch for ethtool. > > > One question - will those actually be called via an ethtool path? In my > poking about through the virtio code, I got the impression those modules > were for "other than networking" sorts of things. I don't know; I just assumed that was why you were adding them! In other contexts such as dev_printk() this string would make even less sense. Ben.
>>> Please use the existing 'not implemented' value, which is the empty >>> string. If you think ethtool should print some helpful message instead >>> of an empty string, please submit a patch for ethtool. >> >> >> One question - will those actually be called via an ethtool path? In my >> poking about through the virtio code, I got the impression those modules >> were for "other than networking" sorts of things. > > I don't know; I just assumed that was why you were adding them! In > other contexts such as dev_printk() this string would make even less > sense. Those were added to make sure there were no dangling references in the config_ops structure defined in those files and that the code calling through wouldn't go off into la-la land. Perhaps it isn't necessary with Rusty's suggestion that I check ".bus_info" against NULL? But that is why those were there, and not simply the instance in virtio_pci.c. I'll spin a v2 regardless. rick -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/lguest/lguest_device.c b/drivers/lguest/lguest_device.c index 0dc30ff..3724d45 100644 --- a/drivers/lguest/lguest_device.c +++ b/drivers/lguest/lguest_device.c @@ -381,6 +381,11 @@ error: return PTR_ERR(vqs[i]); } +static const char *lg_bus_name(struct virtio_device *vdev) +{ + return "Not Implemented"; +} + /* The ops structure which hooks everything together. */ static struct virtio_config_ops lguest_config_ops = { .get_features = lg_get_features, @@ -392,6 +397,7 @@ static struct virtio_config_ops lguest_config_ops = { .reset = lg_reset, .find_vqs = lg_find_vqs, .del_vqs = lg_del_vqs, + .bus_name = lg_bus_name, }; /* diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c index 6ee8410..4dc9d84 100644 --- a/drivers/net/virtio_net.c +++ b/drivers/net/virtio_net.c @@ -39,6 +39,7 @@ module_param(gso, bool, 0444); #define GOOD_COPY_LEN 128 #define VIRTNET_SEND_COMMAND_SG_MAX 2 +#define VIRTNET_DRIVER_VERSION "1.0.0" struct virtnet_stats { struct u64_stats_sync syncp; @@ -889,7 +890,21 @@ static void virtnet_get_ringparam(struct net_device *dev, } + +static void virtnet_get_drvinfo(struct net_device *dev, + struct ethtool_drvinfo *info) +{ + struct virtnet_info *vi = netdev_priv(dev); + struct virtio_device *vdev = vi->vdev; + + strlcpy(info->driver, KBUILD_MODNAME, sizeof(info->driver)); + strlcpy(info->version, VIRTNET_DRIVER_VERSION, sizeof(info->version)); + strlcpy(info->bus_info, virtio_bus_name(vdev), sizeof(info->bus_info)); + +} + static const struct ethtool_ops virtnet_ethtool_ops = { + .get_drvinfo = virtnet_get_drvinfo, .get_link = ethtool_op_get_link, .get_ringparam = virtnet_get_ringparam, }; diff --git a/drivers/s390/kvm/kvm_virtio.c b/drivers/s390/kvm/kvm_virtio.c index 94f49ff..725d90e 100644 --- a/drivers/s390/kvm/kvm_virtio.c +++ b/drivers/s390/kvm/kvm_virtio.c @@ -263,6 +263,11 @@ error: return PTR_ERR(vqs[i]); } +static const char *kvm_bus_name(struct virtio_device *vdev) +{ + return "Not Implemented"; +} + /* * The config ops structure as defined by virtio config */ @@ -276,6 +281,7 @@ static struct virtio_config_ops kvm_vq_configspace_ops = { .reset = kvm_reset, .find_vqs = kvm_find_vqs, .del_vqs = kvm_del_vqs, + .bus_name = kvm_bus_name, }; /* diff --git a/drivers/virtio/virtio_mmio.c b/drivers/virtio/virtio_mmio.c index acc5e43..2f57380 100644 --- a/drivers/virtio/virtio_mmio.c +++ b/drivers/virtio/virtio_mmio.c @@ -361,7 +361,12 @@ static int vm_find_vqs(struct virtio_device *vdev, unsigned nvqs, return 0; } +static const char *vm_bus_name(struct virtio_device *vdev) +{ + struct virtio_mmio_device *vm_dev = to_virtio_mmio_device(vdev); + return vm_dev->pdev->name; +} static struct virtio_config_ops virtio_mmio_config_ops = { .get = vm_get, @@ -373,6 +378,7 @@ static struct virtio_config_ops virtio_mmio_config_ops = { .del_vqs = vm_del_vqs, .get_features = vm_get_features, .finalize_features = vm_finalize_features, + .bus_name = vm_bus_name, }; diff --git a/drivers/virtio/virtio_pci.c b/drivers/virtio/virtio_pci.c index 79a31e5..764ec05 100644 --- a/drivers/virtio/virtio_pci.c +++ b/drivers/virtio/virtio_pci.c @@ -580,6 +580,13 @@ static int vp_find_vqs(struct virtio_device *vdev, unsigned nvqs, false, false); } +static const char *vp_bus_name(struct virtio_device *vdev) +{ + struct virtio_pci_device *vp_dev = to_vp_device(vdev); + + return pci_name(vp_dev->pci_dev); +} + static struct virtio_config_ops virtio_pci_config_ops = { .get = vp_get, .set = vp_set, @@ -590,6 +597,7 @@ static struct virtio_config_ops virtio_pci_config_ops = { .del_vqs = vp_del_vqs, .get_features = vp_get_features, .finalize_features = vp_finalize_features, + .bus_name = vp_bus_name, }; static void virtio_pci_release_dev(struct device *_d) diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h index add4790..63f98d0 100644 --- a/include/linux/virtio_config.h +++ b/include/linux/virtio_config.h @@ -100,6 +100,10 @@ * vdev: the virtio_device * This gives the final feature bits for the device: it can change * the dev->feature bits if it wants. + * @bus_name: return the bus name associated with the device + * vdev: the virtio_device + * This returns a pointer to the bus name a la pci_name from which + * the caller can then copy. */ typedef void vq_callback_t(struct virtqueue *); struct virtio_config_ops { @@ -117,6 +121,7 @@ struct virtio_config_ops { void (*del_vqs)(struct virtio_device *); u32 (*get_features)(struct virtio_device *vdev); void (*finalize_features)(struct virtio_device *vdev); + const char *(*bus_name)(struct virtio_device *vdev); }; /* If driver didn't advertise the feature, it will never appear. */ @@ -182,5 +187,14 @@ struct virtqueue *virtio_find_single_vq(struct virtio_device *vdev, return ERR_PTR(err); return vq; } + +static inline +const char *virtio_bus_name(struct virtio_device *vdev) +{ + if (!vdev->config->bus_name) + return "virtio"; + return vdev->config->bus_name(vdev); +} + #endif /* __KERNEL__ */ #endif /* _LINUX_VIRTIO_CONFIG_H */