net: lib: sntp_simple: Handle responses from previous iterations

SNTP simple runs request iterations with exponential backoff.
If the net interface is a slower connection (ie. CAT M1 modems)
then the request will be sent but the response may take time to
be received, thus causing a timeout and another request to be sent.
Because of the nature of UDP and the fact that the same socket
(source IP/port combo) is being used for both requests, a delayed
response to the first request can be received as the response to the
second request, causing -EINVAL to be returned when the timestamps
mismatch (see subsys/net/lib/sntp/sntp.c). The solution provided
retries receiving the response when the timestamp is mismatched
(without sending an additional request).

Signed-off-by: Marcus Penate <[email protected]>

Author: mpenate-ellenbytech

include/zephyr/net/sntp.h

@@ -73,13 +73,24 @@ int sntp_init(struct sntp_ctx *ctx, struct sockaddr *addr,
  *
  * @param ctx Address of sntp context.
  * @param timeout Timeout of waiting for sntp response (in milliseconds).
- * @param time Timestamp including integer and fractional seconds since
+ * @param ts Timestamp including integer and fractional seconds since
+ * 1 Jan 1970 (output).
+ *
+ * @return 0 if ok, <0 if error (-ETIMEDOUT if timeout).
+ */
+int sntp_query(struct sntp_ctx *ctx, uint32_t timeout, struct sntp_time *ts);
+
+/**
+ * @brief Attempt to receive an SNTP response after issuing a query
+ *
+ * @param ctx Address of sntp context.
+ * @param timeout Timeout of waiting for sntp response (in milliseconds).
+ * @param ts Timestamp including integer and fractional seconds since
  * 1 Jan 1970 (output).
  *
  * @return 0 if ok, <0 if error (-ETIMEDOUT if timeout).
  */
-int sntp_query(struct sntp_ctx *ctx, uint32_t timeout,
-	       struct sntp_time *time);
+int sntp_recv_response(struct sntp_ctx *ctx, uint32_t timeout, struct sntp_time *ts);
 
 /**
  * @brief Release SNTP context

subsys/net/lib/sntp/sntp.c

@@ -70,7 +70,7 @@ static int32_t parse_response(uint8_t *data, uint16_t len, struct sntp_time *exp
 		NET_DBG("Mismatch originate timestamp: %d.%09d, expect: %llu.%09u",
 			ntohl(pkt->orig_tm_s), ntohl(pkt->orig_tm_f), expected_orig_ts->seconds,
 			expected_orig_ts->fraction);
-		return -EINVAL;
+		return -ERANGE;
 	}
 
 	if (pkt->mode != SNTP_MODE_SERVER) {
@@ -151,38 +151,6 @@ static int32_t parse_response(uint8_t *data, uint16_t len, struct sntp_time *exp
 	return 0;
 }
 
-static int sntp_recv_response(struct sntp_ctx *sntp, uint32_t timeout,
-			      struct sntp_time *time)
-{
-	struct sntp_pkt buf = { 0 };
-	int status;
-	int rcvd;
-
-	status = zsock_poll(sntp->sock.fds, sntp->sock.nfds, timeout);
-	if (status < 0) {
-		NET_ERR("Error in poll:%d", errno);
-		return -errno;
-	}
-
-	if (status == 0) {
-		return -ETIMEDOUT;
-	}
-
-	rcvd = zsock_recv(sntp->sock.fd, (uint8_t *)&buf, sizeof(buf), 0);
-	if (rcvd < 0) {
-		return -errno;
-	}
-
-	if (rcvd != sizeof(struct sntp_pkt)) {
-		return -EMSGSIZE;
-	}
-
-	status = parse_response((uint8_t *)&buf, sizeof(buf),
-				&sntp->expected_orig_ts,
-				time);
-	return status;
-}
-
 int sntp_init(struct sntp_ctx *ctx, struct sockaddr *addr, socklen_t addr_len)
 {
 	int ret;
@@ -213,13 +181,13 @@ int sntp_init(struct sntp_ctx *ctx, struct sockaddr *addr, socklen_t addr_len)
 	return 0;
 }
 
-int sntp_query(struct sntp_ctx *ctx, uint32_t timeout, struct sntp_time *time)
+int sntp_query(struct sntp_ctx *ctx, uint32_t timeout, struct sntp_time *ts)
 {
 	struct sntp_pkt tx_pkt = { 0 };
 	int ret = 0;
 	int64_t ts_us = 0;
 
-	if (!ctx || !time) {
+	if (!ctx || !ts) {
 		return -EFAULT;
 	}
 
@@ -239,7 +207,39 @@ int sntp_query(struct sntp_ctx *ctx, uint32_t timeout, struct sntp_time *time)
 		return ret;
 	}
 
-	return sntp_recv_response(ctx, timeout, time);
+	return sntp_recv_response(ctx, timeout, ts);
+}
+
+int sntp_recv_response(struct sntp_ctx *ctx, uint32_t timeout,
+			      struct sntp_time *ts)
+{
+	struct sntp_pkt buf = { 0 };
+	int status;
+	int rcvd;
+
+	status = zsock_poll(ctx->sock.fds, ctx->sock.nfds, timeout);
+	if (status < 0) {
+		NET_ERR("Error in poll:%d", errno);
+		return -errno;
+	}
+
+	if (status == 0) {
+		return -ETIMEDOUT;
+	}
+
+	rcvd = zsock_recv(ctx->sock.fd, (uint8_t *)&buf, sizeof(buf), 0);
+	if (rcvd < 0) {
+		return -errno;
+	}
+
+	if (rcvd != sizeof(struct sntp_pkt)) {
+		return -EMSGSIZE;
+	}
+
+	status = parse_response((uint8_t *)&buf, sizeof(buf),
+				&ctx->expected_orig_ts,
+				ts);
+	return status;
 }
 
 void sntp_close(struct sntp_ctx *ctx)

subsys/net/lib/sntp/sntp_simple.c

@@ -4,6 +4,7 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 #include <errno.h>
+#include <stdbool.h>
 
 #include <zephyr/net/sntp.h>
 #include <zephyr/net/socketutils.h>
@@ -15,6 +16,7 @@ static int sntp_simple_helper(struct sockaddr *addr, socklen_t addr_len, uint32_
 	struct sntp_ctx sntp_ctx;
 	uint64_t deadline;
 	uint32_t iter_timeout;
+	bool first_iter;
 
 	res = sntp_init(&sntp_ctx, addr, addr_len);
 	if (res < 0) {
@@ -29,18 +31,39 @@ static int sntp_simple_helper(struct sockaddr *addr, socklen_t addr_len, uint32_
 
 	/* Timeout for current iteration */
 	iter_timeout = 100;
+	first_iter = true;
 
 	while (k_uptime_get() < deadline) {
 		res = sntp_query(&sntp_ctx, iter_timeout, ts);
 
 		if (res != -ETIMEDOUT) {
-			break;
+			if (false == first_iter && -ERANGE == res) {
+				while (-ERANGE == res) {
+					/* Possible out of order packet received.
+					 * Retry recv with current iteration timeout
+					 * until an error or timeout (flushing the socket
+					 * of old iteration responses until we timeout or
+					 * receive our iteration's response)
+					 */
+					res = sntp_recv_response(&sntp_ctx, iter_timeout, ts);
+				}
+
+				if (res != ETIMEDOUT) {
+					break;
+				}
+			} else {
+				break;
+			}
 		}
 
 		/* Exponential backoff with limit */
 		if (iter_timeout < 1000) {
 			iter_timeout *= 2;
 		}
+
+		if (first_iter) {
+			first_iter = false;
+		}
 	}
 
 	sntp_close(&sntp_ctx);