* Re: [PATCH] 3c59x: support more ethtool_ops [not found] <200501111913.j0BJDnIL009341@hera.kernel.org> @ 2005-01-11 19:42 ` Jeff Garzik 2005-01-12 15:49 ` Steffen Klassert 0 siblings, 1 reply; 3+ messages in thread From: Jeff Garzik @ 2005-01-11 19:42 UTC (permalink / raw) To: klassert, Andrew Morton; +Cc: Linux Kernel Mailing List Linux Kernel Mailing List wrote: > +static void vortex_get_ethtool_stats(struct net_device *dev, > + struct ethtool_stats *stats, u64 *data) > +{ > + struct vortex_private *vp = netdev_priv(dev); > + unsigned long flags; > + > + spin_lock_irqsave(&vp->lock, flags); > + update_stats(dev->base_addr, dev); > + spin_unlock_irqrestore(&vp->lock, flags); > + > + data[0] = vp->stats.rx_packets; > + data[1] = vp->stats.tx_packets; > + data[2] = vp->stats.rx_bytes; > + data[3] = vp->stats.tx_bytes; > + data[4] = vp->stats.collisions; > + data[5] = vp->stats.tx_carrier_errors; > + data[6] = vp->stats.tx_heartbeat_errors; > + data[7] = vp->stats.tx_window_errors; > +} Everything in the patch is correct except for the above. This is very wrong -- get_ethtool_stats() is for NIC-specific stats. The above stats are already available through the generic net stack. Jeff ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] 3c59x: support more ethtool_ops 2005-01-11 19:42 ` [PATCH] 3c59x: support more ethtool_ops Jeff Garzik @ 2005-01-12 15:49 ` Steffen Klassert 2005-01-12 18:38 ` Jeff Garzik 0 siblings, 1 reply; 3+ messages in thread From: Steffen Klassert @ 2005-01-12 15:49 UTC (permalink / raw) To: Jeff Garzik, Andrew Morton; +Cc: Linux Kernel Mailing List On Tue, Jan 11, 2005 at 02:42:48PM -0500, Jeff Garzik wrote: > Linux Kernel Mailing List wrote: > >+static void vortex_get_ethtool_stats(struct net_device *dev, > >+ struct ethtool_stats *stats, u64 *data) > >+{ > >+ struct vortex_private *vp = netdev_priv(dev); > >+ unsigned long flags; > >+ > >+ spin_lock_irqsave(&vp->lock, flags); > >+ update_stats(dev->base_addr, dev); > >+ spin_unlock_irqrestore(&vp->lock, flags); > >+ > >+ data[0] = vp->stats.rx_packets; > >+ data[1] = vp->stats.tx_packets; > >+ data[2] = vp->stats.rx_bytes; > >+ data[3] = vp->stats.tx_bytes; > >+ data[4] = vp->stats.collisions; > >+ data[5] = vp->stats.tx_carrier_errors; > >+ data[6] = vp->stats.tx_heartbeat_errors; > >+ data[7] = vp->stats.tx_window_errors; > >+} > > Everything in the patch is correct except for the above. > > This is very wrong -- get_ethtool_stats() is for NIC-specific stats. > The above stats are already available through the generic net stack. > When I did this I took the e100 driver as an example, here get_ethtool_stats() provides the generic and the NIC specific stats. Thus I added all in vp->stats counted stats (in this case just the generic) to get_ethtool_stats(). But anyway, I will rework this part. What is the expected behavior of get_ethtool_stats()? Provide just the NIC specific stats or all stats as the e100 driver does it? Steffen ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] 3c59x: support more ethtool_ops 2005-01-12 15:49 ` Steffen Klassert @ 2005-01-12 18:38 ` Jeff Garzik 0 siblings, 0 replies; 3+ messages in thread From: Jeff Garzik @ 2005-01-12 18:38 UTC (permalink / raw) To: Steffen Klassert; +Cc: Andrew Morton, Linux Kernel Mailing List Steffen Klassert wrote: > But anyway, I will rework this part. > What is the expected behavior of get_ethtool_stats()? > Provide just the NIC specific stats or all stats as the e100 driver does it? Just the NIC-specific stats. Jeff ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2005-01-12 18:39 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <200501111913.j0BJDnIL009341@hera.kernel.org>
2005-01-11 19:42 ` [PATCH] 3c59x: support more ethtool_ops Jeff Garzik
2005-01-12 15:49 ` Steffen Klassert
2005-01-12 18:38 ` Jeff Garzik
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox