netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Colin Foster <colin.foster@in-advantage.com>
To: Vladimir Oltean <vladimir.oltean@nxp.com>
Cc: netdev@vger.kernel.org, Andrew Lunn <andrew@lunn.ch>,
	Vivien Didelot <vivien.didelot@gmail.com>,
	Florian Fainelli <f.fainelli@gmail.com>,
	Claudiu Manoil <claudiu.manoil@nxp.com>,
	Alexandre Belloni <alexandre.belloni@bootlin.com>,
	UNGLinuxDriver@microchip.com,
	"David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	Maxim Kochetkov <fido_max@inbox.ru>
Subject: Re: [PATCH net 6/8] net: mscc: ocelot: make struct ocelot_stat_layout array indexable
Date: Tue, 16 Aug 2022 23:46:46 -0700	[thread overview]
Message-ID: <YvyO1kjPKPQM0Zw8@euler> (raw)
In-Reply-To: <20220816135352.1431497-7-vladimir.oltean@nxp.com>

On Tue, Aug 16, 2022 at 04:53:50PM +0300, Vladimir Oltean wrote:
> The ocelot counters are 32-bit and require periodic reading, every 2
> seconds, by ocelot_port_update_stats(), so that wraparounds are
> detected.
> 
> Currently, the counters reported by ocelot_get_stats64() come from the
> 32-bit hardware counters directly, rather than from the 64-bit
> accumulated ocelot->stats, and this is a problem for their integrity.
> 
> The strategy is to make ocelot_get_stats64() able to cherry-pick
> individual stats from ocelot->stats the way in which it currently reads
> them out from SYS_COUNT_* registers. But currently it can't, because
> ocelot->stats is an opaque u64 array that's used only to feed data into
> ethtool -S.
> 
> To solve that problem, we need to make ocelot->stats indexable, and
> associate each element with an element of struct ocelot_stat_layout used
> by ethtool -S.
> 
> This makes ocelot_stat_layout a fat (and possibly sparse) array, so we
> need to change the way in which we access it. We no longer need
> OCELOT_STAT_END as a sentinel, because we know the array's size
> (OCELOT_NUM_STATS). We just need to skip the array elements that were
> left unpopulated for the switch revision (ocelot, felix, seville).

Hi Vladimir,

I'm not sure if this is an issue here, and I'm not sure it will ever
be... ocelot_stat_layout as you know relies on consecutive register
addresses to be most efficient. This was indirectly enforced by
*_stats_layout[] always being laid out in ascending order.

If the order of ocelot_stat doesn't match each device's register
offset order, there'll be unnecessary overhead. I tried to test
this just now, but I'll have to deal with a few conflicts that I won't
be able to get to immediately.

Do you see this as a potential issue in the future? Or do you expect all
hardware is simliar enough that they'll all be ordered the same?

Or, because I'm the lucky one running on a slow SPI bus, this will be my
problem to monitor and fix if need be :)


  reply	other threads:[~2022-08-17  6:46 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-08-16 13:53 [PATCH net 0/8] Fixes for Ocelot driver statistics Vladimir Oltean
2022-08-16 13:53 ` [PATCH net 1/8] net: dsa: felix: fix ethtool 256-511 and 512-1023 TX packet counters Vladimir Oltean
2022-08-16 13:53 ` [PATCH net 2/8] net: mscc: ocelot: fix incorrect ndo_get_stats64 " Vladimir Oltean
2022-08-17  6:26   ` Colin Foster
2022-08-16 13:53 ` [PATCH net 3/8] net: mscc: ocelot: fix address of SYS_COUNT_TX_AGING counter Vladimir Oltean
2022-08-16 13:53 ` [PATCH net 4/8] net: mscc: ocelot: turn stats_lock into a spinlock Vladimir Oltean
2022-08-16 13:53 ` [PATCH net 5/8] net: mscc: ocelot: fix race between ndo_get_stats64 and ocelot_check_stats_work Vladimir Oltean
2022-08-16 13:53 ` [PATCH net 6/8] net: mscc: ocelot: make struct ocelot_stat_layout array indexable Vladimir Oltean
2022-08-17  6:46   ` Colin Foster [this message]
2022-08-17 11:06     ` Vladimir Oltean
2022-08-17 13:05       ` Vladimir Oltean
2022-08-17 15:14         ` Colin Foster
2022-08-17 17:42           ` Vladimir Oltean
2022-08-17 20:47             ` Colin Foster
2022-08-17 22:03               ` Vladimir Oltean
2022-08-17 22:07                 ` Colin Foster
2022-08-18  6:01                 ` Colin Foster
2022-08-16 13:53 ` [PATCH net 7/8] net: mscc: ocelot: keep ocelot_stat_layout by reg address, not offset Vladimir Oltean
2022-08-16 13:53 ` [PATCH net 8/8] net: mscc: ocelot: report ndo_get_stats64 from the wraparound-resistant ocelot->stats Vladimir Oltean
2022-08-18  5:20 ` [PATCH net 0/8] Fixes for Ocelot driver statistics patchwork-bot+netdevbpf

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YvyO1kjPKPQM0Zw8@euler \
    --to=colin.foster@in-advantage.com \
    --cc=UNGLinuxDriver@microchip.com \
    --cc=alexandre.belloni@bootlin.com \
    --cc=andrew@lunn.ch \
    --cc=claudiu.manoil@nxp.com \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=f.fainelli@gmail.com \
    --cc=fido_max@inbox.ru \
    --cc=kuba@kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=vivien.didelot@gmail.com \
    --cc=vladimir.oltean@nxp.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).