netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Breno Leitao <leitao@linux.vnet.ibm.com>
To: Michael Chan <mchan@broadcom.com>
Cc: "'Eric Dumazet'" <eric.dumazet@gmail.com>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>
Subject: Re: [PATCH] bnx2: avoid flushing statistics when doing a MTU change
Date: Mon, 30 Nov 2009 11:17:50 -0200	[thread overview]
Message-ID: <4B13C5FE.7060403@linux.vnet.ibm.com> (raw)
In-Reply-To: <C27F8246C663564A84BB7AB34397724218065B9851@IRVEXCHCCR01.corp.ad.broadcom.com>

Hi Michael, Eric, 

Thanks for the reviews. Now stats_extra is inline in the bnx2 structure,
bnx2_open is cleaning stats_extra and I am doing a DMA to the statistics
structure before saving it.

Michael Chan wrote:
> Eric Dumazet wrote:
>> I wonder if you need to renew this stats before copying them ?
>>  (eg calling bnx2_get_stats())
> 
> Calling bnx2_get_stats() won't refresh the stats counters because
> they are DMA'ed once a second.  There's an I/O to immediately DMA
> the counters:
> 
> REG_WR(bp, BNX2_HC_COMMAND, bp->hc_cmd |
>                                 BNX2_HC_COMMAND_STATS_NOW);
> REG_RD(bp, BNX2_HC_COMMAND);
> 
> We then need to add some delay to wait for the DMA to complete.
In the following patch, I just called this I/O as specified above, 
but looking into the code, I saw that the following I/O is just called
when BNX2_FLAG_BROKEN_STATS is set, and I am not sure if my call is correct.

Here is my new patch:

---

[PATCH] bnx2: avoid flushing statistics when doing a MTU change

Actually when bnx2 changes the interface's MTU size, it resets
the chip and consequently flushes the interface statistics.
This patch saves the statistics in a temporary space in order to
maintain the statistics correct after the chip reset.

Signed-off-by: Breno Leitao<leitao@linux.vnet.ibm.com>
---
 drivers/net/bnx2.c |   56 ++++++++++++++++++++++++++++++++++++++-------------
 drivers/net/bnx2.h |    1 +
 2 files changed, 42 insertions(+), 15 deletions(-)

diff --git a/drivers/net/bnx2.c b/drivers/net/bnx2.c
index 7fa4048..3cefee3 100644
--- a/drivers/net/bnx2.c
+++ b/drivers/net/bnx2.c
@@ -6221,6 +6221,8 @@ bnx2_open(struct net_device *dev)
 	else if (bp->flags & BNX2_FLAG_USING_MSIX)
 		printk(KERN_INFO PFX "%s: using MSIX\n", dev->name);
 
+	memset(&bp->stats_extra, 0, sizeof(struct net_device_stats));
+
 	netif_tx_start_all_queues(dev);
 
 	return 0;
@@ -6469,46 +6471,58 @@ bnx2_get_stats(struct net_device *dev)
 	net_stats->rx_packets =
 		GET_NET_STATS(stats_blk->stat_IfHCInUcastPkts) +
 		GET_NET_STATS(stats_blk->stat_IfHCInMulticastPkts) +
-		GET_NET_STATS(stats_blk->stat_IfHCInBroadcastPkts);
+		GET_NET_STATS(stats_blk->stat_IfHCInBroadcastPkts) + 
+		bp->stats_extra.rx_packets;
 
 	net_stats->tx_packets =
 		GET_NET_STATS(stats_blk->stat_IfHCOutUcastPkts) +
 		GET_NET_STATS(stats_blk->stat_IfHCOutMulticastPkts) +
-		GET_NET_STATS(stats_blk->stat_IfHCOutBroadcastPkts);
+		GET_NET_STATS(stats_blk->stat_IfHCOutBroadcastPkts) +
+		bp->stats_extra.tx_packets;
 
 	net_stats->rx_bytes =
-		GET_NET_STATS(stats_blk->stat_IfHCInOctets);
+		GET_NET_STATS(stats_blk->stat_IfHCInOctets) +
+		bp->stats_extra.rx_bytes;
 
 	net_stats->tx_bytes =
-		GET_NET_STATS(stats_blk->stat_IfHCOutOctets);
+		GET_NET_STATS(stats_blk->stat_IfHCOutOctets) +
+		bp->stats_extra.tx_bytes;
 
 	net_stats->multicast =
-		GET_NET_STATS(stats_blk->stat_IfHCOutMulticastPkts);
+		GET_NET_STATS(stats_blk->stat_IfHCOutMulticastPkts) +
+		bp->stats_extra.multicast;
 
 	net_stats->collisions =
-		(unsigned long) stats_blk->stat_EtherStatsCollisions;
+		(unsigned long) stats_blk->stat_EtherStatsCollisions +
+		bp->stats_extra.collisions;
 
 	net_stats->rx_length_errors =
 		(unsigned long) (stats_blk->stat_EtherStatsUndersizePkts +
-		stats_blk->stat_EtherStatsOverrsizePkts);
+		stats_blk->stat_EtherStatsOverrsizePkts) +
+		bp->stats_extra.rx_length_errors;
 
 	net_stats->rx_over_errors =
 		(unsigned long) (stats_blk->stat_IfInFTQDiscards +
-		stats_blk->stat_IfInMBUFDiscards);
+		stats_blk->stat_IfInMBUFDiscards) +
+		bp->stats_extra.rx_over_errors;
 
 	net_stats->rx_frame_errors =
-		(unsigned long) stats_blk->stat_Dot3StatsAlignmentErrors;
+		(unsigned long) stats_blk->stat_Dot3StatsAlignmentErrors +
+		bp->stats_extra.rx_frame_errors;
 
 	net_stats->rx_crc_errors =
-		(unsigned long) stats_blk->stat_Dot3StatsFCSErrors;
+		(unsigned long) stats_blk->stat_Dot3StatsFCSErrors +
+		bp->stats_extra.rx_crc_errors;
 
 	net_stats->rx_errors = net_stats->rx_length_errors +
 		net_stats->rx_over_errors + net_stats->rx_frame_errors +
-		net_stats->rx_crc_errors;
+		net_stats->rx_crc_errors +
+		bp->stats_extra.rx_errors;
 
 	net_stats->tx_aborted_errors =
     		(unsigned long) (stats_blk->stat_Dot3StatsExcessiveCollisions +
-		stats_blk->stat_Dot3StatsLateCollisions);
+		stats_blk->stat_Dot3StatsLateCollisions) +
+		bp->stats_extra.tx_aborted_errors;
 
 	if ((CHIP_NUM(bp) == CHIP_NUM_5706) ||
 	    (CHIP_ID(bp) == CHIP_ID_5708_A0))
@@ -6516,7 +6530,8 @@ bnx2_get_stats(struct net_device *dev)
 	else {
 		net_stats->tx_carrier_errors =
 			(unsigned long)
-			stats_blk->stat_Dot3StatsCarrierSenseErrors;
+			stats_blk->stat_Dot3StatsCarrierSenseErrors +
+			bp->stats_extra.tx_carrier_errors;
 	}
 
 	net_stats->tx_errors =
@@ -6524,11 +6539,13 @@ bnx2_get_stats(struct net_device *dev)
 		stats_blk->stat_emac_tx_stat_dot3statsinternalmactransmiterrors
 		+
 		net_stats->tx_aborted_errors +
-		net_stats->tx_carrier_errors;
+		net_stats->tx_carrier_errors +
+		bp->stats_extra.tx_errors;
 
 	net_stats->rx_missed_errors =
 		(unsigned long) (stats_blk->stat_IfInFTQDiscards +
-		stats_blk->stat_IfInMBUFDiscards + stats_blk->stat_FwRxDrop);
+		stats_blk->stat_IfInMBUFDiscards + stats_blk->stat_FwRxDrop) +
+		bp->stats_extra.rx_missed_errors;
 
 	return net_stats;
 }
@@ -6989,6 +7006,15 @@ bnx2_change_ring_size(struct bnx2 *bp, u32 rx, u32 tx)
 {
 	if (netif_running(bp->dev)) {
 		bnx2_netif_stop(bp);
+
+		/* Save statistics that is going to be reseted */
+		REG_WR(bp, BNX2_HC_COMMAND, bp->hc_cmd |
+				BNX2_HC_COMMAND_STATS_NOW);
+		REG_RD(bp, BNX2_HC_COMMAND);
+		udelay(5);
+		memcpy(&bp->stats_extra, &bp->dev->stats,
+			sizeof(struct net_device_stats));
+
 		bnx2_reset_chip(bp, BNX2_DRV_MSG_CODE_RESET);
 		bnx2_free_skbs(bp);
 		bnx2_free_mem(bp);
diff --git a/drivers/net/bnx2.h b/drivers/net/bnx2.h
index a4d8340..1063d1a 100644
--- a/drivers/net/bnx2.h
+++ b/drivers/net/bnx2.h
@@ -6912,6 +6912,7 @@ struct bnx2 {
 
 	const struct firmware	*mips_firmware;
 	const struct firmware	*rv2p_firmware;
+	struct net_device_stats stats_extra;
 };
 
 #define REG_RD(bp, offset)					\
-- 
1.6.0.2


  parent reply	other threads:[~2009-11-30 13:17 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-11-27 16:59 [PATCH] bnx2: avoid flushing statistics when doing a MTU change leitao
2009-11-30  9:20 ` Michael Chan
2009-11-30 10:30 ` Eric Dumazet
2009-11-30 10:43   ` Michael Chan
2009-11-30 10:49     ` Eric Dumazet
2009-11-30 11:02       ` Michael Chan
2009-11-30 13:17     ` Breno Leitao [this message]
2009-11-30 13:32       ` Eric Dumazet
2009-11-30 14:05         ` Breno Leitao
2009-11-30 18:09           ` Michael Chan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4B13C5FE.7060403@linux.vnet.ibm.com \
    --to=leitao@linux.vnet.ibm.com \
    --cc=eric.dumazet@gmail.com \
    --cc=mchan@broadcom.com \
    --cc=netdev@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).