Skip site navigation (1)Skip section navigation (2)
Date:      Sat, 10 Jun 2017 23:26:25 +0000 (UTC)
From:      Ian Lepore <ian@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r319811 - head/sys/dev/ffec
Message-ID:  <201706102326.v5ANQPhV035342@repo.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: ian
Date: Sat Jun 10 23:26:25 2017
New Revision: 319811
URL: https://svnweb.freebsd.org/changeset/base/319811

Log:
  if_ffec bugfixes related to harvesting of hardware-maintained statistics...
  
  After harvesting the hardware statistics counters and summing them into the
  interface stats, properly clear the hardware counters back to zero.  On imx5
  and earlier hardware it is necessary to disable collection of stats while
  writing zeroes to all the registers.  On imx6 and newer it turns out it's
  not even possible to write zeroes, instead you have to toggle a special
  "zero everything" control bit in a register.
  
  Count incoming packets with a bad start frame delim as input errors, and
  incoming packets dropped due to no fifo space as input drops.
  
  Remove all code related to harvesting the hardware stats less often than
  once per second.  It turns out the 32-bit stats registers are backed by
  16-bit counters under the hood, and they can easily roll over if you only
  harvest them once every 3 seconds like the old code was doing.  Now we just
  read all the regs once a second.
  
  The combination of not properly zeroing the stats registers and 16-bit
  counters sometimes wrapping between harvest calls resulted in basically
  unusable statistics before these changes.

Modified:
  head/sys/dev/ffec/if_ffec.c

Modified: head/sys/dev/ffec/if_ffec.c
==============================================================================
--- head/sys/dev/ffec/if_ffec.c	Sat Jun 10 23:16:00 2017	(r319810)
+++ head/sys/dev/ffec/if_ffec.c	Sat Jun 10 23:26:25 2017	(r319811)
@@ -129,7 +129,6 @@ static struct ofw_compat_data compat_data[] = {
 #define	TX_DESC_SIZE	(sizeof(struct ffec_hwdesc) * TX_DESC_COUNT)
 
 #define	WATCHDOG_TIMEOUT_SECS	5
-#define	STATS_HARVEST_INTERVAL	3
 
 struct ffec_bufmap {
 	struct mbuf	*mbuf;
@@ -160,7 +159,6 @@ struct ffec_softc {
 	boolean_t		is_attached;
 	boolean_t		is_detaching;
 	int			tx_watchdog_count;
-	int			stats_harvest_count;
 
 	bus_dma_tag_t		rxdesc_tag;
 	bus_dmamap_t		rxdesc_map;
@@ -462,22 +460,41 @@ ffec_media_change(struct ifnet * ifp)
 
 static void ffec_clear_stats(struct ffec_softc *sc)
 {
+	uint32_t mibc;
 
-	WR4(sc, FEC_RMON_R_PACKETS, 0);
-	WR4(sc, FEC_RMON_R_MC_PKT, 0);
-	WR4(sc, FEC_RMON_R_CRC_ALIGN, 0);
-	WR4(sc, FEC_RMON_R_UNDERSIZE, 0);
-	WR4(sc, FEC_RMON_R_OVERSIZE, 0);
-	WR4(sc, FEC_RMON_R_FRAG, 0);
-	WR4(sc, FEC_RMON_R_JAB, 0);
-	WR4(sc, FEC_RMON_T_PACKETS, 0);
-	WR4(sc, FEC_RMON_T_MC_PKT, 0);
-	WR4(sc, FEC_RMON_T_CRC_ALIGN, 0);
-	WR4(sc, FEC_RMON_T_UNDERSIZE, 0);
-	WR4(sc, FEC_RMON_T_OVERSIZE , 0);
-	WR4(sc, FEC_RMON_T_FRAG, 0);
-	WR4(sc, FEC_RMON_T_JAB, 0);
-	WR4(sc, FEC_RMON_T_COL, 0);
+	mibc = RD4(sc, FEC_MIBC_REG);
+
+	/*
+	 * On newer hardware the statistic regs are cleared by toggling a bit in
+	 * the mib control register.  On older hardware the clear procedure is
+	 * to disable statistics collection, zero the regs, then re-enable.
+	 */
+	if (sc->fectype == FECTYPE_IMX6 || sc->fectype == FECTYPE_MVF) {
+		WR4(sc, FEC_MIBC_REG, mibc | FEC_MIBC_CLEAR);
+		WR4(sc, FEC_MIBC_REG, mibc & ~FEC_MIBC_CLEAR);
+	} else {
+		WR4(sc, FEC_MIBC_REG, mibc | FEC_MIBC_DIS);
+	
+		WR4(sc, FEC_IEEE_R_DROP, 0);
+		WR4(sc, FEC_IEEE_R_MACERR, 0);
+		WR4(sc, FEC_RMON_R_CRC_ALIGN, 0);
+		WR4(sc, FEC_RMON_R_FRAG, 0);
+		WR4(sc, FEC_RMON_R_JAB, 0);
+		WR4(sc, FEC_RMON_R_MC_PKT, 0);
+		WR4(sc, FEC_RMON_R_OVERSIZE, 0);
+		WR4(sc, FEC_RMON_R_PACKETS, 0);
+		WR4(sc, FEC_RMON_R_UNDERSIZE, 0);
+		WR4(sc, FEC_RMON_T_COL, 0);
+		WR4(sc, FEC_RMON_T_CRC_ALIGN, 0);
+		WR4(sc, FEC_RMON_T_FRAG, 0);
+		WR4(sc, FEC_RMON_T_JAB, 0);
+		WR4(sc, FEC_RMON_T_MC_PKT, 0);
+		WR4(sc, FEC_RMON_T_OVERSIZE , 0);
+		WR4(sc, FEC_RMON_T_PACKETS, 0);
+		WR4(sc, FEC_RMON_T_UNDERSIZE, 0);
+
+		WR4(sc, FEC_MIBC_REG, mibc);
+	}
 }
 
 static void
@@ -485,29 +502,22 @@ ffec_harvest_stats(struct ffec_softc *sc)
 {
 	struct ifnet *ifp;
 
-	/* We don't need to harvest too often. */
-	if (++sc->stats_harvest_count < STATS_HARVEST_INTERVAL)
-		return;
+	ifp = sc->ifp;
 
 	/*
-	 * Try to avoid harvesting unless the IDLE flag is on, but if it has
-	 * been too long just go ahead and do it anyway, the worst that'll
-	 * happen is we'll lose a packet count or two as we clear at the end.
+	 * - FEC_IEEE_R_DROP is "dropped due to invalid start frame delimiter"
+	 *   so it's really just another type of input error.
+	 * - FEC_IEEE_R_MACERR is "no receive fifo space"; count as input drops.
 	 */
-	if (sc->stats_harvest_count < (2 * STATS_HARVEST_INTERVAL) &&
-	    ((RD4(sc, FEC_MIBC_REG) & FEC_MIBC_IDLE) == 0))
-		return;
-
-	sc->stats_harvest_count = 0;
-	ifp = sc->ifp;
-
 	if_inc_counter(ifp, IFCOUNTER_IPACKETS, RD4(sc, FEC_RMON_R_PACKETS));
 	if_inc_counter(ifp, IFCOUNTER_IMCASTS, RD4(sc, FEC_RMON_R_MC_PKT));
 	if_inc_counter(ifp, IFCOUNTER_IERRORS,
 	    RD4(sc, FEC_RMON_R_CRC_ALIGN) + RD4(sc, FEC_RMON_R_UNDERSIZE) +
 	    RD4(sc, FEC_RMON_R_OVERSIZE) + RD4(sc, FEC_RMON_R_FRAG) +
-	    RD4(sc, FEC_RMON_R_JAB));
+	    RD4(sc, FEC_RMON_R_JAB) + RD4(sc, FEC_IEEE_R_DROP));
 
+	if_inc_counter(ifp, IFCOUNTER_IQDROPS, RD4(sc, FEC_IEEE_R_MACERR));
+
 	if_inc_counter(ifp, IFCOUNTER_OPACKETS, RD4(sc, FEC_RMON_T_PACKETS));
 	if_inc_counter(ifp, IFCOUNTER_OMCASTS, RD4(sc, FEC_RMON_T_MC_PKT));
 	if_inc_counter(ifp, IFCOUNTER_OERRORS,
@@ -1016,7 +1026,6 @@ ffec_stop_locked(struct ffec_softc *sc)
 	ifp = sc->ifp;
 	ifp->if_drv_flags &= ~(IFF_DRV_RUNNING | IFF_DRV_OACTIVE);
 	sc->tx_watchdog_count = 0;
-	sc->stats_harvest_count = 0;
 
 	/* 
 	 * Stop the hardware, mask all interrupts, and clear all current
@@ -1194,7 +1203,8 @@ ffec_init_locked(struct ffec_softc *sc)
 	WR4(sc, FEC_IEM_REG, FEC_IER_TXF | FEC_IER_RXF | FEC_IER_EBERR);
 
 	/*
-	 * MIBC - MIB control (hardware stats).
+	 * MIBC - MIB control (hardware stats); clear all statistics regs, then
+	 * enable collection of statistics.
 	 */
 	regval = RD4(sc, FEC_MIBC_REG);
 	WR4(sc, FEC_MIBC_REG, regval | FEC_MIBC_DIS);



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201706102326.v5ANQPhV035342>