Date: Thu, 20 Aug 2009 11:34:12 -0600 From: Manish Vachharajani <manishv@lineratesystems.com> To: Jack Vogel <jfvogel@gmail.com> Cc: Barney Cordoba <barney_cordoba@yahoo.com>, freebsd-net@freebsd.org Subject: Re: Dropped vs. missed packets in the ixgbe driver Message-ID: <5bc218350908201034u553df7feiaead037432279360@mail.gmail.com> 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>
next in thread | previous in thread | raw e-mail | index | archive | help
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<manishv@lineratesystems.com> 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<manishv@lineratesystems.com> 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<jfvogel@gmail.com> 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
>>> <manishv@lineratesystems.com> 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<barney_cordoba@yahoo.c=
om>
>>>> wrote:
>>>> >
>>>> >
>>>> > --- On Wed, 8/19/09, Manish Vachharajani <manishv@lineratesystems.co=
m>
>>>> > wrote:
>>>> >
>>>> >> From: Manish Vachharajani <manishv@lineratesystems.com>
>>>> >> Subject: Re: Dropped vs. missed packets in the ixgbe driver
>>>> >> To: "Barney Cordoba" <barney_cordoba@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.=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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?5bc218350908201034u553df7feiaead037432279360>