Re: [chrony-dev] Support for Multiple PPS Inputs on single PHC
On 2/23/23 4:19 PM, Richard Cochran wrote: On Thu, Feb 23, 2023 at 12:56:34PM -0800, Matt Corallo wrote: There's two separate questions here - multiple readers receiving the same data, and multiple readers receiving data exclusively about one channel. I'd imagine the second is (much?) easier to implement, whereas the first is a bunch of complexity. This second idea would require a new API, so that user could select a particular channel. First idea would only change kernel behavior without changing the API. Fair point. I figured a new IOCTL to filter was a lighter lift, even if a bunch of boilerplate to define it. I'm happy to take a crack at something to get the ball rolling, though not this week. I'm sure I could copy+paste to make a new IOCTL work, but adding relevant queue limiting means I have to go read much more kernel code to figure out which datastructures already exist there :). It sounds like I should go replace the extts queue with a circular buffer, have every reader socket store an index in the buffer, and new sockets read only futures pulses? I assume a new pulse already wakes all select()ers on the sockets so nothing would need to change there. Is there some existing code somewhere I should crib off of or just run and see where I get? Thanks, Matt -- To unsubscribe email chrony-dev-requ...@chrony.tuxfamily.org with "unsubscribe" in the subject. For help email chrony-dev-requ...@chrony.tuxfamily.org with "help" in the subject. Trouble? Email listmas...@chrony.tuxfamily.org.
Re: [chrony-dev] Support for Multiple PPS Inputs on single PHC
On 2/20/23 7:24 AM, Richard Cochran wrote: On Mon, Feb 20, 2023 at 11:08:23AM +0100, Miroslav Lichvar wrote: Does it need to be that way? It seems strange for the kernel to support enabling PPS on multiple channels at the same time, but not allow multiple applications to receive all samples from their channel. It does not need to be that way, but nobody ever wanted multiple readers before. Implementing this would make the kernel side much more complex, as the code would need per-reader tracking of the buffered time stamps, or per-reader fifo buffers, etc. There's two separate questions here - multiple readers receiving the same data, and multiple readers receiving data exclusively about one channel. I'd imagine the second is (much?) easier to implement, whereas the first is a bunch of complexity. At least personally I'm okay with the second, rather than the first, and that fixes the issue for chrony, though it doesn't allow one to, say, get raw samples in one program while having another handle them. Matt -- To unsubscribe email chrony-dev-requ...@chrony.tuxfamily.org with "unsubscribe" in the subject. For help email chrony-dev-requ...@chrony.tuxfamily.org with "help" in the subject. Trouble? Email listmas...@chrony.tuxfamily.org.
Re: [chrony-dev] Support for Multiple PPS Inputs on single PHC
On Thu, Feb 16, 2023 at 02:54:29PM -0800, Richard Cochran wrote: > Each extts in the fifo is delivered only once. If there are multiple > readers, each reader will receive only some of the data. This is > similar to how a pipe behaves. Does it need to be that way? It seems strange for the kernel to support enabling PPS on multiple channels at the same time, but not allow multiple applications to receive all samples from their channel. -- Miroslav Lichvar -- To unsubscribe email chrony-dev-requ...@chrony.tuxfamily.org with "unsubscribe" in the subject. For help email chrony-dev-requ...@chrony.tuxfamily.org with "help" in the subject. Trouble? Email listmas...@chrony.tuxfamily.org.
Re: [chrony-dev] Support for Multiple PPS Inputs on single PHC
I take that as kernel is not likely to change. Since I got around to using this myself I hacked up a patch (next email). Its definitely the "how do I make this work" patch, not something where I spent a while digging into all of chrony's internals to make sure things match how it's "supposed to be done". top seems to think there's no threads to worry about, so I assume the mutex can be dropped with no impact, but I left it cause I wasn't sure. There's probably a cleaner way to do this, and I'm happy to clean up the patch from feedback if it makes sense, or y'all can replace with a more correct solution. Thanks, Matt On 2/16/23 4:58 PM, Matt Corallo wrote: On 2/16/23 2:54 PM, Richard Cochran wrote: On Thu, Feb 16, 2023 at 09:54:56AM -0800, Matt Corallo wrote: As for duplicating the output across sockets, ptp_chardev.c's `ptp_read` is pretty trivial - just pop the next sample off the queue and return it. Tweaking that to copy the sample into every reader is probably above my paygrade (and has a whole host of leak risk I'd probably screw up). `extts_fifo_show` appears to be functionally identical. Each extts in the fifo is delivered only once. If there are multiple readers, each reader will receive only some of the data. This is similar to how a pipe behaves. Right, sorry if the context wasn't clear, I only realized part of the message was removed in the first reply after sending. The question from Miroslav was, basically, "would kernel accept something to only get notified of extts pulses on a given channel, and, if so, how would we go about doing that". The "we get pulses from all extts channels on the same socket" thing is a bit annoying to munge into chrony - it has the concept of "refclocks" which are a single clock, in this case a single pps pulse generator. If you have two of them on the same PTP clock but coming in on different pins/channels it doesn't have a way to express that outside of two refclocks. While we could take pulses from both refclocks on one socket and shove them into some queue and have the refclocks pick that data up its a bunch of complexity on the client side and not super clean in the current codebase. Thanks, Matt -- To unsubscribe email chrony-dev-requ...@chrony.tuxfamily.org with "unsubscribe" in the subject. For help email chrony-dev-requ...@chrony.tuxfamily.org with "help" in the subject. Trouble? Email listmas...@chrony.tuxfamily.org.
Re: [chrony-dev] Support for Multiple PPS Inputs on single PHC
On 2/16/23 2:54 PM, Richard Cochran wrote: On Thu, Feb 16, 2023 at 09:54:56AM -0800, Matt Corallo wrote: As for duplicating the output across sockets, ptp_chardev.c's `ptp_read` is pretty trivial - just pop the next sample off the queue and return it. Tweaking that to copy the sample into every reader is probably above my paygrade (and has a whole host of leak risk I'd probably screw up). `extts_fifo_show` appears to be functionally identical. Each extts in the fifo is delivered only once. If there are multiple readers, each reader will receive only some of the data. This is similar to how a pipe behaves. Right, sorry if the context wasn't clear, I only realized part of the message was removed in the first reply after sending. The question from Miroslav was, basically, "would kernel accept something to only get notified of extts pulses on a given channel, and, if so, how would we go about doing that". The "we get pulses from all extts channels on the same socket" thing is a bit annoying to munge into chrony - it has the concept of "refclocks" which are a single clock, in this case a single pps pulse generator. If you have two of them on the same PTP clock but coming in on different pins/channels it doesn't have a way to express that outside of two refclocks. While we could take pulses from both refclocks on one socket and shove them into some queue and have the refclocks pick that data up its a bunch of complexity on the client side and not super clean in the current codebase. Thanks, Matt -- To unsubscribe email chrony-dev-requ...@chrony.tuxfamily.org with "unsubscribe" in the subject. For help email chrony-dev-requ...@chrony.tuxfamily.org with "help" in the subject. Trouble? Email listmas...@chrony.tuxfamily.org.
Re: [chrony-dev] Support for Multiple PPS Inputs on single PHC
On 2/16/23 12:19 AM, Miroslav Lichvar wrote: On Wed, Feb 15, 2023 at 10:27:15PM -0800, Matt Corallo wrote: My naive solution from skimming the code would be to shove formerly-discarded samples into a global limited queue and check for available timestamps in `phc_poll`. However, I have no idea if the time difference between when the sample was taken by the hardware and when the `HCL_CookTime` call is done would impact accuracy (or maybe the opposite, since we'd then be cooking time with the hardware clock right after taking the HCL sample rather than when the PHC timestamp happens), or if such a patch would simply be rejected as a dirty, dirty hack rather than unifying the PHC read sockets across the devices into one socket (via some global tracking the device -> socket mapping?) and passing the samples out appropriately. Let me know what makes the most sense here. My first thought is that this should be addressed in the kernel, so even different processes having open the PHC device can receive all extts samples. If it turns out it's too difficult to do for the character device (I'm not very familiar with that subsystem), maybe it could be done at least in sysfs (/sys/class/ptp/ptp*/fifo or a new file showing the last event like the PPS assert and clear). I mean my first thought seeing an ioctl on a socket that gives an explicit channel and then receives crap from other channels on the same socket was "wtf" so I went and read the kernel to figure out why first to see if its a driver bug. I can't seem to find *any* documentation for how these ioctls are supposed to work, but it seems the "request" here is kinda misnomer, its really a "configure hardware" request, and is unrelated to future reads on the socket, or really the specific socket at all. As for duplicating the output across sockets, ptp_chardev.c's `ptp_read` is pretty trivial - just pop the next sample off the queue and return it. Tweaking that to copy the sample into every reader is probably above my paygrade (and has a whole host of leak risk I'd probably screw up). `extts_fifo_show` appears to be functionally identical. I've CC'd the MAINTAINERs for ptp to see what they think about this, though it won't let chrony support this without a kernel upgrade - not sure if that's an issue for chrony or not. Matt -- To unsubscribe email chrony-dev-requ...@chrony.tuxfamily.org with "unsubscribe" in the subject. For help email chrony-dev-requ...@chrony.tuxfamily.org with "help" in the subject. Trouble? Email listmas...@chrony.tuxfamily.org.
Re: [chrony-dev] Support for Multiple PPS Inputs on single PHC
On Wed, Feb 15, 2023 at 10:27:15PM -0800, Matt Corallo wrote: > My naive solution from skimming the code would be to shove > formerly-discarded samples into a global limited queue and check for > available timestamps in `phc_poll`. However, I have no idea if the time > difference between when the sample was taken by the hardware and when the > `HCL_CookTime` call is done would impact accuracy (or maybe the opposite, > since we'd then be cooking time with the hardware clock right after taking > the HCL sample rather than when the PHC timestamp happens), or if such a > patch would simply be rejected as a dirty, dirty hack rather than unifying > the PHC read sockets across the devices into one socket (via some global > tracking the device -> socket mapping?) and passing the samples out > appropriately. Let me know what makes the most sense here. My first thought is that this should be addressed in the kernel, so even different processes having open the PHC device can receive all extts samples. If it turns out it's too difficult to do for the character device (I'm not very familiar with that subsystem), maybe it could be done at least in sysfs (/sys/class/ptp/ptp*/fifo or a new file showing the last event like the PPS assert and clear). -- Miroslav Lichvar -- To unsubscribe email chrony-dev-requ...@chrony.tuxfamily.org with "unsubscribe" in the subject. For help email chrony-dev-requ...@chrony.tuxfamily.org with "help" in the subject. Trouble? Email listmas...@chrony.tuxfamily.org.
