Re: [PATCH v2 net-next 1/5] net: dsa: microchip: add rmon grouping for ethtool statistics
From: Alexander Lobakin <aleksander.lobakin@intel.com>
Date: 2023-02-17 15:03:17
Also in:
lkml
From: Rakesh Sankaranarayanan <redacted> 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