From owner-freebsd-net@FreeBSD.ORG Thu Aug 20 17:34:13 2009 Return-Path: Delivered-To: freebsd-net@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 6CD391065691 for ; Thu, 20 Aug 2009 17:34:13 +0000 (UTC) (envelope-from manishv@lineratesystems.com) Received: from mail-vw0-f180.google.com (mail-vw0-f180.google.com [209.85.212.180]) by mx1.freebsd.org (Postfix) with ESMTP id 297818FC7E for ; Thu, 20 Aug 2009 17:34:13 +0000 (UTC) Received: by vws10 with SMTP id 10so72409vws.7 for ; Thu, 20 Aug 2009 10:34:12 -0700 (PDT) MIME-Version: 1.0 Received: by 10.220.12.10 with SMTP id v10mr51985vcv.97.1250789652485; Thu, 20 Aug 2009 10:34:12 -0700 (PDT) In-Reply-To: <5bc218350908201032l44859117obc3203ad91fc5706@mail.gmail.com> References: <5bc218350908191146j2a22f8dcrdecb0b67eedce5c2@mail.gmail.com> <435336.24858.qm@web63908.mail.re1.yahoo.com> <5bc218350908200953p630d99c6u1538999b308c55f9@mail.gmail.com> <2a41acea0908201008y6e8f160dx27b406db7d3081b7@mail.gmail.com> <5bc218350908201023q14c51cer6effadd49cc4c604@mail.gmail.com> <5bc218350908201032l44859117obc3203ad91fc5706@mail.gmail.com> Date: Thu, 20 Aug 2009 11:34:12 -0600 Message-ID: <5bc218350908201034u553df7feiaead037432279360@mail.gmail.com> From: Manish Vachharajani To: Jack Vogel Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Cc: Barney Cordoba , freebsd-net@freebsd.org Subject: Re: Dropped vs. missed packets in the ixgbe driver X-BeenThere: freebsd-net@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Networking and TCP/IP with FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 20 Aug 2009 17:34:13 -0000 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 =3D adapter->ifp;; struct ixgbe_hw *hw =3D &adapter->hw; - u32 missed_rx =3D 0, bprc, lxon, lxoff, total; + u32 missed_rx =3D 0, missed_rx_cum =3D 0, bprc, lxon, lxoff, total= ; adapter->stats.crcerrs +=3D IXGBE_READ_REG(hw, IXGBE_CRCERRS); @@ -4303,6 +4303,7 @@ ixgbe_update_stats_counters(struct adapter *adapter) mp =3D IXGBE_READ_REG(hw, IXGBE_MPC(i)); missed_rx +=3D mp; adapter->stats.mpc[i] +=3D mp; + missed_rx_cum +=3D adapter->stats.mpc[i]; adapter->stats.rnbc[i] +=3D IXGBE_READ_REG(hw, IXGBE_RNBC(i= )); } @@ -4370,7 +4371,7 @@ ixgbe_update_stats_counters(struct adapter *adapter) ifp->if_collisions =3D 0; /* Rx Errors */ - ifp->if_ierrors =3D missed_rx + adapter->stats.crcerrs + + ifp->if_ierrors =3D missed_rx_cum + adapter->stats.crcerrs + adapter->stats.rlec; } On Thu, Aug 20, 2009 at 11:32 AM, Manish Vachharajani wrote: > My co-founder, John, just pointed out the problem. > > The MPC register on ixgbe is clear on read. =A0stats.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.) =A0I 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= ) > =A0{ > =A0 =A0 =A0 =A0struct ifnet =A0 *ifp =3D adapter->ifp;; > =A0 =A0 =A0 =A0struct ixgbe_hw *hw =3D &adapter->hw; > - =A0 =A0 =A0 u32 =A0missed_rx =3D 0, bprc, lxon, lxoff, total; > + =A0 =A0 =A0 u32 =A0missed_rx =3D 0, missed_rx_cum =3D 0, bprc, lxon, lx= off, total; > > =A0 =A0 =A0 =A0adapter->stats.crcerrs +=3D IXGBE_READ_REG(hw, IXGBE_CRCER= RS); > > @@ -4303,6 +4303,7 @@ ixgbe_update_stats_counters(struct adapter *adapter= ) > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0mp =3D IXGBE_READ_REG(hw, IXGBE_MPC(i)); > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0missed_rx +=3D mp; > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0adapter->stats.mpc[i] +=3D mp; > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 missed_rx_cum +=3D adapter->stats.mpc[i]; > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0adapter->stats.rnbc[i] +=3D IXGBE_READ_REG= (hw, IXGBE_RNBC(i)); > =A0 =A0 =A0 =A0} > > (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 wrote: >> I noticed the bogus XON, XOFF numbers. =A0I'm glad to see it will be >> fixed so I can cross it off my todo list. =A0:) =A0I don't think the iss= ue >> 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. =A0At 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. =A0The stats printfs >> activated via sysctl dev.ix.#.stats=3D1 prints stats.mpc[0]. =A0This >> number is large (around 400k on the machine I've been using to find >> the problem). =A0If you look at the code, missed_rx and mpc[i] are >> updated with the same variables. =A0Given 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. =A0But now, I am not sure where the >> problem is. =A0Hopefully, it will be more obvious to you than it is to >> me. =A0I'm sure it is something simple that I am missing. >> >> Manish >> >> On Thu, Aug 20, 2009 at 11:08 AM, Jack Vogel wrote: >>> I've been looking at the code due to another problem of bogus flow cont= rol >>> 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 >>> wrote: >>>> >>>> Oh whoops, sorry didn't see that. =A0So the plot thickens. =A0Why don'= t >>>> these errors show up in the netstat output I forwarded originally? >>>> Ierrs was 0 but the dmesg output clearly shows missed packets. =A0Any >>>> thoughts on what is going on? =A0Looking 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? =A0If so, how d= o >>>> I get the if_ierrors variable from the command line? >>>> >>>> Manish >>>> >>>> On Thu, Aug 20, 2009 at 6:49 AM, Barney Cordoba >>>> wrote: >>>> > >>>> > >>>> > --- On Wed, 8/19/09, Manish Vachharajani >>>> > wrote: >>>> > >>>> >> From: Manish Vachharajani >>>> >> Subject: Re: Dropped vs. missed packets in the ixgbe driver >>>> >> To: "Barney Cordoba" >>>> >> Cc: freebsd-net@freebsd.org >>>> >> Date: Wednesday, August 19, 2009, 2:46 PM >>>> >> Agreed, the errors are reported but >>>> >> missed packets are not.=A0 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.=A0 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: >>>> >> > >>>> >> > =A0 =A0 =A0 =A0ifp->if_ierrors =3D missed_rx + >>>> >> adapter->stats.crcerrs + >>>> >> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0adapter->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-unsubscribe@freebsd.org" >>> >>> >> >> -- >> Manish Vachharajani >> > --=20 Manish Vachharajani Founder LineRate Systems manishv@lineratesystems.com (609)635-9531 M