Yes, in our latest tests, netstat correctly outputs the misses as Ierrs. Manish
On Thu, Aug 20, 2009 at 11:37 AM, Jack Vogel<jfvo...@gmail.com> wrote: > Thanks Manish, I will keep this diff around and work it into my final > changes in the > code, you confirmed this solved the problem you were seeing I assume? > > Cheers, > > Jack > > > On Thu, Aug 20, 2009 at 10:34 AM, Manish Vachharajani > <mani...@lineratesystems.com> wrote: >> >> Whoops, the correct fix is below. Forgot to use missed_rx_cum when >> summing: >> >> diff --git a/fbsd/ixgbe-1.7.4/ixgbe.c b/fbsd/ixgbe-1.7.4/ixgbe.c >> index f1fa728..262d64d 100644 >> --- a/fbsd/ixgbe-1.7.4/ixgbe.c >> +++ b/fbsd/ixgbe-1.7.4/ixgbe.c >> @@ -4294,7 +4294,7 @@ ixgbe_update_stats_counters(struct adapter *adapter) >> { >> struct ifnet *ifp = adapter->ifp;; >> struct ixgbe_hw *hw = &adapter->hw; >> - u32 missed_rx = 0, bprc, lxon, lxoff, total; >> + u32 missed_rx = 0, missed_rx_cum = 0, bprc, lxon, lxoff, total; >> >> adapter->stats.crcerrs += IXGBE_READ_REG(hw, IXGBE_CRCERRS); >> >> @@ -4303,6 +4303,7 @@ ixgbe_update_stats_counters(struct adapter *adapter) >> mp = IXGBE_READ_REG(hw, IXGBE_MPC(i)); >> missed_rx += mp; >> adapter->stats.mpc[i] += mp; >> + missed_rx_cum += adapter->stats.mpc[i]; >> adapter->stats.rnbc[i] += IXGBE_READ_REG(hw, >> IXGBE_RNBC(i)); >> } >> >> @@ -4370,7 +4371,7 @@ ixgbe_update_stats_counters(struct adapter *adapter) >> ifp->if_collisions = 0; >> >> /* Rx Errors */ >> - ifp->if_ierrors = missed_rx + adapter->stats.crcerrs + >> + ifp->if_ierrors = missed_rx_cum + adapter->stats.crcerrs + >> adapter->stats.rlec; >> } >> >> >> >> On Thu, Aug 20, 2009 at 11:32 AM, Manish >> Vachharajani<mani...@lineratesystems.com> wrote: >> > My co-founder, John, just pointed out the problem. >> > >> > The MPC register on ixgbe is clear on read. stats.mpc[i] correctly >> > accumulates the misses, but missed_rx gets set to 0 on any interval >> > where there are no misses and subsequently, if_errors gets set to 0 >> > (assuming no crcerrs or rlecs.) I believe the correct fix is in the >> > patch below: >> > >> > diff --git a/fbsd/ixgbe-1.7.4/ixgbe.c b/fbsd/ixgbe-1.7.4/ixgbe.c >> > index f1fa728..9cbaec6 100644 >> > --- a/fbsd/ixgbe-1.7.4/ixgbe.c >> > +++ b/fbsd/ixgbe-1.7.4/ixgbe.c >> > @@ -4294,7 +4294,7 @@ ixgbe_update_stats_counters(struct adapter >> > *adapter) >> > { >> > struct ifnet *ifp = adapter->ifp;; >> > struct ixgbe_hw *hw = &adapter->hw; >> > - u32 missed_rx = 0, bprc, lxon, lxoff, total; >> > + u32 missed_rx = 0, missed_rx_cum = 0, bprc, lxon, lxoff, total; >> > >> > adapter->stats.crcerrs += IXGBE_READ_REG(hw, IXGBE_CRCERRS); >> > >> > @@ -4303,6 +4303,7 @@ ixgbe_update_stats_counters(struct adapter >> > *adapter) >> > mp = IXGBE_READ_REG(hw, IXGBE_MPC(i)); >> > missed_rx += mp; >> > adapter->stats.mpc[i] += mp; >> > + missed_rx_cum += adapter->stats.mpc[i]; >> > adapter->stats.rnbc[i] += IXGBE_READ_REG(hw, >> > IXGBE_RNBC(i)); >> > } >> > >> > (Note that it may not apply since I've pulled the driver out into a >> > separate directory structure) >> > >> > We need the missed_rx_cum that is the total number of rx misses seen >> > because missed_rx is decremented from gprc to work around a hardware >> > issue and so needs to be the misses seen on this call to the function. >> > >> > Manish >> > >> > >> > On Thu, Aug 20, 2009 at 11:23 AM, Manish >> > Vachharajani<mani...@lineratesystems.com> wrote: >> >> I noticed the bogus XON, XOFF numbers. I'm glad to see it will be >> >> fixed so I can cross it off my todo list. :) I don't think the issue >> >> is related though, but you never know. >> >> >> >> Barney pointed out that missed_rx in the ixgbe_update_stats_counters >> >> function accumulates the missed packet registers into the missed_rx >> >> field. At the end of the function, these are summed into >> >> ifp->if_ierrors which should be reported under Ierrs by netstat -idh. >> >> However that count is zero as reported via netstat. The stats printfs >> >> activated via sysctl dev.ix.#.stats=1 prints stats.mpc[0]. This >> >> number is large (around 400k on the machine I've been using to find >> >> the problem). If you look at the code, missed_rx and mpc[i] are >> >> updated with the same variables. Given that missed_rx is unsigned, I >> >> don't understand how the number fails to make it into ifp->if_ierrors, >> >> but I have yet to dive into debugging this seriously. >> >> >> >> Initially, I thought the fix was simple since I didn't see the >> >> missed_rx getting added to ierrors. But now, I am not sure where the >> >> problem is. Hopefully, it will be more obvious to you than it is to >> >> me. I'm sure it is something simple that I am missing. >> >> >> >> Manish >> >> >> >> On Thu, Aug 20, 2009 at 11:08 AM, Jack Vogel<jfvo...@gmail.com> wrote: >> >>> I've been looking at the code due to another problem of bogus flow >> >>> control >> >>> numbers, and there are some changes needed, things that should be >> >>> 82599 vs >> >>> 82598 and were not seperated properly. Will be forthcoming. Not sure >> >>> if it >> >>> has any relevance to this, but its possible. >> >>> >> >>> Jack >> >>> >> >>> >> >>> On Thu, Aug 20, 2009 at 9:53 AM, Manish Vachharajani >> >>> <mani...@lineratesystems.com> wrote: >> >>>> >> >>>> Oh whoops, sorry didn't see that. So the plot thickens. Why don't >> >>>> these errors show up in the netstat output I forwarded originally? >> >>>> Ierrs was 0 but the dmesg output clearly shows missed packets. Any >> >>>> thoughts on what is going on? Looking at the code, missed_rx should >> >>>> certainly get counted in the ierrors field as you said. >> >>>> >> >>>> Is the Ierrs in the netstat output some other counter? If so, how do >> >>>> I get the if_ierrors variable from the command line? >> >>>> >> >>>> Manish >> >>>> >> >>>> On Thu, Aug 20, 2009 at 6:49 AM, Barney >> >>>> Cordoba<barney_cord...@yahoo.com> >> >>>> wrote: >> >>>> > >> >>>> > >> >>>> > --- On Wed, 8/19/09, Manish Vachharajani >> >>>> > <mani...@lineratesystems.com> >> >>>> > wrote: >> >>>> > >> >>>> >> From: Manish Vachharajani <mani...@lineratesystems.com> >> >>>> >> Subject: Re: Dropped vs. missed packets in the ixgbe driver >> >>>> >> To: "Barney Cordoba" <barney_cord...@yahoo.com> >> >>>> >> Cc: freebsd-net@freebsd.org >> >>>> >> Date: Wednesday, August 19, 2009, 2:46 PM >> >>>> >> Agreed, the errors are reported but >> >>>> >> missed packets are not. The >> >>>> >> question is, is the correct fix to just add stats.mpc[0] to >> >>>> >> if_ierrors >> >>>> >> in that line or to add it to if_iqdrops. The fix is >> >>>> >> easy once we >> >>>> >> agree on what the correct behavior is. >> >>>> >> >> >>>> >> Manish >> >>>> >> >> >>>> >> > Barney wrote: >> >>>> >> > >> >>>> >> > if you look in ixgbe_update_stats_counters at the >> >>>> >> bottom: >> >>>> >> > >> >>>> >> > ifp->if_ierrors = missed_rx + >> >>>> >> adapter->stats.crcerrs + >> >>>> >> > adapter->stats.rlec; >> >>>> >> > >> >>>> >> > the errors are added in. >> >>>> >> > >> >>>> >> > BC >> >>>> > >> >>>> > Huh? missed_rx are the missed packets. So they are counted. >> >>>> > >> >>>> > BC >> >>>> > >> >>>> > >> >>>> > >> >>>> > >> >>>> > >> >>>> >> >>>> >> >>>> >> >>>> -- >> >>>> Manish Vachharajani >> >> >> >>>> _______________________________________________ >> >>>> freebsd-net@freebsd.org mailing list >> >>>> http://lists.freebsd.org/mailman/listinfo/freebsd-net >> >>>> To unsubscribe, send any mail to >> >>>> "freebsd-net-unsubscr...@freebsd.org" >> >>> >> >>> >> >> >> >> -- >> >> Manish Vachharajani >> >> >> > >> >> >> >> -- >> Manish Vachharajani >> Founder >> LineRate Systems >> mani...@lineratesystems.com >> (609)635-9531 M > > -- Manish Vachharajani Founder LineRate Systems mani...@lineratesystems.com (609)635-9531 M _______________________________________________ freebsd-net@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-net To unsubscribe, send any mail to "freebsd-net-unsubscr...@freebsd.org"