rtl8169_get_stats seems to be adding random crap to rx_missed_errors count

rtl8169_get_stats seems to be adding random crap to rx_missed_errors count

[Alex Bennee]

Hi,

I've been seeing odd behaviour in the ifconfig output for my rtl8169
based network device. The ifconfig output had a very high number of
dropped packets in the display which changed massively every time I
checked. I tracked it down to /proc/net/dev and the a few test patches
it seems the rc_missed field is the culprit. I did a test patch of the
form:

+++ b/drivers/net/r8169.c
@@ -3062,10 +3062,16 @@ static struct net_device_stats
*rtl8169_get_stats(struct net_device *dev)
        unsigned long flags;

        if (netif_running(dev)) {
+               u32 missed;
                spin_lock_irqsave(&tp->lock, flags);
-               dev->stats.rx_missed_errors += RTL_R32(RxMissed);
+               missed = RTL_R32(RxMissed);
                RTL_W32(RxMissed, 0);
                spin_unlock_irqrestore(&tp->lock, flags);
+
+               dev->stats.rx_missed_errors += missed;
+               if (missed)
+                       printk(KERN_INFO "rtl8169_get_stats added %d
to missed count\n", missed);
+
        }

        return &dev->stats;

And low and behold I see this behaviour:

[  946.160188] rtl8169_get_stats added 250834943 to missed count
[  946.160354] rtl8169_get_stats added 20768 to missed count
[  946.160411] rtl8169_get_stats added 7032 to missed count
[  946.160425] rtl8169_get_stats added 1677 to missed count
[  946.160439] rtl8169_get_stats added 1577 to missed count
[  946.160452] rtl8169_get_stats added 1543 to missed count
[  946.160465] rtl8169_get_stats added 1517 to missed count
[  946.160479] rtl8169_get_stats added 1572 to missed count
[  946.160492] rtl8169_get_stats added 1586 to missed count
[  946.160504] rtl8169_get_stats added 1284 to missed count
[  948.164206] rtl8169_get_stats added 250864594 to missed count
[  948.164374] rtl8169_get_stats added 21004 to missed count
[  948.164434] rtl8169_get_stats added 7385 to missed count
[  948.164447] rtl8169_get_stats added 1631 to missed count
[  948.164461] rtl8169_get_stats added 1574 to missed count
[  948.164475] rtl8169_get_stats added 1645 to missed count
[  948.164489] rtl8169_get_stats added 1583 to missed count
[  948.164503] rtl8169_get_stats added 1561 to missed count
[  948.164523] rtl8169_get_stats added 2314 to missed count
[  948.164534] rtl8169_get_stats added 1300 to missed count
[  950.168182] rtl8169_get_stats added 250857857 to missed count
[  950.168345] rtl8169_get_stats added 20465 to missed count
[  950.168401] rtl8169_get_stats added 6903 to missed count
[  950.168416] rtl8169_get_stats added 1711 to missed count
[  950.168430] rtl8169_get_stats added 1585 to missed count
[  950.168443] rtl8169_get_stats added 1532 to missed count
[  950.168456] rtl8169_get_stats added 1520 to missed count
[  950.168469] rtl8169_get_stats added 1539 to missed count
[  950.168483] rtl8169_get_stats added 1588 to missed count
[  950.168494] rtl8169_get_stats added 1282 to missed count

It looks very much like the number being read is a decreasing counter
rather than anything else.

Has anyone got a copy of the data sheet and can confirm that:

	RxMissed	= 0x4c,

is the correct offset for the register and it doesn't need any special
handling to be read?




--
Alex, homepage: http://www.bennee.com/~alex/

Re: rtl8169_get_stats seems to be adding random crap to rx_missed_errors count

[Francois Romieu]

Alex Bennee <kernel-hacker@bennee.com> :
[...]

It is fixed by patch #0009 at:

http://userweb.kernel.org/~romieu/r8169/2.6.27-rc9/20081007/

-- 
Ueimor

Re: rtl8169_get_stats seems to be adding random crap to rx_missed_errors count

[David Miller]

From: Francois Romieu <romieu@fr.zoreil.com>
Date: Fri, 10 Oct 2008 19:27:31 +0200

> Alex Bennee <kernel-hacker@bennee.com> :
> [...]
> 
> It is fixed by patch #0009 at:
> 
> http://userweb.kernel.org/~romieu/r8169/2.6.27-rc9/20081007/

Francois, if you have all of this stuff fixed how come you haven't
submitted these properly to netdev for inclusion?

I asked you yesterday to make a patch submission to me so that your
fixes get into the tree.

But you just keep point people at your web site over and over again.

That doesn't work.

Re: rtl8169_get_stats seems to be adding random crap to rx_missed_errors count

[Francois Romieu]

David Miller <davem@davemloft.net> :
[...]
> Francois, if you have all of this stuff fixed how come you haven't
> submitted these properly to netdev for inclusion?

I have submitted the bugfix part, it has been properly included in
Jeff's tree that was rejected as bad timed.

And, no, this stuff was not ready for the previous merge window.

> I asked you yesterday to make a patch submission to me so that your
> fixes get into the tree.

It will be done. Sorry if I am slower than you.

> But you just keep point people at your web site over and over again.

You are the very first one complaining about it.

> That doesn't work.

Give yourself a break (TM). It's friday.

-- 
Ueimor

Re: rtl8169_get_stats seems to be adding random crap to rx_missed_errors count

[Francois Romieu]

Francois Romieu <romieu@fr.zoreil.com> :
[...]

And if you really wonder, I have answered Alex's message before
leaving my workplace while checking for messages from my wife.

Your mail from yesterday was not noticed.

-- 
Ueimor

Re: rtl8169_get_stats seems to be adding random crap to rx_missed_errors count

[Francois Romieu]

Francois Romieu <romieu@fr.zoreil.com> :
> David Miller <davem@davemloft.net> :
> [...]
> > Francois, if you have all of this stuff fixed how come you haven't
> > submitted these properly to netdev for inclusion?
> 
> I have submitted the bugfix part, it has been properly included in
> Jeff's tree that was rejected as bad timed.

Addendum: it has been merged in your net-next tree since.

-- 
Ueimor

Re: rtl8169_get_stats seems to be adding random crap to rx_missed_errors count

[David Miller]

From: Francois Romieu <romieu@fr.zoreil.com>
Date: Fri, 10 Oct 2008 21:53:47 +0200

> David Miller <davem@davemloft.net> :
> [...]
> > That doesn't work.
> 
> Give yourself a break (TM). It's friday.

No such luck, merge window is open.

I can take a break when I'm dead.