[PATCH net 0/2] net: dsa: mv88e6xxx: 88E6171 stats fixes

[PATCH net 0/2] net: dsa: mv88e6xxx: 88E6171 stats fixes

[Florian Fainelli]

These two patches fixes statistics on the 88E6171 present on the Linksys
EA4500 router. These changes were found during testing using a
resurrected version of Andrew's dsa-tests:

https://github.com/lunn/dsa-tests

Florian Fainelli (2):
  net: dsa: mv88e6xxx: fix 88E6171 statistics snapshot
  net: dsa: mv88e6xxx: fix 88E6171 statistics type

 drivers/net/dsa/mv88e6xxx/chip.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

-- 
2.53.0

[PATCH net 1/2] net: dsa: mv88e6xxx: fix 88E6171 statistics snapshot

[Florian Fainelli]

The 88E6171 belongs to the 6351 family and takes the port number in the
low bits of the statistics operation register. The 6320 snapshot helper
instead encodes the port as (port + 1) << 5. On an EA4500, this causes
ethtool -S to return physical port 0's MIB counters for every user port.
Traffic on other ports does not update their reported hardware counters.

Before the snapshot operation was moved into the chip operations, only
the 6320 and 6352 families used the shifted encoding. The refactoring
incorrectly assigned that encoding to the 88E6171 as well.

Use the unshifted snapshot helper for the 88E6171. Tested on an EA4500:
pings through different switch ports now update only the corresponding
port's hardware counters.

Fixes: a605a0fe7115 ("net: dsa: mv88e6xxx: Abstract stats_snapshot into ops structure")
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/net/dsa/mv88e6xxx/chip.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 7f68a0c55802..67daf3b9cb48 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -4610,7 +4610,7 @@ static const struct mv88e6xxx_ops mv88e6171_ops = {
 	.port_disable_pri_override = mv88e6xxx_port_disable_pri_override,
 	.port_get_cmode = mv88e6352_port_get_cmode,
 	.port_setup_message_port = mv88e6xxx_setup_message_port,
-	.stats_snapshot = mv88e6320_g1_stats_snapshot,
+	.stats_snapshot = mv88e6xxx_g1_stats_snapshot,
 	.stats_set_histogram = mv88e6095_g1_stats_set_histogram,
 	.stats_get_sset_count = mv88e6095_stats_get_sset_count,
 	.stats_get_strings = mv88e6095_stats_get_strings,
-- 
2.53.0

[PATCH net 2/2] net: dsa: mv88e6xxx: fix 88E6171 statistics type

[Florian Fainelli]

The 88E6171 uses the 6095 statistics operations, which advertise and name
BANK0 and PORT counters. Its stats_type bitmap instead selects BANK0 and
BANK1. Since statistics collection started using that bitmap, it has
written a different set of counters from the one advertised to ethtool.

For this chip, get_sset_count() accounts for 30 BANK0 counters, three
PORT counters and five ATU/VTU counters. Collection instead writes 30
BANK0 counters, 26 BANK1 counters and five ATU/VTU counters: 61 entries
rather than the advertised 38. This mislabels the tail of the returned
statistics and writes past the advertised statistics buffer.

Select BANK0 and PORT counters to match the chip's statistics operations
and restore the filter used before the bitmap was introduced. Tested on
an EA4500, where the two-bridge forwarding and multicast snooping tests
now pass their hardware counter checks.

Fixes: 5595e3613ea7 ("dsa: mv88e6xxx: Move available stats into info structure")
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/net/dsa/mv88e6xxx/chip.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 67daf3b9cb48..ebd9f69a7ed7 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -5960,7 +5960,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.age_time_coeff = 15000,
 		.g1_irqs = 9,
 		.g2_irqs = 10,
-		.stats_type = STATS_TYPE_BANK0 | STATS_TYPE_BANK1,
+		.stats_type = STATS_TYPE_BANK0 | STATS_TYPE_PORT,
 		.atu_move_port_mask = 0xf,
 		.pvt = true,
 		.multi_chip = true,
-- 
2.53.0

Re: [PATCH net 1/2] net: dsa: mv88e6xxx: fix 88E6171 statistics snapshot

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 1 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] The fix is incomplete.

--

net: dsa: mv88e6xxx: fix 88E6171 statistics snapshot

This patch changes the 88E6171 ops table from the shifted
mv88e6320_g1_stats_snapshot() helper to the unshifted
mv88e6xxx_g1_stats_snapshot() helper. The 6351 family takes the port
number in the low bits of the statistics operation register. With the
shifted helper, ethtool -S returns port 0's MIB counters for every user
port.

> diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
> index 2a975ecd79c74..60d2ce80ede01 100644
> --- a/drivers/net/dsa/mv88e6xxx/chip.c
> +++ b/drivers/net/dsa/mv88e6xxx/chip.c
> @@ -4611,7 +4611,7 @@ static const struct mv88e6xxx_ops mv88e6171_ops = {
>  	.port_disable_pri_override = mv88e6xxx_port_disable_pri_override,
>  	.port_get_cmode = mv88e6352_port_get_cmode,
>  	.port_setup_message_port = mv88e6xxx_setup_message_port,
> -	.stats_snapshot = mv88e6320_g1_stats_snapshot,
> +	.stats_snapshot = mv88e6xxx_g1_stats_snapshot,

[Severity: Medium]
Should the other 6351 family ops tables get the same change?

In chip.c, mv88e6175_ops, mv88e6350_ops and mv88e6351_ops are all marked
/* MV88E6XXX_FAMILY_6351 */. Their info entries set
.family = MV88E6XXX_FAMILY_6351. All three still have:

	.stats_snapshot = mv88e6320_g1_stats_snapshot,

and that helper in global1.c does:

int mv88e6320_g1_stats_snapshot(struct mv88e6xxx_chip *chip, int port)
{
	port = (port + 1) << 5;

	return mv88e6xxx_g1_stats_snapshot(chip, port);
}

For ports 0..6, bits 4:0 of the STATS_OP CAPTURE_PORT write end up as
zero. If those chips decode the port from the low bits, as the commit
message says the 6351 family does, wouldn't every snapshot on
88E6175/88E6350/88E6351 capture port 0?

All stats readers reach the callback through mv88e6xxx_stats_snapshot():
mv88e6xxx_get_ethtool_stats(), get_eth_mac_stats() and get_rmon_stats().
Each of these would then report port 0's counters for every user port on
those chips.

The commit named in the Fixes: tag, a605a0fe7115, appears to have set
mv88e6320_g1_stats_snapshot on all four 6351 family tables. Before that
refactoring, _mv88e6xxx_stats_snapshot() shifted only for:

	mv88e6xxx_6320_family(chip) || mv88e6xxx_6352_family(chip)

so every 6351 family chip used the unshifted encoding.

As far as I can tell, the follow-up patch in the series ("net: dsa:
mv88e6xxx: fix 88E6171 statistics type") only touches the 6171 info
entry. At the end of the series, the three sibling tables would still
use the shifted helper.

>  	.stats_set_histogram = mv88e6095_g1_stats_set_histogram,
>  	.stats_get_sset_count = mv88e6095_stats_get_sset_count,
>  	.stats_get_strings = mv88e6095_stats_get_strings,

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261024182124.296988-1-f.fainelli%40gmail.com

Re: [PATCH net 1/2] net: dsa: mv88e6xxx: fix 88E6171 statistics snapshot

[Andrew Lunn]

On Sat, Oct 24, 2026 at 11:21:23AM -0700, Florian Fainelli wrote:
> The 88E6171 belongs to the 6351 family and takes the port number in the
> low bits of the statistics operation register. The 6320 snapshot helper
> instead encodes the port as (port + 1) << 5. On an EA4500, this causes
> ethtool -S to return physical port 0's MIB counters for every user port.
> Traffic on other ports does not update their reported hardware counters.
> 
> Before the snapshot operation was moved into the chip operations, only
> the 6320 and 6352 families used the shifted encoding. The refactoring
> incorrectly assigned that encoding to the 88E6171 as well.
> 
> Use the unshifted snapshot helper for the 88E6171. Tested on an EA4500:
> pings through different switch ports now update only the corresponding
> port's hardware counters.
> 
> Fixes: a605a0fe7115 ("net: dsa: mv88e6xxx: Abstract stats_snapshot into ops structure")
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
>  drivers/net/dsa/mv88e6xxx/chip.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
> index 7f68a0c55802..67daf3b9cb48 100644
> --- a/drivers/net/dsa/mv88e6xxx/chip.c
> +++ b/drivers/net/dsa/mv88e6xxx/chip.c
> @@ -4610,7 +4610,7 @@ static const struct mv88e6xxx_ops mv88e6171_ops = {
>  	.port_disable_pri_override = mv88e6xxx_port_disable_pri_override,
>  	.port_get_cmode = mv88e6352_port_get_cmode,
>  	.port_setup_message_port = mv88e6xxx_setup_message_port,
> -	.stats_snapshot = mv88e6320_g1_stats_snapshot,
> +	.stats_snapshot = mv88e6xxx_g1_stats_snapshot,
>  	.stats_set_histogram = mv88e6095_g1_stats_set_histogram,
>  	.stats_get_sset_count = mv88e6095_stats_get_sset_count,
>  	.stats_get_strings = mv88e6095_stats_get_strings,

static const struct mv88e6xxx_ops mv88e6171_ops = {
        /* MV88E6XXX_FAMILY_6351 */
...
        .stats_snapshot = mv88e6320_g1_stats_snapshot,

static const struct mv88e6xxx_ops mv88e6175_ops = {
        /* MV88E6XXX_FAMILY_6351 */
...
        .stats_snapshot = mv88e6320_g1_stats_snapshot,

static const struct mv88e6xxx_ops mv88e6350_ops = {
        /* MV88E6XXX_FAMILY_6351 */
...
        .stats_snapshot = mv88e6320_g1_stats_snapshot,

static const struct mv88e6xxx_ops mv88e6351_ops = {
        /* MV88E6XXX_FAMILY_6351 */
...
       .stats_snapshot = mv88e6320_g1_stats_snapshot,

Should these be changed as well?

Looking at DSDT:

static GT_STATUS statsCapture
(
    IN GT_QD_DEV            *dev,
    IN GT_U8             port,
    IN GT_U8             bank
)
...
    if (IS_IN_DEV_GROUP(dev,DEV_RMON_PORT_BITS))
    {
        portNum = (port + 1) << 5;
    }
    else
    {
        portNum = (GT_U16)port;
    }

and

#define DEV_RMON_PORT_BITS    \
        ( DEV_88E6065 | DEV_88E6065 | DEV_88E6352_FAMILY )

So your change looks correct, just not complete.

       Andrew

Re: [PATCH net 2/2] net: dsa: mv88e6xxx: fix 88E6171 statistics type

[Andrew Lunn]

On Sat, Oct 24, 2026 at 11:21:24AM -0700, Florian Fainelli wrote:
> The 88E6171 uses the 6095 statistics operations, which advertise and name
> BANK0 and PORT counters. Its stats_type bitmap instead selects BANK0 and
> BANK1. Since statistics collection started using that bitmap, it has
> written a different set of counters from the one advertised to ethtool.
> 
> For this chip, get_sset_count() accounts for 30 BANK0 counters, three
> PORT counters and five ATU/VTU counters. Collection instead writes 30
> BANK0 counters, 26 BANK1 counters and five ATU/VTU counters: 61 entries
> rather than the advertised 38. This mislabels the tail of the returned
> statistics and writes past the advertised statistics buffer.
> 
> Select BANK0 and PORT counters to match the chip's statistics operations
> and restore the filter used before the bitmap was introduced. Tested on
> an EA4500, where the two-bridge forwarding and multicast snooping tests
> now pass their hardware counter checks.
> 
> Fixes: 5595e3613ea7 ("dsa: mv88e6xxx: Move available stats into info structure")
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew