Re: [PATCH v1] net/idpf: harden PTP frequency adjustment

[Bruce Richardson <bruce.richardson@intel.com>]

On Thu, Mar 12, 2026 at 02:43:44PM -0400, Soumyadeep Hore wrote:
> Normalize ppm to an unsigned magnitude before using it in the
> increment value scaling path.
> 
> This avoids negating INT64_MIN and also prevents subtracting 62
> from the reduced log sum unless the sum is still above the
> overflow threshold reported by Coverity.
> 
> Coverity issue: 501832
> 
> Signed-off-by: Soumyadeep Hore <soumyadeep.hore@intel.com>
> ---
>  drivers/net/intel/idpf/idpf_ethdev.c | 32 +++++++++++++++++-----------
>  1 file changed, 20 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/net/intel/idpf/idpf_ethdev.c b/drivers/net/intel/idpf/idpf_ethdev.c
> index 5e57a45775..1c5bd2ee12 100644
> --- a/drivers/net/intel/idpf/idpf_ethdev.c
> +++ b/drivers/net/intel/idpf/idpf_ethdev.c
> @@ -1007,7 +1007,7 @@ idpf_timesync_adjust_freq(struct rte_eth_dev *dev, int64_t ppm)
>  	struct idpf_ptp *ptp = adapter->ptp;
>  	int64_t incval, diff = 0;
>  	bool negative = false;
> -	uint64_t div, rem;
> +	uint64_t abs_ppm, div, rem;
>  	uint64_t divisor = 1000000ULL << 16;
>  	int shift;
>  	int ret;
> @@ -1016,26 +1016,34 @@ idpf_timesync_adjust_freq(struct rte_eth_dev *dev, int64_t ppm)
>  
>  	if (ppm < 0) {
>  		negative = true;
> -		ppm = -ppm;
> +		abs_ppm = ppm == INT64_MIN ? (uint64_t)INT64_MAX + 1 :
> +			(uint64_t)(-ppm);
> +	} else {
> +		abs_ppm = (uint64_t)ppm;
>  	}
>  
>  	/* can incval * ppm overflow ? */
> -	if (rte_log2_u64(incval) + rte_log2_u64(ppm) > 62) {
> -		rem = ppm % divisor;
> -		div = ppm / divisor;
> +	if (rte_log2_u64(incval) + rte_log2_u64(abs_ppm) > 62) {
> +		rem = abs_ppm % divisor;
> +		div = abs_ppm / divisor;
>  		diff = div * incval;
> -		ppm = rem;
> +		abs_ppm = rem;
> +
> +		if (abs_ppm != 0) {
> +			uint32_t log_sum;
>  
> -		shift = rte_log2_u64(incval) + rte_log2_u64(ppm) - 62;
> -		if (shift > 0) {
> -			/* drop precision */
> -			ppm >>= shift;
> -			divisor >>= shift;
> +			log_sum = rte_log2_u64(incval) + rte_log2_u64(abs_ppm);
> +			if (log_sum > 62) {
> +				shift = log_sum - 62;
> +				/* drop precision */
> +				abs_ppm >>= shift;
> +				divisor >>= shift;
> +			}
>  		}
>  	}
>  
>  	if (divisor)
> -		diff = diff + incval * ppm / divisor;
> +		diff = diff + incval * abs_ppm / divisor;
>  

Asking Claude AI about this code and how to simplify it a bit while also
avoiding the INT64_MIN issue, it suggests using __int128 type, giving much
shorter code:

	__int128 diff;
	int ret;

	/*
	 * ppm is scaled ppm: 1 ppm = 2^16 units (matching Linux adjfine).
	 * Use __int128 to avoid overflow without complex split/shift logic,
	 * and to correctly handle ppm == INT64_MIN.
	 */
	incval = ptp->base_incval;
	diff = ((__int128)incval * ppm) / (1000000LL << 16);
	incval += (int64_t)diff;

	ret = idpf_ptp_adj_dev_clk_fine(adapter, incval);
	if (ret != 0)
		PMD_DRV_LOG(ERR, "PTP failed to set incval, err %d", ret);
	return ret;

Maybe worth considering?

/Bruce