On Mon, 8 Jan 2018 17:46:02 -0800, Jakub Kicinski wrote:
> On Mon, 08 Jan 2018 20:39:13 -0500 (EST), David Miller wrote:
> > From: Jakub Kicinski <kubak...@wp.pl>
> > Date: Mon, 8 Jan 2018 12:04:31 -0800
> >
> > > Ugh, I so didn't review this in time :( I think there is a consensus
> > > that we should avoid duplicating standard stats in ethtool. Especially
> > > those old ones. Like "collisions", I assume this is a modern NIC, are
> > > collisions still a thing?
> >
> > There is no standard way to get per-queue values, and ethtool stats are
> > how pretty much every driver provides it.
>
> Right, agreed. I'm only objecting to this patch (12/20), where we can
> see the telltale code like this:
>
> + const struct rtnl_link_stats64 *net_stats;
> + struct rtnl_link_stats64 temp;
> +
> + net_stats = dev_get_stats(netdev, &temp);
> + for (i = 0; i < HNS3_NETDEV_STATS_COUNT; i++) {
> + stat = (u8 *)net_stats + hns3_netdev_stats[i].stats_offset;
> + *data++ = *(u64 *)stat;
> + }
>
> Where:
>
> +#define HNS3_NETDEV_STAT(_string, _member) { \
> + .stats_string = _string, \
> + .stats_offset = offsetof(struct rtnl_link_stats64, _member) \
> +}
> +
> +static const struct hns3_stats hns3_netdev_stats[] = {
> + /* Rx per-queue statistics */
Oh, I only noticed this extra misleading comment now. Unless each queue
has a netdev, I don't see how these are per-queue.
> + HNS3_NETDEV_STAT("rx_packets", rx_packets),
> + HNS3_NETDEV_STAT("tx_packets", tx_packets),
>
> etc. IOW dumping struct rtnl_link_stats64 to ethtool -S member by
> member.
>
> Let me put the netlink per-queue stats on my soft TODO list :)
>
