On 4/25/24 00:03, Keller, Jacob E wrote:
-----Original Message-----
From: Polchlopek, Mateusz <mateusz.polchlo...@intel.com>
Sent: Monday, April 22, 2024 2:23 AM
To: Rahul Rameshbabu <rrameshb...@nvidia.com>
Cc: intel-wired-...@lists.osuosl.org; net...@vger.kernel.org; ho...@kernel.org;
Nguyen, Anthony L <anthony.l.ngu...@intel.com>; Keller, Jacob E
<jacob.e.kel...@intel.com>; Drewek, Wojciech <wojciech.dre...@intel.com>
Subject: Re: [Intel-wired-lan] [PATCH iwl-next v5 08/12] iavf: periodically
cache
PHC time
On 4/18/2024 9:51 PM, Rahul Rameshbabu wrote:
On Thu, 18 Apr, 2024 01:24:56 -0400 Mateusz Polchlopek
<mateusz.polchlo...@intel.com> wrote:
From: Jacob Keller <jacob.e.kel...@intel.com>
The Rx timestamps reported by hardware may only have 32 bits of storage
for nanosecond time. These timestamps cannot be directly reported to the
Linux stack, as it expects 64bits of time.
To handle this, the timestamps must be extended using an algorithm that
calculates the corrected 64bit timestamp by comparison between the PHC
time and the timestamp. This algorithm requires the PHC time to be
captured within ~2 seconds of when the timestamp was captured.
Instead of trying to read the PHC time in the Rx hotpath, the algorithm
relies on a cached value that is periodically updated.
Keep this cached time up to date by using the PTP .do_aux_work kthread
function.
Seems reasonable.
The iavf_ptp_do_aux_work will reschedule itself about twice a second,
and will check whether or not the cached PTP time needs to be updated.
If so, it issues a VIRTCHNL_OP_1588_PTP_GET_TIME to request the time
from the PF. The jitter and latency involved with this command aren't
important, because the cached time just needs to be kept up to date
within about ~2 seconds.
Reviewed-by: Wojciech Drewek <wojciech.dre...@intel.com>
Signed-off-by: Jacob Keller <jacob.e.kel...@intel.com>
Co-developed-by: Mateusz Polchlopek <mateusz.polchlo...@intel.com>
Signed-off-by: Mateusz Polchlopek <mateusz.polchlo...@intel.com>
---
drivers/net/ethernet/intel/iavf/iavf_ptp.c | 52 ++++++++++++++++++++++
drivers/net/ethernet/intel/iavf/iavf_ptp.h | 1 +
2 files changed, 53 insertions(+)
diff --git a/drivers/net/ethernet/intel/iavf/iavf_ptp.c
b/drivers/net/ethernet/intel/iavf/iavf_ptp.c
<snip>
+/**
+ * iavf_ptp_do_aux_work - Perform periodic work required for PTP support
+ * @ptp: PTP clock info structure
+ *
+ * Handler to take care of periodic work required for PTP operation. This
+ * includes the following tasks:
+ *
+ * 1) updating cached_phc_time
+ *
+ * cached_phc_time is used by the Tx and Rx timestamp flows in order to
+ * perform timestamp extension, by carefully comparing the timestamp
+ * 32bit nanosecond timestamps and determining the corrected 64bit
+ * timestamp value to report to userspace. This algorithm only works if
+ * the cached_phc_time is within ~1 second of the Tx or Rx timestamp
+ * event. This task periodically reads the PHC time and stores it, to
+ * ensure that timestamp extension operates correctly.
+ *
+ * Returns: time in jiffies until the periodic task should be re-scheduled.
+ */
+long iavf_ptp_do_aux_work(struct ptp_clock_info *ptp)
+{
+ struct iavf_adapter *adapter = clock_to_adapter(ptp);
+
+ iavf_ptp_cache_phc_time(adapter);
+
+ /* Check work about twice a second */
+ return msecs_to_jiffies(500);
HZ / 2 might be more intuitive?
I've always found it more intuitive to think in terms of msecs myself, but HZ /
2 is ok if other folks agree.
HZ/2 or HZ/3 as a timer period could be understood without thinking, but
the same stands for 400ms. Problems starts when one thinks about it ;)
For me HZ, which could be both literally and colloquially understood as
"per second" should not mean 1000ms (just evaluate to).
2Hz is a frequency with half second period, but 2*HZ evaluates to 2000ms
which is 4 times more :/
Thanks,
Jake
