Re: [PATCH net-next v1 5/7] ethtool: add helper to prevent invalid statistics exposure to userspace

[Mateusz Polchlopek <mateusz.polchlopek@intel.com>]

On 12/3/2024 8:56 AM, Oleksij Rempel wrote:
> Introduce a new helper function, `ethtool_stat_add`, to update 64-bit
> statistics with proper handling of the reserved value
> `ETHTOOL_STAT_NOT_SET`. This ensures that statistics remain valid and
> are always reported to userspace, even if the driver accidentally sets
> `ETHTOOL_STAT_NOT_SET` during an update.
> 
> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> ---
>   include/linux/ethtool.h | 16 ++++++++++++++++
>   1 file changed, 16 insertions(+)
> 
> diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
> index b0ed740ca749..657bd69ddaf7 100644
> --- a/include/linux/ethtool.h
> +++ b/include/linux/ethtool.h
> @@ -371,6 +371,22 @@ static inline void ethtool_stats_init(u64 *stats, unsigned int n)
>   		stats[n] = ETHTOOL_STAT_NOT_SET;
>   }
>   
> +/**
> + * ethtool_stat_add - Add a value to a u64 statistic with wraparound handling
> + * @stat: Pointer to the statistic to update
> + * @value: Value to add to the statistic
> + *
> + * Adds the specified value to a u64 statistic. If the result of the addition
> + * equals the reserved value (`ETHTOOL_STAT_NOT_SET`), it increments the result
> + * by 1 to avoid the reserved value.
> + */
> +static inline void ethtool_stat_add(u64 *stat, u64 value)
> +{
> +	*stat += value;
> +	if (*stat == ETHTOOL_STAT_NOT_SET)
> +		(*stat)++;
> +}
> +
>   /* Basic IEEE 802.3 MAC statistics (30.3.1.1.*), not otherwise exposed
>    * via a more targeted API.
>    */

The code of function looks good

Reviewed-by: Mateusz Polchlopek <mateusz.polchlopek@intel.com>