Re: [PATCH] 3c59x: support more ethtool_ops

[Steffen Klassert]

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
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/