Bug 12543
| Summary: | [fxp] [patch] cumulative error counters for fxp(4) | ||
|---|---|---|---|
| Product: | Base System | Reporter: | Craig Leres <leres> |
| Component: | kern | Assignee: | Pyun YongHyeon <yongari> |
| Status: | Closed FIXED | ||
| Severity: | Affects Only Me | ||
| Priority: | Normal | ||
| Version: | 3.2-RELEASE | ||
| Hardware: | Any | ||
| OS: | Any | ||
>>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 > >>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
>> 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 > 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
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... > 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 /* Responsible Changed From-To: freebsd-bugs->dg DG was looking at this PR and it's his driver Responsible Changed From-To: dg->freebsd-bugs With bugmeister hat on, reassign from inactive committer. 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. Responsible Changed From-To: freebsd-bugs->yongari Track. |
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.