Re: [PATCH net-next v4 3/3] net: libwx: support vf per-queue statistics via ethtool -S

[Simon Horman <horms@kernel.org>]

On Mon, Jun 08, 2026 at 06:39:46PM +0800, Mengyuan Lou wrote:
> Add per-queue TX/RX packet and byte counters to the VF ethtool stats
> table.
> The stats length is now dynamically determined based on whether the
> device is a VF or PF via wx_stats_len().
> 
> Signed-off-by: Mengyuan Lou <mengyuanlou@net-swift.com>
> ---
>  .../net/ethernet/wangxun/libwx/wx_ethtool.c   | 48 +++++++++++++++----
>  drivers/net/ethernet/wangxun/libwx/wx_hw.c    |  4 ++
>  .../net/ethernet/wangxun/libwx/wx_vf_common.c |  2 +
>  3 files changed, 44 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/net/ethernet/wangxun/libwx/wx_ethtool.c b/drivers/net/ethernet/wangxun/libwx/wx_ethtool.c
> index 30c6ef6103ac..86f6ac63acf7 100644
> --- a/drivers/net/ethernet/wangxun/libwx/wx_ethtool.c
> +++ b/drivers/net/ethernet/wangxun/libwx/wx_ethtool.c
> @@ -48,6 +48,10 @@ static const struct wx_stats wx_gstrings_stats[] = {
>  	WX_STAT("rx_hwtstamp_cleared", rx_hwtstamp_cleared),
>  };
>  
> +static const struct wx_stats wx_gstrings_stats_vf[] = {
> +	WX_STAT("non_eop_descs", non_eop_descs),
> +};
> +

The AI-generated review available at https://netdev-ai.bots.linux.dev/sashiko/
flags that this is user-exposed and probably could benefit from a bit
more explanation in the patch description.

>  static const struct wx_stats wx_gstrings_fdir_stats[] = {
>  	WX_STAT("fdir_match", stats.fdirmatch),
>  	WX_STAT("fdir_miss", stats.fdirmiss),
> @@ -69,15 +73,26 @@ static const struct wx_stats wx_gstrings_rsc_stats[] = {
>  #define WX_QUEUE_STATS_LEN ( \
>  		(WX_NUM_TX_QUEUES + WX_NUM_RX_QUEUES) * \
>  		(sizeof(struct wx_queue_stats) / sizeof(u64)))
> -#define WX_GLOBAL_STATS_LEN  ARRAY_SIZE(wx_gstrings_stats)
>  #define WX_FDIR_STATS_LEN  ARRAY_SIZE(wx_gstrings_fdir_stats)
>  #define WX_RSC_STATS_LEN  ARRAY_SIZE(wx_gstrings_rsc_stats)
> -#define WX_STATS_LEN (WX_GLOBAL_STATS_LEN + WX_QUEUE_STATS_LEN)
> +
> +static inline unsigned int wx_global_stats_len(const struct wx *wx)
> +{
> +	return wx->pdev->is_virtfn ?
> +	       ARRAY_SIZE(wx_gstrings_stats_vf) : ARRAY_SIZE(wx_gstrings_stats);
> +}

Please don't use the inline keyword in .c files unless there is a
demonstrable - usually performance - reason to do so. Rather, please
let the compiler inline code (or not).

It is of course fine to use the inline keyword in .h files.

> +
> +static inline unsigned int wx_stats_len(const struct wx *wx)
> +{
> +	struct net_device *netdev = wx->netdev;

netdev seems unused.

Flagged by AI-generated review on sashiko.dev 

> +
> +	return wx_global_stats_len(wx) + WX_QUEUE_STATS_LEN;
> +}
>  
>  int wx_get_sset_count(struct net_device *netdev, int sset)
>  {
>  	struct wx *wx = netdev_priv(netdev);
> -	int len = WX_STATS_LEN;
> +	int len = wx_stats_len(wx);
>  
>  	switch (sset) {
>  	case ETH_SS_STATS:
> @@ -100,8 +115,11 @@ void wx_get_strings(struct net_device *netdev, u32 stringset, u8 *data)
>  
>  	switch (stringset) {
>  	case ETH_SS_STATS:
> -		for (i = 0; i < WX_GLOBAL_STATS_LEN; i++)
> -			ethtool_puts(&p, wx_gstrings_stats[i].stat_string);
> +		for (i = 0; i < wx_global_stats_len(wx); i++)
> +			if (wx->pdev->is_virtfn)
> +				ethtool_puts(&p, wx_gstrings_stats_vf[i].stat_string);
> +			else
> +				ethtool_puts(&p, wx_gstrings_stats[i].stat_string);

The AI-generated review at https://netdev-ai.bots.linux.dev/sashiko/ also
flags that these seem to cover statistics that can be exposed via
netdev_stat_ops, which is preferred.

If consistency with the PF stats is desired for VF stats, then perhaps
netdev_stat_ops can also be implemented for PF queues.

>  		if (test_bit(WX_FLAG_FDIR_CAPABLE, wx->flags)) {
>  			for (i = 0; i < WX_FDIR_STATS_LEN; i++)
>  				ethtool_puts(&p, wx_gstrings_fdir_stats[i].stat_string);

...