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]

Message-ID: <SN1PR11MB0446E9A84FB312A2C6E6A097EC670@SN1PR11MB0446.namprd11.prod.outlook.com>
Date:   Thu, 14 Feb 2019 19:26:16 +0000
From:   <Tristram.Ha@...rochip.com>
To:     <f.fainelli@...il.com>
CC:     <sergio.paracuellos@...il.com>, <pavel@....cz>,
        <UNGLinuxDriver@...rochip.com>, <netdev@...r.kernel.org>,
        <andrew@...n.ch>
Subject: RE: [PATCH v1 net-next 2/4] net: dsa: microchip: add MIB counter
 reading support

> >> > +		/* read only dropped counters when link is not up */
> >> > +		if (p->link_just_down)
> >> > +			p->link_just_down = 0;
> >> > +		else if (!p->phydev.link)
> >> > +			mib->cnt_ptr = dev->reg_mib_cnt;
> >>
> >> This link_just_down stuff is not clear at all. Why can the drop
> >> counters not be read when the link is up?
> >
> >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.
> 
> Can you use netif_running() to determine that condition? Maintaining your
> own set of variables when the PHY state machine should already determine
> the link state sounds redundant if not error prone.
> 

The driver can store the PHY device pointer passed to it when the port is enabled.  But I am a little worried that pointer can be changed or completely gone as it is out of control of the driver.

> >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.
> >For now that API is called from user space with the ethtool utility, so
> >it may not be called too often and too fast.  But theoretically that
> >API can be called from a program continually.
> >
> >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.
> 
> Some switches have a MIB autocast feature taking a snapshot which AFAIR is
> internally implemented as a fast read register with no contention on other
> registers internally, do you have something similar?

There is no such function in the switch.  Every MIB counter read has to go through a single SPI transfer using indirect access.  There are no table-like stored values that a single SPI transfer can retrieve all.

For i2C the access is even slower, but then I do not expect this access mechanism is used when the switch can do more complex things.