netdev - Re: [PATCH v1 net-next 2/4] net: dsa: microchip: add MIB counter reading support

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] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 13 Feb 2019 04:28:16 +0100
From:   Andrew Lunn <andrew@...n.ch>
To:     Tristram.Ha@...rochip.com
Cc:     sergio.paracuellos@...il.com, f.fainelli@...il.com, pavel@....cz,
        UNGLinuxDriver@...rochip.com, netdev@...r.kernel.org
Subject: Re: [PATCH v1 net-next 2/4] net: dsa: microchip: add MIB counter
 reading support

> All of the MIB counters, except some that may be marked by driver,
> do not get updated when the link is down, so it is a waste of time
> to read them.

Hi Tristram

O.K, so make this clear in the code. Maybe rather than having this
link_just_down, have the adjust link callback update the cached
values for all counters? 

> My intention is the driver eventually reads the MIB counters at
> least every second or faster so that the ethtool API called to show
> MIB counters gets them from memory rather than starting a read
> operation.

The user expects to see the current counters, not some cached values.
For me it is O.K. to frequently read the counters to prevent wrap
around, but each ethtool call should update the counters before
returning them to user space.

> For simple switches that do not need to do anything special the MIB
> read operation does not cause any issue except CPU load, for more
> complicate switches that need to do some background operations too
> many read operation can affect some critical functions.

Sounds like a bad design of the switch, if reading statistics from it
can upset its operation. You might want to consider rate limiting the
ethtool call.

	Andrew