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

Message-ID: <20221206195516.vv57lab7p4iifar5@skbuf>
Date:   Tue, 6 Dec 2022 21:55:16 +0200
From:   Vladimir Oltean <olteanv@...il.com>
To:     Jakub Kicinski <kuba@...nel.org>
Cc:     Jerry.Ray@...rochip.com, 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

On Fri, Dec 02, 2022 at 11:36:22AM -0800, Jakub Kicinski wrote:
> On Fri, 2 Dec 2022 15:22:55 +0000 Jerry.Ray@...rochip.com wrote:
> > >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.