netdev - Re: [PATCH v2] posix-timers: add multi_clock

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Mon, 27 Nov 2023 16:46:03 -0800
From: John Stultz <jstultz@...gle.com>
To: Richard Cochran <richardcochran@...il.com>
Cc: Sagi Maimon <maimon.sagi@...il.com>, reibax@...il.com, davem@...emloft.net, 
	rrameshbabu@...dia.com, netdev@...r.kernel.org, linux-kernel@...r.kernel.org, 
	maheshb@...gle.com, Thomas Gleixner <tglx@...utronix.de>
Subject: Re: [PATCH v2] posix-timers: add multi_clock_gettime system call

On Mon, Nov 27, 2023 at 4:12 PM Richard Cochran
<richardcochran@...il.com> wrote:
>
> On Mon, Nov 27, 2023 at 05:39:01PM +0200, Sagi Maimon wrote:
> >  Some user space applications need to read some clocks.
> >  Each read requires moving from user space to kernel space.
> >  This asymmetry causes the measured offset to have a significant error.
>
> Adding time/clock gurus (jstultz, tglx) on CC for visibility...
>

Thanks for the heads up! (though, "guru" is just the noise I make
standing up these days)

> >  Introduce a new system call multi_clock_gettime, which can be used to measure
> >  the offset between multiple clocks, from variety of types: PHC, virtual PHC
> >  and various system clocks (CLOCK_REALTIME, CLOCK_MONOTONIC, etc).
> >  The offset includes the total time that the driver needs to read the clock
> >  timestamp.

This last bit about "offset includes the total time that the driver
needs to read the clock" is a bit confusing. It seems to suggest there
would be start/stop bookend timestamps so you could bound how long it
took to read all the clocks, but I don't see that in the patch.

> >  New system call allows the reading of a list of clocks - up to PTP_MAX_CLOCKS.
> >  Supported clocks IDs: PHC, virtual PHC and various system clocks.
> >  Up to PTP_MAX_SAMPLES times (per clock) in a single system call read.
> >  The system call returns n_clocks timestamps for each measurement:
> >  - clock 0 timestamp
> >  - ...
> >  - clock n timestamp
> >
> > Signed-off-by: Sagi Maimon <maimon.sagi@...il.com>

Overally, while I understand the intent, I'm pretty hesitant on it
(and "__ptp_multi_clock_get multi_clk_get" has me squinting to find
the actual space amongst all the underscores :).

If the overhead of reading clockids individually is too much, it seems
like the next thing will be folks wanting to export multiple raw
hardware counter values so the counter->ns transformation doesn't get
inbetween each hw clock read, which this interface wouldn't solve, so
we'd have to add yet another interface.

Also, I wonder if trying to get multiple clocks in one read seems
similar to something uio_ring might help with? Though I can't say I'm
very savvy with uio_ring. Have folks looked into that?

thanks
-john