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
| ||
|
Message-ID: <8ad1ec10-0f3f-9c66-a799-23df5a7842c0@gmail.com> Date: Thu, 14 Feb 2019 11:33:17 -0800 From: Florian Fainelli <f.fainelli@...il.com> To: Tristram.Ha@...rochip.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 On 2/14/19 11:26 AM, Tristram.Ha@...rochip.com wrote: >>>>> + /* 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. The per-port network device is accessible from dp->slave so you can do netif_running(dp->slave) from your driver, what are you talking about here? > >>> 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. > Is that something you are considering to change for future designs? -- Florian
Powered by blists - more mailing lists