On Thu, 2020-06-25 at 19:07 -0700, Jeff Kirsher wrote:
> From: Alice Michael <alice.mich...@intel.com>
>
> Implement ethtool interface for the common module.
[]
> diff --git a/drivers/net/ethernet/intel/iecm/iecm_ethtool.c
> b/drivers/net/ethernet/intel/iecm/iecm_ethtool.c
[]
> +/* Stats associated with a Tx queue */
> +static const struct iecm_stats iecm_gstrings_tx_queue_stats[] = {
> + IECM_QUEUE_STAT("%s-%u.packets", q_stats.tx.packets),
> + IECM_QUEUE_STAT("%s-%u.bytes", q_stats.tx.bytes),
> +};
> +
> +static const struct iecm_stats iecm_gstrings_rx_queue_stats[] = {
> + IECM_QUEUE_STAT("%s-%u.packets", q_stats.rx.packets),
> + IECM_QUEUE_STAT("%s-%u.bytes", q_stats.rx.bytes),
> + IECM_QUEUE_STAT("%s-%u.generic_csum", q_stats.rx.generic_csum),
> + IECM_QUEUE_STAT("%s-%u.basic_csum", q_stats.rx.basic_csum),
> + IECM_QUEUE_STAT("%s-%u.csum_err", q_stats.rx.csum_err),
> + IECM_QUEUE_STAT("%s-%u.hsplit_buf_overflow", q_stats.rx.hsplit_hbo),
> +};
> +
> +#define IECM_TX_QUEUE_STATS_LEN
> ARRAY_SIZE(iecm_gstrings_tx_queue_stats)
> +#define IECM_RX_QUEUE_STATS_LEN
> ARRAY_SIZE(iecm_gstrings_rx_queue_stats)
> +
> +/**
> + * __iecm_add_stat_strings - copy stat strings into ethtool buffer
> + * @p: ethtool supplied buffer
> + * @stats: stat definitions array
> + * @size: size of the stats array
> + *
> + * Format and copy the strings described by stats into the buffer pointed at
> + * by p.
> + */
> +static void __iecm_add_stat_strings(u8 **p, const struct iecm_stats stats[],
> + const unsigned int size, ...)
> +{
> + unsigned int i;
> +
> + for (i = 0; i < size; i++) {
> + va_list args;
> +
> + va_start(args, size);
> + vsnprintf((char *)*p, ETH_GSTRING_LEN,
> + stats[i].stat_string, args);
> + *p += ETH_GSTRING_LEN;
> + va_end(args);
> + }
> +}
Slightly dangerous to have a possible mismatch between the varargs
and the actual constant format spec.
Perhaps safer to use something like:
static const struct iecm_stats iecm_gstrings_tx_queue_stats[] = {
IECM_QUEUE_STAT("packets", q_stats.tx.packets),
IECM_QUEUE_STAT("bytes", q_stats.tx.bytes),
};
Perhaps use const char * and unsigned int instead of varargs
so this formats the output without va_start/end
snprintf(*p, ETH_GSTRING_LEN, "%s-%u.%s", type, index,
stats[i].stat_string);
> static void __iecm_add_stat_strings(u8 **p, const struct iecm_stats stats[],
> +
> +/**
> + * iecm_add_stat_strings - copy stat strings into ethtool buffer
> + * @p: ethtool supplied buffer
> + * @stats: stat definitions array
> + *
> + * Format and copy the strings described by the const static stats value into
> + * the buffer pointed at by p.
> + *
> + * The parameter @stats is evaluated twice, so parameters with side effects
> + * should be avoided. Additionally, stats must be an array such that
> + * ARRAY_SIZE can be called on it.
> + */
> +#define iecm_add_stat_strings(p, stats, ...) \
> + __iecm_add_stat_strings(p, stats, ARRAY_SIZE(stats), ## __VA_ARGS__)
> +
>
