The current implementation of PTP_SYS_OFFSET_EXTENDED provides
PHC reads in the form of [pre-TS, PHC, post-TS]. These pre and
post timestamps are useful to measure the width of the PHC read.
However, the current implementation provides these timestamps in
CLOCK_REALTIME only. Since CLOCK_REALTIME is disciplined by NTP
or NTP-like service(s), the value is subjected to change. This
makes some applications that are very sensitive to time change
have these timestamps delivered in different time-base.
This patch updates the gettimex64 / ioctl op PTP_SYS_OFFSET_EXTENDED
to provide these (sandwich) timestamps optionally in
CLOCK_MONOTONIC_RAW timebase while maintaining the default behavior
or giving them in CLOCK_REALTIME.
~# testptp -d /dev/ptp0 -x 3 -y raw
extended timestamp request returned 3 samples
sample # 0: mono-raw time before: 371.548640128
phc time: 371.579671788
mono-raw time after: 371.548640912
sample # 1: mono-raw time before: 371.548642104
phc time: 371.579673346
mono-raw time after: 371.548642490
sample # 2: mono-raw time before: 371.548643320
phc time: 371.579674652
mono-raw time after: 371.548643756
~# testptp -d /dev/ptp0 -x 3
extended timestamp request returned 3 samples
sample # 0: system time before: 1713243413.403474250
phc time: 385.699915490
system time after: 1713243413.403474948
sample # 1: system time before: 1713243413.403476220
phc time: 385.699917168
system time after: 1713243413.403476642
sample # 2: system time before: 1713243413.403477555
phc time: 385.699918442
system time after: 1713243413.403477961
~#
Signed-off-by: Mahesh Bandewar <[email protected]>
---
drivers/ptp/ptp_chardev.c | 5 ++++-
include/linux/ptp_clock_kernel.h | 30 ++++++++++++++++++++++++++----
include/uapi/linux/ptp_clock.h | 7 ++++++-
3 files changed, 36 insertions(+), 6 deletions(-)
diff --git a/drivers/ptp/ptp_chardev.c b/drivers/ptp/ptp_chardev.c
index 7513018c9f9a..34cd0ab79b10 100644
--- a/drivers/ptp/ptp_chardev.c
+++ b/drivers/ptp/ptp_chardev.c
@@ -359,10 +359,13 @@ long ptp_ioctl(struct posix_clock_context *pccontext, unsigned int cmd,
break;
}
if (extoff->n_samples > PTP_MAX_SAMPLES
- || extoff->rsv[0] || extoff->rsv[1] || extoff->rsv[2]) {
+ || extoff->rsv[0] || extoff->rsv[1]
+ || (extoff->clockid != CLOCK_REALTIME
+ && extoff->clockid != CLOCK_MONOTONIC_RAW)) {
err = -EINVAL;
break;
}
+ sts.clockid = extoff->clockid;
for (i = 0; i < extoff->n_samples; i++) {
err = ptp->info->gettimex64(ptp->info, &ts, &sts);
if (err)
diff --git a/include/linux/ptp_clock_kernel.h b/include/linux/ptp_clock_kernel.h
index 6e4b8206c7d0..7563da6db09b 100644
--- a/include/linux/ptp_clock_kernel.h
+++ b/include/linux/ptp_clock_kernel.h
@@ -47,10 +47,12 @@ struct system_device_crosststamp;
* struct ptp_system_timestamp - system time corresponding to a PHC timestamp
* @pre_ts: system timestamp before capturing PHC
* @post_ts: system timestamp after capturing PHC
+ * @clockid: clockid used for cpaturing timestamp
*/
struct ptp_system_timestamp {
struct timespec64 pre_ts;
struct timespec64 post_ts;
+ clockid_t clockid;
};
/**
@@ -457,14 +459,34 @@ static inline ktime_t ptp_convert_timestamp(const ktime_t *hwtstamp,
static inline void ptp_read_system_prets(struct ptp_system_timestamp *sts)
{
- if (sts)
- ktime_get_real_ts64(&sts->pre_ts);
+ if (sts) {
+ switch (sts->clockid) {
+ case CLOCK_REALTIME:
+ ktime_get_real_ts64(&sts->pre_ts);
+ break;
+ case CLOCK_MONOTONIC_RAW:
+ ktime_get_raw_ts64(&sts->pre_ts);
+ break;
+ default:
+ break;
+ }
+ }
}
static inline void ptp_read_system_postts(struct ptp_system_timestamp *sts)
{
- if (sts)
- ktime_get_real_ts64(&sts->post_ts);
+ if (sts) {
+ switch (sts->clockid) {
+ case CLOCK_REALTIME:
+ ktime_get_real_ts64(&sts->post_ts);
+ break;
+ case CLOCK_MONOTONIC_RAW:
+ ktime_get_raw_ts64(&sts->post_ts);
+ break;
+ default:
+ break;
+ }
+ }
}
#endif
diff --git a/include/uapi/linux/ptp_clock.h b/include/uapi/linux/ptp_clock.h
index 053b40d642de..fc5825e72330 100644
--- a/include/uapi/linux/ptp_clock.h
+++ b/include/uapi/linux/ptp_clock.h
@@ -157,7 +157,12 @@ struct ptp_sys_offset {
struct ptp_sys_offset_extended {
unsigned int n_samples; /* Desired number of measurements. */
- unsigned int rsv[3]; /* Reserved for future use. */
+ /* The original implementation provided timestamps (always) in
+ * REALTIME clock-base. Since CLOCK_REALTIME is 0, adding
+ * clockid doesn't break backward compatibility.
+ */
+ clockid_t clockid; /* One of the supported clock-ids */
+ unsigned int rsv[2]; /* Reserved for future use. */
/*
* Array of [system, phc, system] time stamps. The kernel will provide
* 3*n_samples time stamps.
--
2.44.0.683.g7961c838ac-goog
On Tue, 16 Apr 2024 14:59:42 -0700 Mahesh Bandewar wrote:
> if (extoff->n_samples > PTP_MAX_SAMPLES
> - || extoff->rsv[0] || extoff->rsv[1] || extoff->rsv[2]) {
> + || extoff->rsv[0] || extoff->rsv[1]
> + || (extoff->clockid != CLOCK_REALTIME
> + && extoff->clockid != CLOCK_MONOTONIC_RAW)) {
Since you're touching this condition it's probably a good opportunity
to fox the coding style.
On Wed, Apr 17, 2024 at 6:24 PM Jakub Kicinski <[email protected]> wrote:
>
> On Tue, 16 Apr 2024 14:59:42 -0700 Mahesh Bandewar wrote:
> > if (extoff->n_samples > PTP_MAX_SAMPLES
> > - || extoff->rsv[0] || extoff->rsv[1] || extoff->rsv[2]) {
> > + || extoff->rsv[0] || extoff->rsv[1]
> > + || (extoff->clockid != CLOCK_REALTIME
> > + && extoff->clockid != CLOCK_MONOTONIC_RAW)) {
>
> Since you're touching this condition it's probably a good opportunity
> to fox the coding style.
will do! I was just matching the existing style.
On Wed, Apr 17, 2024 at 06:24:45PM -0700, Jakub Kicinski wrote:
> On Tue, 16 Apr 2024 14:59:42 -0700 Mahesh Bandewar wrote:
> > if (extoff->n_samples > PTP_MAX_SAMPLES
> > - || extoff->rsv[0] || extoff->rsv[1] || extoff->rsv[2]) {
> > + || extoff->rsv[0] || extoff->rsv[1]
> > + || (extoff->clockid != CLOCK_REALTIME
> > + && extoff->clockid != CLOCK_MONOTONIC_RAW)) {
>
> Since you're touching this condition it's probably a good opportunity
> to fox the coding style.
What is the CodingStyle violation?
Is it starting a line with || ?
Thanks,
Richard
On Thu, 18 Apr 2024 23:28:48 -0700 Richard Cochran wrote:
> On Wed, Apr 17, 2024 at 06:24:45PM -0700, Jakub Kicinski wrote:
> > On Tue, 16 Apr 2024 14:59:42 -0700 Mahesh Bandewar wrote:
> > > if (extoff->n_samples > PTP_MAX_SAMPLES
> > > - || extoff->rsv[0] || extoff->rsv[1] || extoff->rsv[2]) {
> > > + || extoff->rsv[0] || extoff->rsv[1]
> > > + || (extoff->clockid != CLOCK_REALTIME
> > > + && extoff->clockid != CLOCK_MONOTONIC_RAW)) {
> >
> > Since you're touching this condition it's probably a good opportunity
> > to fox the coding style.
>
> What is the CodingStyle violation?
>
> Is it starting a line with || ?
Yup. Now that it's a mix of || and && the loss of readability
to a kernel-trained eye starts to be impactful.