bnxt_en: Periodically synchronize host time with firmware

[chandrakanth.patil_broadcom.com]

Summary

Synchronize host time with firmware at driver load, then repeat the process every four hours
to prevent firmware time drift. This maintains accuracy for time-dependent features and aids in
debugging long-running systems.

[chandrakanth.patil_broadcom.com]

its been long the patch not reviewed so please review the patch and provide your comments.

[gallatin]

sys/dev/bnxt/bnxt_en/if_bnxt.c

2309

Curious why you chose the faster but less accurate method to get the time. Did you consider nanotime() instead?

From the man page:

The bintime(), microtime(), and nanotime() functions always query the
timecounter to return the current time as precisely as possible.  Whereas
getbintime(), getmicrotime(), and getnanotime() functions are
abstractions which return a less precise, but faster to obtain, time.

[imp]

W/o measuring it, or knowing what the NIC is doing, it's hard to say how good the results from this will be.

Also, it does point out the need in FreeBSD to have a 'time stepped' callback since if the system comes up at the wrong (but maybe plausible) time, we do the NIC time setting, and then the time steps to the right time, the NIC's time will be off for up to 4 hours....

sys/dev/bnxt/bnxt_en/if_bnxt.c

2309

This also needs a comment "Wait until the system time is > 2000 before syncing the NIC to the system."

This test is also terrible. Too many firmwares will bound the clock to 2010 or 2015 (or soon I guess 2020). I'd do 2024 as a minimum date.

But since we're only setting the time in the NIC to the nearest second, the low res version is accurate enough, though it could result in over 1 second error in the NIC's time 1/1000th the time (since we could roll-over the second in the tick interval, we'd truncate to a 1s error). Of course, half the time we have at least half a second of error due to truncation, so maybe that's OK. It's kinda hard to avoid, though you can bound the error to half a second if you add half a second and then round.

2335

I'm not sure what the hardware does here. Does the hardware jam the time into its registers when it gets this message? Or does it 'slew' its time to the system time slowly by adjusting a frequency somewhere?

Doing a force phase change from time to time will introduce discontinuities in the data that's stamped with the timestamp.

And this is only to a second. You should really add 1 to ct.sec if ts.ts_nsec > 500000000 to bound the error to half a second. Though I'd think you'd want to pass the fractional second data down into the NIC....

But if all you have is SET_TIME and no adjust time, adjust frequency, etc, jamming it in like this is about the best you can do... Though a better interface might be to get the time from the NIC on a long time scale (say 30 minutes) and adjust the time only if it's off by more than a second. That would be better from a data point of view, but I don't know what the relative drift is between the NIC and the host, either with ntp steering or without...