Subject: Re: [PATCH] net: mvneta: fix calls to page_pool_get_stats

From: Lorenzo Bianconi <[email-masked]>
Date: Fri, 29 Sep 2023 09:52:19 +0200

> 
> This commit adds the proper checks before calling page_pool_get_stats.
> 
> Fixes: b3fc792 ("net: mvneta: add support for page_pool_get_stats")
> Signed-off-by: Sven Auhagen <[email-masked]>
> Reported-by: Paulo Da Silva <[email-masked]>

Hi Sven,

thx for fixing this issue, just a minor comment inline.

Regards,
Lorenzo

> 
> diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethernet/marvell/mvneta.c
> index d50ac1fc288a..6331f284dc97 100644
> --- a/drivers/net/ethernet/marvell/mvneta.c
> +++ b/drivers/net/ethernet/marvell/mvneta.c
> @@ -4734,13 +4734,16 @@ static void mvneta_ethtool_get_strings(struct net_device *netdev, u32 sset,
>  {
>  	if (sset == ETH_SS_STATS) {
>  		int i;
> +		struct mvneta_port *pp = netdev_priv(netdev);
>  
>  		for (i = 0; i < ARRAY_SIZE(mvneta_statistics); i++)
>  			memcpy(data + i * ETH_GSTRING_LEN,
>  			       mvneta_statistics[i].name, ETH_GSTRING_LEN);
>  
> -		data += ETH_GSTRING_LEN * ARRAY_SIZE(mvneta_statistics);
> -		page_pool_ethtool_stats_get_strings(data);
> +		if (!pp->bm_priv) {
> +			data += ETH_GSTRING_LEN * ARRAY_SIZE(mvneta_statistics);
> +			page_pool_ethtool_stats_get_strings(data);
> +		}
>  	}
>  }
>  
> @@ -4858,8 +4861,10 @@ static void mvneta_ethtool_pp_stats(struct mvneta_port *pp, u64 *data)
>  	struct page_pool_stats stats = {};
>  	int i;
>  
> -	for (i = 0; i < rxq_number; i++)
> -		page_pool_get_stats(pp->rxqs[i].page_pool, &stats);
> +	for (i = 0; i < rxq_number; i++) {
> +		if (pp->rxqs[i].page_pool)
> +			page_pool_get_stats(pp->rxqs[i].page_pool, &stats);

We could move this check inside page_pool_get_stats(), what do you think? I
guess it would be beneficial even for other consumers.

> +	}
>  
>  	page_pool_ethtool_stats_get(data, &stats);
>  }
> @@ -4875,14 +4880,21 @@ static void mvneta_ethtool_get_stats(struct net_device *dev,
>  	for (i = 0; i < ARRAY_SIZE(mvneta_statistics); i++)
>  		*data++ = pp->ethtool_stats[i];
>  
> -	mvneta_ethtool_pp_stats(pp, data);
> +	if (!pp->bm_priv && !pp->is_stopped)
> +		mvneta_ethtool_pp_stats(pp, data);
>  }
>  
>  static int mvneta_ethtool_get_sset_count(struct net_device *dev, int sset)
>  {
> -	if (sset == ETH_SS_STATS)
> -		return ARRAY_SIZE(mvneta_statistics) +
> -		       page_pool_ethtool_stats_get_count();
> +	if (sset == ETH_SS_STATS) {
> +		int count = ARRAY_SIZE(mvneta_statistics);
> +		struct mvneta_port *pp = netdev_priv(dev);
> +
> +		if (!pp->bm_priv)
> +			count += page_pool_ethtool_stats_get_count();
> +
> +		return count;
> +	}
>  
>  	return -EOPNOTSUPP;
>  }
> -- 
> 2.33.1
>