• Vladimir Oltean's avatar
    net: mscc: ocelot: fix stats region batching · 6acc72a4
    Vladimir Oltean authored
    The blamed commit changed struct ocelot_stat_layout :: "u32 offset" to
    "u32 reg".
    
    However, "u32 reg" is not quite a register address, but an enum
    ocelot_reg, which in itself encodes an enum ocelot_target target in the
    upper bits, and an index into the ocelot->map[target][] array in the
    lower bits.
    
    So, whereas the previous code comparison between stats_layout[i].offset
    and last + 1 was correct (because those "offsets" at the time were
    32-bit relative addresses), the new code, comparing layout[i].reg to
    last + 4 is not correct, because the "reg" here is an enum/index, not an
    actual register address.
    
    What we want to compare are indeed register addresses, but to do that,
    we need to actually go through the same motions as
    __ocelot_bulk_read_ix() itself.
    
    With this bug, all statistics counters are deemed by
    ocelot_prepare_stats_regions() as constituting their own region.
    (Truncated) log on VSC9959 (Felix) below (prints added by me):
    
    Before:
    
    region of 1 contiguous counters starting with SYS:STAT:CNT[0x000]
    region of 1 contiguous counters starting with SYS:STAT:CNT[0x001]
    region of 1 contiguous counters starting with SYS:STAT:CNT[0x002]
    ...
    region of 1 contiguous counters starting with SYS:STAT:CNT[0x041]
    region of 1 contiguous counters starting with SYS:STAT:CNT[0x042]
    region of 1 contiguous counters starting with SYS:STAT:CNT[0x080]
    region of 1 contiguous counters starting with SYS:STAT:CNT[0x081]
    ...
    region of 1 contiguous counters starting with SYS:STAT:CNT[0x0ac]
    region of 1 contiguous counters starting with SYS:STAT:CNT[0x100]
    region of 1 contiguous counters starting with SYS:STAT:CNT[0x101]
    ...
    region of 1 contiguous counters starting with SYS:STAT:CNT[0x111]
    
    After:
    
    region of 67 contiguous counters starting with SYS:STAT:CNT[0x000]
    region of 45 contiguous counters starting with SYS:STAT:CNT[0x080]
    region of 18 contiguous counters starting with SYS:STAT:CNT[0x100]
    
    Since commit d87b1c08 ("net: mscc: ocelot: use bulk reads for
    stats") intended bulking as a performance improvement, and since now,
    with trivial-sized regions, performance is even worse than without
    bulking at all, this could easily qualify as a performance regression.
    
    Fixes: d4c36765 ("net: mscc: ocelot: keep ocelot_stat_layout by reg address, not offset")
    Signed-off-by: default avatarVladimir Oltean <vladimir.oltean@nxp.com>
    Acked-by: default avatarColin Foster <colin.foster@in-advantage.com>
    Tested-by: default avatarColin Foster <colin.foster@in-advantage.com>
    Signed-off-by: default avatarJakub Kicinski <kuba@kernel.org>
    6acc72a4
ocelot_stats.c 31.4 KB