Subject: Re: [PATCH net 6/8] net: mscc: ocelot: make struct ocelot_stat_layout array indexable

From: Colin Foster <[email-masked]>
Date: Tue, 16 Aug 2022 23:46:46 -0700

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 :)