Re: [PATCH]: e1000: prevent statistics from getting garbled duringreset.

[Jeff V. Merkey]

Linas Vepstas wrote:

> On Thu, Mar 30, 2006 at 03:39:28PM -0600, Linas Vepstas wrote:
>  
>
> > Please review, sign-off/ack, and forward upstream.
> > --linas
> >    
>
> Per feedback, here's a slightly more human-readable version.
> --linas
>
> [PATCH]: e1000: prevent statistics from getting garbled during reset.
>
> If a PCI bus error/fault triggers a PCI bus reset, attempts to get the
> ethernet packet count statistics from the hardware will fail, returning
> garbage data upstream.  This patch skips statistics data collection
> if the PCI device is not on the bus. 
>
> This patch presumes that an earlier patch,
> [PATCH] PCI Error Recovery: e1000 network device driver
> has already been applied.
>
> Signed-off-by: Linas Vepstas <linas@xxxxxxxxxxxxxx>
>
> ----
> drivers/net/e1000/e1000_main.c |    6 +++++-
> 1 files changed, 5 insertions(+), 1 deletion(-)
>
> Index: linux-2.6.16-git6/drivers/net/e1000/e1000_main.c
> ===================================================================
> --- linux-2.6.16-git6.orig/drivers/net/e1000/e1000_main.c	2006-03-30 17:51:37.924162779 -0600
> +++ linux-2.6.16-git6/drivers/net/e1000/e1000_main.c	2006-03-30 17:54:07.659188391 -0600
> @@ -3069,14 +3069,18 @@ void
> e1000_update_stats(struct e1000_adapter *adapter)
> {
> 	struct e1000_hw *hw = &adapter->hw;
> +	struct pci_dev *pdev = adapter->pdev;
> 	unsigned long flags;
> 	uint16_t phy_tmp;
>
> #define PHY_IDLE_ERROR_COUNT_MASK 0x00FF
>
> -	/* Prevent stats update while adapter is being reset */
> +	/* Prevent stats update while adapter is being reset,
> +	 * or if the pci connection is down. */
> 	if (adapter->link_speed == 0)
> 		return;
> +   if (pdev->error_state && pdev->error_state != pci_channel_io_normal)
> +		return;
>
> 	spin_lock_irqsave(&adapter->stats_lock, flags);
>
> -
> 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/
>
>  

The driver also needs to be fixed to allow clearing of the stats (like 
all the other adapter drivers). At present, when I run performance
and packet drop counts on the cards, I cannot reset the stats with this 
code because the driver stores them in the e100_adapter
structure. This is busted.

This function:

int clear_network_device_stats(BYTE *name)
{
register int i;

i = get_dev_index_by_name(name);
if (i != -1)
{
if (ndevs[i])
{
struct net_device_stats *stats;

stats = (ndevs[i]->get_stats)(ndevs[i]);
if (stats)
{
stats->rx_packets = 0;
stats->tx_packets = 0;
stats->rx_bytes = 0;
stats->tx_bytes = 0;
stats->multicast = 0;
stats->collisions = 0;
stats->rx_errors = 0;
stats->rx_length_errors = 0;
stats->rx_crc_errors = 0;
stats->rx_frame_errors = 0;
stats->rx_fifo_errors = 0;
stats->rx_missed_errors = 0;
stats->tx_errors = 0;
stats->tx_aborted_errors = 0;
stats->tx_window_errors = 0;
stats->tx_carrier_errors = 0;
P_Print("solera: stats cleared for %s\n", ndevs[i]->name);
return 0;
}
}
return 0;
}
return -1;
}

does not work on e1000 due to this section of code:


void
e1000_update_stats(struct e1000_adapter *adapter)
{
struct e1000_hw *hw = &adapter->hw;
unsigned long flags;
uint16_t phy_tmp;

#define PHY_IDLE_ERROR_COUNT_MASK 0x00FF

spin_lock_irqsave(&adapter->stats_lock, flags);

/* these counters are modified from e1000_adjust_tbi_stats,
* called from the interrupt context, so they must only
* be written while holding adapter->stats_lock
*/

adapter->stats.crcerrs += E1000_READ_REG(hw, CRCERRS);
adapter->stats.gprc += E1000_READ_REG(hw, GPRC);
adapter->stats.gorcl += E1000_READ_REG(hw, GORCL);
adapter->stats.gorch += E1000_READ_REG(hw, GORCH);
adapter->stats.bprc += E1000_READ_REG(hw, BPRC);
adapter->stats.mprc += E1000_READ_REG(hw, MPRC);
adapter->stats.roc += E1000_READ_REG(hw, ROC);
adapter->stats.prc64 += E1000_READ_REG(hw, PRC64);
adapter->stats.prc127 += E1000_READ_REG(hw, PRC127);
adapter->stats.prc255 += E1000_READ_REG(hw, PRC255);
adapter->stats.prc511 += E1000_READ_REG(hw, PRC511);
adapter->stats.prc1023 += E1000_READ_REG(hw, PRC1023);
adapter->stats.prc1522 += E1000_READ_REG(hw, PRC1522);

adapter->stats.symerrs += E1000_READ_REG(hw, SYMERRS);
adapter->stats.mpc += E1000_READ_REG(hw, MPC);
adapter->stats.scc += E1000_READ_REG(hw, SCC);
adapter->stats.ecol += E1000_READ_REG(hw, ECOL);
adapter->stats.mcc += E1000_READ_REG(hw, MCC);
adapter->stats.latecol += E1000_READ_REG(hw, LATECOL);
adapter->stats.dc += E1000_READ_REG(hw, DC);
adapter->stats.sec += E1000_READ_REG(hw, SEC);
adapter->stats.rlec += E1000_READ_REG(hw, RLEC);
adapter->stats.xonrxc += E1000_READ_REG(hw, XONRXC);
adapter->stats.xontxc += E1000_READ_REG(hw, XONTXC);
adapter->stats.xoffrxc += E1000_READ_REG(hw, XOFFRXC);
adapter->stats.xofftxc += E1000_READ_REG(hw, XOFFTXC);
adapter->stats.fcruc += E1000_READ_REG(hw, FCRUC);
adapter->stats.gptc += E1000_READ_REG(hw, GPTC);
adapter->stats.gotcl += E1000_READ_REG(hw, GOTCL);
adapter->stats.gotch += E1000_READ_REG(hw, GOTCH);
adapter->stats.rnbc += E1000_READ_REG(hw, RNBC);
adapter->stats.ruc += E1000_READ_REG(hw, RUC);
adapter->stats.rfc += E1000_READ_REG(hw, RFC);
adapter->stats.rjc += E1000_READ_REG(hw, RJC);
adapter->stats.torl += E1000_READ_REG(hw, TORL);
adapter->stats.torh += E1000_READ_REG(hw, TORH);
adapter->stats.totl += E1000_READ_REG(hw, TOTL);
adapter->stats.toth += E1000_READ_REG(hw, TOTH);
adapter->stats.tpr += E1000_READ_REG(hw, TPR);
adapter->stats.ptc64 += E1000_READ_REG(hw, PTC64);
adapter->stats.ptc127 += E1000_READ_REG(hw, PTC127);
adapter->stats.ptc255 += E1000_READ_REG(hw, PTC255);
adapter->stats.ptc511 += E1000_READ_REG(hw, PTC511);
adapter->stats.ptc1023 += E1000_READ_REG(hw, PTC1023);
adapter->stats.ptc1522 += E1000_READ_REG(hw, PTC1522);
adapter->stats.mptc += E1000_READ_REG(hw, MPTC);
adapter->stats.bptc += E1000_READ_REG(hw, BPTC);

//NOTE These stats need to be stored in the stats structure so they can 
be cleared by
statistics monitoring programs.

/* used for adaptive IFS */

hw->tx_packet_delta = E1000_READ_REG(hw, TPT);
adapter->stats.tpt += hw->tx_packet_delta;
hw->collision_delta = E1000_READ_REG(hw, COLC);
adapter->stats.colc += hw->collision_delta;

if(hw->mac_type >= e1000_82543) {
adapter->stats.algnerrc += E1000_READ_REG(hw, ALGNERRC);
adapter->stats.rxerrc += E1000_READ_REG(hw, RXERRC);
adapter->stats.tncrs += E1000_READ_REG(hw, TNCRS);
adapter->stats.cexterr += E1000_READ_REG(hw, CEXTERR);
adapter->stats.tsctc += E1000_READ_REG(hw, TSCTC);
adapter->stats.tsctfc += E1000_READ_REG(hw, TSCTFC);
}

/* Fill out the OS statistics structure */

adapter->net_stats.rx_packets = adapter->stats.gprc;
adapter->net_stats.tx_packets = adapter->stats.gptc;
adapter->net_stats.rx_bytes = adapter->stats.gorcl;
adapter->net_stats.tx_bytes = adapter->stats.gotcl;
adapter->net_stats.multicast = adapter->stats.mprc;
adapter->net_stats.collisions = adapter->stats.colc;

/* Rx Errors */

adapter->net_stats.rx_errors = adapter->stats.rxerrc +
adapter->stats.crcerrs + adapter->stats.algnerrc +
adapter->stats.rlec + adapter->stats.rnbc +
adapter->stats.mpc + adapter->stats.cexterr;
adapter->net_stats.rx_dropped = adapter->stats.rnbc;
adapter->net_stats.rx_length_errors = adapter->stats.rlec;
adapter->net_stats.rx_crc_errors = adapter->stats.crcerrs;
adapter->net_stats.rx_frame_errors = adapter->stats.algnerrc;
adapter->net_stats.rx_fifo_errors = adapter->stats.mpc;
adapter->net_stats.rx_missed_errors = adapter->stats.mpc;

/* Tx Errors */

adapter->net_stats.tx_errors = adapter->stats.ecol +
adapter->stats.latecol;
adapter->net_stats.tx_aborted_errors = adapter->stats.ecol;
adapter->net_stats.tx_window_errors = adapter->stats.latecol;
adapter->net_stats.tx_carrier_errors = adapter->stats.tncrs;

/* Tx Dropped needs to be maintained elsewhere */

/* Phy Stats */

if(hw->media_type == e1000_media_type_copper) {
if((adapter->link_speed == SPEED_1000) &&
(!e1000_read_phy_reg(hw, PHY_1000T_STATUS, &phy_tmp))) {
phy_tmp &= PHY_IDLE_ERROR_COUNT_MASK;
adapter->phy_stats.idle_errors += phy_tmp;
}

if((hw->mac_type <= e1000_82546) &&
(hw->phy_type == e1000_phy_m88) &&
!e1000_read_phy_reg(hw, M88E1000_RX_ERR_CNTR, &phy_tmp))
adapter->phy_stats.receive_errors += phy_tmp;
}

spin_unlock_irqrestore(&adapter->stats_lock, flags);
}

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