You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2021/01/25 03:57:24 UTC

[GitHub] [incubator-nuttx-apps] xiaoxiang781216 commented on a change in pull request #570: netutils/ntpclient: add more features

xiaoxiang781216 commented on a change in pull request #570:
URL: https://github.com/apache/incubator-nuttx-apps/pull/570#discussion_r563447397



##########
File path: netutils/ntpclient/ntpclient.c
##########
@@ -103,10 +143,53 @@ enum ntpc_daemon_e
 
 struct ntpc_daemon_s
 {
-  uint8_t state; /* See enum ntpc_daemon_e */
-  sem_t lock;    /* Used to protect the whole structure */
-  sem_t sync;    /* Used to synchronize start and stop events */
-  pid_t pid;     /* Task ID of the NTP daemon */
+  uint8_t state;       /* See enum ntpc_daemon_e */
+  sem_t lock;          /* Used to protect the whole structure */
+  sem_t sync;          /* Used to synchronize start and stop events */
+  pid_t pid;           /* Task ID of the NTP daemon */
+  sq_queue_t kod_list; /* KoD excluded server addresses */
+  int family;          /* Allowed address family */
+};
+
+union ntp_addr_u
+{
+  struct sockaddr sa;
+#ifdef CONFIG_NET_IPv4
+  struct sockaddr_in in4;
+#endif
+#ifdef CONFIG_NET_IPv6
+  struct sockaddr_in6 in6;
+#endif
+};
+
+/* NTP offset. */
+
+struct ntp_sample_s
+{
+  int64_t offset;
+  int64_t delay;
+  union ntp_addr_u srv_addr;
+} packet_struct;
+
+/* Server address list. */
+
+struct ntp_servers_s
+{
+  union ntp_addr_u list[CONFIG_NETUTILS_NTPCLIENT_NUM_SAMPLES];
+  size_t num;
+  size_t pos;
+  char *hostlist_str;
+  char *hostlist_saveptr;
+  char *hostnext;
+  const char *ntp_servers;

Review comment:
       need add FAR for all pointer type

##########
File path: netutils/ntpclient/ntpclient.c
##########
@@ -577,6 +1503,23 @@ int ntpc_start(void)
   return g_ntpc_daemon.pid;
 }
 
+/****************************************************************************
+ * Name: ntpc_start
+ *
+ * Description:
+ *   Start the NTP daemon
+ *
+ * Returned Value:
+ *   On success, the non-negative task ID of the NTPC daemon is returned;
+ *   On failure, a negated errno value is returned.
+ *
+ ****************************************************************************/
+
+int ntpc_start(void)
+{
+  return ntpc_start_with_list(NULL);

Review comment:
       should we change NULL to CONFIG_NETUTILS_NTPCLIENT_SERVER? and remvoe the check from ntp task.

##########
File path: netutils/ntpclient/ntpclient.c
##########
@@ -186,328 +436,982 @@ static void ntpc_settime(FAR uint8_t *timestamp)
    *   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
    */
 
-  seconds = ntpc_getuint32(timestamp);
+  /* Get seconds part. */
 
-  /* Translate seconds to account for the difference in the origin time */
+  return time >> 32;
+}
 
-  if (seconds > NTP2UNIX_TRANSLATION)
-    {
-      seconds -= NTP2UNIX_TRANSLATION;
-    }
+/****************************************************************************
+ * Name: ntp_nsecpart
+ ****************************************************************************/
 
-  /* Conversion of the fractional part to nanoseconds:
-   *
-   *  NSec = (f * 1,000,000,000) / 4,294,967,296
-   *       = (f * (5**9 * 2**9) / (2**32)
-   *       = (f * 5**9) / (2**23)
-   *       = (f * 1,953,125) / 8,388,608
-   */
+static uint32_t ntp_nsecpart(uint64_t time)
+{
+  /* Get fraction part converted to nanoseconds. */
 
-  frac = ntpc_getuint32(timestamp + 4);
-#ifdef CONFIG_HAVE_LONG_LONG
-  /* if we have 64-bit long long values, then the computation is easy */
+  return ((time & 0xffffffffu) * NSEC_PER_SEC) >> 32;
+}
 
-  tmp  = ((uint64_t)frac * 1953125) >> 23;
-  nsec = (uint32_t)tmp;
+/****************************************************************************
+ * Name: timespec2ntp
+ *
+ * Convert UNIX timespec timestamp to NTP 64-bit fixed point timestamp.
+ *
+ ****************************************************************************/
 
-#else
-  /* If we don't have 64 bit integer types, then the calculation is a little
-   * more complex:
-   *
-   * Let f         = a    << 16 + b
-   *     1,953,125 = 0x1d << 16 + 0xcd65
-   * NSec << 23 =  ((a << 16) + b) * ((0x1d << 16) + 0xcd65)
-   *            = (a << 16) * 0x1d << 16) +
-   *              (a << 16) * 0xcd65 +
-   *              b         * 0x1d << 16) +
-   *              b         * 0xcd65;
-   */
+static uint64_t timespec2ntp(const struct timespec *ts)
+{
+  uint64_t ntp_time;
 
-  /* Break the fractional part up into two values */
+  /* Set fraction part. */
 
-  a16  = frac >> 16;
-  b0   = frac & 0xffff;
+  ntp_time = ((uint64_t)(ts->tv_nsec) << 32) / NSEC_PER_SEC;
 
-  /* Get the b32 and b0 terms
-   *
-   * t32 = (a << 16) * 0x1d << 16)
-   * t0  = b * 0xcd65
-   */
+  DEBUGASSERT((ntp_time >> 32) == 0);
 
-  t32  = 0x001d * a16;
-  t0   = 0xcd65 * b0;
+  /* Set seconds part. */
 
-  /* Get the first b16 term
-   *
-   * (a << 16) * 0xcd65
-   */
+  ntp_time += (uint64_t)(ts->tv_sec) << 32;
 
-  t16  = 0xcd65 * a16;
+  return ntp_time;
+}
 
-  /* Add the upper 16-bits to the b32 accumulator */
+/****************************************************************************
+ * Name: ntp_gettime
+ *
+ * Get time in NTP epoch, stored in fixed point uint64_t format (upper 32-bit
+ * seconds, lower 32-bit fraction)
+ ****************************************************************************/
 
-  t32 += (t16 >> 16);
+static uint64_t ntp_localtime(void)
+{
+  int err = errno;
+  struct timespec currts;
 
-  /* Add the lower 16-bits to the b0 accumulator, handling carry to the b32
-   * accumulator
-   */
+  /* Get current clock in NTP epoch. */
 
-  t16  <<= 16;
-  if (t0 > (0xffffffff - t16))
-    {
-      t32++;
-    }
+  clock_gettime(CLOCK_REALTIME, &currts);
+  currts.tv_sec += NTP2UNIX_TRANSLATION;
 
-  t0 += t16;
+  /* Restore errno. */
 
-  /* Get the second b16 term
-   *
-   * b * (0x1d << 16)
-   */
+  errno = err;
 
-  t16  = 0x001d * b0;
+  return timespec2ntp(&currts);
+}
 
-  /* Add the upper 16-bits to the b32 accumulator */
+/****************************************************************************
+ * Name: ntpc_calculate_offset
+ *
+ * Description:
+ *   Calculate NTP time offset and round-trip delay
+ *
+ ****************************************************************************/
 
-  t32 += (t16 >> 16);
+static void ntpc_calculate_offset(int64_t *offset, int64_t *delay,
+                                  uint64_t local_xmittime,
+                                  uint64_t local_recvtime,
+                                  FAR const uint8_t *remote_recv,
+                                  FAR const uint8_t *remote_xmit)
+{
+  uint64_t remote_recvtime;
+  uint64_t remote_xmittime;
 
-  /* Add the lower 16-bits to the b0 accumulator, handling carry to the b32
-   * accumulator
+  /* Two timestamps from server, when request was received, and response
+   * send.
    */
 
-  t16  <<= 16;
-  if (t0 > (0xffffffff - t16))
-    {
-      t32++;
-    }
-
-  t0 += t16;
+  remote_recvtime = ntpc_getuint64(remote_recv);
+  remote_xmittime = ntpc_getuint64(remote_xmit);
 
-  /* t32 and t0 represent the 64 bit product.  Now shift right by 23 bits to
-   * accomplish the divide by by 2**23.
+  /* Calculate offset of local time compared to remote time.
+   * See: https://www.eecis.udel.edu/~mills/time.html
+   *      http://nicolas.aimon.fr/2014/12/05/timesync/
    */
 
-  nsec = (t32 << (32 - 23)) + (t0 >> 23);
-#endif
-
-  /* Set the system time */
+  *offset = (int64_t)((remote_recvtime - local_xmittime) +
+                     (remote_xmittime - local_recvtime)) / 2;
 
-  tp.tv_sec  = seconds;
-  tp.tv_nsec = nsec;
-  clock_settime(CLOCK_REALTIME, &tp);
+  /* Calculate roundtrip delay. */
 
-  ninfo("Set time to %ju seconds\n", (intmax_t)tp.tv_sec);
+  *delay = (local_recvtime - local_xmittime) -
+          (remote_xmittime - remote_recvtime);
 }
 
 /****************************************************************************
- * Name: ntpc_daemon
+ * Name: ntpc_settime
  *
  * Description:
- *   This the NTP client daemon.  This is a *very* minimal
- *   implementation! An NTP request is and the system clock is set when the
- *   response is received
+ *   Given the NTP time offset, adjust the system time
  *
  ****************************************************************************/
 
-static int ntpc_daemon(int argc, char **argv)
+static void ntpc_settime(int64_t offset, struct timespec *start_realtime,
+                         struct timespec *start_monotonic)
 {
-  struct sockaddr_in server;
-  struct ntp_datagram_s pkt;
-  struct timeval tv;
-
-#ifdef CONFIG_LIBC_NETDB
-  struct hostent *he;
-  struct in_addr **addr_list;
+  struct timespec tp;
+  struct timespec curr_realtime;
+#ifdef CONFIG_CLOCK_MONOTONIC
+  struct timespec curr_monotonic;
+  int64_t diffms_real;
+  int64_t diffms_mono;
+  int64_t diff_diff_ms;
 #endif
 
-  socklen_t socklen;
-  ssize_t nbytes;
-  int exitcode = EXIT_SUCCESS;
-  int retry = 0;
-  int sd;
-  int ret;
-
-  /* Indicate that we have started */
+  /* Get the system times */
 
-  g_ntpc_daemon.state = NTP_RUNNING;
-  sem_post(&g_ntpc_daemon.sync);
+  clock_gettime(CLOCK_REALTIME, &curr_realtime);
 
-  /* Create a datagram socket  */
+#ifdef CONFIG_CLOCK_MONOTONIC
+  clock_gettime(CLOCK_MONOTONIC, &curr_monotonic);
 
-  sd = socket(AF_INET, SOCK_DGRAM, 0);
-  if (sd < 0)
-    {
-      nerr("ERROR: socket failed: %d\n", errno);
+  /* Check differences between monotonic and realtime. */
 
-      g_ntpc_daemon.state = NTP_STOPPED;
-      sem_post(&g_ntpc_daemon.sync);
-      return EXIT_FAILURE;
-    }
+  diffms_real = curr_realtime.tv_sec - start_realtime->tv_sec;
+  diffms_real *= 1000;
+  diffms_real += (int64_t)(curr_realtime.tv_nsec -
+                           start_realtime->tv_nsec) / (1000 * 1000);
 
-  /* Setup a receive timeout on the socket */
+  diffms_mono = curr_monotonic.tv_sec - start_monotonic->tv_sec;
+  diffms_mono *= 1000;
+  diffms_mono += (int64_t)(curr_monotonic.tv_nsec -
+                           start_monotonic->tv_nsec) / (1000 * 1000);
 
-  tv.tv_sec  = 5;
-  tv.tv_usec = 0;
+  /* Detect if real-time has been altered by other task. */
 
-  ret = setsockopt(sd, SOL_SOCKET, SO_RCVTIMEO, &tv, sizeof(struct timeval));
-  if (ret < 0)
+  diff_diff_ms = diffms_real - diffms_mono;
+  if (diff_diff_ms < 0)
     {
-      nerr("ERROR: setsockopt failed: %d\n", errno);
-
-      g_ntpc_daemon.state = NTP_STOPPED;
-      sem_post(&g_ntpc_daemon.sync);
-      return EXIT_FAILURE;
+      diff_diff_ms = -diff_diff_ms;
     }
 
-  /* Setup or sockaddr_in struct with information about the server we are
-   * going to ask the time from.
-   */
-
-  memset(&server, 0, sizeof(struct sockaddr_in));
-  server.sin_family      = AF_INET;
-  server.sin_port        = htons(CONFIG_NETUTILS_NTPCLIENT_PORTNO);
-
-#ifndef CONFIG_LIBC_NETDB
-  server.sin_addr.s_addr = htonl(CONFIG_NETUTILS_NTPCLIENT_SERVERIP);
-#else
-  he = gethostbyname(CONFIG_NETUTILS_NTPCLIENT_SERVER);
-  if (he != NULL && he->h_addrtype == AF_INET)
+  if (diff_diff_ms >= 1000)
     {
-      addr_list = (struct in_addr **)he->h_addr_list;
-      server.sin_addr.s_addr = addr_list[0]->s_addr;
-      ninfo("INFO: '%s' resolved to: %s\n",
-            CONFIG_NETUTILS_NTPCLIENT_SERVER,
-            inet_ntoa(server.sin_addr));
-    }
-  else
-    {
-      nerr("ERROR: Failed to resolve '%s'\n",
-           CONFIG_NETUTILS_NTPCLIENT_SERVER);
+      nwarn("System time altered by other task by %ju msecs, "
+            "do not apply offset.\n", (intmax_t)diff_diff_ms);
 
-      g_ntpc_daemon.state = NTP_STOPPED;
-      sem_post(&g_ntpc_daemon.sync);
-      return EXIT_FAILURE;
+      return;
     }
+#else
+  UNUSED(start_monotonic);
 #endif
 
-  /* Here we do the communication with the NTP server.  This is a very simple
-   * client architecture.  A request is sent and then a NTP packet is
-   * received and used to set the current time.
-   *
-   * NOTE that the scheduler is locked whenever this loop runs.  That
-   * assures both:  (1) that there are no asynchronous stop requests and
-   * (2) that we are not suspended while in critical moments when we about
-   * to set the new time.  This sounds harsh, but this function is suspended
-   * most of the time either: (1) sending a datagram, (2) receiving a
-   * datagram, or (3) waiting for the next poll cycle.
-   *
-   * TODO: The first datagram that is sent is usually lost.  That is because
-   * the MAC address of the NTP server is not in the ARP table.  This is
-   * particularly bad here because the request will not be sent again until
-   * the long delay expires leaving the system with bad time for a long time
-   * initially.  Solutions:
-   *
-   * 1. Fix send logic so that it assures that the ARP request has been
-   *    sent and the entry is in the ARP table before sending the packet
-   *    (best).
-   * 2. Add some ad hoc logic here so that there is no delay until at least
-   *    one good time is received.
-   */
+  /* Apply offset */
 
-  sched_lock();
-  while (g_ntpc_daemon.state != NTP_STOP_REQUESTED)
+  tp = curr_realtime;
+  tp.tv_sec  += ntp_secpart(offset);
+  tp.tv_nsec += ntp_nsecpart(offset);
+  while (tp.tv_nsec >= NSEC_PER_SEC)
     {
-      /* Format the transmit datagram */
-
-      memset(&pkt, 0, sizeof(pkt));
-      pkt.lvm = MKLVM(0, NTP_VERSION, 3);
+      tp.tv_nsec -= NSEC_PER_SEC;
+      tp.tv_sec++;
+    }
 
-      ninfo("Sending a NTP packet\n");
+  /* Set the system time */
 
-      ret = sendto(sd, &pkt, sizeof(struct ntp_datagram_s),
-                   0, (FAR struct sockaddr *)&server,
-                   sizeof(struct sockaddr_in));
+  clock_settime(CLOCK_REALTIME, &tp);
 
-      if (ret < 0)
-        {
-          /* Check if we received a signal.  That is not an error but
-           * other error events will terminate the client.
-           */
+  ninfo("Set time to %ju.%03ld seconds (offset: %s%lu.%03lu).\n",
+        (intmax_t)tp.tv_sec, tp.tv_nsec / NSEC_PER_MSEC,
+        offset < 0 ? "-" : "",
+        (unsigned long)ntp_secpart(int64abs(offset)),
+        ntp_nsecpart(int64abs(offset)) / NSEC_PER_MSEC);
+}
 
-          int errval = errno;
-          if (errval != EINTR)
-            {
-              nerr("ERROR: sendto() failed: %d\n", errval);
-              exitcode = EXIT_FAILURE;
-              break;
-            }
+/****************************************************************************
+ * Name: ntp_address_in_kod_list
+ *
+ * Description: Check if address is in KoD KoD exclusion list.
+ *
+ ****************************************************************************/
 
-          /* Go back to the top of the loop if we were interrupted
-           * by a signal.  The signal might mean that we were
-           * requested to stop(?)
-           */
+static bool ntp_address_in_kod_list(const union ntp_addr_u *server_addr)
+{
+  struct ntp_kod_exclude_s *entry;
 
-          continue;
+  entry = (void *)sq_peek(&g_ntpc_daemon.kod_list);
+  while (entry)
+    {
+      if (memcmp(&entry->addr, server_addr, sizeof(*server_addr)) == 0)
+        {
+          return true;
         }
 
-      /* Attempt to receive a packet (with a timeout that was set up via
-       * setsockopt() above)
-       */
+      entry = (void *)sq_next(&entry->node);
+    }
 
-      socklen = sizeof(struct sockaddr_in);
-      nbytes = recvfrom(sd, (void *)&pkt, sizeof(struct ntp_datagram_s),
-                        0, (FAR struct sockaddr *)&server, &socklen);
+  return false;
+}
 
-      /* Check if the received message was long enough to be a valid NTP
-       * datagram.
-       */
+/****************************************************************************
+ * Name: ntp_is_kiss_of_death
+ *
+ * Description: Check if this is KoD response from the server. If it is,
+ * add server to KoD exclusion list and return 'true'.
+ *
+ ****************************************************************************/
+
+static bool ntp_is_kiss_of_death(const struct ntp_datagram_s *recv,
+                                 const union ntp_addr_u *server_addr)
+{
+  /* KoD only specified for v4. */
+
+  if (GETVN(recv->lvm) != NTP_VERSION_V4)
+    {
+      if (recv->stratum == 0)
+        {
+          /* Stratum 0 is unspecified on v3, so ignore packet. */
 
-      if (nbytes >= (ssize_t)NTP_DATAGRAM_MINSIZE)
+          return true;
+        }
+      else
         {
-          ninfo("Setting time\n");
-          ntpc_settime(pkt.recvtimestamp);
-          retry = 0;
+          return false;
         }
+    }
 
-      /* Check for errors.  Note that properly received, short datagrams
-       * are simply ignored.
-       */
+  /* KoD message if stratum == 0. */
 
-      else if (nbytes < 0)
-        {
-          /* Check if we received a signal.  That is not an error but
-           * other error events will terminate the client.
-           */
+  if (recv->stratum != 0)
+    {
+      return false;
+    }
 
-          int errval = errno;
-          if (errval != EINTR)
-            {
-              /* Allow up to three retries */
+  /* KoD message received. */
 
-              if (++retry < 3)
-                {
-                  continue;
-                }
+  /* Check if we need to add server to access exclusion list. */
 
-              /* Then declare the failure */
+  if (strncmp((char *)recv->refid, "DENY", 4) == 0 ||
+      strncmp((char *)recv->refid, "RSTR", 4) == 0 ||
+      strncmp((char *)recv->refid, "RATE", 4) == 0)
+    {
+      struct ntp_kod_exclude_s *entry;
 
-              nerr("ERROR: recvfrom() failed: %d\n", errval);
-              exitcode = EXIT_FAILURE;
-              break;
-            }
+      entry = calloc(1, sizeof(*entry));
+      if (entry)
+        {
+          entry->addr = *server_addr;
+          sq_addlast(&entry->node, &g_ntpc_daemon.kod_list);
+        }
+    }
+
+  return true;
+}
+
+/****************************************************************************
+ * Name: ntpc_verify_recvd_ntp_datagram
+ ****************************************************************************/
+
+static bool ntpc_verify_recvd_ntp_datagram(const struct ntp_datagram_s *xmit,
+                                           const struct ntp_datagram_s *recv,
+                                           size_t nbytes,
+                                           const union ntp_addr_u *xmitaddr,
+                                           const union ntp_addr_u *recvaddr,
+                                           size_t recvaddrlen)
+{
+  time_t buildtime;
+  time_t seconds;
+
+  if (recvaddr->sa.sa_family != xmitaddr->sa.sa_family)
+    {
+      ninfo("wrong address family\n");
+
+      return false;
+    }
+
+#ifdef CONFIG_NET_IPv4
+  if (recvaddr->sa.sa_family == AF_INET)
+    {
+      if (recvaddrlen != sizeof(struct sockaddr_in) ||
+          xmitaddr->in4.sin_addr.s_addr != recvaddr->in4.sin_addr.s_addr ||
+          xmitaddr->in4.sin_port != recvaddr->in4.sin_port)
+        {
+          ninfo("response from wrong peer\n");
+
+          return false;
         }
+    }
+#endif
+
+#ifdef CONFIG_NET_IPv6
+  if (recvaddr->sa.sa_family == AF_INET6)
+    {
+      if (recvaddrlen != sizeof(struct sockaddr_in6) ||
+          memcmp(&xmitaddr->in6.sin6_addr,
+                 &recvaddr->in6.sin6_addr, sizeof(struct in6_addr)) != 0 ||
+          xmitaddr->in6.sin6_port != recvaddr->in6.sin6_port)
+        {
+          ninfo("response from wrong peer\n");
+
+          return false;
+        }
+    }
+#endif /* CONFIG_NET_IPv6 */
+
+  if (nbytes < NTP_DATAGRAM_MINSIZE)
+    {
+      /* Too short. */
+
+      ninfo("too short response\n");
+
+      return false;
+    }
+
+  if (GETVN(recv->lvm) != NTP_VERSION_V3 &&
+      GETVN(recv->lvm) != NTP_VERSION_V4)
+    {
+      /* Wrong version. */
+
+      ninfo("wrong version: %d\n", GETVN(recv->lvm));
+
+      return false;
+    }
+
+  if (GETMODE(recv->lvm) != 4)
+    {
+      /* Response not in server mode. */
+
+      ninfo("wrong mode: %d\n", GETMODE(recv->lvm));
+
+      return false;
+    }
+
+  if (ntp_is_kiss_of_death(recv, xmitaddr))
+    {
+      /* KoD, Kiss-o'-Death. Ignore response. */
+
+      ninfo("kiss-of-death response\n");
+
+      return false;
+    }
+
+  if (GETLI(recv->lvm) == 3)
+    {
+      /* Clock not synchronized. */
 
-      /* A full implementation of an NTP client would require much more.  I
-       * think we can skip most of that here.
+      ninfo("LI: not synchronized\n");
+
+      return false;
+    }
+
+  if (memcmp(recv->origtimestamp, xmit->xmittimestamp, 8) != 0)
+    {
+      /* "The Originate Timestamp in the server reply should match the
+       * Transmit Timestamp used in the client request."
        */
 
-      if (g_ntpc_daemon.state == NTP_RUNNING)
+      ninfo("xmittimestamp mismatch\n");
+
+      return false;
+    }
+
+  if (ntpc_getuint32(recv->reftimestamp) == 0)
+    {
+      /* Invalid timestamp. */
+
+      ninfo("invalid reftimestamp, 0x%08lx\n",
+            (unsigned long)ntpc_getuint32(recv->reftimestamp));
+
+      return false;
+    }
+
+  if (ntpc_getuint32(recv->recvtimestamp) == 0)
+    {
+      /* Invalid timestamp. */
+
+      ninfo("invalid recvtimestamp, 0x%08lx\n",
+            (unsigned long)ntpc_getuint32(recv->recvtimestamp));
+
+      return false;
+    }
+
+  if (ntpc_getuint32(recv->xmittimestamp) == 0)
+    {
+      /* Invalid timestamp. */
+
+      ninfo("invalid xmittimestamp, 0x%08lx\n",
+            (unsigned long)ntpc_getuint32(recv->xmittimestamp));
+
+      return false;
+    }
+
+  if ((int64_t)(ntpc_getuint64(recv->xmittimestamp) -
+                ntpc_getuint64(recv->recvtimestamp)) < 0)
+    {
+      /* Remote received our request after sending response? */
+
+      ninfo("invalid xmittimestamp & recvtimestamp pair\n");
+
+      return false;
+    }
+
+  buildtime = ntpc_get_compile_timestamp();
+  seconds = ntpc_getuint32(recv->recvtimestamp);
+  if (seconds > NTP2UNIX_TRANSLATION)
+    {
+      seconds -= NTP2UNIX_TRANSLATION;
+    }
+
+  if (seconds < buildtime)
+    {
+      /* Invalid timestamp. */
+
+      ninfo("invalid recvtimestamp, 0x%08lx\n",
+            (unsigned long)ntpc_getuint32(recv->recvtimestamp));
+
+      return false;
+    }
+
+  return true;
+}
+
+/****************************************************************************
+ * Name: ntpc_create_dgram_socket
+ ****************************************************************************/
+
+static int ntpc_create_dgram_socket(int domain)
+{
+  struct timeval tv;
+  int ret;
+  int sd;
+  int err;
+
+  /* Create a datagram socket  */
+
+  sd = socket(domain, SOCK_DGRAM, 0);
+  if (sd < 0)
+    {
+      err = errno;
+      nerr("ERROR: socket failed: %d\n", err);
+      errno = err;
+      return sd;
+    }
+
+  /* Setup a send timeout on the socket */
+
+  tv.tv_sec  = 5;
+  tv.tv_usec = 0;
+
+  ret = setsockopt(sd, SOL_SOCKET, SO_SNDTIMEO, &tv, sizeof(struct timeval));
+  if (ret < 0)
+    {
+      err = errno;
+      nerr("ERROR: setsockopt(SO_SNDTIMEO) failed: %d\n", errno);
+      goto err_close;
+    }
+
+  /* Setup a receive timeout on the socket */
+
+  tv.tv_sec  = 5;
+  tv.tv_usec = 0;
+
+  ret = setsockopt(sd, SOL_SOCKET, SO_RCVTIMEO, &tv, sizeof(struct timeval));
+  if (ret < 0)
+    {
+      err = errno;
+      nerr("ERROR: setsockopt(SO_RCVTIMEO) failed: %d\n", errno);
+      goto err_close;
+    }
+
+  return sd;
+
+err_close:
+  close(sd);
+  errno = err;
+  return ret;
+}
+
+/****************************************************************************
+ * Name: ntp_gethostip_multi
+ ****************************************************************************/
+
+static int ntp_gethostip_multi(const char *hostname,
+                               union ntp_addr_u *ipaddr, size_t nipaddr)
+{
+  struct addrinfo hints;
+  FAR struct addrinfo *info;
+  FAR struct addrinfo *next;
+  int ret;
+
+  memset(&hints, 0, sizeof(struct addrinfo));
+  hints.ai_family   = g_ntpc_daemon.family;
+  hints.ai_socktype = SOCK_DGRAM;
+  hints.ai_protocol = IPPROTO_UDP;
+  hints.ai_flags    = AI_NUMERICSERV;
+
+  ret = getaddrinfo(hostname, STR(CONFIG_NETUTILS_NTPCLIENT_PORTNO),
+                    &hints, &info);
+  if (ret != OK)
+    {
+      nerr("ERROR: getaddrinfo(%s): %d: %s\n", hostname,
+           ret, gai_strerror(ret));
+      return ERROR;
+    }
+
+  ret = 0;
+  for (next = info; next != NULL; next = next->ai_next)
+    {
+      if (ret >= nipaddr)
+        {
+          break;
+        }
+
+      memcpy(ipaddr, next->ai_addr, next->ai_addrlen);
+      ret++;
+      ipaddr++;
+    }
+
+  freeaddrinfo(info);
+  return ret;
+}
+
+/****************************************************************************
+ * Name: ntp_get_next_hostip
+ ****************************************************************************/
+
+static int ntp_get_next_hostip(struct ntp_servers_s *srvs,
+                               union ntp_addr_u *addr)
+{
+  int ret;
+
+  if (srvs->pos >= srvs->num)
+    {
+      char *hostname;
+
+      srvs->pos = 0;
+      srvs->num = 0;
+
+      /* Get next hostname. */
+
+      if (srvs->hostnext == NULL)
+        {
+          if (srvs->hostlist_str)
+            {
+              free(srvs->hostlist_str);
+              srvs->hostlist_str = NULL;
+            }
+
+          /* Allocate hostname list buffer */
+
+          srvs->hostlist_str = strdup(srvs->ntp_servers);
+          if (!srvs->hostlist_str)
+            {
+              return ERROR;
+            }
+
+          srvs->hostlist_saveptr = NULL;
+#ifndef __clang_analyzer__ /* Silence false 'possible memory leak'. */
+          srvs->hostnext =
+              strtok_r(srvs->hostlist_str, ";", &srvs->hostlist_saveptr);
+#endif
+        }
+
+      hostname = srvs->hostnext;
+      srvs->hostnext = strtok_r(NULL, ";", &srvs->hostlist_saveptr);
+
+      if (!hostname)
+        {
+          /* Invalid configuration. */
+
+          errno = EINVAL;
+          return ERROR;
+        }
+
+      /* Refresh DNS for new IP-addresses. */
+
+      ret = ntp_gethostip_multi(hostname, srvs->list,
+                                ARRAY_SIZE(srvs->list));
+      if (ret <= 0)
+        {
+          return ERROR;
+        }
+
+      srvs->num = ret;
+    }
+
+  *addr = srvs->list[srvs->pos++];
+
+  return OK;
+}
+
+/****************************************************************************
+ * Name: ntpc_get_ntp_sample
+ ****************************************************************************/
+
+static int ntpc_get_ntp_sample(struct ntp_servers_s *srvs,
+                               struct ntp_sample_s *samples, int curr_idx)
+{
+  struct ntp_sample_s *sample = &samples[curr_idx];
+  uint64_t xmit_time, recv_time;
+  union ntp_addr_u server;
+  union ntp_addr_u recvaddr;
+  struct ntp_datagram_s xmit;
+  struct ntp_datagram_s recv;
+  socklen_t socklen;
+  ssize_t nbytes;
+  int errval;
+  int retry = 0;
+  int nsamples = curr_idx;
+  bool addr_ok;
+  int ret;
+  int sd = -1;
+  int i;
+
+  /* Setup an ntp_addr_u with information about the server we are
+   * going to ask the time from.
+   */
+
+  memset(&server, 0, sizeof(server));
+
+  do
+    {
+      addr_ok = true;
+
+      ret = ntp_get_next_hostip(srvs, &server);
+      if (ret < 0)
+        {
+          errval = errno;
+
+          nerr("ERROR: ntp_get_next_hostip() failed: %d\n", errval);
+
+          goto sock_error;
+        }
+
+      /* Make sure that server not in exclusion list. */
+
+      if (ntp_address_in_kod_list(&server))
+        {
+          if (retry < MAX_SERVER_SELECTION_RETRIES)
+            {
+              ninfo("on KoD list. retry DNS.\n");
+
+              retry++;
+              addr_ok = false;
+              continue;
+            }
+          else
+            {
+              errval = -EALREADY;
+              goto sock_error;
+            }
+        }
+
+      /* Make sure that this sample is from new server. */
+
+      for (i = 0; i < nsamples; i++)
         {
-          ninfo("Waiting for %d seconds\n",
-                CONFIG_NETUTILS_NTPCLIENT_POLLDELAYSEC);
+          if (memcmp(&server, &samples[i].srv_addr, sizeof(server)) == 0)
+            {
+              /* Already have sample from this server, retry DNS. */
+
+              ninfo("retry DNS\n");
+
+              if (retry < MAX_SERVER_SELECTION_RETRIES)
+                {
+                  retry++;
+                  addr_ok = false;
+                  break;
+                }
+              else
+                {
+                  /* Accept same server if cannot get DNS for other server. */
+
+                  break;
+                }
+            }
+        }
+    }
+  while (!addr_ok);
+
+  /* Open socket. */
+
+  sd = ntpc_create_dgram_socket(server.sa.sa_family);
+  if (sd < 0)
+    {
+      errval = errno;
+
+      nerr("ERROR: ntpc_create_dgram_socket() failed: %d\n", errval);
+
+      goto sock_error;
+    }
+
+  /* Format the transmit datagram */
+
+  memset(&xmit, 0, sizeof(xmit));
+  xmit.lvm = MKLVM(0, NTP_VERSION, 3);
+
+  ninfo("Sending a NTPv%d packet\n", NTP_VERSION);
+
+  xmit_time = ntp_localtime();
+  ntpc_setuint64(xmit.xmittimestamp, xmit_time);
+
+  socklen = (server.sa.sa_family == AF_INET) ? sizeof(struct sockaddr_in)
+                                             : sizeof(struct sockaddr_in6);
+  ret = sendto(sd, &xmit, sizeof(struct ntp_datagram_s),
+               0, &server.sa, socklen);
+  if (ret < 0)
+    {
+      errval = errno;
+
+      nerr("ERROR: sendto() failed: %d\n", errval);
+
+      goto sock_error;
+    }
+
+  /* Attempt to receive a packet (with a timeout that was set up via
+   * setsockopt() above)
+   */
+
+  nbytes = recvfrom(sd, (void *)&recv, sizeof(struct ntp_datagram_s),
+                    0, &recvaddr.sa, &socklen);
+  recv_time = ntp_localtime();
+
+  /* Check if the received message was long enough to be a valid NTP
+   * datagram.
+   */
+
+  if (nbytes > 0 && ntpc_verify_recvd_ntp_datagram(
+                          &xmit, &recv, nbytes, &server, &recvaddr, socklen))
+    {
+      close(sd);
+      sd = -1;
+
+      ninfo("Calculate offset\n");
+
+      memset(sample, 0, sizeof(struct ntp_sample_s));
+
+      sample->srv_addr = server;
+
+      ntpc_calculate_offset(&sample->offset, &sample->delay,
+                            xmit_time, recv_time, recv.recvtimestamp,
+                            recv.xmittimestamp);
+
+      return OK;
+    }
+  else
+    {
+      /* Check for errors.  Short datagrams are handled as error. */
+
+      errval = errno;
 
-          sleep(CONFIG_NETUTILS_NTPCLIENT_POLLDELAYSEC);
+      if (nbytes >= 0)
+        {
+          errval = EMSGSIZE;
+        }
+      else
+        {
+          nerr("ERROR: recvfrom() failed: %d\n", errval);
         }
+
+      goto sock_error;
+    }
+
+sock_error:
+  if (sd >= 0)
+    {
+      close(sd);
+      sd = -1;
+    }
+
+  errno = errval;
+  return ERROR;
+}
+
+/****************************************************************************
+ * Name: ntpc_daemon
+ *
+ * Description:
+ *   This the the NTP client daemon.  This is a *very* minimal
+ *   implementation! An NTP request is and the system clock is set when the
+ *   response is received
+ *
+ ****************************************************************************/
+
+static int ntpc_daemon(int argc, char **argv)

Review comment:
       add FAR for all pointer

##########
File path: netutils/ntpclient/ntpclient.c
##########
@@ -524,7 +1429,22 @@ static int ntpc_daemon(int argc, char **argv)
  ****************************************************************************/
 
 /****************************************************************************
- * Name: ntpc_start
+ * Name: ntpc_dualstack_family()
+ *
+ * Description:
+ *   Set the protocol family used (AF_INET, AF_INET6 or AF_UNSPEC)
+ *
+ ****************************************************************************/
+
+void ntpc_dualstack_family(int family)
+{
+  sem_wait(&g_ntpc_daemon.lock);
+  g_ntpc_daemon.family = family;

Review comment:
       why not use Kconfig for dualstack support? it more like a compile time decision.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org