diff mbox series

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

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

Commit Message

Rakesh Sankaranarayanan Feb. 17, 2023, 11:02 a.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 | 90 +++++++++++++++++++++++++
 drivers/net/dsa/microchip/ksz_ethtool.h |  4 ++
 4 files changed, 109 insertions(+)

Comments

Alexander Lobakin Feb. 17, 2023, 3:01 p.m. UTC | #1 
From: Rakesh Sankaranarayanan <rakesh.sankaranarayanan@microchip.com>
Date: Fri, 17 Feb 2023 16:32:09 +0530

>     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>

[...]

> +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]);

(for example here)

> +		++mib->cnt_ptr;

Reason for the pre-increment? :)

> +	}
> +
> +	mac_stats->FramesTransmittedOK = ctr[KSZ8_TX_MCAST] +
> +					 ctr[KSZ8_TX_BCAST] +
> +					 ctr[KSZ8_TX_UCAST] +
> +					 ctr[KSZ8_TX_PAUSE] -
> +					 ctr[KSZ8_TX_DISCARDS];
[...]

Thanks,
Olek
Vladimir Oltean Feb. 17, 2023, 4:42 p.m. UTC | #2 
On Fri, Feb 17, 2023 at 04:01:15PM +0100, Alexander Lobakin wrote:
> > +		++mib->cnt_ptr;
> 
> Reason for the pre-increment? :)

because it's kool

Somebody not that long ago suggested that pre-increment is "less resource consuming":
https://patchwork.kernel.org/project/netdevbpf/patch/677a5e37aab97a4f992d35c41329733c5f3082fb.1675407169.git.daniel@makrotopia.org/#25197216

Of course, when pressed to explain more, he stopped responding.
Alexander Lobakin Feb. 24, 2023, 4:07 p.m. UTC | #3 
From: Vladimir Oltean <olteanv@gmail.com>
Date: Fri, 17 Feb 2023 18:42:27 +0200

> On Fri, Feb 17, 2023 at 04:01:15PM +0100, Alexander Lobakin wrote:
>>> +		++mib->cnt_ptr;
>>
>> Reason for the pre-increment? :)
> 
> because it's kool

:D The most common reason for it usually ._.

> 
> Somebody not that long ago suggested that pre-increment is "less resource consuming":
> https://patchwork.kernel.org/project/netdevbpf/patch/677a5e37aab97a4f992d35c41329733c5f3082fb.1675407169.git.daniel@makrotopia.org/#25197216
> 
> Of course, when pressed to explain more, he stopped responding.

Haha, classics.
It's not so common for people to show up back in the thread after you
ask them to show godbolt / asm code comparison.

Olek
Vladimir Oltean Feb. 24, 2023, 9:53 p.m. UTC | #4 
On Fri, Feb 24, 2023 at 05:07:01PM +0100, Alexander Lobakin wrote:
> It's not so common for people to show up back in the thread after you
> ask them to show godbolt / asm code comparison.

idk what godbolt is, but if it's some sort of online compiler, then I
suppose it's of limited usefulness for the Linux kernel.

Easiest way to see a disassembly (also has C code interleaved) would be
this:

make drivers/net/dsa/microchip/ksz_ethtool.lst

This is also useful to see precisely which instruction went boom in case
there's a NULL pointer dereference or something like that.
Alexander Lobakin Feb. 27, 2023, 2:31 p.m. UTC | #5 
From: Vladimir Oltean <olteanv@gmail.com>
Date: Fri, 24 Feb 2023 23:53:49 +0200

> On Fri, Feb 24, 2023 at 05:07:01PM +0100, Alexander Lobakin wrote:
>> It's not so common for people to show up back in the thread after you
>> ask them to show godbolt / asm code comparison.
> 
> idk what godbolt is, but if it's some sort of online compiler, then I
> suppose it's of limited usefulness for the Linux kernel.

Yeah it's an online compiler, which can spit out assembly code. For
testing non-complex stuff or macros it's often enough.

> 
> Easiest way to see a disassembly (also has C code interleaved) would be
> this:
> 
> make drivers/net/dsa/microchip/ksz_ethtool.lst

Oh, nice! I didn't know Kbuild has capability of listing the assembly
code built-in. I was adding it manually to Makefiles when needed >_<
Thanks! :D

> 
> This is also useful to see precisely which instruction went boom in case
> there's a NULL pointer dereference or something like that.

Thanks,
Olek
Andrew Lunn Feb. 27, 2023, 2:52 p.m. UTC | #6 
> > Easiest way to see a disassembly (also has C code interleaved) would be
> > this:
> > 
> > make drivers/net/dsa/microchip/ksz_ethtool.lst
> 
> Oh, nice! I didn't know Kbuild has capability of listing the assembly
> code built-in. I was adding it manually to Makefiles when needed >_<
> Thanks! :D

You can also do

make drivers/net/dsa/microchip/ksz_ethtool.o
make drivers/net/dsa/microchip/ksz_ethtool.S

etc to get any of the intermediary files from the build process.

Also

make drivers/net/dsa/microchip/

will build everything in that subdirectory and below. That can be much
faster, especially when you have an allmodconf configuration and it
needs to check 1000s of modules before getting around to building the
one module you just changed. FYI: the trailing / is important.

       Andrew
Alexander Lobakin Feb. 28, 2023, 10:53 a.m. UTC | #7 
From: Andrew Lunn <andrew@lunn.ch>
Date: Mon, 27 Feb 2023 15:52:09 +0100

>>> Easiest way to see a disassembly (also has C code interleaved) would be
>>> this:
>>>
>>> make drivers/net/dsa/microchip/ksz_ethtool.lst
>>
>> Oh, nice! I didn't know Kbuild has capability of listing the assembly
>> code built-in. I was adding it manually to Makefiles when needed >_<
>> Thanks! :D
> 
> You can also do
> 
> make drivers/net/dsa/microchip/ksz_ethtool.o
> make drivers/net/dsa/microchip/ksz_ethtool.S
> 
> etc to get any of the intermediary files from the build process.
> 
> Also
> 
> make drivers/net/dsa/microchip/

I was aware of this and .o, but didn't know about .lst and .S, hehe.
Yeah, this helps a lot. Sometimes I do

make W=1 <folder/file to recheck>

so that if a driver builds cleanly before my changes, it should do so
after them as well. `make W=1` for the whole kernel is still noisy
currently.

> 
> will build everything in that subdirectory and below. That can be much
> faster, especially when you have an allmodconf configuration and it
> needs to check 1000s of modules before getting around to building the
> one module you just changed. FYI: the trailing / is important.
> 
>        Andrew

Thanks,
Olek
diff mbox series

Patch

diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
index 91fc7eed79f0..e4a51f13afa4 100644
--- a/drivers/net/dsa/microchip/ksz_common.c
+++ b/drivers/net/dsa/microchip/ksz_common.c
@@ -207,6 +207,7 @@  static const struct ksz_dev_ops ksz8_dev_ops = {
 	.fdb_dump = ksz8_fdb_dump,
 	.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,
 	.mdb_add = ksz8_mdb_add,
 	.mdb_del = ksz8_mdb_del,
 	.vlan_filtering = ksz8_port_vlan_filtering,
@@ -246,6 +247,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,
@@ -284,6 +286,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,
@@ -1756,6 +1759,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)
 {
@@ -3214,6 +3226,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,
 	.get_ts_info		= ksz_get_ts_info,
diff --git a/drivers/net/dsa/microchip/ksz_common.h b/drivers/net/dsa/microchip/ksz_common.h
index 7b0219947c7a..738e81923c31 100644
--- a/drivers/net/dsa/microchip/ksz_common.h
+++ b/drivers/net/dsa/microchip/ksz_common.h
@@ -366,6 +366,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 122c4371810a..42954bbfb9b4 100644
--- a/drivers/net/dsa/microchip/ksz_ethtool.c
+++ b/drivers/net/dsa/microchip/ksz_ethtool.c
@@ -160,6 +160,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] -
+					 ctr[KSZ8_TX_DISCARDS];
+	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] +
+				      ctr[KSZ8_RX_DISCARDS];
+	mac_stats->FrameCheckSequenceErrors = ctr[KSZ8_RX_CRC_ERR];
+	mac_stats->AlignmentErrors = ctr[KSZ8_RX_ALIGN_ERR];
+	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];
+
+	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)
@@ -220,3 +264,49 @@  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] -
+					 ctr[KSZ9477_TX_DISCARDS];
+	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] +
+				      ctr[KSZ9477_RX_DISCARDS];
+	mac_stats->FrameCheckSequenceErrors = ctr[KSZ9477_RX_CRC_ERR];
+	mac_stats->AlignmentErrors = ctr[KSZ9477_RX_ALIGN_ERR];
+	mac_stats->OctetsTransmittedOK = ctr[KSZ9477_TX_TOTAL] -
+					 (18 * mac_stats->FramesTransmittedOK);
+	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];
+
+	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