Pass the expected socket family into UdpTimedRx() so it can pick the

right socket, rather than listening on both.

Implement TODO_Cancel()

Only wake the time_unix task when needed.

Fix sockaddr comparison for IPv6[1]

Grap userland timestamp, in case SCM_TIMESTAMP* does not work/isn't
available.[1]

Undefined timestamp fix for simfile reading[1]

Minor fixes from github

[1] From Ronan Flood

README.rst

@@ -135,7 +135,7 @@ Packet traces and simulations
 
 You can also run the program two other ways::
 
-	./Ntimed-client --poll-server some_ntp_server some_other_ntp_server
+	./ntimed-client --poll-server some_ntp_server some_other_ntp_server
 
 This will *not* steer your clock, but it will query the servers as
 if it should have steered your clock.  (Ntpd should *not* be running
@@ -145,7 +145,7 @@ but you can control that with "-d 3600" for one hour etc.
 If you save the output into a file (redirect stdout or use '-t filename'),
 you can use it as input for a simulation run::
 
-	./Ntimed-client --sim-client -s filename -t /tmp/_
+	./ntimed-client --sim-client -s filename -t /tmp/_
 
 And then run::
 
@@ -200,7 +200,7 @@ in dire straits after Dave Mills retired.
 Thanks for giving me money and free hands to do what I thought was
 best -- even though I am a "BSD-guy".
 
-Thanks to Harlan Steen for keeping the NTPD flame burning, however
+Thanks to Harlan Stenn for keeping the NTPD flame burning, however
 stormy the last decade has been.
 
 I trust The Network Time Foundation will take as good care of Ntimed

configure

@@ -67,6 +67,7 @@ SRCS='
 	ocx_stdio.c
 	param.c
 	pll_std.c
+	suckaddr.c
 	time_sim.c
 	time_stuff.c
 	time_unix.c
@@ -100,7 +101,7 @@ if $BSD ; then
 	echo '.include <bsd.prog.mk>'
 	) > Makefile
 
-	msg=", remeber to run 'make depend'"
+	msg=", remember to run 'make depend'"
 else
 	(
 	echo '# Portable Makefile generated by configure'

ntimed.h

@@ -89,6 +89,10 @@ extern pll_f *PLL;
 
 void PLL_Init(void);
 
+/* suckaddr.c -- Sockaddr utils ***************************************/
+
+int SA_Equal(const void *sa1, size_t sl1, const void *sa2, size_t sl2);
+
 /* time_sim.c -- Simulated timebase ***********************************/
 
 extern double Time_Sim_delta;
@@ -145,11 +149,16 @@ typedef enum todo_e todo_f(struct ocx *, struct todolist *, void *priv);
 
 struct todolist *TODO_NewList(void);
 
-struct todo *TODO_ScheduleRel(struct todolist *, todo_f *func, void *priv,
+uintptr_t TODO_ScheduleRel(struct todolist *,
-    double when, double repeat, const char *fmt, ...);
+    todo_f *func, void *priv,
-struct todo *TODO_ScheduleAbs(struct todolist *, todo_f *func, void *priv,
+    double when, double repeat,
-    const struct timestamp *when, double repeat, const char *fmt, ...);
+    const char *fmt, ...) __printflike(6, 7);
+uintptr_t TODO_ScheduleAbs(struct todolist *,
+    todo_f *func, void *priv,
+    const struct timestamp *when, double repeat,
+    const char *fmt, ...) __printflike(6, 7);
 enum todo_e TODO_Run(struct ocx *ocx, struct todolist *);
+void TODO_Cancel(struct todolist *tdl, uintptr_t *);
 
 /* combine_delta.c -- Source Combiner based on delta-pdfs *************/
 

ntp_peer.c

@@ -133,7 +133,7 @@ NTP_Peer_Poll(struct ocx *ocx, const struct udp_socket *usc,
 		(void)TB_Now(&t1);
 		d = TS_Diff(&t1, &t0);
 
-		i = UdpTimedRx(ocx, usc, &rss, &rssl, &t2,
+		i = UdpTimedRx(ocx, usc, np->sa->sa_family, &rss, &rssl, &t2,
 		    buf, sizeof buf, tmo - d);
 
 		if (i == 0)
@@ -149,7 +149,7 @@ NTP_Peer_Poll(struct ocx *ocx, const struct udp_socket *usc,
 		}
 
 		/* Ignore packets from other hosts */
-		if (np->sa_len != rssl || memcmp(np->sa, &rss, rssl))
+		if (!SA_Equal(np->sa, np->sa_len, &rss, rssl))
 			continue;
 
 		AN(NTP_Packet_Unpack(np->rx_pkt, buf, i));

ntp_tools.c

@@ -82,13 +82,13 @@ NTP_Tool_Format(char *p, ssize_t len, const struct ntp_packet *pkt)
 
 	e = p + len;
 
-	p += snprintf(p, e - p, "[%u", pkt->ntp_leap);
+	p += snprintf(p, e - p, "[%d", pkt->ntp_leap);
 	assert(p < e);
 
 	p += snprintf(p, e - p, " %u", pkt->ntp_version);
 	assert(p < e);
 
-	p += snprintf(p, e - p, " %u", pkt->ntp_mode);
+	p += snprintf(p, e - p, " %d", pkt->ntp_mode);
 	assert(p < e);
 
 	p += snprintf(p, e - p, " %3u", pkt->ntp_stratum);
@@ -196,6 +196,7 @@ NTP_Tool_Scan(struct ntp_packet *pkt, const char *buf)
 	if (d_fields[6] != 0.0) {
 		pkt->ts_rx = pkt->ntp_transmit;
 		TS_Add(&pkt->ts_rx, d_fields[6]);
-	}
+	} else
+		INIT_OBJ(&pkt->ts_rx, TIMESTAMP_MAGIC);
 	return (0);
 }

ocx_stdio.c

@@ -52,7 +52,7 @@
  *
  * About this implementation:
  *
- * This is a very naiive implementation spitting things out to stdout/stderr,
+ * This is a very naive implementation spitting things out to stdout/stderr,
  * knowing that we are a single threaded program.
  *
  * XXX: Pull in sbufs to do it right.

suckaddr.c

@@ -0,0 +1,84 @@
+/*-
+ * Copyright (c) 2014 Poul-Henning Kamp
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND
+ * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED.  IN NO EVENT SHALL AUTHOR OR CONTRIBUTORS BE LIABLE
+ * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+ * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
+ * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
+ * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
+ * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
+ * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+ * SUCH DAMAGE.
+ *
+ * The world has not enough cuss-words to precisely convey how broken the
+ * the struct sockaddr concept is.
+ */
+
+#include <string.h>
+
+#include <sys/types.h>		/* Compat for OpenBSD */
+#include <sys/socket.h>
+
+#include <netinet/in.h>
+
+#include "ntimed.h"
+
+int
+SA_Equal(const void *sa1, size_t sl1, const void *sa2, size_t sl2)
+{
+	const struct sockaddr *s1, *s2;
+	const struct sockaddr_in *s41, *s42;
+	const struct sockaddr_in6 *s61, *s62;
+
+	AN(sa1);
+	AN(sa2);
+	assert(sl1 >= sizeof(struct sockaddr));
+	assert(sl2 >= sizeof(struct sockaddr));
+
+	s1 = sa1;
+	s2 = sa2;
+	if (s1->sa_family != s2->sa_family)
+		return (0);
+
+	if (s1->sa_family == AF_INET) {
+		assert(sl1 >= sizeof(struct sockaddr_in));
+		assert(sl2 >= sizeof(struct sockaddr_in));
+		s41 = sa1;
+		s42 = sa2;
+		if (s41->sin_port != s42->sin_port)
+			return (0);
+		if (memcmp(&s41->sin_addr, &s42->sin_addr,
+		      sizeof s41->sin_addr))
+			return (0);
+		return (1);
+	}
+
+	if (s1->sa_family == AF_INET6) {
+		assert(sl1 >= sizeof(struct sockaddr_in6));
+		assert(sl2 >= sizeof(struct sockaddr_in6));
+		s61 = sa1;
+		s62 = sa2;
+		if (s61->sin6_port != s62->sin6_port)
+			return (0);
+		if (s61->sin6_scope_id != s62->sin6_scope_id)
+			return (0);
+		if (memcmp(&s61->sin6_addr, &s62->sin6_addr,
+		    sizeof s61->sin6_addr))
+			return (0);
+		return (1);
+	}
+	return (0);
+}

time_unix.c

@@ -46,6 +46,9 @@ static double adj_offset = 0;
 static double adj_duration = 0;
 static double adj_freq = 0;
 
+static uintptr_t ticker;
+static struct todolist *kt_tdl;
+
 // #undef CLOCK_REALTIME		/* Test old unix code */
 
 /**********************************************************************
@@ -70,7 +73,7 @@ static double adj_freq = 0;
  */
 
 static void
-kt_setfreq(double frequency)
+kt_setfreq(struct ocx *ocx, double frequency)
 {
 	struct timex tx;
 	int i;
@@ -90,46 +93,47 @@ kt_setfreq(double frequency)
 	tx.freq = (long)floor(frequency * (65536 * 1e6));
 	errno = 0;
 	i = ntp_adjtime(&tx);
+	Put(ocx, OCX_TRACE, "KERNPLL %.6e %d\n", frequency, i);
 	/* XXX: what is the correct error test here ? */
 	assert(i >= 0);
 }
 
 static enum todo_e __match_proto__(todo_f)
-kt_pll(struct ocx *ocx, struct todolist *tdl, void *priv)
+kt_ticker(struct ocx *ocx, struct todolist *tdl, void *priv)
 {
-	double d, freq;
-	struct timestamp ts;
-	char buf[40];
 
 	(void)ocx;
 	AN(tdl);
 	AZ(priv);
-	freq = adj_freq;
+	kt_setfreq(ocx, adj_freq);
-	if (adj_duration > 0.0) {
+	ticker = 0;
-		d = adj_offset / adj_duration;
-		freq += d;
-		adj_offset -= d;
-		adj_duration -= 1.0;
-	}
-	TB_Now(&ts);
-	TS_Format(buf, sizeof buf, &ts);
-	Put(ocx, OCX_TRACE, "KERNTIME_PLL %s %.6e %.6e %.3e %.6e\n",
-	    buf, adj_freq, adj_offset, adj_duration, freq);
-	kt_setfreq(freq);
 	return (TODO_OK);
 }
 
 static void __match_proto__(tb_adjust_f)
 kt_adjust(struct ocx *ocx, double offset, double duration, double frequency)
 {
+	double freq;
 
 	(void)ocx;
+	assert(duration >= 0.0);
+
+	if (ticker)
+		TODO_Cancel(kt_tdl, &ticker);
 
 	adj_offset = offset;
 	adj_duration = floor(duration);
 	if (adj_offset > 0.0 && adj_duration == 0.0)
 		adj_duration = 1.0;
 	adj_freq = frequency;
+
+	freq = adj_freq;
+	if (adj_duration > 0.0)
+		freq += adj_offset / adj_duration;
+	kt_setfreq(ocx, freq);
+	if (adj_duration > 0.0)
+		ticker = TODO_ScheduleRel(kt_tdl, kt_ticker, NULL,
+		    adj_duration, 0.0, "KT_TICK");
 }
 
 /**********************************************************************/
@@ -229,18 +233,15 @@ kt_sleep(double dur)
 void
 Time_Unix(struct todolist *tdl)
 {
-	struct timestamp ts;
 
+	AN(tdl);
 	TB_Step = kt_step;
 	TB_Adjust = kt_adjust;
 	TB_Sleep = kt_sleep;
 	TB_Now = kt_now;
+	kt_tdl = tdl;
 
-	/* Aim the userland PLL wrangler at 125msec before the second */
+	/* XXX: test if we have perms */
-	TB_Now(&ts);
-	ts.sec += 1;
-	ts.frac = 14ULL << 60ULL;
-	(void)TODO_ScheduleAbs(tdl, kt_pll, NULL, &ts, 1.0, "SIMPLL");
 }
 
 /**********************************************************************

todo.c

@@ -28,6 +28,8 @@
  *
  * This is a simple "TODO-list" scheduler for calling things at certain
  * times.  Jobs can be one-shot or repeated and repeated jobs can abort.
+ *
+ * For ease of debugging, TODO jobs have a name.
  */
 
 #include <stdarg.h>
@@ -81,13 +83,30 @@ todo_insert(struct todolist *tdl, struct todo *tp)
 }
 
 /**********************************************************************
- * Return a pointer to the todo item, so the API can support cancellation.
- * (Cancellation will be necessary for NTP-peer removal)
- *
- * For ease of debugging, TODO jobs have a name.
  */
 
-struct todo *
+void
+TODO_Cancel(struct todolist *tdl, uintptr_t *tp)
+{
+	struct todo *tp2;
+
+	CHECK_OBJ_NOTNULL(tdl, TODOLIST_MAGIC);
+	AN(tp);
+	AN(*tp);
+
+	TAILQ_FOREACH(tp2, &tdl->todolist, list)
+		if ((uintptr_t)tp2 == *tp)
+			break;
+	CHECK_OBJ_NOTNULL(tp2, TODO_MAGIC);
+	TAILQ_REMOVE(&tdl->todolist, tp2, list);
+	FREE_OBJ(tp2);
+	*tp = 0;
+}
+
+/**********************************************************************
+ */
+
+uintptr_t
 TODO_ScheduleAbs(struct todolist *tdl, todo_f *func, void *priv,
     const struct timestamp *when, double repeat, const char *fmt, ...)
 {
@@ -110,10 +129,10 @@ TODO_ScheduleAbs(struct todolist *tdl, todo_f *func, void *priv,
 	(void)vsnprintf(tp->what, sizeof tp->what, fmt, ap);
 	va_end(ap);
 	todo_insert(tdl, tp);
-	return (tp);
+	return ((uintptr_t)tp);
 }
 
-struct todo *
+uintptr_t
 TODO_ScheduleRel(struct todolist *tdl, todo_f *func, void *priv,
     double when, double repeat, const char *fmt, ...)
 {
@@ -137,7 +156,7 @@ TODO_ScheduleRel(struct todolist *tdl, todo_f *func, void *priv,
 	(void)vsnprintf(tp->what, sizeof tp->what, fmt, ap);
 	va_end(ap);
 	todo_insert(tdl, tp);
-	return (tp);
+	return ((uintptr_t)tp);
 }
 
 /**********************************************************************

udp.c

@@ -86,6 +86,7 @@ UdpTimedSocket(struct ocx *ocx)
 
 ssize_t
 UdpTimedRx(struct ocx *ocx, const struct udp_socket *usc,
+    sa_family_t fam,
     struct sockaddr_storage *ss, socklen_t *sl,
     struct timestamp *ts, void *buf, ssize_t len, double tmo)
 {
@@ -94,9 +95,9 @@ UdpTimedRx(struct ocx *ocx, const struct udp_socket *usc,
 	struct cmsghdr *cmsg;
 	u_char ctrl[1024];
 	ssize_t rl;
-	int i, j;
+	int i;
 	int tmo_msec;
-	struct pollfd pfd[2];
+	struct pollfd pfd[1];
 
 	CHECK_OBJ_NOTNULL(usc, UDP_SOCKET_MAGIC);
 	AN(ss);
@@ -105,18 +106,15 @@ UdpTimedRx(struct ocx *ocx, const struct udp_socket *usc,
 	AN(buf);
 	assert(len > 0);
 
+	if (fam == AF_INET)
+		pfd[0].fd = usc->fd4;
+	else if (fam == AF_INET6)
+		pfd[0].fd = usc->fd6;
+	else
+		WRONG("Wrong family in UdpTimedRx");
 
-	j = 0;
+	pfd[0].events = POLLIN;
-	if (usc->fd4 >= 0) {
+	pfd[0].revents = 0;
-		pfd[j].fd = usc->fd4;
-		pfd[j].events = POLLIN;
-		pfd[j++].revents = 0;
-	}
-	if (usc->fd6 >= 0) {
-		pfd[j].fd = usc->fd6;
-		pfd[j].events = POLLIN;
-		pfd[j++].revents = 0;
-	}
 
 	if (tmo == 0.0) {
 		tmo_msec = -1;
@@ -125,7 +123,7 @@ UdpTimedRx(struct ocx *ocx, const struct udp_socket *usc,
 		if (tmo_msec <= 0)
 			tmo_msec = 0;
 	}
-	i = poll(pfd, j, tmo_msec);
+	i = poll(pfd, 1, tmo_msec);
 
 	if (i < 0)
 		Fail(ocx, 1, "poll(2) failed\n");
@@ -133,6 +131,9 @@ UdpTimedRx(struct ocx *ocx, const struct udp_socket *usc,
 	if (i == 0)
 		return (0);
 
+	/* Grab a timestamp in case none of the SCM_TIMESTAMP* works */
+	TB_Now(ts);
+
 	memset(&msg, 0, sizeof msg);
 	msg.msg_name = (void*)ss;
 	msg.msg_namelen = sizeof *ss;
@@ -145,19 +146,17 @@ UdpTimedRx(struct ocx *ocx, const struct udp_socket *usc,
 	memset(ctrl, 0, sizeof ctrl);
 	cmsg = (void*)ctrl;
 
-	for (i = 0; i < j; i++)
+	rl = recvmsg(pfd[0].fd, &msg, 0);
-		if (pfd[i].revents & POLLIN)
-			break;
-
-	if (i == j)
-		return (0);
-
-	rl = recvmsg(pfd[i].fd, &msg, 0);
 	if (rl <= 0)
 		return (rl);
 
 	*sl = msg.msg_namelen;
 
+	if (msg.msg_flags != 0) {
+		Debug(ocx, "msg_flags = 0x%x", msg.msg_flags);
+		return (-1);
+	}
+
 	for(;cmsg != NULL; cmsg = CMSG_NXTHDR(&msg, cmsg)) {
 #ifdef SCM_TIMESTAMPNS
 		if (cmsg->cmsg_level == SOL_SOCKET &&

udp.h

@@ -36,6 +36,7 @@
 
 struct udp_socket *UdpTimedSocket(struct ocx *ocx);
 ssize_t UdpTimedRx(struct ocx *, const struct udp_socket *,
+    sa_family_t fam,
     struct sockaddr_storage *, socklen_t *,
     struct timestamp *,
     void *, ssize_t len,