From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.0 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id D521DC433E0 for ; Fri, 26 Jun 2020 03:29:31 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id BEB962084D for ; Fri, 26 Jun 2020 03:29:31 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728339AbgFZD3a (ORCPT ); Thu, 25 Jun 2020 23:29:30 -0400 Received: from smtprelay0249.hostedemail.com ([216.40.44.249]:56424 "EHLO smtprelay.hostedemail.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1728333AbgFZD3a (ORCPT ); Thu, 25 Jun 2020 23:29:30 -0400 Received: from filter.hostedemail.com (clb03-v110.bra.tucows.net [216.40.38.60]) by smtprelay01.hostedemail.com (Postfix) with ESMTP id 07C1C100E7B40; Fri, 26 Jun 2020 03:29:29 +0000 (UTC) X-Session-Marker: 6A6F6540706572636865732E636F6D X-HE-Tag: pail55_290cbf926e52 X-Filterd-Recvd-Size: 4319 Received: from XPS-9350.home (unknown [47.151.133.149]) (Authenticated sender: joe@perches.com) by omf20.hostedemail.com (Postfix) with ESMTPA; Fri, 26 Jun 2020 03:29:26 +0000 (UTC) Message-ID: <2b2a00cc0198328f1a0f3c9ccb6004a611a60011.camel@perches.com> Subject: Re: [net-next v3 13/15] iecm: Add ethtool From: Joe Perches To: Jeff Kirsher , davem@davemloft.net Cc: Alice Michael , netdev@vger.kernel.org, nhorman@redhat.com, sassmann@redhat.com, Alan Brady , Phani Burra , Joshua Hay , Madhu Chittim , Pavan Kumar Linga , Donald Skidmore , Jesse Brandeburg , Sridhar Samudrala Date: Thu, 25 Jun 2020 20:29:25 -0700 In-Reply-To: <20200626020737.775377-14-jeffrey.t.kirsher@intel.com> References: <20200626020737.775377-1-jeffrey.t.kirsher@intel.com> <20200626020737.775377-14-jeffrey.t.kirsher@intel.com> Content-Type: text/plain; charset="ISO-8859-1" User-Agent: Evolution 3.36.2-0ubuntu1 MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org On Thu, 2020-06-25 at 19:07 -0700, Jeff Kirsher wrote: > From: Alice Michael > > 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__) > + >