diff mbox series

[net-next,V2,3/4] octeontx2-pf: ethtool: Implement get_fec_stats

Message ID 20221129051409.20034-4-hkelam@marvell.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series CN10KB MAC block support | expand

Checks

Context Check Description
netdev/tree_selection success Clearly marked for net-next
netdev/fixes_present success Fixes tag not required for -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: 0 this patch: 0
netdev/cc_maintainers success CCed 9 of 9 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 No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 113 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Hariprasad Kelam Nov. 29, 2022, 5:14 a.m. UTC 
The current implementation is such that FEC statistics are reported as
part of ethtool statistics that is the user can fetch them from the
below command

"ethtool -S eth0"

Fec Corrected Errors:
Fec Uncorrected Errors:

This patch removes this logic and registers a callback for get_fec_stats
such that FEC stats can be queried from the below command
"ethtool -I --show-fec eth0"

Signed-off-by: Hariprasad Kelam <hkelam@marvell.com>
Signed-off-by: Sunil Kovvuri Goutham <sgoutham@marvell.com>
---
 .../marvell/octeontx2/nic/otx2_ethtool.c      | 75 ++++++++++---------
 1 file changed, 41 insertions(+), 34 deletions(-)

Comments

Andrew Lunn Nov. 29, 2022, 6:45 p.m. UTC | #1 
On Tue, Nov 29, 2022 at 10:44:08AM +0530, Hariprasad Kelam wrote:
> The current implementation is such that FEC statistics are reported as
> part of ethtool statistics that is the user can fetch them from the
> below command
> 
> "ethtool -S eth0"
> 
> Fec Corrected Errors:
> Fec Uncorrected Errors:
> 
> This patch removes this logic

What might break the ABI, if anybody is using these statistics, or has
a dumb parser and just looks at entry 42 which is now a different
entry.

So i think you need to keep these two, and additionally report them
the correct way via get_fec_stats.

    Andrew
Hariprasad Kelam Nov. 30, 2022, 8:54 a.m. UTC | #2 
On Tue, Nov 29, 2022 at 10:44:08AM +0530, Hariprasad Kelam wrote:
> The current implementation is such that FEC statistics are reported as 
> part of ethtool statistics that is the user can fetch them from the 
> below command
> 
> "ethtool -S eth0"
> 
> Fec Corrected Errors:
> Fec Uncorrected Errors:
> 
> This patch removes this logic

What might break the ABI, if anybody is using these statistics, or has a dumb parser and just looks at entry 42 which is now a different entry.

So i think you need to keep these two, and additionally report them the correct way via get_fec_stats.

Make sense, that's a good point .
will wait for a day for other feedback and submit V3.

Thanks,
Hariprasad k

    Andrew
diff mbox series

Patch

diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_ethtool.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_ethtool.c
index 0eb74e8c553d..0dd7bce2b866 100644
--- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_ethtool.c
+++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_ethtool.c
@@ -135,10 +135,6 @@  static void otx2_get_strings(struct net_device *netdev, u32 sset, u8 *data)
 
 	strcpy(data, "reset_count");
 	data += ETH_GSTRING_LEN;
-	sprintf(data, "Fec Corrected Errors: ");
-	data += ETH_GSTRING_LEN;
-	sprintf(data, "Fec Uncorrected Errors: ");
-	data += ETH_GSTRING_LEN;
 }
 
 static void otx2_get_qset_stats(struct otx2_nic *pfvf,
@@ -193,8 +189,6 @@  static void otx2_get_ethtool_stats(struct net_device *netdev,
 				   struct ethtool_stats *stats, u64 *data)
 {
 	struct otx2_nic *pfvf = netdev_priv(netdev);
-	u64 fec_corr_blks, fec_uncorr_blks;
-	struct cgx_fw_data *rsp;
 	int stat;
 
 	otx2_get_dev_stats(pfvf);
@@ -217,32 +211,6 @@  static void otx2_get_ethtool_stats(struct net_device *netdev,
 	}
 
 	*(data++) = pfvf->reset_count;
-
-	fec_corr_blks = pfvf->hw.cgx_fec_corr_blks;
-	fec_uncorr_blks = pfvf->hw.cgx_fec_uncorr_blks;
-
-	rsp = otx2_get_fwdata(pfvf);
-	if (!IS_ERR(rsp) && rsp->fwdata.phy.misc.has_fec_stats &&
-	    !otx2_get_phy_fec_stats(pfvf)) {
-		/* Fetch fwdata again because it's been recently populated with
-		 * latest PHY FEC stats.
-		 */
-		rsp = otx2_get_fwdata(pfvf);
-		if (!IS_ERR(rsp)) {
-			struct fec_stats_s *p = &rsp->fwdata.phy.fec_stats;
-
-			if (pfvf->linfo.fec == OTX2_FEC_BASER) {
-				fec_corr_blks   = p->brfec_corr_blks;
-				fec_uncorr_blks = p->brfec_uncorr_blks;
-			} else {
-				fec_corr_blks   = p->rsfec_corr_cws;
-				fec_uncorr_blks = p->rsfec_uncorr_cws;
-			}
-		}
-	}
-
-	*(data++) = fec_corr_blks;
-	*(data++) = fec_uncorr_blks;
 }
 
 static int otx2_get_sset_count(struct net_device *netdev, int sset)
@@ -257,10 +225,9 @@  static int otx2_get_sset_count(struct net_device *netdev, int sset)
 		       (pfvf->hw.rx_queues + pfvf->hw.tx_queues);
 	if (!test_bit(CN10K_RPM, &pfvf->hw.cap_flag))
 		mac_stats = CGX_RX_STATS_COUNT + CGX_TX_STATS_COUNT;
-	otx2_update_lmac_fec_stats(pfvf);
 
 	return otx2_n_dev_stats + otx2_n_drv_stats + qstats_count +
-	       mac_stats + OTX2_FEC_STATS_CNT + 1;
+	       mac_stats + 1;
 }
 
 /* Get no of queues device supports and current queue count */
@@ -1268,6 +1235,45 @@  static int otx2_set_link_ksettings(struct net_device *netdev,
 	return err;
 }
 
+static void otx2_get_fec_stats(struct net_device *netdev,
+			       struct ethtool_fec_stats *fec_stats)
+{
+	struct otx2_nic *pfvf = netdev_priv(netdev);
+	struct cgx_fw_data *rsp;
+
+	otx2_update_lmac_fec_stats(pfvf);
+
+	rsp = otx2_get_fwdata(pfvf);
+	if (!IS_ERR(rsp) && rsp->fwdata.phy.misc.has_fec_stats &&
+	    !otx2_get_phy_fec_stats(pfvf)) {
+		/* Fetch fwdata again because it's been recently populated with
+		 * latest PHY FEC stats.
+		 */
+		rsp = otx2_get_fwdata(pfvf);
+		if (!IS_ERR(rsp)) {
+			struct fec_stats_s *p = &rsp->fwdata.phy.fec_stats;
+
+			if (pfvf->linfo.fec == OTX2_FEC_BASER) {
+				fec_stats->corrected_blocks.total     =
+					p->brfec_corr_blks;
+				fec_stats->uncorrectable_blocks.total =
+					p->brfec_uncorr_blks;
+			} else {
+				fec_stats->corrected_blocks.total     =
+					p->rsfec_corr_cws;
+				fec_stats->uncorrectable_blocks.total =
+					p->rsfec_uncorr_cws;
+			}
+		}
+	} else {
+		/* Report MAC FEC stats */
+		fec_stats->corrected_blocks.total     =
+			pfvf->hw.cgx_fec_corr_blks;
+		fec_stats->uncorrectable_blocks.total =
+			pfvf->hw.cgx_fec_uncorr_blks;
+	}
+}
+
 static const struct ethtool_ops otx2_ethtool_ops = {
 	.supported_coalesce_params = ETHTOOL_COALESCE_USECS |
 				     ETHTOOL_COALESCE_MAX_FRAMES |
@@ -1298,6 +1304,7 @@  static const struct ethtool_ops otx2_ethtool_ops = {
 	.get_pauseparam		= otx2_get_pauseparam,
 	.set_pauseparam		= otx2_set_pauseparam,
 	.get_ts_info		= otx2_get_ts_info,
+	.get_fec_stats		= otx2_get_fec_stats,
 	.get_fecparam		= otx2_get_fecparam,
 	.set_fecparam		= otx2_set_fecparam,
 	.get_link_ksettings     = otx2_get_link_ksettings,