| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <e56a994e-e329-4cac-9fff-e57139408769@linux.dev>
Date: Thu, 22 Aug 2024 13:26:24 +0100
From: Vadim Fedorenko <vadim.fedorenko@...ux.dev>
To: Mahesh Bandewar <maheshb@...gle.com>, Netdev <netdev@...r.kernel.org>,
David Miller <davem@...emloft.net>, Jakub Kicinski <kuba@...nel.org>,
Eric Dumazet <edumazet@...gle.com>, Paolo Abeni <pabeni@...hat.com>,
Thomas Gleixner <tglx@...utronix.de>,
Richard Cochran <richardcochran@...il.com>, Arnd Bergmann <arnd@...db.de>,
Sagi Maimon <maimon.sagi@...il.com>
Cc: Jonathan Corbet <corbet@....net>, John Stultz <jstultz@...gle.com>,
Mahesh Bandewar <mahesh@...dewar.net>
Subject: Re: [PATCHv4 net-next] ptp/ioctl: support MONOTONIC_RAW timestamps
for PTP_SYS_OFFSET_EXTENDED
On 02/05/2024 22:10, Mahesh Bandewar wrote:
> The ability to read the PHC (Physical Hardware Clock) alongside
> multiple system clocks is currently dependent on the specific
> hardware architecture. This limitation restricts the use of
> PTP_SYS_OFFSET_PRECISE to certain hardware configurations.
>
> The generic soultion which would work across all architectures
> is to read the PHC along with the latency to perform PHC-read as
> offered by PTP_SYS_OFFSET_EXTENDED which provides pre and post
> timestamps. However, these timestamps are currently limited
> to the CLOCK_REALTIME timebase. Since CLOCK_REALTIME is affected
> by NTP (or similar time synchronization services), it can
> experience significant jumps forward or backward. This hinders
> the precise latency measurements that PTP_SYS_OFFSET_EXTENDED
> is designed to provide.
>
> This problem could be addressed by supporting MONOTONIC_RAW
> timestamps within PTP_SYS_OFFSET_EXTENDED. Unlike CLOCK_REALTIME
> or CLOCK_MONOTONIC, the MONOTONIC_RAW timebase is unaffected
> by NTP adjustments.
>
> This enhancement can be implemented by utilizing one of the three
> reserved words within the PTP_SYS_OFFSET_EXTENDED struct to pass
> the clock-id for timestamps. The current behavior aligns with
> clock-id for CLOCK_REALTIME timebase (value of 0), ensuring
> backward compatibility of the UAPI.
>
> Signed-off-by: Mahesh Bandewar <maheshb@...gle.com>
Hi Manesh!
Any chance there will be v5 soon with the comments addressed?
I don't see any strong objections to the patch, and it looks
like we can benefit from having MONOTONIC_RAW clock for our
project too.
Thanks,
Vadim
> ---
> v1 -> v2
> * Code-style fixes.
> v2 -> v3
> * Reword commit log
> * Fix the compilation issue by using __kernel_clockid instead of clockid_t
> which has kernel only scope.
> v3 -> v4
> * Typo/comment fixes.
>
> drivers/ptp/ptp_chardev.c | 7 +++++--
> include/linux/ptp_clock_kernel.h | 30 ++++++++++++++++++++++++++----
> include/uapi/linux/ptp_clock.h | 27 +++++++++++++++++++++------
> 3 files changed, 52 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/ptp/ptp_chardev.c b/drivers/ptp/ptp_chardev.c
> index 7513018c9f9a..c109109c9e8e 100644
> --- a/drivers/ptp/ptp_chardev.c
> +++ b/drivers/ptp/ptp_chardev.c
> @@ -358,11 +358,14 @@ long ptp_ioctl(struct posix_clock_context *pccontext, unsigned int cmd,
> extoff = NULL;
> break;
> }
> - if (extoff->n_samples > PTP_MAX_SAMPLES
> - || extoff->rsv[0] || extoff->rsv[1] || extoff->rsv[2]) {
> + if (extoff->n_samples > PTP_MAX_SAMPLES ||
> + 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..74ded5f95d95 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: clock-base used for capturing the system timestamps
> */
> 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..5e3d70fbc499 100644
> --- a/include/uapi/linux/ptp_clock.h
> +++ b/include/uapi/linux/ptp_clock.h
> @@ -155,13 +155,28 @@ struct ptp_sys_offset {
> struct ptp_clock_time ts[2 * PTP_MAX_SAMPLES + 1];
> };
>
> +/*
> + * ptp_sys_offset_extended - data structure for IOCTL operation
> + * PTP_SYS_OFFSET_EXTENDED
> + *
> + * @n_samples: Desired number of measurements.
> + * @clockid: clockid of a clock-base used for pre/post timestamps.
> + * @rsv: Reserved for future use.
> + * @ts: Array of samples in the form [pre-TS, PHC, post-TS]. The
> + * kernel provides @n_samples.
> + *
> + * History:
> + * v1: Initial implementation.
> + *
> + * v2: Use the first word of the reserved-field for @clockid. That's
> + * backward compatible since v1 expects all three reserved words
> + * (@rsv[3]) to be 0 while the clockid (first word in v2) for
> + * CLOCK_REALTIME is '0'.
> + */
> struct ptp_sys_offset_extended {
> - unsigned int n_samples; /* Desired number of measurements. */
> - unsigned int rsv[3]; /* Reserved for future use. */
> - /*
> - * Array of [system, phc, system] time stamps. The kernel will provide
> - * 3*n_samples time stamps.
> - */
> + unsigned int n_samples;
> + __kernel_clockid_t clockid;
> + unsigned int rsv[2];
> struct ptp_clock_time ts[PTP_MAX_SAMPLES][3];
> };
>
Powered by blists - more mailing lists