2023-02-17 11:02:25

by Rakesh Sankaranarayanan

[permalink] [raw]
Subject: [PATCH v2 net-next 0/5] add ethtool categorized statistics

Patch series contain following changes:
- add categorized ethtool statistics for Microchip KSZ series switches,
support "eth-mac", "eth-phy", "eth-ctrl", "rmon" parameters with
ethtool statistics command. mib parameter index are same for all
KSZ family switches except KSZ8830. So, functions can be re-used
across all KSZ Families (except KSZ8830) and LAN937x series. Create
separate functions for KSZ8830 with their mib parameters.
- Remove num_alus member from ksz_chip_data structure since it is unused

v2
- updated all constants as capital
- removed counters that are not supported in hardware
- updated the FramesTransmittedOK and OctetsTransmittedOK counters as
per standards

v1
- Initial submission

Rakesh Sankaranarayanan (5):
net: dsa: microchip: add rmon grouping for ethtool statistics
net: dsa: microchip: add eth ctrl grouping for ethtool statistics
net: dsa: microchip: add eth mac grouping for ethtool statistics
net: dsa: microchip: add eth phy grouping for ethtool statistics
net: dsa: microchip: remove num_alus_variable

drivers/net/dsa/microchip/Makefile | 1 +
drivers/net/dsa/microchip/ksz_common.c | 70 +++--
drivers/net/dsa/microchip/ksz_common.h | 10 +-
drivers/net/dsa/microchip/ksz_ethtool.c | 348 ++++++++++++++++++++++++
drivers/net/dsa/microchip/ksz_ethtool.h | 31 +++
5 files changed, 443 insertions(+), 17 deletions(-)
create mode 100644 drivers/net/dsa/microchip/ksz_ethtool.c
create mode 100644 drivers/net/dsa/microchip/ksz_ethtool.h

--
2.34.1



2023-02-17 11:02:57

by Rakesh Sankaranarayanan

[permalink] [raw]
Subject: [PATCH v2 net-next 1/5] net: dsa: microchip: add rmon grouping for ethtool statistics

Add support for ethtool standard device statistics grouping. Support rmon
statistics grouping using rmon groups parameter in ethtool command. rmon
provides packet size based range grouping. Common mib parameters are used
across all KSZ series swtches for packet size statistics, except for
KSZ8830. KSZ series have mib counters for packets with size:
- less than 64 Bytes,
- 65 to 127 Bytes,
- 128 to 255 Bytes,
- 256 to 511 Bytes,
- 512 to 1023 Bytes,
- 1024 to 1522 Bytes,
- 1523 to 2000 Bytes and
- More than 2001 Bytes
KSZ8830 have mib counters upto 1024-1522 range only. Since no other change,
common range used across all KSZ series, but used upto only upto 1024-1522
for KSZ8830.

Co-developed-by: Thangaraj Samynathan <[email protected]>
Signed-off-by: Rakesh Sankaranarayanan <[email protected]>
---
drivers/net/dsa/microchip/Makefile | 1 +
drivers/net/dsa/microchip/ksz_common.c | 15 ++
drivers/net/dsa/microchip/ksz_common.h | 3 +
drivers/net/dsa/microchip/ksz_ethtool.c | 180 ++++++++++++++++++++++++
drivers/net/dsa/microchip/ksz_ethtool.h | 18 +++
5 files changed, 217 insertions(+)
create mode 100644 drivers/net/dsa/microchip/ksz_ethtool.c
create mode 100644 drivers/net/dsa/microchip/ksz_ethtool.h

diff --git a/drivers/net/dsa/microchip/Makefile b/drivers/net/dsa/microchip/Makefile
index 48360cc9fc68..2b698ac39a1f 100644
--- a/drivers/net/dsa/microchip/Makefile
+++ b/drivers/net/dsa/microchip/Makefile
@@ -1,6 +1,7 @@
# SPDX-License-Identifier: GPL-2.0-only
obj-$(CONFIG_NET_DSA_MICROCHIP_KSZ_COMMON) += ksz_switch.o
ksz_switch-objs := ksz_common.o
+ksz_switch-objs += ksz_ethtool.o
ksz_switch-objs += ksz9477.o
ksz_switch-objs += ksz8795.o
ksz_switch-objs += lan937x_main.o
diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
index 729b36eeb2c4..61f4e23d8849 100644
--- a/drivers/net/dsa/microchip/ksz_common.c
+++ b/drivers/net/dsa/microchip/ksz_common.c
@@ -27,6 +27,7 @@
#include <net/switchdev.h>

#include "ksz_common.h"
+#include "ksz_ethtool.h"
#include "ksz_ptp.h"
#include "ksz8.h"
#include "ksz9477.h"
@@ -204,6 +205,7 @@ static const struct ksz_dev_ops ksz8_dev_ops = {
.freeze_mib = ksz8_freeze_mib,
.port_init_cnt = ksz8_port_init_cnt,
.fdb_dump = ksz8_fdb_dump,
+ .get_rmon_stats = ksz8_get_rmon_stats,
.mdb_add = ksz8_mdb_add,
.mdb_del = ksz8_mdb_del,
.vlan_filtering = ksz8_port_vlan_filtering,
@@ -241,6 +243,7 @@ static const struct ksz_dev_ops ksz9477_dev_ops = {
.r_mib_stat64 = ksz_r_mib_stats64,
.freeze_mib = ksz9477_freeze_mib,
.port_init_cnt = ksz9477_port_init_cnt,
+ .get_rmon_stats = ksz9477_get_rmon_stats,
.vlan_filtering = ksz9477_port_vlan_filtering,
.vlan_add = ksz9477_port_vlan_add,
.vlan_del = ksz9477_port_vlan_del,
@@ -277,6 +280,7 @@ static const struct ksz_dev_ops lan937x_dev_ops = {
.r_mib_stat64 = ksz_r_mib_stats64,
.freeze_mib = ksz9477_freeze_mib,
.port_init_cnt = ksz9477_port_init_cnt,
+ .get_rmon_stats = ksz9477_get_rmon_stats,
.vlan_filtering = ksz9477_port_vlan_filtering,
.vlan_add = ksz9477_port_vlan_add,
.vlan_del = ksz9477_port_vlan_del,
@@ -1730,6 +1734,16 @@ static void ksz_get_pause_stats(struct dsa_switch *ds, int port,
spin_unlock(&mib->stats64_lock);
}

+static void ksz_get_rmon_stats(struct dsa_switch *ds, int port,
+ struct ethtool_rmon_stats *rmon_stats,
+ const struct ethtool_rmon_hist_range **ranges)
+{
+ struct ksz_device *dev = ds->priv;
+
+ if (dev->dev_ops->get_rmon_stats)
+ dev->dev_ops->get_rmon_stats(dev, port, rmon_stats, ranges);
+}
+
static void ksz_get_strings(struct dsa_switch *ds, int port,
u32 stringset, uint8_t *buf)
{
@@ -3186,6 +3200,7 @@ static const struct dsa_switch_ops ksz_switch_ops = {
.port_mirror_del = ksz_port_mirror_del,
.get_stats64 = ksz_get_stats64,
.get_pause_stats = ksz_get_pause_stats,
+ .get_rmon_stats = ksz_get_rmon_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 d2d5761d58e9..a4e53431218c 100644
--- a/drivers/net/dsa/microchip/ksz_common.h
+++ b/drivers/net/dsa/microchip/ksz_common.h
@@ -361,6 +361,9 @@ struct ksz_dev_ops {
int (*reset)(struct ksz_device *dev);
int (*init)(struct ksz_device *dev);
void (*exit)(struct ksz_device *dev);
+ void (*get_rmon_stats)(struct ksz_device *dev, int port,
+ struct ethtool_rmon_stats *rmon_stats,
+ const struct ethtool_rmon_hist_range **ranges);
};

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
new file mode 100644
index 000000000000..0f3f18545858
--- /dev/null
+++ b/drivers/net/dsa/microchip/ksz_ethtool.c
@@ -0,0 +1,180 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Microchip KSZ series ethtool statistics
+ *
+ * Copyright (C) 2022 Microchip Technology Inc.
+ */
+
+#include "ksz_common.h"
+#include "ksz_ethtool.h"
+
+enum ksz8_mib_entry {
+ KSZ8_RX,
+ KSZ8_RX_HI,
+ KSZ8_RX_UNDERSIZE,
+ KSZ8_RX_FRAGMENTS,
+ KSZ8_RX_OVERSIZE,
+ KSZ8_RX_JABBERS,
+ KSZ8_RX_SYMBOL_ERR,
+ KSZ8_RX_CRC_ERR,
+ KSZ8_RX_ALIGN_ERR,
+ KSZ8_RX_MAC_CTL,
+ KSZ8_RX_PAUSE,
+ KSZ8_RX_BCAST,
+ KSZ8_RX_MCAST,
+ KSZ8_RX_UCAST,
+ KSZ8_RX_64_OR_LESS,
+ KSZ8_RX_65_127,
+ KSZ8_RX_128_255,
+ KSZ8_RX_256_511,
+ KSZ8_RX_512_1023,
+ KSZ8_RX_1024_1522,
+ KSZ8_TX,
+ KSZ8_TX_HI,
+ KSZ8_TX_LATE_COL,
+ KSZ8_TX_PAUSE,
+ KSZ8_TX_BCAST,
+ KSZ8_TX_MCAST,
+ KSZ8_TX_UCAST,
+ KSZ8_TX_DEFERRED,
+ KSZ8_TX_TOTAL_COL,
+ KSZ8_TX_EXC_COL,
+ KSZ8_TX_SINGLE_COL,
+ KSZ8_TX_MULT_COL,
+ KSZ8_RX_DISCARDS,
+ KSZ8_TX_DISCARDS,
+};
+
+enum ksz9477_mib_entry {
+ KSZ9477_RX_HI,
+ KSZ9477_RX_UNDERSIZE,
+ KSZ9477_RX_FRAGMENTS,
+ KSZ9477_RX_OVERSIZE,
+ KSZ9477_RX_JABBERS,
+ KSZ9477_RX_SYMBOL_ERR,
+ KSZ9477_RX_CRC_ERR,
+ KSZ9477_RX_ALIGN_ERR,
+ KSZ9477_RX_MAC_CTL,
+ KSZ9477_RX_PAUSE,
+ KSZ9477_RX_BCAST,
+ KSZ9477_RX_MCAST,
+ KSZ9477_RX_UCAST,
+ KSZ9477_RX_64_OR_LESS,
+ KSZ9477_RX_65_127,
+ KSZ9477_RX_128_255,
+ KSZ9477_RX_256_511,
+ KSZ9477_RX_512_1023,
+ KSZ9477_RX_1024_1522,
+ KSZ9477_RX_1523_2000,
+ KSZ9477_RX_2001,
+ KSZ9477_TX_HI,
+ KSZ9477_TX_LATE_COL,
+ KSZ9477_TX_PAUSE,
+ KSZ9477_TX_BCAST,
+ KSZ9477_TX_MCAST,
+ KSZ9477_TX_UCAST,
+ KSZ9477_TX_DEFERRED,
+ KSZ9477_TX_TOTAL_COL,
+ KSZ9477_TX_EXC_COL,
+ KSZ9477_TX_SINGLE_COL,
+ KSZ9477_TX_MULT_COL,
+ KSZ9477_RX_TOTAL,
+ KSZ9477_TX_TOTAL,
+ KSZ9477_RX_DISCARDS,
+ KSZ9477_TX_DISCARDS,
+};
+
+static const struct ethtool_rmon_hist_range ksz_rmon_ranges[] = {
+ { 0, 64 },
+ { 65, 127 },
+ { 128, 255 },
+ { 256, 511 },
+ { 512, 1023 },
+ { 1024, 1522 },
+ { 1523, 2000 },
+ { 2001, 9000 },
+ {}
+};
+
+#define KSZ8_HIST_LEN 6
+#define KSZ9477_HIST_LEN 8
+
+void ksz8_get_rmon_stats(struct ksz_device *dev, int port,
+ struct ethtool_rmon_stats *rmon_stats,
+ const struct ethtool_rmon_hist_range **ranges)
+{
+ struct ksz_port_mib *mib;
+ u64 *cnt;
+ u8 i;
+
+ mib = &dev->ports[port].mib;
+
+ mutex_lock(&mib->cnt_mutex);
+
+ cnt = &mib->counters[KSZ8_RX_UNDERSIZE];
+ dev->dev_ops->r_mib_pkt(dev, port, KSZ8_RX_UNDERSIZE, NULL, cnt);
+ rmon_stats->undersize_pkts = *cnt;
+
+ cnt = &mib->counters[KSZ8_RX_OVERSIZE];
+ dev->dev_ops->r_mib_pkt(dev, port, KSZ8_RX_OVERSIZE, NULL, cnt);
+ rmon_stats->oversize_pkts = *cnt;
+
+ cnt = &mib->counters[KSZ8_RX_FRAGMENTS];
+ dev->dev_ops->r_mib_pkt(dev, port, KSZ8_RX_FRAGMENTS, NULL, cnt);
+ rmon_stats->fragments = *cnt;
+
+ cnt = &mib->counters[KSZ8_RX_JABBERS];
+ dev->dev_ops->r_mib_pkt(dev, port, KSZ8_RX_JABBERS, NULL, cnt);
+ rmon_stats->jabbers = *cnt;
+
+ for (i = 0; i < KSZ8_HIST_LEN; i++) {
+ cnt = &mib->counters[KSZ8_RX_64_OR_LESS + i];
+ dev->dev_ops->r_mib_pkt(dev, port,
+ (KSZ8_RX_64_OR_LESS + i), NULL, cnt);
+ rmon_stats->hist[i] = *cnt;
+ }
+
+ mutex_unlock(&mib->cnt_mutex);
+
+ *ranges = ksz_rmon_ranges;
+}
+
+void ksz9477_get_rmon_stats(struct ksz_device *dev, int port,
+ struct ethtool_rmon_stats *rmon_stats,
+ const struct ethtool_rmon_hist_range **ranges)
+{
+ struct ksz_port_mib *mib;
+ u64 *cnt;
+ u8 i;
+
+ mib = &dev->ports[port].mib;
+
+ mutex_lock(&mib->cnt_mutex);
+
+ cnt = &mib->counters[KSZ9477_RX_UNDERSIZE];
+ dev->dev_ops->r_mib_pkt(dev, port, KSZ9477_RX_UNDERSIZE, NULL, cnt);
+ rmon_stats->undersize_pkts = *cnt;
+
+ cnt = &mib->counters[KSZ9477_RX_OVERSIZE];
+ dev->dev_ops->r_mib_pkt(dev, port, KSZ9477_RX_OVERSIZE, NULL, cnt);
+ rmon_stats->oversize_pkts = *cnt;
+
+ cnt = &mib->counters[KSZ9477_RX_FRAGMENTS];
+ dev->dev_ops->r_mib_pkt(dev, port, KSZ9477_RX_FRAGMENTS, NULL, cnt);
+ rmon_stats->fragments = *cnt;
+
+ cnt = &mib->counters[KSZ9477_RX_JABBERS];
+ dev->dev_ops->r_mib_pkt(dev, port, KSZ9477_RX_JABBERS, NULL, cnt);
+ rmon_stats->jabbers = *cnt;
+
+ for (i = 0; i < KSZ9477_HIST_LEN; i++) {
+ cnt = &mib->counters[KSZ9477_RX_64_OR_LESS + i];
+ dev->dev_ops->r_mib_pkt(dev, port,
+ (KSZ9477_RX_64_OR_LESS + i), NULL, cnt);
+ rmon_stats->hist[i] = *cnt;
+ }
+
+ mutex_unlock(&mib->cnt_mutex);
+
+ *ranges = ksz_rmon_ranges;
+}
diff --git a/drivers/net/dsa/microchip/ksz_ethtool.h b/drivers/net/dsa/microchip/ksz_ethtool.h
new file mode 100644
index 000000000000..6927e2f143f8
--- /dev/null
+++ b/drivers/net/dsa/microchip/ksz_ethtool.h
@@ -0,0 +1,18 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Microchip KSZ series ethtool statistics
+ *
+ * Copyright (C) 2022 Microchip Technology Inc.
+ */
+
+#ifndef __KSZ_ETHTOOL_H
+#define __KSZ_ETHTOOL_H
+
+void ksz8_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_rmon_stats(struct ksz_device *dev, int port,
+ struct ethtool_rmon_stats *rmon_stats,
+ const struct ethtool_rmon_hist_range **ranges);
+#endif
--
2.34.1


2023-02-17 11:02:57

by Rakesh Sankaranarayanan

[permalink] [raw]
Subject: [PATCH v2 net-next 2/5] net: dsa: microchip: add eth ctrl grouping for ethtool statistics

Add support for ethtool standard device statistics grouping. Support
ethernet mac ctrl statistics grouping using eth-ctrl groups parameter
in ethtool command.

Signed-off-by: Rakesh Sankaranarayanan <[email protected]>
---
drivers/net/dsa/microchip/ksz_common.c | 13 ++++++++
drivers/net/dsa/microchip/ksz_common.h | 2 ++
drivers/net/dsa/microchip/ksz_ethtool.c | 42 +++++++++++++++++++++++++
drivers/net/dsa/microchip/ksz_ethtool.h | 4 +++
4 files changed, 61 insertions(+)

diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
index 61f4e23d8849..91fc7eed79f0 100644
--- a/drivers/net/dsa/microchip/ksz_common.c
+++ b/drivers/net/dsa/microchip/ksz_common.c
@@ -206,6 +206,7 @@ static const struct ksz_dev_ops ksz8_dev_ops = {
.port_init_cnt = ksz8_port_init_cnt,
.fdb_dump = ksz8_fdb_dump,
.get_rmon_stats = ksz8_get_rmon_stats,
+ .get_eth_ctrl_stats = ksz8_get_eth_ctrl_stats,
.mdb_add = ksz8_mdb_add,
.mdb_del = ksz8_mdb_del,
.vlan_filtering = ksz8_port_vlan_filtering,
@@ -244,6 +245,7 @@ static const struct ksz_dev_ops ksz9477_dev_ops = {
.freeze_mib = ksz9477_freeze_mib,
.port_init_cnt = ksz9477_port_init_cnt,
.get_rmon_stats = ksz9477_get_rmon_stats,
+ .get_eth_ctrl_stats = ksz9477_get_eth_ctrl_stats,
.vlan_filtering = ksz9477_port_vlan_filtering,
.vlan_add = ksz9477_port_vlan_add,
.vlan_del = ksz9477_port_vlan_del,
@@ -281,6 +283,7 @@ static const struct ksz_dev_ops lan937x_dev_ops = {
.freeze_mib = ksz9477_freeze_mib,
.port_init_cnt = ksz9477_port_init_cnt,
.get_rmon_stats = ksz9477_get_rmon_stats,
+ .get_eth_ctrl_stats = ksz9477_get_eth_ctrl_stats,
.vlan_filtering = ksz9477_port_vlan_filtering,
.vlan_add = ksz9477_port_vlan_add,
.vlan_del = ksz9477_port_vlan_del,
@@ -1744,6 +1747,15 @@ static void ksz_get_rmon_stats(struct dsa_switch *ds, int port,
dev->dev_ops->get_rmon_stats(dev, port, rmon_stats, ranges);
}

+static void ksz_get_eth_ctrl_stats(struct dsa_switch *ds, int port,
+ struct ethtool_eth_ctrl_stats *ctrl_stats)
+{
+ struct ksz_device *dev = ds->priv;
+
+ if (dev->dev_ops->get_eth_ctrl_stats)
+ dev->dev_ops->get_eth_ctrl_stats(dev, port, ctrl_stats);
+}
+
static void ksz_get_strings(struct dsa_switch *ds, int port,
u32 stringset, uint8_t *buf)
{
@@ -3201,6 +3213,7 @@ static const struct dsa_switch_ops ksz_switch_ops = {
.get_stats64 = ksz_get_stats64,
.get_pause_stats = ksz_get_pause_stats,
.get_rmon_stats = ksz_get_rmon_stats,
+ .get_eth_ctrl_stats = ksz_get_eth_ctrl_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 a4e53431218c..7b0219947c7a 100644
--- a/drivers/net/dsa/microchip/ksz_common.h
+++ b/drivers/net/dsa/microchip/ksz_common.h
@@ -364,6 +364,8 @@ struct ksz_dev_ops {
void (*get_rmon_stats)(struct ksz_device *dev, int port,
struct ethtool_rmon_stats *rmon_stats,
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);
};

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 0f3f18545858..122c4371810a 100644
--- a/drivers/net/dsa/microchip/ksz_ethtool.c
+++ b/drivers/net/dsa/microchip/ksz_ethtool.c
@@ -139,6 +139,27 @@ void ksz8_get_rmon_stats(struct ksz_device *dev, int port,
*ranges = ksz_rmon_ranges;
}

+void ksz8_get_eth_ctrl_stats(struct ksz_device *dev, int port,
+ struct ethtool_eth_ctrl_stats *ctrl_stats)
+{
+ struct ksz_port_mib *mib;
+ u64 *cnt;
+
+ mib = &dev->ports[port].mib;
+
+ mutex_lock(&mib->cnt_mutex);
+
+ cnt = &mib->counters[KSZ8_TX_PAUSE];
+ dev->dev_ops->r_mib_pkt(dev, port, KSZ8_TX_PAUSE, NULL, cnt);
+ ctrl_stats->MACControlFramesTransmitted = *cnt;
+
+ cnt = &mib->counters[KSZ8_RX_PAUSE];
+ dev->dev_ops->r_mib_pkt(dev, port, KSZ8_RX_PAUSE, NULL, cnt);
+ ctrl_stats->MACControlFramesReceived = *cnt;
+
+ 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)
@@ -178,3 +199,24 @@ void ksz9477_get_rmon_stats(struct ksz_device *dev, int port,

*ranges = ksz_rmon_ranges;
}
+
+void ksz9477_get_eth_ctrl_stats(struct ksz_device *dev, int port,
+ struct ethtool_eth_ctrl_stats *ctrl_stats)
+{
+ struct ksz_port_mib *mib;
+ u64 *cnt;
+
+ mib = &dev->ports[port].mib;
+
+ mutex_lock(&mib->cnt_mutex);
+
+ cnt = &mib->counters[KSZ9477_TX_PAUSE];
+ dev->dev_ops->r_mib_pkt(dev, port, KSZ9477_TX_PAUSE, NULL, cnt);
+ ctrl_stats->MACControlFramesTransmitted = *cnt;
+
+ cnt = &mib->counters[KSZ9477_RX_PAUSE];
+ dev->dev_ops->r_mib_pkt(dev, port, KSZ9477_RX_PAUSE, NULL, cnt);
+ ctrl_stats->MACControlFramesReceived = *cnt;
+
+ mutex_unlock(&mib->cnt_mutex);
+}
diff --git a/drivers/net/dsa/microchip/ksz_ethtool.h b/drivers/net/dsa/microchip/ksz_ethtool.h
index 6927e2f143f8..18dc155d60b9 100644
--- a/drivers/net/dsa/microchip/ksz_ethtool.h
+++ b/drivers/net/dsa/microchip/ksz_ethtool.h
@@ -11,8 +11,12 @@
void ksz8_get_rmon_stats(struct ksz_device *dev, int port,
struct ethtool_rmon_stats *rmon_stats,
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 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);
#endif
--
2.34.1


2023-02-17 11:02:57

by Rakesh Sankaranarayanan

[permalink] [raw]
Subject: [PATCH v2 net-next 3/5] net: dsa: microchip: add eth mac grouping for ethtool statistics

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 <[email protected]>
---
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(+)

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


2023-02-17 11:02:58

by Rakesh Sankaranarayanan

[permalink] [raw]
Subject: [PATCH v2 net-next 4/5] net: dsa: microchip: add eth phy grouping for ethtool statistics

Add support for ethtool standard device statistics grouping. Support
ethernet phy statistics grouping using eth-phy groups parameter in
ethtool command.

Signed-off-by: Rakesh Sankaranarayanan <[email protected]>
---
drivers/net/dsa/microchip/ksz_common.c | 13 +++++++++
drivers/net/dsa/microchip/ksz_common.h | 2 ++
drivers/net/dsa/microchip/ksz_ethtool.c | 36 +++++++++++++++++++++++++
drivers/net/dsa/microchip/ksz_ethtool.h | 5 ++++
4 files changed, 56 insertions(+)

diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
index e4a51f13afa4..01adcbeffaaa 100644
--- a/drivers/net/dsa/microchip/ksz_common.c
+++ b/drivers/net/dsa/microchip/ksz_common.c
@@ -208,6 +208,7 @@ static const struct ksz_dev_ops ksz8_dev_ops = {
.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,
+ .get_eth_phy_stats = ksz8_get_eth_phy_stats,
.mdb_add = ksz8_mdb_add,
.mdb_del = ksz8_mdb_del,
.vlan_filtering = ksz8_port_vlan_filtering,
@@ -248,6 +249,7 @@ static const struct ksz_dev_ops ksz9477_dev_ops = {
.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,
+ .get_eth_phy_stats = ksz9477_get_eth_phy_stats,
.vlan_filtering = ksz9477_port_vlan_filtering,
.vlan_add = ksz9477_port_vlan_add,
.vlan_del = ksz9477_port_vlan_del,
@@ -287,6 +289,7 @@ static const struct ksz_dev_ops lan937x_dev_ops = {
.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,
+ .get_eth_phy_stats = ksz9477_get_eth_phy_stats,
.vlan_filtering = ksz9477_port_vlan_filtering,
.vlan_add = ksz9477_port_vlan_add,
.vlan_del = ksz9477_port_vlan_del,
@@ -1768,6 +1771,15 @@ static void ksz_get_eth_mac_stats(struct dsa_switch *ds, int port,
dev->dev_ops->get_eth_mac_stats(dev, port, mac_stats);
}

+static void ksz_get_eth_phy_stats(struct dsa_switch *ds, int port,
+ struct ethtool_eth_phy_stats *phy_stats)
+{
+ struct ksz_device *dev = ds->priv;
+
+ if (dev->dev_ops->get_eth_phy_stats)
+ dev->dev_ops->get_eth_phy_stats(dev, port, phy_stats);
+}
+
static void ksz_get_strings(struct dsa_switch *ds, int port,
u32 stringset, uint8_t *buf)
{
@@ -3227,6 +3239,7 @@ static const struct dsa_switch_ops ksz_switch_ops = {
.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,
+ .get_eth_phy_stats = ksz_get_eth_phy_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 738e81923c31..8a71e035b699 100644
--- a/drivers/net/dsa/microchip/ksz_common.h
+++ b/drivers/net/dsa/microchip/ksz_common.h
@@ -368,6 +368,8 @@ struct ksz_dev_ops {
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);
+ void (*get_eth_phy_stats)(struct ksz_device *dev, int port,
+ struct ethtool_eth_phy_stats *phy_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 42954bbfb9b4..c0b95d78e41e 100644
--- a/drivers/net/dsa/microchip/ksz_ethtool.c
+++ b/drivers/net/dsa/microchip/ksz_ethtool.c
@@ -204,6 +204,24 @@ void ksz8_get_eth_mac_stats(struct ksz_device *dev, int port,
mutex_unlock(&mib->cnt_mutex);
}

+void ksz8_get_eth_phy_stats(struct ksz_device *dev, int port,
+ struct ethtool_eth_phy_stats *phy_stats)
+{
+ struct ksz_port_mib *mib;
+ u64 *cnt;
+
+ mib = &dev->ports[port].mib;
+
+ mutex_lock(&mib->cnt_mutex);
+
+ cnt = &mib->counters[KSZ8_RX_SYMBOL_ERR];
+ dev->dev_ops->r_mib_pkt(dev, port, KSZ8_RX_SYMBOL_ERR, NULL, cnt);
+
+ phy_stats->SymbolErrorDuringCarrier = *cnt;
+
+ 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)
@@ -310,3 +328,21 @@ void ksz9477_get_eth_mac_stats(struct ksz_device *dev, int port,

mutex_unlock(&mib->cnt_mutex);
}
+
+void ksz9477_get_eth_phy_stats(struct ksz_device *dev, int port,
+ struct ethtool_eth_phy_stats *phy_stats)
+{
+ struct ksz_port_mib *mib;
+ u64 *cnt;
+
+ mib = &dev->ports[port].mib;
+
+ mutex_lock(&mib->cnt_mutex);
+
+ cnt = &mib->counters[KSZ9477_RX_SYMBOL_ERR];
+ dev->dev_ops->r_mib_pkt(dev, port, KSZ9477_RX_SYMBOL_ERR, NULL, cnt);
+
+ phy_stats->SymbolErrorDuringCarrier = *cnt;
+
+ mutex_unlock(&mib->cnt_mutex);
+}
diff --git a/drivers/net/dsa/microchip/ksz_ethtool.h b/drivers/net/dsa/microchip/ksz_ethtool.h
index 2dcfe8922b4e..042a0b38a899 100644
--- a/drivers/net/dsa/microchip/ksz_ethtool.h
+++ b/drivers/net/dsa/microchip/ksz_ethtool.h
@@ -15,6 +15,8 @@ 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 ksz8_get_eth_phy_stats(struct ksz_device *dev, int port,
+ struct ethtool_eth_phy_stats *phy_stats);

void ksz9477_get_rmon_stats(struct ksz_device *dev, int port,
struct ethtool_rmon_stats *rmon_stats,
@@ -23,4 +25,7 @@ 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);
+void ksz9477_get_eth_phy_stats(struct ksz_device *dev, int port,
+ struct ethtool_eth_phy_stats *phy_stats);
+
#endif
--
2.34.1


2023-02-17 11:02:58

by Rakesh Sankaranarayanan

[permalink] [raw]
Subject: [PATCH v2 net-next 5/5] net: dsa: microchip: remove num_alus_variable

Remove num_alus variable from ksz_chip_data structure since
it is unused now.

Signed-off-by: Rakesh Sankaranarayanan <[email protected]>
---
drivers/net/dsa/microchip/ksz_common.c | 16 ----------------
drivers/net/dsa/microchip/ksz_common.h | 1 -
2 files changed, 17 deletions(-)

diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
index 01adcbeffaaa..152f68eda355 100644
--- a/drivers/net/dsa/microchip/ksz_common.c
+++ b/drivers/net/dsa/microchip/ksz_common.c
@@ -1095,7 +1095,6 @@ const struct ksz_chip_data ksz_switch_chips[] = {
.chip_id = KSZ8563_CHIP_ID,
.dev_name = "KSZ8563",
.num_vlans = 4096,
- .num_alus = 4096,
.num_statics = 16,
.cpu_ports = 0x07, /* can be configured as cpu port */
.port_cnt = 3, /* total port count */
@@ -1124,7 +1123,6 @@ const struct ksz_chip_data ksz_switch_chips[] = {
.chip_id = KSZ8795_CHIP_ID,
.dev_name = "KSZ8795",
.num_vlans = 4096,
- .num_alus = 0,
.num_statics = 8,
.cpu_ports = 0x10, /* can be configured as cpu port */
.port_cnt = 5, /* total cpu and user ports */
@@ -1163,7 +1161,6 @@ const struct ksz_chip_data ksz_switch_chips[] = {
.chip_id = KSZ8794_CHIP_ID,
.dev_name = "KSZ8794",
.num_vlans = 4096,
- .num_alus = 0,
.num_statics = 8,
.cpu_ports = 0x10, /* can be configured as cpu port */
.port_cnt = 5, /* total cpu and user ports */
@@ -1188,7 +1185,6 @@ const struct ksz_chip_data ksz_switch_chips[] = {
.chip_id = KSZ8765_CHIP_ID,
.dev_name = "KSZ8765",
.num_vlans = 4096,
- .num_alus = 0,
.num_statics = 8,
.cpu_ports = 0x10, /* can be configured as cpu port */
.port_cnt = 5, /* total cpu and user ports */
@@ -1213,7 +1209,6 @@ const struct ksz_chip_data ksz_switch_chips[] = {
.chip_id = KSZ8830_CHIP_ID,
.dev_name = "KSZ8863/KSZ8873",
.num_vlans = 16,
- .num_alus = 0,
.num_statics = 8,
.cpu_ports = 0x4, /* can be configured as cpu port */
.port_cnt = 3,
@@ -1234,7 +1229,6 @@ const struct ksz_chip_data ksz_switch_chips[] = {
.chip_id = KSZ9477_CHIP_ID,
.dev_name = "KSZ9477",
.num_vlans = 4096,
- .num_alus = 4096,
.num_statics = 16,
.cpu_ports = 0x7F, /* can be configured as cpu port */
.port_cnt = 7, /* total physical port count */
@@ -1268,7 +1262,6 @@ const struct ksz_chip_data ksz_switch_chips[] = {
.chip_id = KSZ9896_CHIP_ID,
.dev_name = "KSZ9896",
.num_vlans = 4096,
- .num_alus = 4096,
.num_statics = 16,
.cpu_ports = 0x3F, /* can be configured as cpu port */
.port_cnt = 6, /* total physical port count */
@@ -1301,7 +1294,6 @@ const struct ksz_chip_data ksz_switch_chips[] = {
.chip_id = KSZ9897_CHIP_ID,
.dev_name = "KSZ9897",
.num_vlans = 4096,
- .num_alus = 4096,
.num_statics = 16,
.cpu_ports = 0x7F, /* can be configured as cpu port */
.port_cnt = 7, /* total physical port count */
@@ -1332,7 +1324,6 @@ const struct ksz_chip_data ksz_switch_chips[] = {
.chip_id = KSZ9893_CHIP_ID,
.dev_name = "KSZ9893",
.num_vlans = 4096,
- .num_alus = 4096,
.num_statics = 16,
.cpu_ports = 0x07, /* can be configured as cpu port */
.port_cnt = 3, /* total port count */
@@ -1358,7 +1349,6 @@ const struct ksz_chip_data ksz_switch_chips[] = {
.chip_id = KSZ9563_CHIP_ID,
.dev_name = "KSZ9563",
.num_vlans = 4096,
- .num_alus = 4096,
.num_statics = 16,
.cpu_ports = 0x07, /* can be configured as cpu port */
.port_cnt = 3, /* total port count */
@@ -1385,7 +1375,6 @@ const struct ksz_chip_data ksz_switch_chips[] = {
.chip_id = KSZ9567_CHIP_ID,
.dev_name = "KSZ9567",
.num_vlans = 4096,
- .num_alus = 4096,
.num_statics = 16,
.cpu_ports = 0x7F, /* can be configured as cpu port */
.port_cnt = 7, /* total physical port count */
@@ -1417,7 +1406,6 @@ const struct ksz_chip_data ksz_switch_chips[] = {
.chip_id = LAN9370_CHIP_ID,
.dev_name = "LAN9370",
.num_vlans = 4096,
- .num_alus = 1024,
.num_statics = 256,
.cpu_ports = 0x10, /* can be configured as cpu port */
.port_cnt = 5, /* total physical port count */
@@ -1443,7 +1431,6 @@ const struct ksz_chip_data ksz_switch_chips[] = {
.chip_id = LAN9371_CHIP_ID,
.dev_name = "LAN9371",
.num_vlans = 4096,
- .num_alus = 1024,
.num_statics = 256,
.cpu_ports = 0x30, /* can be configured as cpu port */
.port_cnt = 6, /* total physical port count */
@@ -1469,7 +1456,6 @@ const struct ksz_chip_data ksz_switch_chips[] = {
.chip_id = LAN9372_CHIP_ID,
.dev_name = "LAN9372",
.num_vlans = 4096,
- .num_alus = 1024,
.num_statics = 256,
.cpu_ports = 0x30, /* can be configured as cpu port */
.port_cnt = 8, /* total physical port count */
@@ -1499,7 +1485,6 @@ const struct ksz_chip_data ksz_switch_chips[] = {
.chip_id = LAN9373_CHIP_ID,
.dev_name = "LAN9373",
.num_vlans = 4096,
- .num_alus = 1024,
.num_statics = 256,
.cpu_ports = 0x38, /* can be configured as cpu port */
.port_cnt = 5, /* total physical port count */
@@ -1529,7 +1514,6 @@ const struct ksz_chip_data ksz_switch_chips[] = {
.chip_id = LAN9374_CHIP_ID,
.dev_name = "LAN9374",
.num_vlans = 4096,
- .num_alus = 1024,
.num_statics = 256,
.cpu_ports = 0x30, /* can be configured as cpu port */
.port_cnt = 8, /* total physical port count */
diff --git a/drivers/net/dsa/microchip/ksz_common.h b/drivers/net/dsa/microchip/ksz_common.h
index 8a71e035b699..40c4f5f2d9d5 100644
--- a/drivers/net/dsa/microchip/ksz_common.h
+++ b/drivers/net/dsa/microchip/ksz_common.h
@@ -44,7 +44,6 @@ struct ksz_chip_data {
u32 chip_id;
const char *dev_name;
int num_vlans;
- int num_alus;
int num_statics;
int cpu_ports;
int port_cnt;
--
2.34.1


2023-02-17 14:57:31

by Alexander Lobakin

[permalink] [raw]
Subject: Re: [PATCH v2 net-next 0/5] add ethtool categorized statistics

From: Rakesh Sankaranarayanan <[email protected]>
Date: Fri, 17 Feb 2023 16:32:06 +0530

> [PATCH v2 net-next 0/5] add ethtool categorized statistics

I'd like to see the cover letter's subject prefixed as well, e.g.

[PATCH v2 net-next 0/5] net: dsa: microchip: add ethtool categorized
statistics

...or so, depending on the usual prefix for ksz.
Otherwise, it looks like you're adding something generic and only
realize it targets a particular driver only after opening the thread itself.

> Patch series contain following changes:
> - add categorized ethtool statistics for Microchip KSZ series switches,
> support "eth-mac", "eth-phy", "eth-ctrl", "rmon" parameters with
> ethtool statistics command. mib parameter index are same for all
> KSZ family switches except KSZ8830. So, functions can be re-used
> across all KSZ Families (except KSZ8830) and LAN937x series. Create
> separate functions for KSZ8830 with their mib parameters.
> - Remove num_alus member from ksz_chip_data structure since it is unused
>
> v2
> - updated all constants as capital
> - removed counters that are not supported in hardware
> - updated the FramesTransmittedOK and OctetsTransmittedOK counters as
> per standards
>
> v1
> - Initial submission
>
> Rakesh Sankaranarayanan (5):
> net: dsa: microchip: add rmon grouping for ethtool statistics
> net: dsa: microchip: add eth ctrl grouping for ethtool statistics
> net: dsa: microchip: add eth mac grouping for ethtool statistics
> net: dsa: microchip: add eth phy grouping for ethtool statistics
> net: dsa: microchip: remove num_alus_variable
>
> drivers/net/dsa/microchip/Makefile | 1 +
> drivers/net/dsa/microchip/ksz_common.c | 70 +++--
> drivers/net/dsa/microchip/ksz_common.h | 10 +-
> drivers/net/dsa/microchip/ksz_ethtool.c | 348 ++++++++++++++++++++++++
> drivers/net/dsa/microchip/ksz_ethtool.h | 31 +++
> 5 files changed, 443 insertions(+), 17 deletions(-)
> create mode 100644 drivers/net/dsa/microchip/ksz_ethtool.c
> create mode 100644 drivers/net/dsa/microchip/ksz_ethtool.h
>
Thanks,
Olek

2023-02-17 15:03:17

by Alexander Lobakin

[permalink] [raw]
Subject: Re: [PATCH v2 net-next 1/5] net: dsa: microchip: add rmon grouping for ethtool statistics

From: Rakesh Sankaranarayanan <[email protected]>
Date: Fri, 17 Feb 2023 16:32:07 +0530

> Add support for ethtool standard device statistics grouping. Support rmon
> statistics grouping using rmon groups parameter in ethtool command. rmon
> provides packet size based range grouping. Common mib parameters are used
> across all KSZ series swtches for packet size statistics, except for
> KSZ8830. KSZ series have mib counters for packets with size:

[...]

> +void ksz8_get_rmon_stats(struct ksz_device *dev, int port,
> + struct ethtool_rmon_stats *rmon_stats,
> + const struct ethtool_rmon_hist_range **ranges)
> +{
> + struct ksz_port_mib *mib;
> + u64 *cnt;

Nit: I guess it can be const since you only read it (in every such
callback)?

> + u8 i;
> +
> + mib = &dev->ports[port].mib;
> +
> + mutex_lock(&mib->cnt_mutex);
> +
> + cnt = &mib->counters[KSZ8_RX_UNDERSIZE];
> + dev->dev_ops->r_mib_pkt(dev, port, KSZ8_RX_UNDERSIZE, NULL, cnt);
> + rmon_stats->undersize_pkts = *cnt;
> +
> + cnt = &mib->counters[KSZ8_RX_OVERSIZE];
> + dev->dev_ops->r_mib_pkt(dev, port, KSZ8_RX_OVERSIZE, NULL, cnt);
> + rmon_stats->oversize_pkts = *cnt;
> +
> + cnt = &mib->counters[KSZ8_RX_FRAGMENTS];
> + dev->dev_ops->r_mib_pkt(dev, port, KSZ8_RX_FRAGMENTS, NULL, cnt);
> + rmon_stats->fragments = *cnt;
> +
> + cnt = &mib->counters[KSZ8_RX_JABBERS];
> + dev->dev_ops->r_mib_pkt(dev, port, KSZ8_RX_JABBERS, NULL, cnt);
> + rmon_stats->jabbers = *cnt;
> +
> + for (i = 0; i < KSZ8_HIST_LEN; i++) {
> + cnt = &mib->counters[KSZ8_RX_64_OR_LESS + i];
> + dev->dev_ops->r_mib_pkt(dev, port,
> + (KSZ8_RX_64_OR_LESS + i), NULL, cnt);

Weird linewrap. Please align the following lines with the opening brace
of the first one, e.g.

dev->dev_ops->r_mib_pkt(dev, port,
KSZ8_RX_64_OR_LESS + i, NULL,
cnt);

BUT I don't see why you need those braces around `macro + i` and without
them you can fit it into the previous line I believe.

> + rmon_stats->hist[i] = *cnt;
> + }
> +
> + mutex_unlock(&mib->cnt_mutex);
> +
> + *ranges = ksz_rmon_ranges;
> +}
> +
> +void ksz9477_get_rmon_stats(struct ksz_device *dev, int port,
> + struct ethtool_rmon_stats *rmon_stats,
> + const struct ethtool_rmon_hist_range **ranges)
> +{
> + struct ksz_port_mib *mib;
> + u64 *cnt;
> + u8 i;
> +
> + mib = &dev->ports[port].mib;
> +
> + mutex_lock(&mib->cnt_mutex);
> +
> + cnt = &mib->counters[KSZ9477_RX_UNDERSIZE];
> + dev->dev_ops->r_mib_pkt(dev, port, KSZ9477_RX_UNDERSIZE, NULL, cnt);
> + rmon_stats->undersize_pkts = *cnt;
> +
> + cnt = &mib->counters[KSZ9477_RX_OVERSIZE];
> + dev->dev_ops->r_mib_pkt(dev, port, KSZ9477_RX_OVERSIZE, NULL, cnt);
> + rmon_stats->oversize_pkts = *cnt;
> +
> + cnt = &mib->counters[KSZ9477_RX_FRAGMENTS];
> + dev->dev_ops->r_mib_pkt(dev, port, KSZ9477_RX_FRAGMENTS, NULL, cnt);
> + rmon_stats->fragments = *cnt;
> +
> + cnt = &mib->counters[KSZ9477_RX_JABBERS];
> + dev->dev_ops->r_mib_pkt(dev, port, KSZ9477_RX_JABBERS, NULL, cnt);
> + rmon_stats->jabbers = *cnt;
> +
> + for (i = 0; i < KSZ9477_HIST_LEN; i++) {
> + cnt = &mib->counters[KSZ9477_RX_64_OR_LESS + i];
> + dev->dev_ops->r_mib_pkt(dev, port,
> + (KSZ9477_RX_64_OR_LESS + i), NULL, cnt);

(same, and please check all other places in the series)

> + rmon_stats->hist[i] = *cnt;
> + }
> +
> + mutex_unlock(&mib->cnt_mutex);
> +
> + *ranges = ksz_rmon_ranges;
> +}
[...]

Thanks,
Olek

2023-02-17 15:03:46

by Alexander Lobakin

[permalink] [raw]
Subject: Re: [PATCH v2 net-next 3/5] net: dsa: microchip: add eth mac grouping for ethtool statistics

From: Rakesh Sankaranarayanan <[email protected]>
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 <[email protected]>

[...]

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

2023-02-17 16:42:51

by Vladimir Oltean

[permalink] [raw]
Subject: Re: [PATCH v2 net-next 3/5] net: dsa: microchip: add eth mac grouping for ethtool statistics

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.

2023-02-17 16:48:59

by Vladimir Oltean

[permalink] [raw]
Subject: Re: [PATCH v2 net-next 0/5] add ethtool categorized statistics

On Fri, Feb 17, 2023 at 03:54:48PM +0100, Alexander Lobakin wrote:
> From: Rakesh Sankaranarayanan <[email protected]>
> Date: Fri, 17 Feb 2023 16:32:06 +0530
>
> > [PATCH v2 net-next 0/5] add ethtool categorized statistics
>
> I'd like to see the cover letter's subject prefixed as well, e.g.
>
> [PATCH v2 net-next 0/5] net: dsa: microchip: add ethtool categorized
> statistics
>
> ...or so, depending on the usual prefix for ksz.
> Otherwise, it looks like you're adding something generic and only
> realize it targets a particular driver only after opening the thread itself.

+1

some people just look at the KSZ DSA driver all day, and so in their mind, it
then becomes implicit in the subject. But the cover letter description gets
turned by the netdev maintainers into a branch name for a merge commit, see:

fa15072b650a Merge branch 'sfc-devlink-support-for-ef100'
e9ab2559e2c5 Merge branch 'net-sched-transition-actions-to-pcpu-stats-and-rcu'
10d13421a6ae Merge branch 'net-core-commmon-prints-for-promisc'
a1d83abc8f2f Merge branch 'net-sched-retire-some-tc-qdiscs-and-classifiers'

and so, the naming of the cover letter has non-zero importance.

I agree that the contents of this patch set is absolutely disappointing
for someone reading the title and expecting some new ethtool counters.

2023-02-17 16:53:57

by Vladimir Oltean

[permalink] [raw]
Subject: Re: [PATCH v2 net-next 1/5] net: dsa: microchip: add rmon grouping for ethtool statistics

On Fri, Feb 17, 2023 at 04:32:07PM +0530, Rakesh Sankaranarayanan wrote:
> Add support for ethtool standard device statistics grouping. Support rmon
> statistics grouping using rmon groups parameter in ethtool command. rmon
> provides packet size based range grouping. Common mib parameters are used
> across all KSZ series swtches for packet size statistics, except for
> KSZ8830. KSZ series have mib counters for packets with size:
> - less than 64 Bytes,
> - 65 to 127 Bytes,
> - 128 to 255 Bytes,
> - 256 to 511 Bytes,
> - 512 to 1023 Bytes,
> - 1024 to 1522 Bytes,
> - 1523 to 2000 Bytes and
> - More than 2001 Bytes
> KSZ8830 have mib counters upto 1024-1522 range only. Since no other change,
> common range used across all KSZ series, but used upto only upto 1024-1522
> for KSZ8830.

Why are all commit messages indented in this way? Please keep the
default text indentation at 0 characters. I have never seen this style
in "git log".

>
> Co-developed-by: Thangaraj Samynathan <[email protected]>

Documentation/process/submitting-patches.rst:

Co-developed-by: states that the patch was co-created by multiple developers;
it is used to give attribution to co-authors (in addition to the author
attributed by the From: tag) when several people work on a single patch. Since
Co-developed-by: denotes authorship, every Co-developed-by: must be immediately
followed by a Signed-off-by: of the associated co-author. Standard sign-off
procedure applies, i.e. the ordering of Signed-off-by: tags should reflect the
chronological history of the patch insofar as possible, regardless of whether
the author is attributed via From: or Co-developed-by:. Notably, the last
Signed-off-by: must always be that of the developer submitting the patch.

2023-02-17 17:08:34

by Vladimir Oltean

[permalink] [raw]
Subject: Re: [PATCH v2 net-next 2/5] net: dsa: microchip: add eth ctrl grouping for ethtool statistics

On Fri, Feb 17, 2023 at 04:32:08PM +0530, Rakesh Sankaranarayanan wrote:
> +void ksz8_get_eth_ctrl_stats(struct ksz_device *dev, int port,
> + struct ethtool_eth_ctrl_stats *ctrl_stats)
> +{
> + struct ksz_port_mib *mib;
> + u64 *cnt;
> +
> + mib = &dev->ports[port].mib;
> +
> + mutex_lock(&mib->cnt_mutex);
> +
> + cnt = &mib->counters[KSZ8_TX_PAUSE];
> + dev->dev_ops->r_mib_pkt(dev, port, KSZ8_TX_PAUSE, NULL, cnt);
> + ctrl_stats->MACControlFramesTransmitted = *cnt;
> +
> + cnt = &mib->counters[KSZ8_RX_PAUSE];
> + dev->dev_ops->r_mib_pkt(dev, port, KSZ8_RX_PAUSE, NULL, cnt);
> + ctrl_stats->MACControlFramesReceived = *cnt;
> +
> + mutex_unlock(&mib->cnt_mutex);
> +}

These should be reported as standard pause stats as well (ethtool -I --show-pause swpN).

2023-02-17 17:16:51

by Vladimir Oltean

[permalink] [raw]
Subject: Re: [PATCH v2 net-next 5/5] net: dsa: microchip: remove num_alus_variable

On Fri, Feb 17, 2023 at 04:32:11PM +0530, Rakesh Sankaranarayanan wrote:
> Remove num_alus variable from ksz_chip_data structure since
> it is unused now.

now=since when?

"git log -Gnum_alus drivers/net/dsa/microchip/" says it was never used.

2023-02-17 17:40:15

by Andrew Lunn

[permalink] [raw]
Subject: Re: [PATCH v2 net-next 1/5] net: dsa: microchip: add rmon grouping for ethtool statistics

On Fri, Feb 17, 2023 at 06:53:46PM +0200, Vladimir Oltean wrote:
> On Fri, Feb 17, 2023 at 04:32:07PM +0530, Rakesh Sankaranarayanan wrote:
> > Add support for ethtool standard device statistics grouping. Support rmon
> > statistics grouping using rmon groups parameter in ethtool command. rmon
> > provides packet size based range grouping. Common mib parameters are used
> > across all KSZ series swtches for packet size statistics, except for
> > KSZ8830. KSZ series have mib counters for packets with size:
> > - less than 64 Bytes,
> > - 65 to 127 Bytes,
> > - 128 to 255 Bytes,
> > - 256 to 511 Bytes,
> > - 512 to 1023 Bytes,
> > - 1024 to 1522 Bytes,
> > - 1523 to 2000 Bytes and
> > - More than 2001 Bytes
> > KSZ8830 have mib counters upto 1024-1522 range only. Since no other change,
> > common range used across all KSZ series, but used upto only upto 1024-1522
> > for KSZ8830.
>
> Why are all commit messages indented in this way? Please keep the
> default text indentation at 0 characters. I have never seen this style
> in "git log".

I can make a guess

git show HEAD

notice how the commit message is indented. So if you where to
cut/paste that, you get the extra indent. It is better to do:

git commit -C 8e757b50555f3ae ; git commit --am

to copy a commit message from 8e757b50555f3ae, and then edit it to
suite.

Andrew

2023-02-21 04:38:07

by Rakesh Sankaranarayanan

[permalink] [raw]
Subject: Re: [PATCH v2 net-next 1/5] net: dsa: microchip: add rmon grouping for ethtool statistics

On Fri, 2023-02-17 at 15:59 +0100, Alexander Lobakin wrote:
> [Some people who received this message don't often get email from
> [email protected]. Learn why this is important at
> https://aka.ms/LearnAboutSenderIdentification ]
>
> EXTERNAL EMAIL: Do not click links or open attachments unless you
> know the content is safe
>
> From: Rakesh Sankaranarayanan <[email protected]>
> Date: Fri, 17 Feb 2023 16:32:07 +0530
>
> >     Add support for ethtool standard device statistics grouping.
> > Support rmon
> >     statistics grouping using rmon groups parameter in ethtool
> > command. rmon
> >     provides packet size based range grouping. Common mib
> > parameters are used
> >     across all KSZ series swtches for packet size statistics,
> > except for
> >     KSZ8830. KSZ series have mib counters for packets with size:
>
> [...]
>
> > +void ksz8_get_rmon_stats(struct ksz_device *dev, int port,
> > +                      struct ethtool_rmon_stats *rmon_stats,
> > +                      const struct ethtool_rmon_hist_range
> > **ranges)
> > +{
> > +     struct ksz_port_mib *mib;
> > +     u64 *cnt;
>
> Nit: I guess it can be const since you only read it (in every such
> callback)?
This variable is getting updated on every call back after reading from
registers, so we cannot have it as constant.

>
> > +     u8 i;
> > +
> > +     mib = &dev->ports[port].mib;
> > +
> > +     mutex_lock(&mib->cnt_mutex);
> > +
> > +     cnt = &mib->counters[KSZ8_RX_UNDERSIZE];
> > +     dev->dev_ops->r_mib_pkt(dev, port, KSZ8_RX_UNDERSIZE, NULL,
> > cnt);
> > +     rmon_stats->undersize_pkts = *cnt;
> > +
> > +     cnt = &mib->counters[KSZ8_RX_OVERSIZE];
> > +     dev->dev_ops->r_mib_pkt(dev, port, KSZ8_RX_OVERSIZE, NULL,
> > cnt);
> > +     rmon_stats->oversize_pkts = *cnt;
> > +
> > +     cnt = &mib->counters[KSZ8_RX_FRAGMENTS];
> > +     dev->dev_ops->r_mib_pkt(dev, port, KSZ8_RX_FRAGMENTS, NULL,
> > cnt);
> > +     rmon_stats->fragments = *cnt;
> > +
> > +     cnt = &mib->counters[KSZ8_RX_JABBERS];
> > +     dev->dev_ops->r_mib_pkt(dev, port, KSZ8_RX_JABBERS, NULL,
> > cnt);
> > +     rmon_stats->jabbers = *cnt;
> > +
> > +     for (i = 0; i < KSZ8_HIST_LEN; i++) {
> > +             cnt = &mib->counters[KSZ8_RX_64_OR_LESS + i];
> > +             dev->dev_ops->r_mib_pkt(dev, port,
> > +                             (KSZ8_RX_64_OR_LESS + i), NULL, cnt);
>
> Weird linewrap. Please align the following lines with the opening
> brace
> of the first one, e.g.
>
>                 dev->dev_ops->r_mib_pkt(dev, port,
>                                         KSZ8_RX_64_OR_LESS + i, NULL,
>                                         cnt);
>
> BUT I don't see why you need those braces around `macro + i` and
> without
> them you can fit it into the previous line I believe.
Will update it in next revision
>
> > +             rmon_stats->hist[i] = *cnt;
> > +     }
> > +
> > +     mutex_unlock(&mib->cnt_mutex);
> > +
> > +     *ranges = ksz_rmon_ranges;
> > +}
> > +
> > +void ksz9477_get_rmon_stats(struct ksz_device *dev, int port,
> > +                         struct ethtool_rmon_stats *rmon_stats,
> > +                         const struct ethtool_rmon_hist_range
> > **ranges)
> > +{
> > +     struct ksz_port_mib *mib;
> > +     u64 *cnt;
> > +     u8 i;
> > +
> > +     mib = &dev->ports[port].mib;
> > +
> > +     mutex_lock(&mib->cnt_mutex);
> > +
> > +     cnt = &mib->counters[KSZ9477_RX_UNDERSIZE];
> > +     dev->dev_ops->r_mib_pkt(dev, port, KSZ9477_RX_UNDERSIZE,
> > NULL, cnt);
> > +     rmon_stats->undersize_pkts = *cnt;
> > +
> > +     cnt = &mib->counters[KSZ9477_RX_OVERSIZE];
> > +     dev->dev_ops->r_mib_pkt(dev, port, KSZ9477_RX_OVERSIZE, NULL,
> > cnt);
> > +     rmon_stats->oversize_pkts = *cnt;
> > +
> > +     cnt = &mib->counters[KSZ9477_RX_FRAGMENTS];
> > +     dev->dev_ops->r_mib_pkt(dev, port, KSZ9477_RX_FRAGMENTS,
> > NULL, cnt);
> > +     rmon_stats->fragments = *cnt;
> > +
> > +     cnt = &mib->counters[KSZ9477_RX_JABBERS];
> > +     dev->dev_ops->r_mib_pkt(dev, port, KSZ9477_RX_JABBERS, NULL,
> > cnt);
> > +     rmon_stats->jabbers = *cnt;
> > +
> > +     for (i = 0; i < KSZ9477_HIST_LEN; i++) {
> > +             cnt = &mib->counters[KSZ9477_RX_64_OR_LESS + i];
> > +             dev->dev_ops->r_mib_pkt(dev, port,
> > +                             (KSZ9477_RX_64_OR_LESS + i), NULL,
> > cnt);
>
> (same, and please check all other places in the series)
sure, will update in next version.
>
> > +             rmon_stats->hist[i] = *cnt;
> > +     }
> > +
> > +     mutex_unlock(&mib->cnt_mutex);
> > +
> > +     *ranges = ksz_rmon_ranges;
> > +}
> [...]
>
> Thanks,
> Olek

2023-02-21 04:40:47

by Rakesh Sankaranarayanan

[permalink] [raw]
Subject: Re: [PATCH v2 net-next 1/5] net: dsa: microchip: add rmon grouping for ethtool statistics

Hi Vlad,
On Fri, 2023-02-17 at 18:53 +0200, Vladimir Oltean wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you
> know the content is safe
>
> On Fri, Feb 17, 2023 at 04:32:07PM +0530, Rakesh Sankaranarayanan
> wrote:
> >     Add support for ethtool standard device statistics grouping.
> > Support rmon
> >     statistics grouping using rmon groups parameter in ethtool
> > command. rmon
> >     provides packet size based range grouping. Common mib
> > parameters are used
> >     across all KSZ series swtches for packet size statistics,
> > except for
> >     KSZ8830. KSZ series have mib counters for packets with size:
> >     - less than 64 Bytes,
> >     - 65 to 127 Bytes,
> >     - 128 to 255 Bytes,
> >     - 256 to 511 Bytes,
> >     - 512 to 1023 Bytes,
> >     - 1024 to 1522 Bytes,
> >     - 1523 to 2000 Bytes and
> >     - More than 2001 Bytes
> >     KSZ8830 have mib counters upto 1024-1522 range only. Since no
> > other change,
> >     common range used across all KSZ series, but used upto only
> > upto 1024-1522
> >     for KSZ8830.
>
> Why are all commit messages indented in this way? Please keep the
> default text indentation at 0 characters. I have never seen this
> style
> in "git log".
Sure, will update the indentation in next version
>
> >
> > Co-developed-by: Thangaraj Samynathan <[email protected]>
>
> Documentation/process/submitting-patches.rst:
>
> Co-developed-by: states that the patch was co-created by multiple
> developers;
> it is used to give attribution to co-authors (in addition to the
> author
> attributed by the From: tag) when several people work on a single
> patch.  Since
> Co-developed-by: denotes authorship, every Co-developed-by: must be
> immediately
> followed by a Signed-off-by: of the associated co-author.  Standard
> sign-off
> procedure applies, i.e. the ordering of Signed-off-by: tags should
> reflect the
> chronological history of the patch insofar as possible, regardless of
> whether
> the author is attributed via From: or Co-developed-by:.  Notably, the
> last
> Signed-off-by: must always be that of the developer submitting the
> patch.
Will add Signed-off-by information in next revision

2023-02-21 07:14:18

by Rakesh Sankaranarayanan

[permalink] [raw]
Subject: Re: [PATCH v2 net-next 2/5] net: dsa: microchip: add eth ctrl grouping for ethtool statistics

Hi Vlad,
On Fri, 2023-02-17 at 19:08 +0200, Vladimir Oltean wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you
> know the content is safe
>
> On Fri, Feb 17, 2023 at 04:32:08PM +0530, Rakesh Sankaranarayanan
> wrote:
> > +void ksz8_get_eth_ctrl_stats(struct ksz_device *dev, int port,
> > +                          struct ethtool_eth_ctrl_stats
> > *ctrl_stats)
> > +{
> > +     struct ksz_port_mib *mib;
> > +     u64 *cnt;
> > +
> > +     mib = &dev->ports[port].mib;
> > +
> > +     mutex_lock(&mib->cnt_mutex);
> > +
> > +     cnt = &mib->counters[KSZ8_TX_PAUSE];
> > +     dev->dev_ops->r_mib_pkt(dev, port, KSZ8_TX_PAUSE, NULL, cnt);
> > +     ctrl_stats->MACControlFramesTransmitted = *cnt;
> > +
> > +     cnt = &mib->counters[KSZ8_RX_PAUSE];
> > +     dev->dev_ops->r_mib_pkt(dev, port, KSZ8_RX_PAUSE, NULL, cnt);
> > +     ctrl_stats->MACControlFramesReceived = *cnt;
> > +
> > +     mutex_unlock(&mib->cnt_mutex);
> > +}
>
> These should be reported as standard pause stats as well (ethtool -I
> --show-pause swpN).
Yes, these are reported as standards stats through get_pause_stats call
back.
static void ksz_get_pause_stats(struct dsa_switch *ds, int port,
struct ethtool_pause_stats
*pause_stats)
{
struct ksz_device *dev = ds->priv;
struct ksz_port_mib *mib;

mib = &dev->ports[port].mib;

spin_lock(&mib->stats64_lock);
memcpy(pause_stats, &mib->pause_stats, sizeof(*pause_stats));
spin_unlock(&mib->stats64_lock);
}


2023-02-21 07:25:39

by Rakesh Sankaranarayanan

[permalink] [raw]
Subject: Re: [PATCH v2 net-next 5/5] net: dsa: microchip: remove num_alus_variable

Hi Vlad,
On Fri, 2023-02-17 at 19:16 +0200, Vladimir Oltean wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you
> know the content is safe
>
> On Fri, Feb 17, 2023 at 04:32:11PM +0530, Rakesh Sankaranarayanan
> wrote:
> >     Remove num_alus variable from ksz_chip_data structure since
> >     it is unused now.
>
> now=since when?
>
> "git log -Gnum_alus drivers/net/dsa/microchip/" says it was never
> used.
Yes, this variable is never used after declaration, will update commit
message accordingly

2023-02-24 16:08:42

by Alexander Lobakin

[permalink] [raw]
Subject: Re: [PATCH v2 net-next 3/5] net: dsa: microchip: add eth mac grouping for ethtool statistics

From: Vladimir Oltean <[email protected]>
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

2023-02-24 21:54:08

by Vladimir Oltean

[permalink] [raw]
Subject: Re: [PATCH v2 net-next 3/5] net: dsa: microchip: add eth mac grouping for ethtool statistics

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.

2023-02-27 14:34:42

by Alexander Lobakin

[permalink] [raw]
Subject: Re: [PATCH v2 net-next 3/5] net: dsa: microchip: add eth mac grouping for ethtool statistics

From: Vladimir Oltean <[email protected]>
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

2023-02-27 14:52:18

by Andrew Lunn

[permalink] [raw]
Subject: Re: [PATCH v2 net-next 3/5] net: dsa: microchip: add eth mac grouping for ethtool statistics

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

2023-02-28 10:56:23

by Alexander Lobakin

[permalink] [raw]
Subject: Re: [PATCH v2 net-next 3/5] net: dsa: microchip: add eth mac grouping for ethtool statistics

From: Andrew Lunn <[email protected]>
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