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

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

On Tue, Apr 07, 2026 at 04:04:05PM -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>
> ---
> v2:
> -Addressed Bruce's comments
> ---

The code is a lot cleaner, but unfortunately the 32-bit builds are failing
due to a lack of 128-bit type. I'd suggest keeping thge 128-bit calculation
for simplicity on 64-bit platforms, and maybe adding back your original
calculation for 32-bit systems.

/Bruce

>  drivers/net/intel/idpf/idpf_ethdev.c | 38 ++++------------------------
>  1 file changed, 5 insertions(+), 33 deletions(-)
> 
> diff --git a/drivers/net/intel/idpf/idpf_ethdev.c b/drivers/net/intel/idpf/idpf_ethdev.c
> index 5e57a45775..23a2af6332 100644
> --- a/drivers/net/intel/idpf/idpf_ethdev.c
> +++ b/drivers/net/intel/idpf/idpf_ethdev.c
> @@ -1005,42 +1005,14 @@ idpf_timesync_adjust_freq(struct rte_eth_dev *dev, int64_t ppm)
>  	struct idpf_vport *vport = dev->data->dev_private;
>  	struct idpf_adapter *adapter = vport->adapter;
>  	struct idpf_ptp *ptp = adapter->ptp;
> -	int64_t incval, diff = 0;
> -	bool negative = false;
> -	uint64_t div, rem;
> -	uint64_t divisor = 1000000ULL << 16;
> -	int shift;
> +	uint64_t incval;
> +	__int128 diff;
>  	int ret;
>  
> +	/* ppm is scaled by 2^16 to match Linux adjfine. */
>  	incval = ptp->base_incval;
> -
> -	if (ppm < 0) {
> -		negative = true;
> -		ppm = -ppm;
> -	}
> -
> -	/* can incval * ppm overflow ? */
> -	if (rte_log2_u64(incval) + rte_log2_u64(ppm) > 62) {
> -		rem = ppm % divisor;
> -		div = ppm / divisor;
> -		diff = div * incval;
> -		ppm = rem;
> -
> -		shift = rte_log2_u64(incval) + rte_log2_u64(ppm) - 62;
> -		if (shift > 0) {
> -			/* drop precision */
> -			ppm >>= shift;
> -			divisor >>= shift;
> -		}
> -	}
> -
> -	if (divisor)
> -		diff = diff + incval * ppm / divisor;
> -
> -	if (negative)
> -		incval -= diff;
> -	else
> -		incval += diff;
> +	diff = ((__int128)incval * ppm) / (1000000LL << 16);
> +	incval += (int64_t)diff;
>  
>  	ret = idpf_ptp_adj_dev_clk_fine(adapter, incval);
>  	if (ret) {
> -- 
> 2.47.1
>