Messages in this thread | | | Date | Fri, 17 Feb 2023 15:59:12 +0100 | Subject | Re: [PATCH v2 net-next 1/5] net: dsa: microchip: add rmon grouping for ethtool statistics | From | Alexander Lobakin <> |
| |
From: Rakesh Sankaranarayanan <rakesh.sankaranarayanan@microchip.com> 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
| |