Dropped vs. missed packets in the ixgbe driver
Manish Vachharajani
manishv at lineratesystems.com
Thu Aug 20 17:39:01 UTC 2009
Yes, in our latest tests, netstat correctly outputs the misses as Ierrs.
Manish
On Thu, Aug 20, 2009 at 11:37 AM, Jack Vogel<jfvogel at 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
> <manishv at 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 at 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 at 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 at 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 at 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 at yahoo.com>
>> >>>> wrote:
>> >>>> >
>> >>>> >
>> >>>> > --- On Wed, 8/19/09, Manish Vachharajani
>> >>>> > <manishv at lineratesystems.com>
>> >>>> > wrote:
>> >>>> >
>> >>>> >> From: Manish Vachharajani <manishv at lineratesystems.com>
>> >>>> >> Subject: Re: Dropped vs. missed packets in the ixgbe driver
>> >>>> >> To: "Barney Cordoba" <barney_cordoba at yahoo.com>
>> >>>> >> Cc: freebsd-net at 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 at freebsd.org mailing list
>> >>>> http://lists.freebsd.org/mailman/listinfo/freebsd-net
>> >>>> To unsubscribe, send any mail to
>> >>>> "freebsd-net-unsubscribe at freebsd.org"
>> >>>
>> >>>
>> >>
>> >> --
>> >> Manish Vachharajani
>> >>
>> >
>>
>>
>>
>> --
>> Manish Vachharajani
>> Founder
>> LineRate Systems
>> manishv at lineratesystems.com
>> (609)635-9531 M
>
>
--
Manish Vachharajani
Founder
LineRate Systems
manishv at lineratesystems.com
(609)635-9531 M
More information about the freebsd-net
mailing list