Re: [PATCH net-next 5/8] ice: register 1588 PTP clock device object for E810 devices

On Mon, 14 Jun 2021 19:50:23 +0000 Keller, Jacob E wrote:

quoted

quoted

Hmmm.. I thought ppb was a s64, not an s32.

In general, I believe max_adj is usually capped at 1 billion anyways,
since it doesn't make sense to slow a clock by more than 1billioln ppb,
and increasing it more than that isn't really useful either.  

Do you mean it's capped somewhere in the code to 1B?

I'm no time expert but this is not probability where 1 is a magic
value, adjusting clock by 1 - 1ppb vs 1 + 1ppb makes little difference,
no? Both mean something is super fishy with the nominal or expected
frequency, but the hardware can do that and more.

Flipping the question, if adjusting by large ppb values is not correct,
why not cap the adjustment at the value which would prevent the u64
overflow?  

Large ppb values are sometimes used when you want to slew a clock to
bring it in sync when its a few milliseconds to seconds off, without
performing a time jump (so that you maintain monotonic increasing
time).

Ah, you're right, ptp4l will explicitly cap the freq adjustments
based on max_adj from sysfs, so setting max_adj too low could impact
the convergence time in strange scenarios.

That being said, we are supposed to be checking max_adj, except that
you're right the conversion to ppb could overflow, and there's no
check prior to the conversion from scaled_ppm to ppb.