netdev - Re: [PATCH RFC] net: mvneta: add ethtool statistics

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20151008154211.GG32532@n2100.arm.linux.org.uk>
Date:	Thu, 8 Oct 2015 16:42:12 +0100
From:	Russell King - ARM Linux <linux@....linux.org.uk>
To:	Andrew Lunn <andrew@...n.ch>
Cc:	Thomas Petazzoni <thomas.petazzoni@...e-electrons.com>,
	netdev@...r.kernel.org
Subject: Re: [PATCH RFC] net: mvneta: add ethtool statistics

On Thu, Oct 08, 2015 at 05:32:57PM +0200, Andrew Lunn wrote:
> > > This would suggest the driver is not clearing the statistics when it
> > > loads. Should it?
> > 
> > That's an interesting question, one which I have no answer for.
> 
> So i took a look at the other Marvell drivers. mv643xx_eth.c, skge and
> sky2.c all have code to clear the statistics. So for consistency, i
> think nvneta should as well.
> 
> But it gets stranger. mvneta has:
> 
> static void mvneta_mib_counters_clear(struct mvneta_port *pp)
> {
>         int i;
>         u32 dummy;
> 
>         /* Perform dummy reads from MIB counters */
>         for (i = 0; i < MVNETA_MIB_LATE_COLLISION; i += 4)
>                 dummy = mvreg_read(pp, (MVNETA_MIB_COUNTERS_BASE + i));
> }

That's called from mvneta_port_up(), which is called every time the link
comes up or the MTU is changed.  That's really not nice.  However, it
seems ineffectual because it's not accessing any documented registers:

#define MVNETA_MIB_COUNTERS_BASE                 0x3080
#define      MVNETA_MIB_LATE_COLLISION           0x7c

However, the documented counters are from 0x3000 to 0x307c.

> Looks like somebody did not finish this?

All you need to do is to read the counter to clear it, so the above
should result in it being cleared as above.

Maybe the above code can be fixed to read the proper counters, and
called from a more appropriate place (driver initialisation?)

-- 
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html