diff mbox

[-next] sundance: Add initial ethtool stats support

Message ID 4CB54C4F.5070205@kernel.org
State Changes Requested, archived
Delegated to: David Miller
Headers show

Commit Message

Denis Kirjanov Oct. 13, 2010, 6:06 a.m. UTC 
On 10/13/2010 12:57 AM, Joe Perches wrote:
> On Wed, 2010-10-13 at 00:36 +0400, Denis Kirjanov wrote:
> 
>> diff --git a/drivers/net/sundance.c b/drivers/net/sundance.c
> 
> Hi Denis.
>  
> Just a few trivial misspellings of deffered -> deferred

Hi Joe,
thanks for pointing that out.

[PATCH -next v4] sundance: Add ethtool stats support

Add ethtool stats support

Signed-off-by: Denis Kirjanov <dkirjanov@kernel.org>
---
V2:
 check for the ETH_SS_STATS in get_string()
 use xstats struct for ethtool stats
V3:
 make counters 64-bits wide
V4:
 fixed some misspellings 

 drivers/net/sundance.c |   90 ++++++++++++++++++++++++++++++++++++++++++++----
 1 files changed, 83 insertions(+), 7 deletions(-)

Comments

Eric Dumazet Oct. 13, 2010, 6:33 a.m. UTC | #1 
Le mercredi 13 octobre 2010 à 10:06 +0400, Denis Kirjanov a écrit :

> @@ -1494,13 +1506,23 @@ static struct net_device_stats *get_stats(struct net_device *dev)
>  	dev->stats.rx_missed_errors	+= ioread8(ioaddr + RxMissed);
>  	dev->stats.tx_packets += ioread16(ioaddr + TxFramesOK);
>  	dev->stats.rx_packets += ioread16(ioaddr + RxFramesOK);
> -	dev->stats.collisions += ioread8(ioaddr + StatsLateColl);
> -	dev->stats.collisions += ioread8(ioaddr + StatsMultiColl);
> -	dev->stats.collisions += ioread8(ioaddr + StatsOneColl);
>  	dev->stats.tx_carrier_errors += ioread8(ioaddr + StatsCarrierError);
> -	ioread8(ioaddr + StatsTxDefer);
> -	for (i = StatsTxDefer; i <= StatsMcastRx; i++)
> -		ioread8(ioaddr + i);
> +
> +	np->xstats.tx_multiple_collisions += ioread8(ioaddr + StatsMultiColl);
> +	np->xstats.tx_single_collisions += ioread8(ioaddr + StatsOneColl);
> +	np->xstats.tx_late_collisions += ioread8(ioaddr + StatsLateColl);
> +	dev->stats.collisions += np->xstats.tx_multiple_collisions
> +		+ np->xstats.tx_single_collisions
> +		+ np->xstats.tx_late_collisions;


Oh well..., really ?

Each time we are calling get_stats(), you do

dev->stats.collisions += (number of accumulated collisions since device
is up)

instead of

dev->stats.collisions += (number of accumulated collisions since last
time we read counters)


Can you see the problem ?



--
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
Denis Kirjanov Oct. 13, 2010, 7:02 a.m. UTC | #2 
On 10/13/2010 10:33 AM, Eric Dumazet wrote:
> Le mercredi 13 octobre 2010 à 10:06 +0400, Denis Kirjanov a écrit :
> 
>> @@ -1494,13 +1506,23 @@ static struct net_device_stats *get_stats(struct net_device *dev)
>>  	dev->stats.rx_missed_errors	+= ioread8(ioaddr + RxMissed);
>>  	dev->stats.tx_packets += ioread16(ioaddr + TxFramesOK);
>>  	dev->stats.rx_packets += ioread16(ioaddr + RxFramesOK);
>> -	dev->stats.collisions += ioread8(ioaddr + StatsLateColl);
>> -	dev->stats.collisions += ioread8(ioaddr + StatsMultiColl);
>> -	dev->stats.collisions += ioread8(ioaddr + StatsOneColl);
>>  	dev->stats.tx_carrier_errors += ioread8(ioaddr + StatsCarrierError);
>> -	ioread8(ioaddr + StatsTxDefer);
>> -	for (i = StatsTxDefer; i <= StatsMcastRx; i++)
>> -		ioread8(ioaddr + i);
>> +
>> +	np->xstats.tx_multiple_collisions += ioread8(ioaddr + StatsMultiColl);
>> +	np->xstats.tx_single_collisions += ioread8(ioaddr + StatsOneColl);
>> +	np->xstats.tx_late_collisions += ioread8(ioaddr + StatsLateColl);
>> +	dev->stats.collisions += np->xstats.tx_multiple_collisions
>> +		+ np->xstats.tx_single_collisions
>> +		+ np->xstats.tx_late_collisions;
> 
> 
> Oh well..., really ?
> 
> Each time we are calling get_stats(), you do
> 
> dev->stats.collisions += (number of accumulated collisions since device
> is up)
> 
> instead of
> 
> dev->stats.collisions += (number of accumulated collisions since last
> time we read counters)
> 
> 
> Can you see the problem ?

Oops, thanks Eric!
I'll fix it up and resend
> 
> 

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

Patch

diff --git a/drivers/net/sundance.c b/drivers/net/sundance.c
index 4283cc5..406ca45 100644
--- a/drivers/net/sundance.c
+++ b/drivers/net/sundance.c
@@ -363,6 +363,19 @@  struct netdev_private {
         dma_addr_t tx_ring_dma;
         dma_addr_t rx_ring_dma;
 	struct timer_list timer;		/* Media monitoring timer. */
+	/* ethtool extra stats */
+	struct {
+		u64 tx_multiple_collisions;
+		u64 tx_single_collisions;
+		u64 tx_late_collisions;
+		u64 tx_defered;
+		u64 tx_defered_excessive;
+		u64 tx_aborted;
+		u64 tx_bcasts;
+		u64 rx_bcasts;
+		u64 tx_mcasts;
+		u64 rx_mcasts;
+	} xstats;
 	/* Frequently used values: keep some adjacent for cache effect. */
 	spinlock_t lock;
 	int msg_enable;
@@ -1486,7 +1499,6 @@  static struct net_device_stats *get_stats(struct net_device *dev)
 {
 	struct netdev_private *np = netdev_priv(dev);
 	void __iomem *ioaddr = np->base;
-	int i;
 	unsigned long flags;
 
 	spin_lock_irqsave(&np->statlock, flags);
@@ -1494,13 +1506,23 @@  static struct net_device_stats *get_stats(struct net_device *dev)
 	dev->stats.rx_missed_errors	+= ioread8(ioaddr + RxMissed);
 	dev->stats.tx_packets += ioread16(ioaddr + TxFramesOK);
 	dev->stats.rx_packets += ioread16(ioaddr + RxFramesOK);
-	dev->stats.collisions += ioread8(ioaddr + StatsLateColl);
-	dev->stats.collisions += ioread8(ioaddr + StatsMultiColl);
-	dev->stats.collisions += ioread8(ioaddr + StatsOneColl);
 	dev->stats.tx_carrier_errors += ioread8(ioaddr + StatsCarrierError);
-	ioread8(ioaddr + StatsTxDefer);
-	for (i = StatsTxDefer; i <= StatsMcastRx; i++)
-		ioread8(ioaddr + i);
+
+	np->xstats.tx_multiple_collisions += ioread8(ioaddr + StatsMultiColl);
+	np->xstats.tx_single_collisions += ioread8(ioaddr + StatsOneColl);
+	np->xstats.tx_late_collisions += ioread8(ioaddr + StatsLateColl);
+	dev->stats.collisions += np->xstats.tx_multiple_collisions
+		+ np->xstats.tx_single_collisions
+		+ np->xstats.tx_late_collisions;
+
+	np->xstats.tx_defered += ioread8(ioaddr + StatsTxDefer);
+	np->xstats.tx_defered_excessive += ioread8(ioaddr + StatsTxXSDefer);
+	np->xstats.tx_aborted += ioread8(ioaddr + StatsTxAbort);
+	np->xstats.tx_bcasts += ioread8(ioaddr + StatsBcastTx);
+	np->xstats.rx_bcasts += ioread8(ioaddr + StatsBcastRx);
+	np->xstats.tx_mcasts += ioread8(ioaddr + StatsMcastTx);
+	np->xstats.rx_mcasts += ioread8(ioaddr + StatsMcastRx);
+
 	dev->stats.tx_bytes += ioread16(ioaddr + TxOctetsLow);
 	dev->stats.tx_bytes += ioread16(ioaddr + TxOctetsHigh) << 16;
 	dev->stats.rx_bytes += ioread16(ioaddr + RxOctetsLow);
@@ -1566,6 +1588,21 @@  static int __set_mac_addr(struct net_device *dev)
 	return 0;
 }
 
+static const struct {
+	const char name[ETH_GSTRING_LEN];
+} sundance_stats[] = {
+	{ "tx_multiple_collisions" },
+	{ "tx_single_collisions" },
+	{ "tx_late_collisions" },
+	{ "tx_defered" },
+	{ "tx_defered_excessive" },
+	{ "tx_aborted" },
+	{ "tx_bcasts" },
+	{ "rx_bcasts" },
+	{ "tx_mcasts" },
+	{ "rx_mcasts" },
+};
+
 static int check_if_running(struct net_device *dev)
 {
 	if (!netif_running(dev))
@@ -1624,6 +1661,42 @@  static void set_msglevel(struct net_device *dev, u32 val)
 	np->msg_enable = val;
 }
 
+static void get_strings(struct net_device *dev, u32 stringset,
+		u8 *data)
+{
+	if (stringset == ETH_SS_STATS)
+		memcpy(data, sundance_stats, sizeof(sundance_stats));
+}
+
+static int get_sset_count(struct net_device *dev, int sset)
+{
+	switch (sset) {
+	case ETH_SS_STATS:
+		return ARRAY_SIZE(sundance_stats);
+	default:
+		return -EOPNOTSUPP;
+	}
+}
+
+static void get_ethtool_stats(struct net_device *dev,
+		struct ethtool_stats *stats, u64 *data)
+{
+	struct netdev_private *np = netdev_priv(dev);
+	int i = 0;
+
+	get_stats(dev);
+	data[i++] = np->xstats.tx_multiple_collisions;
+	data[i++] = np->xstats.tx_single_collisions;
+	data[i++] = np->xstats.tx_late_collisions;
+	data[i++] = np->xstats.tx_defered;
+	data[i++] = np->xstats.tx_defered_excessive;
+	data[i++] = np->xstats.tx_aborted;
+	data[i++] = np->xstats.tx_bcasts;
+	data[i++] = np->xstats.rx_bcasts;
+	data[i++] = np->xstats.tx_mcasts;
+	data[i++] = np->xstats.rx_mcasts;
+}
+
 static const struct ethtool_ops ethtool_ops = {
 	.begin = check_if_running,
 	.get_drvinfo = get_drvinfo,
@@ -1633,6 +1706,9 @@  static const struct ethtool_ops ethtool_ops = {
 	.get_link = get_link,
 	.get_msglevel = get_msglevel,
 	.set_msglevel = set_msglevel,
+	.get_strings = get_strings,
+	.get_sset_count = get_sset_count,
+	.get_ethtool_stats = get_ethtool_stats,
 };
 
 static int netdev_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)