Messages in this thread | | | From | <> | Subject | Re: [PATCH v2 net-next 1/5] net: dsa: microchip: add rmon grouping for ethtool statistics | Date | Tue, 21 Feb 2023 04:37:54 +0000 |
| |
On Fri, 2023-02-17 at 15:59 +0100, Alexander Lobakin wrote: > [Some people who received this message don't often get email from > aleksander.lobakin@intel.com. 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 <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)? 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
| |