diff mbox series

[net,2/8] net: mscc: ocelot: fix incorrect ndo_get_stats64 packet counters

Message ID 20220816135352.1431497-3-vladimir.oltean@nxp.com (mailing list archive)
State Accepted
Commit 5152de7b79ab0be150f5966481b0c8f996192531
Delegated to: Netdev Maintainers
Headers show
Series Fixes for Ocelot driver statistics | expand

Checks

Context Check Description
netdev/tree_selection success Clearly marked for net
netdev/fixes_present success Fixes tag present in non-next series
netdev/subject_prefix success Link
netdev/cover_letter success Series has a cover letter
netdev/patch_count success Link
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 1360 this patch: 1360
netdev/cc_maintainers success CCed 13 of 13 maintainers
netdev/build_clang success Errors and warnings before: 0 this patch: 0
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success Fixes tag looks correct
netdev/build_allmodconfig_warn success Errors and warnings before: 1360 this patch: 1360
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 131 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Vladimir Oltean Aug. 16, 2022, 1:53 p.m. UTC 
Reading stats using the SYS_COUNT_* register definitions is only used by
ocelot_get_stats64() from the ocelot switchdev driver, however,
currently the bucket definitions are incorrect.

Separately, on both RX and TX, we have the following problems:
- a 256-1023 bucket which actually tracks the 256-511 packets
- the 1024-1526 bucket actually tracks the 512-1023 packets
- the 1527-max bucket actually tracks the 1024-1526 packets

=> nobody tracks the packets from the real 1527-max bucket

Additionally, the RX_PAUSE, RX_CONTROL, RX_LONGS and RX_CLASSIFIED_DROPS
all track the wrong thing. However this doesn't seem to have any
consequence, since ocelot_get_stats64() doesn't use these.

Even though this problem only manifests itself for the switchdev driver,
we cannot split the fix for ocelot and for DSA, since it requires fixing
the bucket definitions from enum ocelot_reg, which makes us necessarily
adapt the structures from felix and seville as well.

Fixes: 84705fc16552 ("net: dsa: felix: introduce support for Seville VSC9953 switch")
Fixes: 56051948773e ("net: dsa: ocelot: add driver for Felix switch family")
Fixes: a556c76adc05 ("net: mscc: Add initial Ocelot switch support")
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 drivers/net/dsa/ocelot/felix_vsc9959.c   | 20 ++++++++++++--------
 drivers/net/dsa/ocelot/seville_vsc9953.c | 16 +++++++++-------
 drivers/net/ethernet/mscc/ocelot_net.c   |  6 ++++--
 drivers/net/ethernet/mscc/vsc7514_regs.c | 24 +++++++++++++-----------
 include/soc/mscc/ocelot.h                |  6 ++++--
 5 files changed, 42 insertions(+), 30 deletions(-)

Comments

Colin Foster Aug. 17, 2022, 6:26 a.m. UTC | #1 
> --- a/drivers/net/ethernet/mscc/vsc7514_regs.c
> +++ b/drivers/net/ethernet/mscc/vsc7514_regs.c
> @@ -180,13 +180,14 @@ const u32 vsc7514_sys_regmap[] = {
>  	REG(SYS_COUNT_RX_64,				0x000024),
>  	REG(SYS_COUNT_RX_65_127,			0x000028),
>  	REG(SYS_COUNT_RX_128_255,			0x00002c),
> -	REG(SYS_COUNT_RX_256_1023,			0x000030),
> -	REG(SYS_COUNT_RX_1024_1526,			0x000034),
> -	REG(SYS_COUNT_RX_1527_MAX,			0x000038),
> -	REG(SYS_COUNT_RX_PAUSE,				0x00003c),
> -	REG(SYS_COUNT_RX_CONTROL,			0x000040),
> -	REG(SYS_COUNT_RX_LONGS,				0x000044),
> -	REG(SYS_COUNT_RX_CLASSIFIED_DROPS,		0x000048),
> +	REG(SYS_COUNT_RX_256_511,			0x000030),
> +	REG(SYS_COUNT_RX_512_1023,			0x000034),
> +	REG(SYS_COUNT_RX_1024_1526,			0x000038),
> +	REG(SYS_COUNT_RX_1527_MAX,			0x00003c),
> +	REG(SYS_COUNT_RX_PAUSE,				0x000040),
> +	REG(SYS_COUNT_RX_CONTROL,			0x000044),
> +	REG(SYS_COUNT_RX_LONGS,				0x000048),
> +	REG(SYS_COUNT_RX_CLASSIFIED_DROPS,		0x00004c),

Hi Vladimir,

Good catch! From a 7512/7514 point, these all look good. There's a
couple conflicts I'll have to deal with to test the whole series.

>  	REG(SYS_COUNT_TX_OCTETS,			0x000100),
>  	REG(SYS_COUNT_TX_UNICAST,			0x000104),
>  	REG(SYS_COUNT_TX_MULTICAST,			0x000108),
> @@ -196,10 +197,11 @@ const u32 vsc7514_sys_regmap[] = {
>  	REG(SYS_COUNT_TX_PAUSE,				0x000118),
>  	REG(SYS_COUNT_TX_64,				0x00011c),
>  	REG(SYS_COUNT_TX_65_127,			0x000120),
> -	REG(SYS_COUNT_TX_128_511,			0x000124),
> -	REG(SYS_COUNT_TX_512_1023,			0x000128),
> -	REG(SYS_COUNT_TX_1024_1526,			0x00012c),
> -	REG(SYS_COUNT_TX_1527_MAX,			0x000130),
> +	REG(SYS_COUNT_TX_128_255,			0x000124),
> +	REG(SYS_COUNT_TX_256_511,			0x000128),
> +	REG(SYS_COUNT_TX_512_1023,			0x00012c),
> +	REG(SYS_COUNT_TX_1024_1526,			0x000130),
> +	REG(SYS_COUNT_TX_1527_MAX,			0x000134),
>  	REG(SYS_COUNT_TX_AGING,				0x000170),
>  	REG(SYS_RESET_CFG,				0x000508),
>  	REG(SYS_CMID,					0x00050c),
> diff --git a/include/soc/mscc/ocelot.h b/include/soc/mscc/ocelot.h
> index ac151ecc7f19..e7e5b06deb2d 100644
> --- a/include/soc/mscc/ocelot.h
> +++ b/include/soc/mscc/ocelot.h
> @@ -335,7 +335,8 @@ enum ocelot_reg {
>  	SYS_COUNT_RX_64,
>  	SYS_COUNT_RX_65_127,
>  	SYS_COUNT_RX_128_255,
> -	SYS_COUNT_RX_256_1023,
> +	SYS_COUNT_RX_256_511,
> +	SYS_COUNT_RX_512_1023,
>  	SYS_COUNT_RX_1024_1526,
>  	SYS_COUNT_RX_1527_MAX,
>  	SYS_COUNT_RX_PAUSE,
> @@ -351,7 +352,8 @@ enum ocelot_reg {
>  	SYS_COUNT_TX_PAUSE,
>  	SYS_COUNT_TX_64,
>  	SYS_COUNT_TX_65_127,
> -	SYS_COUNT_TX_128_511,
> +	SYS_COUNT_TX_128_255,
> +	SYS_COUNT_TX_256_511,
>  	SYS_COUNT_TX_512_1023,
>  	SYS_COUNT_TX_1024_1526,
>  	SYS_COUNT_TX_1527_MAX,
> -- 
> 2.34.1
>
diff mbox series

Patch

diff --git a/drivers/net/dsa/ocelot/felix_vsc9959.c b/drivers/net/dsa/ocelot/felix_vsc9959.c
index 5859ef3b242c..e1ebe21cad00 100644
--- a/drivers/net/dsa/ocelot/felix_vsc9959.c
+++ b/drivers/net/dsa/ocelot/felix_vsc9959.c
@@ -281,19 +281,23 @@  static const u32 vsc9959_sys_regmap[] = {
 	REG(SYS_COUNT_RX_64,			0x000024),
 	REG(SYS_COUNT_RX_65_127,		0x000028),
 	REG(SYS_COUNT_RX_128_255,		0x00002c),
-	REG(SYS_COUNT_RX_256_1023,		0x000030),
-	REG(SYS_COUNT_RX_1024_1526,		0x000034),
-	REG(SYS_COUNT_RX_1527_MAX,		0x000038),
-	REG(SYS_COUNT_RX_LONGS,			0x000044),
+	REG(SYS_COUNT_RX_256_511,		0x000030),
+	REG(SYS_COUNT_RX_512_1023,		0x000034),
+	REG(SYS_COUNT_RX_1024_1526,		0x000038),
+	REG(SYS_COUNT_RX_1527_MAX,		0x00003c),
+	REG(SYS_COUNT_RX_PAUSE,			0x000040),
+	REG(SYS_COUNT_RX_CONTROL,		0x000044),
+	REG(SYS_COUNT_RX_LONGS,			0x000048),
 	REG(SYS_COUNT_TX_OCTETS,		0x000200),
 	REG(SYS_COUNT_TX_COLLISION,		0x000210),
 	REG(SYS_COUNT_TX_DROPS,			0x000214),
 	REG(SYS_COUNT_TX_64,			0x00021c),
 	REG(SYS_COUNT_TX_65_127,		0x000220),
-	REG(SYS_COUNT_TX_128_511,		0x000224),
-	REG(SYS_COUNT_TX_512_1023,		0x000228),
-	REG(SYS_COUNT_TX_1024_1526,		0x00022c),
-	REG(SYS_COUNT_TX_1527_MAX,		0x000230),
+	REG(SYS_COUNT_TX_128_255,		0x000224),
+	REG(SYS_COUNT_TX_256_511,		0x000228),
+	REG(SYS_COUNT_TX_512_1023,		0x00022c),
+	REG(SYS_COUNT_TX_1024_1526,		0x000230),
+	REG(SYS_COUNT_TX_1527_MAX,		0x000234),
 	REG(SYS_COUNT_TX_AGING,			0x000278),
 	REG(SYS_RESET_CFG,			0x000e00),
 	REG(SYS_SR_ETYPE_CFG,			0x000e04),
diff --git a/drivers/net/dsa/ocelot/seville_vsc9953.c b/drivers/net/dsa/ocelot/seville_vsc9953.c
index ea0649211356..ebe9ddbbe2b7 100644
--- a/drivers/net/dsa/ocelot/seville_vsc9953.c
+++ b/drivers/net/dsa/ocelot/seville_vsc9953.c
@@ -277,19 +277,21 @@  static const u32 vsc9953_sys_regmap[] = {
 	REG(SYS_COUNT_RX_64,			0x000024),
 	REG(SYS_COUNT_RX_65_127,		0x000028),
 	REG(SYS_COUNT_RX_128_255,		0x00002c),
-	REG(SYS_COUNT_RX_256_1023,		0x000030),
-	REG(SYS_COUNT_RX_1024_1526,		0x000034),
-	REG(SYS_COUNT_RX_1527_MAX,		0x000038),
+	REG(SYS_COUNT_RX_256_511,		0x000030),
+	REG(SYS_COUNT_RX_512_1023,		0x000034),
+	REG(SYS_COUNT_RX_1024_1526,		0x000038),
+	REG(SYS_COUNT_RX_1527_MAX,		0x00003c),
 	REG(SYS_COUNT_RX_LONGS,			0x000048),
 	REG(SYS_COUNT_TX_OCTETS,		0x000100),
 	REG(SYS_COUNT_TX_COLLISION,		0x000110),
 	REG(SYS_COUNT_TX_DROPS,			0x000114),
 	REG(SYS_COUNT_TX_64,			0x00011c),
 	REG(SYS_COUNT_TX_65_127,		0x000120),
-	REG(SYS_COUNT_TX_128_511,		0x000124),
-	REG(SYS_COUNT_TX_512_1023,		0x000128),
-	REG(SYS_COUNT_TX_1024_1526,		0x00012c),
-	REG(SYS_COUNT_TX_1527_MAX,		0x000130),
+	REG(SYS_COUNT_TX_128_255,		0x000124),
+	REG(SYS_COUNT_TX_256_511,		0x000128),
+	REG(SYS_COUNT_TX_512_1023,		0x00012c),
+	REG(SYS_COUNT_TX_1024_1526,		0x000130),
+	REG(SYS_COUNT_TX_1527_MAX,		0x000134),
 	REG(SYS_COUNT_TX_AGING,			0x000178),
 	REG(SYS_RESET_CFG,			0x000318),
 	REG_RESERVED(SYS_SR_ETYPE_CFG),
diff --git a/drivers/net/ethernet/mscc/ocelot_net.c b/drivers/net/ethernet/mscc/ocelot_net.c
index 5e6136e80282..9d8cea16245e 100644
--- a/drivers/net/ethernet/mscc/ocelot_net.c
+++ b/drivers/net/ethernet/mscc/ocelot_net.c
@@ -739,7 +739,8 @@  static void ocelot_get_stats64(struct net_device *dev,
 			    ocelot_read(ocelot, SYS_COUNT_RX_64) +
 			    ocelot_read(ocelot, SYS_COUNT_RX_65_127) +
 			    ocelot_read(ocelot, SYS_COUNT_RX_128_255) +
-			    ocelot_read(ocelot, SYS_COUNT_RX_256_1023) +
+			    ocelot_read(ocelot, SYS_COUNT_RX_256_511) +
+			    ocelot_read(ocelot, SYS_COUNT_RX_512_1023) +
 			    ocelot_read(ocelot, SYS_COUNT_RX_1024_1526) +
 			    ocelot_read(ocelot, SYS_COUNT_RX_1527_MAX);
 	stats->multicast = ocelot_read(ocelot, SYS_COUNT_RX_MULTICAST);
@@ -749,7 +750,8 @@  static void ocelot_get_stats64(struct net_device *dev,
 	stats->tx_bytes = ocelot_read(ocelot, SYS_COUNT_TX_OCTETS);
 	stats->tx_packets = ocelot_read(ocelot, SYS_COUNT_TX_64) +
 			    ocelot_read(ocelot, SYS_COUNT_TX_65_127) +
-			    ocelot_read(ocelot, SYS_COUNT_TX_128_511) +
+			    ocelot_read(ocelot, SYS_COUNT_TX_128_255) +
+			    ocelot_read(ocelot, SYS_COUNT_TX_256_511) +
 			    ocelot_read(ocelot, SYS_COUNT_TX_512_1023) +
 			    ocelot_read(ocelot, SYS_COUNT_TX_1024_1526) +
 			    ocelot_read(ocelot, SYS_COUNT_TX_1527_MAX);
diff --git a/drivers/net/ethernet/mscc/vsc7514_regs.c b/drivers/net/ethernet/mscc/vsc7514_regs.c
index c2af4eb8ca5d..38ab20b48cd4 100644
--- a/drivers/net/ethernet/mscc/vsc7514_regs.c
+++ b/drivers/net/ethernet/mscc/vsc7514_regs.c
@@ -180,13 +180,14 @@  const u32 vsc7514_sys_regmap[] = {
 	REG(SYS_COUNT_RX_64,				0x000024),
 	REG(SYS_COUNT_RX_65_127,			0x000028),
 	REG(SYS_COUNT_RX_128_255,			0x00002c),
-	REG(SYS_COUNT_RX_256_1023,			0x000030),
-	REG(SYS_COUNT_RX_1024_1526,			0x000034),
-	REG(SYS_COUNT_RX_1527_MAX,			0x000038),
-	REG(SYS_COUNT_RX_PAUSE,				0x00003c),
-	REG(SYS_COUNT_RX_CONTROL,			0x000040),
-	REG(SYS_COUNT_RX_LONGS,				0x000044),
-	REG(SYS_COUNT_RX_CLASSIFIED_DROPS,		0x000048),
+	REG(SYS_COUNT_RX_256_511,			0x000030),
+	REG(SYS_COUNT_RX_512_1023,			0x000034),
+	REG(SYS_COUNT_RX_1024_1526,			0x000038),
+	REG(SYS_COUNT_RX_1527_MAX,			0x00003c),
+	REG(SYS_COUNT_RX_PAUSE,				0x000040),
+	REG(SYS_COUNT_RX_CONTROL,			0x000044),
+	REG(SYS_COUNT_RX_LONGS,				0x000048),
+	REG(SYS_COUNT_RX_CLASSIFIED_DROPS,		0x00004c),
 	REG(SYS_COUNT_TX_OCTETS,			0x000100),
 	REG(SYS_COUNT_TX_UNICAST,			0x000104),
 	REG(SYS_COUNT_TX_MULTICAST,			0x000108),
@@ -196,10 +197,11 @@  const u32 vsc7514_sys_regmap[] = {
 	REG(SYS_COUNT_TX_PAUSE,				0x000118),
 	REG(SYS_COUNT_TX_64,				0x00011c),
 	REG(SYS_COUNT_TX_65_127,			0x000120),
-	REG(SYS_COUNT_TX_128_511,			0x000124),
-	REG(SYS_COUNT_TX_512_1023,			0x000128),
-	REG(SYS_COUNT_TX_1024_1526,			0x00012c),
-	REG(SYS_COUNT_TX_1527_MAX,			0x000130),
+	REG(SYS_COUNT_TX_128_255,			0x000124),
+	REG(SYS_COUNT_TX_256_511,			0x000128),
+	REG(SYS_COUNT_TX_512_1023,			0x00012c),
+	REG(SYS_COUNT_TX_1024_1526,			0x000130),
+	REG(SYS_COUNT_TX_1527_MAX,			0x000134),
 	REG(SYS_COUNT_TX_AGING,				0x000170),
 	REG(SYS_RESET_CFG,				0x000508),
 	REG(SYS_CMID,					0x00050c),
diff --git a/include/soc/mscc/ocelot.h b/include/soc/mscc/ocelot.h
index ac151ecc7f19..e7e5b06deb2d 100644
--- a/include/soc/mscc/ocelot.h
+++ b/include/soc/mscc/ocelot.h
@@ -335,7 +335,8 @@  enum ocelot_reg {
 	SYS_COUNT_RX_64,
 	SYS_COUNT_RX_65_127,
 	SYS_COUNT_RX_128_255,
-	SYS_COUNT_RX_256_1023,
+	SYS_COUNT_RX_256_511,
+	SYS_COUNT_RX_512_1023,
 	SYS_COUNT_RX_1024_1526,
 	SYS_COUNT_RX_1527_MAX,
 	SYS_COUNT_RX_PAUSE,
@@ -351,7 +352,8 @@  enum ocelot_reg {
 	SYS_COUNT_TX_PAUSE,
 	SYS_COUNT_TX_64,
 	SYS_COUNT_TX_65_127,
-	SYS_COUNT_TX_128_511,
+	SYS_COUNT_TX_128_255,
+	SYS_COUNT_TX_256_511,
 	SYS_COUNT_TX_512_1023,
 	SYS_COUNT_TX_1024_1526,
 	SYS_COUNT_TX_1527_MAX,