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 <[email protected]>
> ---
> 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
