Bug 12543 - [fxp] [patch] cumulative error counters for fxp(4)
Summary: [fxp] [patch] cumulative error counters for fxp(4)
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: 3.2-RELEASE
Hardware: Any Any
: Normal Affects Only Me
Assignee: Pyun YongHyeon
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 1999-07-07 04:40 UTC by Craig Leres
Modified: 2010-09-20 00:17 UTC (History)
0 users

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Craig Leres freebsd_committer 1999-07-07 04:40:02 UTC
	It would be really sweet if one could look at the long term
	error counters for the intel 10/100 card. This information
	can be invaluable in debugging network problems.

Fix: Add a new fxp_stats struct and add in new amounts fxp_stats_update().

RCS file: RCS/if_fxpvar.h,v
retrieving revision 1.1


-----BEGIN PGP SIGNATURE-----
Version: 2.6.2

iQCVAwUBN4LL2L2JLbUEFcrxAQHcwQQAtENXZEtPG9GCEiCrkbOh6f0455BV4ujJ
Jh7G4iPN66uqhOs3NemfPSvbLm5gLrnAM5Rhto/TDfi2LTdDZcH7B9dYZEMvPm59
bgRY1AvMKdwvtpjcg6ucuA6vN9gq6lSy3kArx9q3oYSVvETCdWdw2KMlVjk2jBfE
f7cc5n2FjlA=
=RtNR
-----END PGP SIGNATURE-------TDubOKGS9I68hy13b7qf5TWNkZX5FT87Wp24gwRraySwkEjB
Content-Type: text/plain; name="file.diff"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment; filename="file.diff"

diff -c -r1.1 if_fxpvar.h
*** /tmp/,RCSt1X53005	Tue Jul  6 20:33:45 1999
- --- if_fxpvar.h	Tue Jul  6 20:33:36 1999
***************
*** 56,61 ****
- --- 56,62 ----
  	int need_mcsetup;		/* multicast filter needs programming */
  	struct fxp_cb_tx *cbl_last;	/* last active TxCB in list */
  	struct fxp_stats *fxp_stats;	/* Pointer to interface stats */
+ 	struct fxp_stats fxp_tstats;	/* interface stats totals */
  	int rx_idle_secs;		/* # of seconds RX has been idle */
  	struct callout_handle stat_ch;	/* Handle for canceling our stat timeout */
  	struct fxp_cb_tx *cbl_base;	/* base of TxCB list */
*** if_fxp.c.virgin	Tue Jul  6 20:34:12 1999
- --- if_fxp.c.new	Tue Jul  6 20:36:00 1999
***************
*** 1058,1069 ****
- --- 1058,1073 ----
  					    FXP_RFA_STATUS_IAMATCH) &&
  					    (eh->ether_dhost[0] & 1)
  					    == 0) {
+ #ifdef BRIDGE
  dropit:
+ #endif
  					    if (m)
  						m_freem(m);
  					    goto rcvloop;
  					}
+ #ifdef BRIDGE
  getit:
+ #endif
  					m->m_data +=
  					    sizeof(struct ether_header);
  					m->m_len -=
***************
*** 1105,1111 ****
  {
  	struct fxp_softc *sc = arg;
  	struct ifnet *ifp = &sc->sc_if;
! 	struct fxp_stats *sp = sc->fxp_stats;
  	struct fxp_cb_tx *txp;
  	int s;
  
- --- 1109,1115 ----
  {
  	struct fxp_softc *sc = arg;
  	struct ifnet *ifp = &sc->sc_if;
! 	struct fxp_stats *sp = sc->fxp_stats, *sp2 = &sc->fxp_tstats;
  	struct fxp_cb_tx *txp;
  	int s;
  
***************
*** 1134,1139 ****
- --- 1138,1162 ----
  		if (tx_threshold < 192)
  			tx_threshold += 64;
  	}
+ 
+ 	/* Update totals */
+ 	sp2->tx_good += sp->tx_good;
+ 	sp2->tx_maxcols += sp->tx_maxcols;
+ 	sp2->tx_latecols += sp->tx_latecols;
+ 	sp2->tx_underruns += sp->tx_underruns;
+ 	sp2->tx_lostcrs += sp->tx_lostcrs;
+ 	sp2->tx_deffered += sp->tx_deffered;
+ 	sp2->tx_single_collisions += sp->tx_single_collisions;
+ 	sp2->tx_multiple_collisions += sp->tx_multiple_collisions;
+ 	sp2->tx_total_collisions += sp->tx_total_collisions;
+ 	sp2->rx_good += sp->rx_good;
+ 	sp2->rx_crc_errors += sp->rx_crc_errors;
+ 	sp2->rx_alignment_errors += sp->rx_alignment_errors;
+ 	sp2->rx_rnr_errors += sp->rx_rnr_errors;
+ 	sp2->rx_overrun_errors += sp->rx_overrun_errors;
+ 	sp2->rx_cdt_errors += sp->rx_cdt_errors;
+ 	sp2->rx_shortframes += sp->rx_shortframes;
+ 
  	s = splimp();
  	/*
  	 * Release any xmit buffers that have completed DMA. This isn't
How-To-Repeat: 
	The fxp driver only keeps around the last set of incremental
	changes to the error counters.
Comment 1 dg 1999-07-07 09:38:05 UTC
>>Description:
>
>	It would be really sweet if one could look at the long term
>	error counters for the intel 10/100 card. This information
>	can be invaluable in debugging network problems.
>
>>How-To-Repeat:
>
>	The fxp driver only keeps around the last set of incremental
>	changes to the error counters.
>
>>Fix:
>	
>	Add a new fxp_stats struct and add in new amounts fxp_stats_update().

   Hello, Craig. I actually had counters like that in an early version of the
driver but decided to remove them for performance reasons. I wonder if these
should be hidden behind a compile time option?

-DG

David Greenman
Co-founder/Principal Architect, The FreeBSD Project - http://www.freebsd.org
Creator of high-performance Internet servers - http://www.terasolutions.com
Comment 2 Craig Leres freebsd_committer 1999-07-07 10:40:03 UTC
> >>Description:
> >
> >	It would be really sweet if one could look at the long term
> >	error counters for the intel 10/100 card. This information
> >	can be invaluable in debugging network problems.
> >
> >>How-To-Repeat:
> >
> >	The fxp driver only keeps around the last set of incremental
> >	changes to the error counters.
> >
> >>Fix:
> >	
> >	Add a new fxp_stats struct and add in new amounts fxp_stats_update().
> 
>    Hello, Craig. I actually had counters like that in an early version of the
> driver but decided to remove them for performance reasons. I wonder if these
> should be hidden behind a compile time option?

That would be fine with me.

It would be nice if there was some way to tell at run time that the
stats were available so that a C program or gdb script could tell if
the stats are available.

		Craig
Comment 3 dg 1999-07-07 11:10:19 UTC
>>    Hello, Craig. I actually had counters like that in an early version of the
>> driver but decided to remove them for performance reasons. I wonder if these
>> should be hidden behind a compile time option?
>
>That would be fine with me.
>
>It would be nice if there was some way to tell at run time that the
>stats were available so that a C program or gdb script could tell if
>the stats are available.

   Perhaps it would also be better if the stats were stored as a ISO 8802.3
MIB and made available via sysctl like what is done inside the if_ed driver.
The struct for that looks like this:

struct ifmib_iso_8802_3 {
        u_int32_t       dot3StatsAlignmentErrors;
        u_int32_t       dot3StatsFCSErrors;
        u_int32_t       dot3StatsSingleCollisionFrames;
        u_int32_t       dot3StatsMultipleCollisionFrames;
        u_int32_t       dot3StatsSQETestErrors;
        u_int32_t       dot3StatsDeferredTransmissions;
        u_int32_t       dot3StatsLateCollisions;
        u_int32_t       dot3StatsExcessiveCollisions;
        u_int32_t       dot3StatsInternalMacTransmitErrors;
        u_int32_t       dot3StatsCarrierSenseErrors;
        u_int32_t       dot3StatsFrameTooLongs;
        u_int32_t       dot3StatsInternalMacReceiveErrors;
        u_int32_t       dot3StatsEtherChipSet;
        /* Matt Thomas wants this one, not included in RFC 1650: */
        u_int32_t       dot3StatsMissedFrames;

        u_int32_t       dot3StatsCollFrequencies[16]; /* NB: index origin */

        u_int32_t       dot3Compliance;
#define DOT3COMPLIANCE_STATS    1
#define DOT3COMPLIANCE_COLLS    2
};

   ...Garrett may be able to add more to this thought since he was the one
who implemented the above.

-DG

David Greenman
Co-founder/Principal Architect, The FreeBSD Project - http://www.freebsd.org
Creator of high-performance Internet servers - http://www.terasolutions.com
Comment 4 Craig Leres freebsd_committer 1999-07-07 21:02:53 UTC
>    Perhaps it would also be better if the stats were stored as a ISO 8802.3
> MIB and made available via sysctl like what is done inside the if_ed driver.
> The struct for that looks like this:

Ok, let me hack something up and send you a copy...

		Craig
Comment 5 gpalmer freebsd_committer 1999-07-08 15:58:12 UTC
David Greenman wrote in message ID
<199907071020.DAA13817@freefall.freebsd.org>:
>     Perhaps it would also be better if the stats were stored as a ISO 8802.3
>  MIB and made available via sysctl like what is done inside the if_ed driver.
>  The struct for that looks like this:

An excellent idea, and the dot3stats structure from the MIB should
probably be made the standard for statistics collection in all our
ethernet drivers...
Comment 6 Craig Leres freebsd_committer 1999-07-10 22:27:45 UTC
>    Perhaps it would also be better if the stats were stored as a ISO 8802.3
> MIB and made available via sysctl like what is done inside the if_ed driver.

Ok, here's a version (diff'ed against 3.2-RELEASE) that does this. It
also adds a new bpf hook that allows packet capture interfaces to
report missed packets. If you want to ifdef the stats collection, it's
ok with me but the expense of adding up a few counters is minor
compared to fetching the stat struct over the bus (which you have to do
anyway for collision stats). Plus, this is only done once a second.

One question I had is do you know what the dot3Compliance field means?
It's not in RFC 1650 but I think it's an indication of what stats are
collected and I'm not sure just setting it to DOT3COMPLIANCE_COLLS is
%100 correct.

Please take a look at let me know what you think.

		Craig
------
*** if_fxp.c.virgin	Tue Jul  6 20:34:12 1999
--- if_fxp.c.new	Sat Jul 10 14:04:17 1999
***************
*** 32,37 ****
--- 32,41 ----
  
  /*
   * Intel EtherExpress Pro/100B PCI Fast Ethernet driver
+  * as described on:
+  *
+  *	http://developer.intel.com/design/network/mature/82557.htm
+  *
   */
  
  #include "bpfilter.h"
***************
*** 45,50 ****
--- 49,55 ----
  
  #include <net/if.h>
  #include <net/if_dl.h>
+ #include <net/if_mib.h>
  #include <net/if_media.h>
  
  #ifdef NS
***************
*** 585,590 ****
--- 590,600 ----
  	ifp->if_ioctl = fxp_ioctl;
  	ifp->if_start = fxp_start;
  	ifp->if_watchdog = fxp_watchdog;
+ 	ifp->if_linkmib = &sc->mibdata;
+ 	ifp->if_linkmiblen = sizeof(sc->mibdata);
+ 	sc->mibdata.dot3StatsEtherChipSet =
+ 	    DOT3CHIPSET(dot3VendorIntel, dot3ChipSetIntel82557);
+ 	sc->mibdata.dot3Compliance = DOT3COMPLIANCE_COLLS;
  
  	/*
  	 * Attach the interface.
***************
*** 1058,1069 ****
--- 1068,1083 ----
  					    FXP_RFA_STATUS_IAMATCH) &&
  					    (eh->ether_dhost[0] & 1)
  					    == 0) {
+ #ifde BRIDGE
  dropit:
+ #endif
  					    if (m)
  						m_freem(m);
  					    goto rcvloop;
  					}
+ #ifdef BRIDGE
  getit:
+ #endif
  					m->m_data +=
  					    sizeof(struct ether_header);
  					m->m_len -=
***************
*** 1107,1113 ****
  	struct ifnet *ifp = &sc->sc_if;
  	struct fxp_stats *sp = sc->fxp_stats;
  	struct fxp_cb_tx *txp;
! 	int s;
  
  	ifp->if_opackets += sp->tx_good;
  	ifp->if_collisions += sp->tx_total_collisions;
--- 1121,1128 ----
  	struct ifnet *ifp = &sc->sc_if;
  	struct fxp_stats *sp = sc->fxp_stats;
  	struct fxp_cb_tx *txp;
! 	struct ifmib_iso_8802_3 *mib = &sc->mibdata;
! 	int s, missed;
  
  	ifp->if_opackets += sp->tx_good;
  	ifp->if_collisions += sp->tx_total_collisions;
***************
*** 1134,1140 ****
--- 1149,1188 ----
  		if (tx_threshold < 192)
  			tx_threshold += 64;
  	}
+ 
+ 	/* Update RFC1650 totals */
+ 	mib->dot3StatsAlignmentErrors += sp->rx_alignment_errors;
+ 	mib->dot3StatsFCSErrors += sp->rx_crc_errors;
+ 	mib->dot3StatsSingleCollisionFrames += sp->tx_single_collisions;
+ 	mib->dot3StatsCollFrequencies[0] += sp->tx_single_collisions;
+ 	mib->dot3StatsMultipleCollisionFrames += sp->tx_multiple_collisions;
+ 	mib->dot3StatsDeferredTransmissions += sp->tx_deffered;
+ 	mib->dot3StatsLateCollisions += sp->tx_latecols;
+ 	mib->dot3StatsExcessiveCollisions += sp->tx_maxcols;
+ 	mib->dot3StatsCollFrequencies[15] += sp->tx_maxcols;
+ 	mib->dot3StatsInternalMacTransmitErrors += sp->tx_underruns;
+ 	mib->dot3StatsCarrierSenseErrors += sp->tx_lostcrs;
+ 	mib->dot3StatsInternalMacReceiveErrors += sp->rx_overrun_errors;
+ 	missed = sp->rx_overrun_errors + sp->rx_rnr_errors;
+ 	mib->dot3StatsMissedFrames += missed;
+ #ifdef notdef
+ 	mib->dot3StatsFrameTooLongs += ???;
+ 	/* Doesn't apply to non-aui ethernet */
+ 	mib->dot3StatsSQETestErrors += 0;
+ #endif
+ 
+ #ifdef notdef
+ 	/* Things the 82557 tells us about we can't report */
+ 	sp->rx_cdt_errors;	/* collisions during frame reception */
+ 	sp->rx_shortframes;	/* frame shorter than minimum */
+ #endif
+ 
  	s = splimp();
+ #if NBPFILTER > 0
+ 	/* Call bpf_dropcnt() at elevated ipl */
+ 	if (ifp->if_bpf != NULL && missed != 0)
+ 		bpf_dropcnt(ifp, missed);
+ #endif
  	/*
  	 * Release any xmit buffers that have completed DMA. This isn't
  	 * strictly necessary to do here, but it's advantagous for mbufs
RCS file: RCS/if_fxpvar.h,v
retrieving revision 1.1
diff -c -r1.1 if_fxpvar.h
*** /tmp/,RCSt1D84823	Sat Jul 10 14:08:50 1999
--- if_fxpvar.h	Wed Jul  7 12:52:32 1999
***************
*** 65,70 ****
--- 65,71 ----
  	int phy_primary_addr;		/* address of primary PHY */
  	int phy_primary_device;		/* device type of primary PHY */
  	int phy_10Mbps_only;		/* PHY is 10Mbps-only device */
+ 	struct ifmib_iso_8802_3 mibdata; /* stuff for network mgmt */
  };
  
  /* Macros to ease CSR access. */
*** bpf.c.virgin	Sat Jul 10 14:09:23 1999
--- bpf.c.new	Sat Jul 10 14:10:44 1999
***************
*** 1107,1112 ****
--- 1107,1127 ----
  }
  
  /*
+  * Hook for drivers to report dropped packets
+  */
+ void
+ bpf_dropcnt(ifp, drop)
+ 	struct ifnet *ifp;
+ 	u_int drop;
+ {
+ 	struct bpf_if *bp = ifp->if_bpf;
+ 	struct bpf_d *d;
+ 
+ 	for (d = bp->bif_dlist; d != NULL; d = d->bd_next)
+ 		d->bd_dcount += drop;
+ }
+ 
+ /*
   * Move the packet data from interface memory (pkt) into the
   * store buffer.  Return 1 if it's time to wakeup a listener (buffer full),
   * otherwise 0.  "copy" is the routine called to do the actual data
*** bpf.h.virgin	Sat Jul 10 14:12:34 1999
--- bpf.h.new	Sat Jul 10 14:13:01 1999
***************
*** 230,235 ****
--- 230,236 ----
  void	 bpfattach __P((struct ifnet *, u_int, u_int));
  void	 bpfilterattach __P((int));
  u_int	 bpf_filter __P((struct bpf_insn *, u_char *, u_int, u_int));
+ void	 bpf_dropcnt __P((struct ifnet *, u_int));
  #endif
  
  /*
Comment 7 cpiazza freebsd_committer 2000-01-14 06:37:32 UTC
Responsible Changed
From-To: freebsd-bugs->dg

DG was looking at this PR and it's his driver 
Comment 8 Mark Linimon freebsd_committer 2005-10-24 06:01:06 UTC
Responsible Changed
From-To: dg->freebsd-bugs

With bugmeister hat on, reassign from inactive committer.
Comment 9 Pyun YongHyeon freebsd_committer 2010-09-20 00:16:54 UTC
State Changed
From-To: open->closed

Close. 
fxp(4) implemented all MAC counters supported by controller with 
sysctl(8) and these counters are not cleared unless you unload 
driver. Having a common interface for MAC counters would be great 
in theory but it's really hard to do that because there are too 
many controllers that provides more/less counters than that of 
RFC 1650. Personally I think it's better to leave it to driver to 
expose controller specific MAC counters. 


Comment 10 Pyun YongHyeon freebsd_committer 2010-09-20 00:16:54 UTC
Responsible Changed
From-To: freebsd-bugs->yongari

Track.