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