Re: [PATCH v2 net-next 16/19] ionic: Add driver stats

[Jakub Kicinski <jakub.kicinski@netronome.com>]

On Fri, 28 Jun 2019 14:39:31 -0700, Shannon Nelson wrote:
> Add in the detailed statistics for ethtool -S that the driver
> keeps as it processes packets.  Display of the additional
> debug statistics can be enabled through the ethtool priv-flags
> feature.
> 
> Signed-off-by: Shannon Nelson <snelson@pensando.io>

> diff --git a/drivers/net/ethernet/pensando/ionic/Makefile b/drivers/net/ethernet/pensando/ionic/Makefile
> index 0e2dc53f08d4..4f3cfbf36c23 100644
> --- a/drivers/net/ethernet/pensando/ionic/Makefile
> +++ b/drivers/net/ethernet/pensando/ionic/Makefile
> @@ -4,4 +4,5 @@
>  obj-$(CONFIG_IONIC) := ionic.o
>  
>  ionic-y := ionic_main.o ionic_bus_pci.o ionic_dev.o ionic_ethtool.o \
> -	   ionic_lif.o ionic_rx_filter.o ionic_txrx.o ionic_debugfs.o
> +	   ionic_lif.o ionic_rx_filter.o ionic_txrx.o ionic_debugfs.o \
> +	   ionic_stats.o
> diff --git a/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c b/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c
> index 2bbe5819387b..518e79c90fca 100644
> --- a/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c
> +++ b/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c
> @@ -8,6 +8,84 @@
>  #include "ionic_bus.h"
>  #include "ionic_lif.h"
>  #include "ionic_ethtool.h"
> +#include "ionic_stats.h"
> +
> +static const char ionic_priv_flags_strings[][ETH_GSTRING_LEN] = {
> +#define PRIV_F_SW_DBG_STATS		BIT(0)
> +	"sw-dbg-stats",
> +};
> +#define PRIV_FLAGS_COUNT ARRAY_SIZE(ionic_priv_flags_strings)
> +
> +static void ionic_get_stats_strings(struct lif *lif, u8 *buf)
> +{
> +	u32 i;
> +
> +	for (i = 0; i < ionic_num_stats_grps; i++)
> +		ionic_stats_groups[i].get_strings(lif, &buf);
> +}
> +
> +static void ionic_get_stats(struct net_device *netdev,
> +			    struct ethtool_stats *stats, u64 *buf)
> +{
> +	struct lif *lif;
> +	u32 i;
> +
> +	lif = netdev_priv(netdev);
> +
> +	memset(buf, 0, stats->n_stats * sizeof(*buf));
> +	for (i = 0; i < ionic_num_stats_grps; i++)
> +		ionic_stats_groups[i].get_values(lif, &buf);
> +}
> +
> +static int ionic_get_stats_count(struct lif *lif)
> +{
> +	int i, num_stats = 0;
> +
> +	for (i = 0; i < ionic_num_stats_grps; i++)
> +		num_stats += ionic_stats_groups[i].get_count(lif);
> +
> +	return num_stats;
> +}
> +
> +static int ionic_get_sset_count(struct net_device *netdev, int sset)
> +{
> +	struct lif *lif = netdev_priv(netdev);
> +	int count = 0;
> +
> +	switch (sset) {
> +	case ETH_SS_STATS:
> +		count = ionic_get_stats_count(lif);
> +		break;
> +	case ETH_SS_TEST:
> +		break;
> +	case ETH_SS_PRIV_FLAGS:
> +		count = PRIV_FLAGS_COUNT;
> +		break;
> +	default:
> +		return -EOPNOTSUPP;
> +	}
> +	return count;
> +}
> +
> +static void ionic_get_strings(struct net_device *netdev,
> +			      u32 sset, u8 *buf)
> +{
> +	struct lif *lif = netdev_priv(netdev);
> +
> +	switch (sset) {
> +	case ETH_SS_STATS:
> +		ionic_get_stats_strings(lif, buf);
> +		break;
> +	case ETH_SS_PRIV_FLAGS:
> +		memcpy(buf, ionic_priv_flags_strings,
> +		       PRIV_FLAGS_COUNT * ETH_GSTRING_LEN);
> +		break;
> +	case ETH_SS_TEST:
> +		// IONIC_TODO
> +	default:
> +		netdev_err(netdev, "Invalid sset %d\n", sset);

Not really an error, as long as sset_count() returns a 0 nothing will
happen.  Also you can drop the SS_TEST if you don't report it.

> +	}
> +}

> diff --git a/drivers/net/ethernet/pensando/ionic/ionic_stats.h b/drivers/net/ethernet/pensando/ionic/ionic_stats.h
> new file mode 100644
> index 000000000000..b5487e7fd4fb
> --- /dev/null
> +++ b/drivers/net/ethernet/pensando/ionic/ionic_stats.h
> @@ -0,0 +1,53 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/* Copyright(c) 2017 - 2019 Pensando Systems, Inc */
> +
> +#ifndef _IONIC_STATS_H_
> +#define _IONIC_STATS_H_
> +
> +#define IONIC_STAT_TO_OFFSET(type, stat_name) (offsetof(type, stat_name))
> +
> +#define IONIC_STAT_DESC(type, stat_name) { \
> +	.name = #stat_name, \
> +	.offset = IONIC_STAT_TO_OFFSET(type, stat_name) \
> +}
> +
> +#define IONIC_LIF_STAT_DESC(stat_name) \
> +	IONIC_STAT_DESC(struct lif_sw_stats, stat_name)
> +
> +#define IONIC_TX_STAT_DESC(stat_name) \
> +	IONIC_STAT_DESC(struct tx_stats, stat_name)
> +
> +#define IONIC_RX_STAT_DESC(stat_name) \
> +	IONIC_STAT_DESC(struct rx_stats, stat_name)
> +
> +#define IONIC_TX_Q_STAT_DESC(stat_name) \
> +	IONIC_STAT_DESC(struct queue, stat_name)
> +
> +#define IONIC_CQ_STAT_DESC(stat_name) \
> +	IONIC_STAT_DESC(struct cq, stat_name)
> +
> +#define IONIC_INTR_STAT_DESC(stat_name) \
> +	IONIC_STAT_DESC(struct intr, stat_name)
> +
> +#define IONIC_NAPI_STAT_DESC(stat_name) \
> +	IONIC_STAT_DESC(struct napi_stats, stat_name)
> +
> +/* Interface structure for a particalar stats group */
> +struct ionic_stats_group_intf {
> +	void (*get_strings)(struct lif *lif, u8 **buf);
> +	void (*get_values)(struct lif *lif, u64 **buf);
> +	u64 (*get_count)(struct lif *lif);
> +};
> +
> +extern const struct ionic_stats_group_intf ionic_stats_groups[];
> +extern const int ionic_num_stats_grps;
> +
> +#define IONIC_READ_STAT64(base_ptr, desc_ptr) \
> +	(*((u64 *)(((u8 *)(base_ptr)) + (desc_ptr)->offset)))
> +
> +struct ionic_stat_desc {
> +	char name[ETH_GSTRING_LEN];
> +	u64 offset;
> +};
> +
> +#endif // _IONIC_STATS_H_

Perhaps worth grepping the driver for C++ style comments?