netdev - RE: [PATCH net-next v4] dsa: lan9303: Add 3 ethtool stats

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: <MWHPR11MB1693C9E7D1B2AB14B5256F19EF1B9@MWHPR11MB1693.namprd11.prod.outlook.com>
Date:   Tue, 6 Dec 2022 22:38:58 +0000
From:   <Jerry.Ray@...rochip.com>
To:     <olteanv@...il.com>, <kuba@...nel.org>
CC:     <andrew@...n.ch>, <f.fainelli@...il.com>, <davem@...emloft.net>,
        <edumazet@...gle.com>, <pabeni@...hat.com>,
        <netdev@...r.kernel.org>
Subject: RE: [PATCH net-next v4] dsa: lan9303: Add 3 ethtool stats

> > > >Huh? I'm guessing you're referring to some patches you have queued
> > > >already and don't want to rebase across? Or some project planning?
> > > >Otherwise I don't see a connection :S
> > >
> > > In looking around at other implementations, I see where the link_up
> > > and link_down are used to start or clean up the periodic workqueue
> > > used to retrieve and accumulate the mib stats into the driver.  Can't tell
> > > if that's a requirement or only needed when the device interface is
> > > considered too slow.  The device interface is not atomic.
> >
> > Atomic as in it reads over a bus which requires sleeping?
> > Yes, the stats ndo can't sleep because of the old procfs interface
> > which ifconfig uses and which is invoked under the RCU lock.
> 
> Jerry, did you respond to this (what do you mean by "device interface is
> not atomic")?
> 
> Still not clear why transitioning to phylink is a requirement for
> standardized statistics. You get your link up/link down events with
> adjust_link too (phydev->link).
> 
> Some other drivers only perform periodic stats readout for ports that
> are up, because those are the only ports where the stats can ever
> change. That's about all that there is to know. There's no requirement
> one way or another, it's an optimization.
> 

Yes, Jakub's understanding is correct.  The device is accessed over the
mdio bus and as such, will sleep on hardware accesses.  This means that the
get_stats64 will need to respond with the mib info cached by the driver rather
than reading directly from the device.  This adds to the driver.

The phylink up/down DSA hooks will be used in controlling the workqueue
threads that periodically read and cache the stats info.

You are correct - It's not a hard requirement to first migrate to PHYLINK.
It's more of a logical progression of bringing the driver up-to-date. Having
adjust_link API still defined, you get the 
  "Using legacy PHYLIB callbacks. Please migrate to PHYLINK!"
message on board power-up.