mlx5 M_TSTMP accuracy looses quite a bit of precision so lets fix it.

[rrs]

Summary

The way that the clock is synchronized between the system and the current mlx5 for the purposes of the M_TSTMP
being carried we loose a lot of precision. Instead lets change the math that calculates this to separate out
the seconds/nanoseconds and operate on the two values so we don't get overflow instead of just
shifting the value down and loosing precision.

Test Plan

One can test with BB logs to see the timestamp and I have also tested this
with added query mechanisms (not included in the patch) so that I could compare
old and new timestamps generated and all of those to the real floating point
result. This new algorithm now gets us to be within the 1/10ths of a nanosecond
where you would expect it to be for integer math.

[• hselasky]

sys/dev/mlx5/mlx5_en/mlx5_en_rx.c

253

Why can't you subtract the HW timestamps AS-IS, and then do the division and modulus on the result? Will save one round of expensive division and modulus?

261

Can negative values happen in res_s ?

[rrs]

sys/dev/mlx5/mlx5_en/mlx5_en_rx.c

253

Good point

261

No I don't think its possible because the min time you can set in the callout is 1 second. And
then you have to have some time after that. So it should never be negative here... If you want
we could add a check for that.. maybe a KASSERT would be best.

[kib]

sys/dev/mlx5/mlx5_en/mlx5_en_rx.c

241

Is this an advanced English, or a typo in 'equivalent'?

406–409

Why the check is needed now?

[• hselasky]

sys/dev/mlx5/mlx5_en/mlx5_en_rx.c

261

No reason to panic! Maybe a warn_once() from the LinuxKPI is better?

[rrs]

sys/dev/mlx5/mlx5_en/mlx5_en_rx.c

406–409

If you can't get sync (check the lockless code) and see gen = 0, you return 0.
In such a case you *cannot* convert the timestamp so you need to *not* set the bit.

[rrs]

sys/dev/mlx5/mlx5_en/mlx5_en_rx.c

241

No its my very poor spelling ;)

[rrs]

Fix the spelling pointed out by kib. And take Hans suggestion to cut out a / and *.

[• hselasky]

sys/dev/mlx5/mlx5_en/mlx5_en_rx.c

258

Can hw_clk_div be zero?

274

Should remove all of this comments and commented code.

[rrs]

sys/dev/mlx5/mlx5_en/mlx5_en_rx.c

232–249

Hans

Here is the comment I was referring too about divisor.. same divisor basically except
its >> 10

258

No it cannot.. Same for the original code I believe there is a comment in the
block somewhere to that effect. The reason behind it makes sense since
you have 1 second or more (min timeout granularity for the callout is 1 second)
between clbr_hw_curr and clbr_hw_prev and the only way that could happen
is if the hw clock stopped. And I believe the comment put in originally by yall
says that that case is covered by gen being set to 0 and thus a return of
0 (which is why I added the if 0 you drop the M_TSTMP).

274

Sure I left it in for your reference.

[• hselasky]

Code looks sane to me.

sys/dev/mlx5/mlx5_en/mlx5_en_main.c

4801

Are you sure you don't need to cast to ULL or uint64_t to have a proper 64-bit multiplication here?

[rrs]

sys/dev/mlx5/mlx5_en/mlx5_en_main.c

4801

Let me add that, I doubt its needed but it would not hurt :)

[rrs]

Address Hans ULL comment for clock freq

[rrs]

Take out the commented out old code.

[kib]

sys/dev/mlx5/mlx5_en/mlx5_en_main.c

4801

The cast is definitely not needed.

[• hselasky]

I'm good with this. @kib : Make sure to push the patch internally aswell.