linux-kernel - Re: [PATCH net-next 07/12] net: dsa: rzn1-a5psw: add statistics 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: <Yll1IsIYKHC/n+sg@lunn.ch>
Date:   Fri, 15 Apr 2022 15:37:38 +0200
From:   Andrew Lunn <andrew@...n.ch>
To:     Clément Léger <clement.leger@...tlin.com>
Cc:     Vivien Didelot <vivien.didelot@...il.com>,
        Florian Fainelli <f.fainelli@...il.com>,
        Vladimir Oltean <olteanv@...il.com>,
        "David S . Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzk+dt@...nel.org>,
        Geert Uytterhoeven <geert+renesas@...der.be>,
        Magnus Damm <magnus.damm@...il.com>,
        Heiner Kallweit <hkallweit1@...il.com>,
        Russell King <linux@...linux.org.uk>,
        Thomas Petazzoni <thomas.petazzoni@...tlin.com>,
        Herve Codina <herve.codina@...tlin.com>,
        Miquèl Raynal <miquel.raynal@...tlin.com>,
        Milan Stevanovic <milan.stevanovic@...com>,
        Jimmy Lalande <jimmy.lalande@...com>,
        linux-kernel@...r.kernel.org, devicetree@...r.kernel.org,
        linux-renesas-soc@...r.kernel.org, netdev@...r.kernel.org
Subject: Re: [PATCH net-next 07/12] net: dsa: rzn1-a5psw: add statistics
 support

> > > +static void a5psw_get_ethtool_stats(struct dsa_switch *ds, int port,
> > > +				    uint64_t *data)
> > > +{
> > > +	struct a5psw *a5psw = ds->priv;
> > > +	u32 reg_lo, reg_hi;
> > > +	unsigned int u;
> > > +
> > > +	for (u = 0; u < ARRAY_SIZE(a5psw_stats); u++) {
> > > +		/* A5PSW_STATS_HIWORD is global and thus, access must be
> > > +		 * exclusive
> > > +		 */  
> > 
> > Could you explain that a bit more. The RTNL lock will prevent two
> > parallel calls to this function.
> 
> Ok, I wasn't sure of the locking applicable here.

In general, RTNL protects you for any user space management like
operation on the driver. In this case, if you look in net/ethtool, you
will find the IOCTL handler code takes RTNL before calling into the
main IOCTL dispatcher. If you want to be paranoid/document the
assumption, you can add an ASSERT_RTNL().

The semantics for some of the other statistics Vladimir requested can
be slightly different. One of them is in atomic context, because a
spinlock is held. But i don't remember if RTNL is also held. This is
less of an issue for your switch, since it uses MMIO, however many
switches need to perform blocking IO over MDIO, SPI, IC2 etc to get
stats, which you cannot do in atomic context. So they end up returning
cached values.

Look in the mailing list for past discussion for details.

    Andrew