netdev - Re: [PATCH net v1] net: stmmac: Fix queue statistics reading

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Sun, 15 Jan 2023 17:48:10 +0100
From:   Andrew Lunn <andrew@...n.ch>
To:     Kurt Kanzenbach <kurt@...utronix.de>
Cc:     Giuseppe Cavallaro <peppe.cavallaro@...com>,
        Alexandre Torgue <alexandre.torgue@...s.st.com>,
        Jose Abreu <joabreu@...opsys.com>,
        "David S. Miller" <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>,
        Maxime Coquelin <mcoquelin.stm32@...il.com>,
        netdev@...r.kernel.org, linux-stm32@...md-mailman.stormreply.com,
        linux-arm-kernel@...ts.infradead.org,
        Vijayakannan Ayyathurai <vijayakannan.ayyathurai@...el.com>,
        Wong Vee Khee <vee.khee.wong@...ux.intel.com>
Subject: Re: [PATCH net v1] net: stmmac: Fix queue statistics reading

On Sun, Jan 15, 2023 at 12:25:51PM +0100, Kurt Kanzenbach wrote:
> Hi Andrew,
> 
> On Sat Jan 14 2023, Andrew Lunn wrote:
> > On Sat, Jan 14, 2023 at 01:04:37PM +0100, Kurt Kanzenbach wrote:
> >> Correct queue statistics reading. All queue statistics are stored as unsigned
> >> long values. The retrieval for ethtool fetches these values as u64. However, on
> >> some systems the size of the counters are 32 bit.
> >
> >> @@ -551,16 +551,16 @@ static void stmmac_get_per_qstats(struct stmmac_priv *priv, u64 *data)
> >>  		p = (char *)priv + offsetof(struct stmmac_priv,
> >>  					    xstats.txq_stats[q].tx_pkt_n);
> >>  		for (stat = 0; stat < STMMAC_TXQ_STATS; stat++) {
> >> -			*data++ = (*(u64 *)p);
> >> -			p += sizeof(u64 *);
> >> +			*data++ = (*(unsigned long *)p);
> >> +			p += sizeof(unsigned long);
> >
> > As you said in the comment, the register is 32 bits.
> 
> Maybe the commit message is a bit misleading. There are no registers
> involved here.

Ah!

In that case, yes, unsigned long. Maybe reword the commit message to
mention the values are being read from struct stmmac_txq_stats and
struct stmmac_rxq_stats which use unsigned long. That would avoid my
confusion of thinking it is being read from a register.

With that change made, you can add my Reviewed-by.

    Andrew