Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 20 Aug 2009 10:37:23 -0700
From:      Jack Vogel <jfvogel@gmail.com>
To:        Manish Vachharajani <manishv@lineratesystems.com>
Cc:        Barney Cordoba <barney_cordoba@yahoo.com>, freebsd-net@freebsd.org
Subject:   Re: Dropped vs. missed packets in the ixgbe driver
Message-ID:  <2a41acea0908201037n10505b04le924f29efd5398a7@mail.gmail.com>
In-Reply-To: <5bc218350908201034u553df7feiaead037432279360@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> <5bc218350908201034u553df7feiaead037432279360@mail.gmail.com>

next in thread | previous in thread | raw e-mail | index | archive | help
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 <
manishv@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<manishv@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<manishv@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<jfvogel@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
> >>> <manishv@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_cordoba@yahoo.com>
> >>>> wrote:
> >>>> >
> >>>> >
> >>>> > --- On Wed, 8/19/09, Manish Vachharajani <
> manishv@lineratesystems.com>
> >>>> > 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.  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-unsubscribe@freebsd.org
> "
> >>>
> >>>
> >>
> >> --
> >> Manish Vachharajani
> >>
> >
>
>
>
> --
> 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?2a41acea0908201037n10505b04le924f29efd5398a7>