diff mbox series

[RFC,net-next,3/5] net: dsa: microchip: add eth mac grouping for ethtool statistics

Message ID 20221130132902.2984580-4-rakesh.sankaranarayanan@microchip.com
State New
Headers
Series add ethtool categorized statistics |

Commit Message

Rakesh Sankaranarayanan Nov. 30, 2022, 1:29 p.m. UTC 
  Add support for ethtool standard device statistics grouping.
Support ethernet mac statistics grouping using eth-mac groups
parameter in ethtool command.

Signed-off-by: Rakesh Sankaranarayanan <rakesh.sankaranarayanan@microchip.com>
---
 drivers/net/dsa/microchip/ksz_common.c  | 13 ++++
 drivers/net/dsa/microchip/ksz_common.h  |  2 +
 drivers/net/dsa/microchip/ksz_ethtool.c | 88 +++++++++++++++++++++++++
 drivers/net/dsa/microchip/ksz_ethtool.h |  4 ++
 4 files changed, 107 insertions(+)

Comments

Jakub Kicinski Dec. 2, 2022, 4:02 a.m. UTC | #1 
On Wed, 30 Nov 2022 18:59:00 +0530 Rakesh Sankaranarayanan wrote:
> +	mac_stats->FramesTransmittedOK = ctr[ksz9477_tx_mcast] +
> +					 ctr[ksz9477_tx_bcast] +
> +					 ctr[ksz9477_tx_ucast] +
> +					 ctr[ksz9477_tx_pause];

do control frames count towards FramesTransmittedOK?
Please check the standard I don't recall.

> +	mac_stats->SingleCollisionFrames = ctr[ksz9477_tx_single_col];
> +	mac_stats->MultipleCollisionFrames = ctr[ksz9477_tx_mult_col];
> +	mac_stats->FramesReceivedOK = ctr[ksz9477_rx_mcast] +
> +				      ctr[ksz9477_rx_bcast] +
> +				      ctr[ksz9477_rx_ucast] +
> +				      ctr[ksz9477_rx_pause];
> +	mac_stats->FrameCheckSequenceErrors = ctr[ksz9477_rx_crc_err];
> +	mac_stats->AlignmentErrors = ctr[ksz9477_rx_align_err];
> +	mac_stats->OctetsTransmittedOK = ctr[ksz9477_tx_total_col];

OctetsTransmittedOK = ksz9477_tx_total_col[lisons] ?

> +	mac_stats->InRangeLengthErrors = ctr[ksz9477_rx_oversize];

You use the same counter for RMON oversize statistic, the two
definitely have different semantics, please check the standard
and the datasheet.

Remember that you don't have to fill in all the stats, if the HW does
not maintain a matching statistic - leave the field be. Kernel will
not report to user space unset fields.
Rakesh Sankaranarayanan Dec. 2, 2022, 11:53 a.m. UTC | #2 
Hi Jakub,

Thanks for the review comment.

On Thu, 2022-12-01 at 20:02 -0800, Jakub Kicinski wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you
> know the content is safe
> 
> On Wed, 30 Nov 2022 18:59:00 +0530 Rakesh Sankaranarayanan wrote:
> > +     mac_stats->FramesTransmittedOK = ctr[ksz9477_tx_mcast] +
> > +                                      ctr[ksz9477_tx_bcast] +
> > +                                      ctr[ksz9477_tx_ucast] +
> > +                                      ctr[ksz9477_tx_pause];
> 
> do control frames count towards FramesTransmittedOK?
> Please check the standard I don't recall.
> 
Yeah, I will check with the documentation.

> > +     mac_stats->SingleCollisionFrames =
> > ctr[ksz9477_tx_single_col];
> > +     mac_stats->MultipleCollisionFrames =
> > ctr[ksz9477_tx_mult_col];
> > +     mac_stats->FramesReceivedOK = ctr[ksz9477_rx_mcast] +
> > +                                   ctr[ksz9477_rx_bcast] +
> > +                                   ctr[ksz9477_rx_ucast] +
> > +                                   ctr[ksz9477_rx_pause];
> > +     mac_stats->FrameCheckSequenceErrors =
> > ctr[ksz9477_rx_crc_err];
> > +     mac_stats->AlignmentErrors = ctr[ksz9477_rx_align_err];
> > +     mac_stats->OctetsTransmittedOK = ctr[ksz9477_tx_total_col];
> 
> OctetsTransmittedOK = ksz9477_tx_total_col[lisons] ?
> 
Sorry about this. It should be ksz9477_tx_total. I will review all the
parameters again for avoiding such mistakes.

> > +     mac_stats->InRangeLengthErrors = ctr[ksz9477_rx_oversize];
> 
> You use the same counter for RMON oversize statistic, the two
> definitely have different semantics, please check the standard
> and the datasheet.
> 
> Remember that you don't have to fill in all the stats, if the HW does
> not maintain a matching statistic - leave the field be. Kernel will
> not report to user space unset fields.

Sure Jacub, I will review my code against documentation and submit the
updated revision.

Thanks,
Rakesh S
Vladimir Oltean Dec. 7, 2022, 1:21 p.m. UTC | #3 
On Fri, Dec 02, 2022 at 11:53:33AM +0000, Rakesh.Sankaranarayanan@microchip.com wrote:
> Hi Jakub,
> 
> Thanks for the review comment.
> 
> On Thu, 2022-12-01 at 20:02 -0800, Jakub Kicinski wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you
> > know the content is safe
> > 
> > On Wed, 30 Nov 2022 18:59:00 +0530 Rakesh Sankaranarayanan wrote:
> > > +     mac_stats->FramesTransmittedOK = ctr[ksz9477_tx_mcast] +
> > > +                                      ctr[ksz9477_tx_bcast] +
> > > +                                      ctr[ksz9477_tx_ucast] +
> > > +                                      ctr[ksz9477_tx_pause];
> > 
> > do control frames count towards FramesTransmittedOK?
> > Please check the standard I don't recall.
> > 
> Yeah, I will check with the documentation.

Oleksij said that the hardware counts pause frames for the byte
counters, so at least for consistency, they should be counted in frame
counters too.
https://patchwork.kernel.org/project/netdevbpf/patch/20221205052904.2834962-1-o.rempel@pengutronix.de/
diff mbox series

Patch

diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
index 9b13a38d553d..ceb3c4f120bd 100644
--- a/drivers/net/dsa/microchip/ksz_common.c
+++ b/drivers/net/dsa/microchip/ksz_common.c
@@ -160,6 +160,7 @@  static const struct ksz_dev_ops ksz8_dev_ops = {
 	.port_init_cnt = ksz8_port_init_cnt,
 	.get_rmon_stats = ksz8_get_rmon_stats,
 	.get_eth_ctrl_stats = ksz8_get_eth_ctrl_stats,
+	.get_eth_mac_stats = ksz8_get_eth_mac_stats,
 	.fdb_dump = ksz8_fdb_dump,
 	.mdb_add = ksz8_mdb_add,
 	.mdb_del = ksz8_mdb_del,
@@ -199,6 +200,7 @@  static const struct ksz_dev_ops ksz9477_dev_ops = {
 	.port_init_cnt = ksz9477_port_init_cnt,
 	.get_rmon_stats = ksz9477_get_rmon_stats,
 	.get_eth_ctrl_stats = ksz9477_get_eth_ctrl_stats,
+	.get_eth_mac_stats = ksz9477_get_eth_mac_stats,
 	.vlan_filtering = ksz9477_port_vlan_filtering,
 	.vlan_add = ksz9477_port_vlan_add,
 	.vlan_del = ksz9477_port_vlan_del,
@@ -237,6 +239,7 @@  static const struct ksz_dev_ops lan937x_dev_ops = {
 	.port_init_cnt = ksz9477_port_init_cnt,
 	.get_rmon_stats = ksz9477_get_rmon_stats,
 	.get_eth_ctrl_stats = ksz9477_get_eth_ctrl_stats,
+	.get_eth_mac_stats = ksz9477_get_eth_mac_stats,
 	.vlan_filtering = ksz9477_port_vlan_filtering,
 	.vlan_add = ksz9477_port_vlan_add,
 	.vlan_del = ksz9477_port_vlan_del,
@@ -1635,6 +1638,15 @@  static void ksz_get_eth_ctrl_stats(struct dsa_switch *ds, int port,
 		dev->dev_ops->get_eth_ctrl_stats(dev, port, ctrl_stats);
 }
 
+static void ksz_get_eth_mac_stats(struct dsa_switch *ds, int port,
+				  struct ethtool_eth_mac_stats *mac_stats)
+{
+	struct ksz_device *dev = ds->priv;
+
+	if (dev->dev_ops->get_eth_mac_stats)
+		dev->dev_ops->get_eth_mac_stats(dev, port, mac_stats);
+}
+
 static void ksz_get_strings(struct dsa_switch *ds, int port,
 			    u32 stringset, uint8_t *buf)
 {
@@ -2887,6 +2899,7 @@  static const struct dsa_switch_ops ksz_switch_ops = {
 	.get_pause_stats	= ksz_get_pause_stats,
 	.get_rmon_stats		= ksz_get_rmon_stats,
 	.get_eth_ctrl_stats	= ksz_get_eth_ctrl_stats,
+	.get_eth_mac_stats	= ksz_get_eth_mac_stats,
 	.port_change_mtu	= ksz_change_mtu,
 	.port_max_mtu		= ksz_max_mtu,
 };
diff --git a/drivers/net/dsa/microchip/ksz_common.h b/drivers/net/dsa/microchip/ksz_common.h
index 07627ff1a749..5b77f98483a9 100644
--- a/drivers/net/dsa/microchip/ksz_common.h
+++ b/drivers/net/dsa/microchip/ksz_common.h
@@ -344,6 +344,8 @@  struct ksz_dev_ops {
 			       const struct ethtool_rmon_hist_range **ranges);
 	void (*get_eth_ctrl_stats)(struct ksz_device *dev, int port,
 				   struct ethtool_eth_ctrl_stats *ctrl_stats);
+	void (*get_eth_mac_stats)(struct ksz_device *dev, int port,
+				  struct ethtool_eth_mac_stats *mac_stats);
 };
 
 struct ksz_device *ksz_switch_alloc(struct device *base, void *priv);
diff --git a/drivers/net/dsa/microchip/ksz_ethtool.c b/drivers/net/dsa/microchip/ksz_ethtool.c
index 4c9ca21e1806..96529fea8e84 100644
--- a/drivers/net/dsa/microchip/ksz_ethtool.c
+++ b/drivers/net/dsa/microchip/ksz_ethtool.c
@@ -159,6 +159,50 @@  void ksz8_get_eth_ctrl_stats(struct ksz_device *dev, int port,
 	mutex_unlock(&mib->cnt_mutex);
 }
 
+void ksz8_get_eth_mac_stats(struct ksz_device *dev, int port,
+			    struct ethtool_eth_mac_stats *mac_stats)
+{
+	struct ksz_port_mib *mib;
+	u64 *ctr;
+
+	mib = &dev->ports[port].mib;
+
+	mutex_lock(&mib->cnt_mutex);
+	ctr = mib->counters;
+
+	while (mib->cnt_ptr < dev->info->mib_cnt) {
+		dev->dev_ops->r_mib_pkt(dev, port, mib->cnt_ptr,
+			       NULL, &mib->counters[mib->cnt_ptr]);
+		++mib->cnt_ptr;
+	}
+
+	mac_stats->FramesTransmittedOK = ctr[ksz8_tx_mcast] +
+					 ctr[ksz8_tx_bcast] +
+					 ctr[ksz8_tx_ucast] +
+					 ctr[ksz8_tx_pause];
+	mac_stats->SingleCollisionFrames = ctr[ksz8_tx_single_col];
+	mac_stats->MultipleCollisionFrames = ctr[ksz8_tx_mult_col];
+	mac_stats->FramesReceivedOK = ctr[ksz8_rx_mcast] +
+				      ctr[ksz8_rx_bcast] +
+				      ctr[ksz8_rx_ucast] +
+				      ctr[ksz8_rx_pause];
+	mac_stats->FrameCheckSequenceErrors = ctr[ksz8_rx_crc_err];
+	mac_stats->AlignmentErrors = ctr[ksz8_rx_align_err];
+	mac_stats->OctetsTransmittedOK = ctr[ksz8_tx_total_col];
+	mac_stats->FramesWithDeferredXmissions = ctr[ksz8_tx_deferred];
+	mac_stats->LateCollisions = ctr[ksz8_tx_late_col];
+	mac_stats->FramesAbortedDueToXSColls = ctr[ksz8_tx_exc_col];
+	mac_stats->MulticastFramesXmittedOK = ctr[ksz8_tx_mcast];
+	mac_stats->BroadcastFramesXmittedOK = ctr[ksz8_tx_bcast];
+	mac_stats->MulticastFramesReceivedOK = ctr[ksz8_rx_mcast];
+	mac_stats->BroadcastFramesReceivedOK = ctr[ksz8_rx_bcast];
+	mac_stats->InRangeLengthErrors = ctr[ksz8_rx_oversize];
+
+	mib->cnt_ptr = 0;
+
+	mutex_unlock(&mib->cnt_mutex);
+}
+
 void ksz9477_get_rmon_stats(struct ksz_device *dev, int port,
 			    struct ethtool_rmon_stats *rmon_stats,
 			    const struct ethtool_rmon_hist_range **ranges)
@@ -218,3 +262,47 @@  void ksz9477_get_eth_ctrl_stats(struct ksz_device *dev, int port,
 
 	mutex_unlock(&mib->cnt_mutex);
 }
+
+void ksz9477_get_eth_mac_stats(struct ksz_device *dev, int port,
+			       struct ethtool_eth_mac_stats *mac_stats)
+{
+	struct ksz_port_mib *mib;
+	u64 *ctr;
+
+	mib = &dev->ports[port].mib;
+	ctr = mib->counters;
+
+	mutex_lock(&mib->cnt_mutex);
+
+	while (mib->cnt_ptr < dev->info->mib_cnt) {
+		dev->dev_ops->r_mib_pkt(dev, port, mib->cnt_ptr,
+			       NULL, &mib->counters[mib->cnt_ptr]);
+		++mib->cnt_ptr;
+	}
+
+	mac_stats->FramesTransmittedOK = ctr[ksz9477_tx_mcast] +
+					 ctr[ksz9477_tx_bcast] +
+					 ctr[ksz9477_tx_ucast] +
+					 ctr[ksz9477_tx_pause];
+	mac_stats->SingleCollisionFrames = ctr[ksz9477_tx_single_col];
+	mac_stats->MultipleCollisionFrames = ctr[ksz9477_tx_mult_col];
+	mac_stats->FramesReceivedOK = ctr[ksz9477_rx_mcast] +
+				      ctr[ksz9477_rx_bcast] +
+				      ctr[ksz9477_rx_ucast] +
+				      ctr[ksz9477_rx_pause];
+	mac_stats->FrameCheckSequenceErrors = ctr[ksz9477_rx_crc_err];
+	mac_stats->AlignmentErrors = ctr[ksz9477_rx_align_err];
+	mac_stats->OctetsTransmittedOK = ctr[ksz9477_tx_total_col];
+	mac_stats->FramesWithDeferredXmissions = ctr[ksz9477_tx_deferred];
+	mac_stats->LateCollisions = ctr[ksz9477_tx_late_col];
+	mac_stats->FramesAbortedDueToXSColls = ctr[ksz9477_tx_exc_col];
+	mac_stats->MulticastFramesXmittedOK = ctr[ksz9477_tx_mcast];
+	mac_stats->BroadcastFramesXmittedOK = ctr[ksz9477_tx_bcast];
+	mac_stats->MulticastFramesReceivedOK = ctr[ksz9477_rx_mcast];
+	mac_stats->BroadcastFramesReceivedOK = ctr[ksz9477_rx_bcast];
+	mac_stats->InRangeLengthErrors = ctr[ksz9477_rx_oversize];
+
+	mib->cnt_ptr = 0;
+
+	mutex_unlock(&mib->cnt_mutex);
+}
diff --git a/drivers/net/dsa/microchip/ksz_ethtool.h b/drivers/net/dsa/microchip/ksz_ethtool.h
index 18dc155d60b9..2dcfe8922b4e 100644
--- a/drivers/net/dsa/microchip/ksz_ethtool.h
+++ b/drivers/net/dsa/microchip/ksz_ethtool.h
@@ -13,10 +13,14 @@  void ksz8_get_rmon_stats(struct ksz_device *dev, int port,
 			 const struct ethtool_rmon_hist_range **ranges);
 void ksz8_get_eth_ctrl_stats(struct ksz_device *dev, int port,
 			     struct ethtool_eth_ctrl_stats *ctrl_stats);
+void ksz8_get_eth_mac_stats(struct ksz_device *dev, int port,
+			    struct ethtool_eth_mac_stats *mac_stats);
 
 void ksz9477_get_rmon_stats(struct ksz_device *dev, int port,
 			    struct ethtool_rmon_stats *rmon_stats,
 			    const struct ethtool_rmon_hist_range **ranges);
 void ksz9477_get_eth_ctrl_stats(struct ksz_device *dev, int port,
 				struct ethtool_eth_ctrl_stats *ctrl_stats);
+void ksz9477_get_eth_mac_stats(struct ksz_device *dev, int port,
+			       struct ethtool_eth_mac_stats *mac_stats);
 #endif