Re: [Linuxptp-devel] [PATCH] phc2sys: provide missing kernel headers for sysoff functionality
On Thu, Jun 25, 2020 at 01:21:34AM +0300, Vladimir Oltean wrote: > Currently it is very finicky to deploy linuxptp in an automated build > system and make KBUILD_OUTPUT pick up the output of "make > headers_install" in order for the application to make full use of the > features exposed by the runtime kernel. And the toolchain/libc will > almost certainly never contain recent enough kernel headers to be of any > use here. And there's no good reason for that: the application can probe > at runtime for the sysoff methods supported by the kernel anyway. > > So let's provide the kernel definitions for sysoff, sysoff_precise and > sysoff_extended, such that SYSOFF_COMPILE_TIME_MISSING is not something > that will bother us any longer. > > Signed-off-by: Vladimir Oltean > --- > Changes since RFC: > Made sure that KBUILD_OUTPUT=$(path to linux-3.0.y headers) still > compiles properly. Thanks for following up... Applied. Thanks, Richard ___ Linuxptp-devel mailing list Linuxptp-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
[Linuxptp-devel] [PATCH] phc2sys: provide missing kernel headers for sysoff functionality
Currently it is very finicky to deploy linuxptp in an automated build
system and make KBUILD_OUTPUT pick up the output of "make
headers_install" in order for the application to make full use of the
features exposed by the runtime kernel. And the toolchain/libc will
almost certainly never contain recent enough kernel headers to be of any
use here. And there's no good reason for that: the application can probe
at runtime for the sysoff methods supported by the kernel anyway.
So let's provide the kernel definitions for sysoff, sysoff_precise and
sysoff_extended, such that SYSOFF_COMPILE_TIME_MISSING is not something
that will bother us any longer.
Signed-off-by: Vladimir Oltean
---
Changes since RFC:
Made sure that KBUILD_OUTPUT=$(path to linux-3.0.y headers) still
compiles properly.
missing.h | 52
sysoff.c | 27 +--
sysoff.h | 2 +-
3 files changed, 54 insertions(+), 27 deletions(-)
diff --git a/missing.h b/missing.h
index bc708cbbbe8a..35eaf155fc51 100644
--- a/missing.h
+++ b/missing.h
@@ -97,6 +97,58 @@ struct compat_ptp_clock_caps {
#endif /*LINUX_VERSION_CODE < 5.8*/
+#ifndef PTP_MAX_SAMPLES
+#define PTP_MAX_SAMPLES 25 /* Maximum allowed offset measurement samples. */
+#endif /* PTP_MAX_SAMPLES */
+
+#ifndef PTP_SYS_OFFSET
+
+#define PTP_SYS_OFFSET _IOW(PTP_CLK_MAGIC, 5, struct ptp_sys_offset)
+
+struct ptp_sys_offset {
+ unsigned int n_samples; /* Desired number of measurements. */
+ unsigned int rsv[3];/* Reserved for future use. */
+ /*
+* Array of interleaved system/phc time stamps. The kernel
+* will provide 2*n_samples + 1 time stamps, with the last
+* one as a system time stamp.
+*/
+ struct ptp_clock_time ts[2 * PTP_MAX_SAMPLES + 1];
+};
+
+#endif /* PTP_SYS_OFFSET */
+
+#ifndef PTP_SYS_OFFSET_PRECISE
+
+#define PTP_SYS_OFFSET_PRECISE \
+ _IOWR(PTP_CLK_MAGIC, 8, struct ptp_sys_offset_precise)
+
+struct ptp_sys_offset_precise {
+ struct ptp_clock_time device;
+ struct ptp_clock_time sys_realtime;
+ struct ptp_clock_time sys_monoraw;
+ unsigned int rsv[4];/* Reserved for future use. */
+};
+
+#endif /* PTP_SYS_OFFSET_PRECISE */
+
+#ifndef PTP_SYS_OFFSET_EXTENDED
+
+#define PTP_SYS_OFFSET_EXTENDED \
+ _IOWR(PTP_CLK_MAGIC, 9, struct ptp_sys_offset_extended)
+
+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.
+*/
+ struct ptp_clock_time ts[PTP_MAX_SAMPLES][3];
+};
+
+#endif /* PTP_SYS_OFFSET_EXTENDED */
+
#ifndef PTP_PIN_SETFUNC
enum ptp_pin_function {
diff --git a/sysoff.c b/sysoff.c
index ec77a0d4dc63..274385974c04 100644
--- a/sysoff.c
+++ b/sysoff.c
@@ -27,8 +27,6 @@
#define NS_PER_SEC 10LL
-#ifdef PTP_SYS_OFFSET
-
static int64_t pctns(struct ptp_clock_time *t)
{
return t->sec * NS_PER_SEC + t->nsec;
@@ -36,7 +34,6 @@ static int64_t pctns(struct ptp_clock_time *t)
static int sysoff_precise(int fd, int64_t *result, uint64_t *ts)
{
-#ifdef PTP_SYS_OFFSET_PRECISE
struct ptp_sys_offset_precise pso;
memset(&pso, 0, sizeof(pso));
if (ioctl(fd, PTP_SYS_OFFSET_PRECISE, &pso)) {
@@ -46,9 +43,6 @@ static int sysoff_precise(int fd, int64_t *result, uint64_t
*ts)
*result = pctns(&pso.sys_realtime) - pctns(&pso.device);
*ts = pctns(&pso.sys_realtime);
return SYSOFF_PRECISE;
-#else
- return SYSOFF_COMPILE_TIME_MISSING;
-#endif
}
static int64_t sysoff_estimate(struct ptp_clock_time *pct, int extended,
@@ -99,7 +93,6 @@ static int64_t sysoff_estimate(struct ptp_clock_time *pct,
int extended,
static int sysoff_extended(int fd, int n_samples,
int64_t *result, uint64_t *ts, int64_t *delay)
{
-#ifdef PTP_SYS_OFFSET_EXTENDED
struct ptp_sys_offset_extended pso;
memset(&pso, 0, sizeof(pso));
pso.n_samples = n_samples;
@@ -109,9 +102,6 @@ static int sysoff_extended(int fd, int n_samples,
}
*result = sysoff_estimate(&pso.ts[0][0], 1, n_samples, ts, delay);
return SYSOFF_EXTENDED;
-#else
- return SYSOFF_COMPILE_TIME_MISSING;
-#endif
}
static int sysoff_basic(int fd, int n_samples,
@@ -140,7 +130,7 @@ int sysoff_measure(int fd, int method, int n_samples,
case SYSOFF_BASIC:
return sysoff_basic(fd, n_samples, result, ts, delay);
}
- return SYSOFF_COMPILE_TIME_MISSING;
+ return SYSOFF_RUN_TIME_MISSING;
}
int sysoff_probe(int fd, int n_samples)
@@ -164,18 +154,3 @@ int sysoff_probe(int fd, int n_samples)
return SYSOFF_RUN_TIME_MISSING;
}
-
-#else /* !PTP_SYS_OFFSET */
-
-int sysoff_measure(int fd, int method, int n_samples,
- int64_t
Re: [Linuxptp-devel] [PATCH v2 2/3] clock: dump unexpected packets received on the error queues of sockets
On Wed, 24 Jun 2020 at 21:22, Richard Cochran wrote: > > On Wed, Jun 24, 2020 at 09:16:42PM +0300, Vladimir Oltean wrote: > > Treating it as an event on a data queue is a bug. > > Yeah, I guess we have just been lucky all this time. I can sense some irony here, but I don't see your point though? I mean, take any board which has a DSA master that supports PTP timestamping and a DSA switch that also supports PTP timestamping, and run that "tcpdump -j adapter_unsynced" command on the DSA master. If you don't have my patch that prevents that from happening: https://patchwork.ozlabs.org/project/netdev/patch/20191228133046.9406-3-olte...@gmail.com/ then what'll happen is exactly what happened to me. Although in my case it was due to a driver bug, and in the tcpdump case it is due to a systematic limitation of the SO_TIMESTAMPING API. Not only that, but when you're coming from the angle that "tcpdump broke ptp4l", then you at least have some sort of rope to cling on to (if you want to, that is). And I _guarantee_ to you that somebody out there ran into that oddity at least once before, said "huh, weird, what a buggy system" and went on with their life. So, not really worth it to investigate, when there's something you can do to avoid the problem. But just blowing off the fact that it _is_ easy to reproduce even without "odd" driver bugs is pushing it a bit, IMO. So, I guess we _have_ been lucky all this time, in the "ignorance is bliss" sense. The ptp4l program does the same thing in both cases, and it isn't something that is correct given its options. Thanks, -Vladimir ___ Linuxptp-devel mailing list Linuxptp-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
Re: [Linuxptp-devel] [PATCH v2 2/3] clock: dump unexpected packets received on the error queues of sockets
On Wed, Jun 24, 2020 at 09:16:42PM +0300, Vladimir Oltean wrote: > Treating it as an event on a data queue is a bug. Yeah, I guess we have just been lucky all this time. ___ Linuxptp-devel mailing list Linuxptp-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
Re: [Linuxptp-devel] [PATCH v2 2/3] clock: dump unexpected packets received on the error queues of sockets
On Wed, 24 Jun 2020 at 21:12, Richard Cochran wrote: > > On Wed, Jun 24, 2020 at 09:01:33PM +0300, Vladimir Oltean wrote: > > I think it boils down to something very simple. > > The kernel is waking up a user process with revents = POLLIN | > > POLLERR. That has a certain meaning. > > Then the process says "Oh, yay, I have revents = POLLIN. Let me > > process that!". That has an entirely different meaning. > > I agree. > > In this specific case, if POLLERR appears, it means that some unknown > error occurred, and the application can't do anything about it beyond > throwing a generic fault. This is what I would call "defensive" > programming. > Treating it in a meaningful way for what it is, or for what it can be in the future (an event on an error/control socket) is defensive programming. Treating it as an event on a data queue is a bug. > If and when the kernel introduces new POLLERR events, we will of > course implement the appropriate handlers. But as of today there are > no such events to handle. > > Thanks, > Richard > Thanks, -Vladimir ___ Linuxptp-devel mailing list Linuxptp-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
Re: [Linuxptp-devel] [PATCH v2 2/3] clock: dump unexpected packets received on the error queues of sockets
On Wed, Jun 24, 2020 at 09:01:33PM +0300, Vladimir Oltean wrote: > I think it boils down to something very simple. > The kernel is waking up a user process with revents = POLLIN | > POLLERR. That has a certain meaning. > Then the process says "Oh, yay, I have revents = POLLIN. Let me > process that!". That has an entirely different meaning. I agree. In this specific case, if POLLERR appears, it means that some unknown error occurred, and the application can't do anything about it beyond throwing a generic fault. This is what I would call "defensive" programming. If and when the kernel introduces new POLLERR events, we will of course implement the appropriate handlers. But as of today there are no such events to handle. Thanks, Richard ___ Linuxptp-devel mailing list Linuxptp-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
Re: [Linuxptp-devel] [PATCH v2 2/3] clock: dump unexpected packets received on the error queues of sockets
On Wed, 24 Jun 2020 at 20:56, Richard Cochran wrote: > > On Wed, Jun 24, 2020 at 08:21:22PM +0300, Vladimir Oltean wrote: > > > What contract, who said that this control channel is _only_ for TX > > timestamps, and for how many TX timestamps it is? > > Um, there can't be more time stamps than transmitted frames. > > > If a future kernel decided to send more data to programs who opted in > > to the error queue, and that kernel decision were to break ptp4l, > > could you honestly say it's the kernel's fault? > > You are missing the point entirely. This is about poll(2) and not > about the socket error queue. POLLERR might possibly one day acquire > some kind of push semantics (i.e. unwanted by user space) on the kind > of DGRAM sockets we use. In addition, if ever a pipe-like transport > appears (and I think it not unlikely), then POLLERR will definitely > appear, and this patch prepares for that day. > > If a given driver is spewing out extra time stamps, then it is most > definitely the kernel's fault! > > Thanks, > Richard > > I think it boils down to something very simple. The kernel is waking up a user process with revents = POLLIN | POLLERR. That has a certain meaning. Then the process says "Oh, yay, I have revents = POLLIN. Let me process that!". That has an entirely different meaning. Timestamps are just excuses here. Thanks, -Vladimir ___ Linuxptp-devel mailing list Linuxptp-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
Re: [Linuxptp-devel] [PATCH v2 2/3] clock: dump unexpected packets received on the error queues of sockets
On Wed, Jun 24, 2020 at 05:52:39PM +, Keller, Jacob E wrote: > Sure, but the POLLERR could (in theory, not in current practice) return other > events/messages besides timestamps. > > It was invented/created prior to the Tx timestamps, wasn't it? Yes, and this is the point! Thanks, Richard ___ Linuxptp-devel mailing list Linuxptp-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
Re: [Linuxptp-devel] [PATCH v2 2/3] clock: dump unexpected packets received on the error queues of sockets
On Wed, Jun 24, 2020 at 08:21:22PM +0300, Vladimir Oltean wrote: > What contract, who said that this control channel is _only_ for TX > timestamps, and for how many TX timestamps it is? Um, there can't be more time stamps than transmitted frames. > If a future kernel decided to send more data to programs who opted in > to the error queue, and that kernel decision were to break ptp4l, > could you honestly say it's the kernel's fault? You are missing the point entirely. This is about poll(2) and not about the socket error queue. POLLERR might possibly one day acquire some kind of push semantics (i.e. unwanted by user space) on the kind of DGRAM sockets we use. In addition, if ever a pipe-like transport appears (and I think it not unlikely), then POLLERR will definitely appear, and this patch prepares for that day. If a given driver is spewing out extra time stamps, then it is most definitely the kernel's fault! Thanks, Richard ___ Linuxptp-devel mailing list Linuxptp-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
Re: [Linuxptp-devel] [PATCH v2 2/3] clock: dump unexpected packets received on the error queues of sockets
> -Original Message- > From: Richard Cochran > Sent: Wednesday, June 24, 2020 9:58 AM > To: Vladimir Oltean > Cc: linuxptp-devel@lists.sourceforge.net; Keller, Jacob E > > Subject: Re: [PATCH v2 2/3] clock: dump unexpected packets received on the > error queues of sockets > > On Wed, Jun 24, 2020 at 07:36:47PM +0300, Vladimir Oltean wrote: > > I am reading this as: "let's be defensive even in the case where the > > kernel decides to go nuts and push us a packet on the error queue even > > if we didn't enable SO_SELECT_ERR_QUEUE". But that isn't at all what's > > going on. As stated, we opted in this SO_SELECT_ERR_QUEUE game because > > we need TX timestamps. > > But, at this point in the program, we know that no tx time stamp is > outstanding. We always send one Tx event, then immediately fetch the > time stamp. This is carefully synchronized by the program. It is > important to do this because many drivers support exactly one Tx time > stamp at a time. > > The kernel must not fabricate transmit time stamps! That would be > breaking the contract. Sure, but the POLLERR could (in theory, not in current practice) return other events/messages besides timestamps. It was invented/created prior to the Tx timestamps, wasn't it? > > > So we need to be correct, and play by the API's > > rules, which means treat the POLLERR returned event. It is a > > correctness issue, not a defense issue. > > I think you are splitting hairs here, but I disagree that the program > was incorrect. There is no reason _today_ for poll to return a > POLLERR event from this call, but, in general, I don't believe this is > guaranteed by anything. > Right. Today there's no other messages that I am aware of. > Would you prefer me leaving your name off the commit message? > > Thanks, > Richard > > ___ Linuxptp-devel mailing list Linuxptp-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
Re: [Linuxptp-devel] [PATCH v2 2/3] clock: dump unexpected packets received on the error queues of sockets
On Wed, 24 Jun 2020 at 19:58, Richard Cochran wrote: > > On Wed, Jun 24, 2020 at 07:36:47PM +0300, Vladimir Oltean wrote: > > I am reading this as: "let's be defensive even in the case where the > > kernel decides to go nuts and push us a packet on the error queue even > > if we didn't enable SO_SELECT_ERR_QUEUE". But that isn't at all what's > > going on. As stated, we opted in this SO_SELECT_ERR_QUEUE game because > > we need TX timestamps. > > But, at this point in the program, we know that no tx time stamp is > outstanding. We always send one Tx event, then immediately fetch the > time stamp. This is carefully synchronized by the program. It is > important to do this because many drivers support exactly one Tx time > stamp at a time. > > The kernel must not fabricate transmit time stamps! That would be > breaking the contract. > Luckily the author of that socket option, and 1 of the 2 people on CC on that patch, are present in this email thread: https://patchwork.ozlabs.org/project/netdev/patch/20130327220028.13267.89112.st...@jekeller-hub.jf.intel.com/ So, I think you or Jacob would be in a good position to answer: What contract, who said that this control channel is _only_ for TX timestamps, and for how many TX timestamps it is? If a future kernel decided to send more data to programs who opted in to the error queue, and that kernel decision were to break ptp4l, could you honestly say it's the kernel's fault? I mean, those packets are already associated with a cmsg, you're supposed to only filter the one you're interested in, and ignore everything else. But ptp4l is clearly not doing that. > > So we need to be correct, and play by the API's > > rules, which means treat the POLLERR returned event. It is a > > correctness issue, not a defense issue. > > I think you are splitting hairs here, but I disagree that the program > was incorrect. There is no reason _today_ for poll to return a > POLLERR event from this call, but, in general, I don't believe this is > guaranteed by anything. > I am splitting the hairs I have left, really. Life would have been _so_much_simpler_ if ptp4l just had this POLLERR check. Who am I to guess what's going on if ptp4l gets a POLLERR event and treats it like a POLLIN? > Would you prefer me leaving your name off the commit message? > If we can't reconcile the fact that this is fixing an API misuse, then you can take my name off of it, sure. I care more about seeing the code modification go in, than I do about seeing my name on it. > Thanks, > Richard > > > Thanks, -Vladimir ___ Linuxptp-devel mailing list Linuxptp-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
Re: [Linuxptp-devel] [PATCH v2 3/3] port: print sync/follow-up mismatch events
On Mon, Jun 15, 2020 at 06:23:21PM +0300, Vladimir Oltean wrote: > ptp4l is too silent when receiving, for whatever reason, out of order > messages. If the reordering is persistent (which is either a broken > network, or a broken kernel), the behavior looks like a complete > synchronization stall, since the application is designed to never > attempt to recover from such a condition. > > At least save some people some debugging hours and print when the > application reaches this code path. Since it's a debugging tool, we > don't want to create false alarms when the occasional packet gets > reordered in a production system, but have this information readily > available when the program's log level is set to debug, instead of > having users fish for it with code instrumentation. > > Signed-off-by: Vladimir Oltean Applied. Thanks, Richard ___ Linuxptp-devel mailing list Linuxptp-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
Re: [Linuxptp-devel] [PATCH v2 1/3] ptp4l: call recvmsg() with the MSG_DONTWAIT flag
On Mon, Jun 15, 2020 at 06:23:19PM +0300, Vladimir Oltean wrote: > The application's main event loop (clock_poll) is woken up by poll() and > dispatches the socket receive queue events to the corresponding ports as > needed. > > So it is a bug if poll() wakes up the process for data availability on a > socket's receive queue, and then recvmsg(), called immediately > afterwards, goes to sleep trying to retrieve it. This patch will > generate an error that will be propagated to the user if this condition > happens. > > Can it happen? > > As of this patch, ptp4l uses the SO_SELECT_ERR_QUEUE socket option, > which means that poll() will wake the process up, with revents == > (POLLIN | POLLERR), if data is available in the error queue. But > clock_poll() does not check POLLERR, just POLLIN, and draws the wrong > conclusion that there is data available in the receive queue (when it is > in fact available in the error queue). > > When the above condition happens, recvmsg() will sleep typically for a > whole sync interval waiting for data on the event socket, and will be > woken up when the new real frame arrives. It will not dequeue follow-up > messages during this time (which are sent to the general message socket) > and when it does, it will already be late for them (their seqid will be > out of order). So it will drop them and everything that comes after. The > synchronization process will fail. > > The above condition shouldn't typically happen, but exceptional kernel > events will trigger it. It helps to be strict in ptp4l in order for > those events to not blow up in even stranger symptoms unrelated to the > root cause of the problem. > > Signed-off-by: Vladimir Oltean Applied. Thanks, Richard ___ Linuxptp-devel mailing list Linuxptp-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
Re: [Linuxptp-devel] [PATCH] Eliminate isort
On Sun, Jun 21, 2020 at 11:14:45AM +0200, Georg Sauthoff wrote: > This saves a few bytes of static storage and less instructions are > executed when looking for the best offset. > > Signed-off-by: Georg Sauthoff Applied. Thanks, Richard ___ Linuxptp-devel mailing list Linuxptp-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
Re: [Linuxptp-devel] [PATCH v2 2/3] clock: dump unexpected packets received on the error queues of sockets
On Wed, Jun 24, 2020 at 07:36:47PM +0300, Vladimir Oltean wrote: > I am reading this as: "let's be defensive even in the case where the > kernel decides to go nuts and push us a packet on the error queue even > if we didn't enable SO_SELECT_ERR_QUEUE". But that isn't at all what's > going on. As stated, we opted in this SO_SELECT_ERR_QUEUE game because > we need TX timestamps. But, at this point in the program, we know that no tx time stamp is outstanding. We always send one Tx event, then immediately fetch the time stamp. This is carefully synchronized by the program. It is important to do this because many drivers support exactly one Tx time stamp at a time. The kernel must not fabricate transmit time stamps! That would be breaking the contract. > So we need to be correct, and play by the API's > rules, which means treat the POLLERR returned event. It is a > correctness issue, not a defense issue. I think you are splitting hairs here, but I disagree that the program was incorrect. There is no reason _today_ for poll to return a POLLERR event from this call, but, in general, I don't believe this is guaranteed by anything. Would you prefer me leaving your name off the commit message? Thanks, Richard ___ Linuxptp-devel mailing list Linuxptp-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
Re: [Linuxptp-devel] [PATCH RFC] phc2sys: provide missing kernel headers for sysoff functionality
On Tue, Jun 23, 2020 at 12:27:27PM +0300, Vladimir Oltean wrote: > Currently it is very finicky to deploy linuxptp in an automated build > system and make KBUILD_OUTPUT pick up the output of "make > headers_install" in order for the application to make full use of the > features exposed by the runtime kernel. And the toolchain/libc will > almost certainly never contain recent enough kernel headers to be of any > use here. And there's no good reason for that: the application can probe > at runtime for the sysoff methods supported by the kernel anyway. > > So let's provide the kernel definitions for sysoff, sysoff_precise and > sysoff_extended, such that SYSOFF_COMPILE_TIME_MISSING is not something > that will bother us any longer. I like the general idea, but this breaks when compiling against Linux 3.0 headers. We support Linux 3.0 and newer, and we do, in fact, have users still on that kernel! In file included from /home/richard/git/linuxptp/phc.h:22, from /home/richard/git/linuxptp/phc.c:29: /home/richard/git/linuxptp/missing.h:112:31: error: ‘PTP_MAX_SAMPLES’ undeclared here (not in a function) struct ptp_clock_time ts[2 * PTP_MAX_SAMPLES + 1]; ^~~ make: *** [: phc.o] Error 1 make: *** Waiting for unfinished jobs In file included from /home/richard/git/linuxptp/rtnl.c:31: /home/richard/git/linuxptp/missing.h:112:31: error: ‘PTP_MAX_SAMPLES’ undeclared here (not in a function) struct ptp_clock_time ts[2 * PTP_MAX_SAMPLES + 1]; ^~~ and so on ... ___ Linuxptp-devel mailing list Linuxptp-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
Re: [Linuxptp-devel] [PATCH v2 2/3] clock: dump unexpected packets received on the error queues of sockets
Hi Richard, On Wed, 24 Jun 2020 at 19:27, Richard Cochran wrote: > > On Fri, Jun 19, 2020 at 10:45:23PM +0300, Vladimir Oltean wrote: > > Yes, much better. > > May I add your tag to the commit message, like this? I think the commit description is slightly lossy and a bit misleading. > > Catch unexpected socket polling errors. This part is ok. > > The poll(2) system call may set POLLERR in the returned events. Normally > no errors are returned unless specifically requested by setting an > appropriate socket option. Socket option which _is_ set, that's how we get our TX timestamps, not sure why you see the need to mention this. > Nevertheless, the poll(2) API is quite generic, > and there is no guarantee that the kernel networking stack might push an > error event one day. This patch adds defensive code in order to catch any > unexpected error condition. > I am reading this as: "let's be defensive even in the case where the kernel decides to go nuts and push us a packet on the error queue even if we didn't enable SO_SELECT_ERR_QUEUE". But that isn't at all what's going on. As stated, we opted in this SO_SELECT_ERR_QUEUE game because we need TX timestamps. So we need to be correct, and play by the API's rules, which means treat the POLLERR returned event. It is a correctness issue, not a defense issue. > Suggested-by: Vladimir Oltean > Signed-off-by: Richard Cochran > > Thoughts? -Vladimir ___ Linuxptp-devel mailing list Linuxptp-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
Re: [Linuxptp-devel] [PATCH v2 2/3] clock: dump unexpected packets received on the error queues of sockets
On Fri, Jun 19, 2020 at 10:45:23PM +0300, Vladimir Oltean wrote: > Yes, much better. May I add your tag to the commit message, like this? Catch unexpected socket polling errors. The poll(2) system call may set POLLERR in the returned events. Normally no errors are returned unless specifically requested by setting an appropriate socket option. Nevertheless, the poll(2) API is quite generic, and there is no guarantee that the kernel networking stack might push an error event one day. This patch adds defensive code in order to catch any unexpected error condition. Suggested-by: Vladimir Oltean Signed-off-by: Richard Cochran ___ Linuxptp-devel mailing list Linuxptp-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
