Merge branch 'update/lwip_sntp_comment_i32_timestamp' into 'master'

lwip: Document 32 bit NTP timestamp converstion to 64b time_t

Closes IDF-5956 and IDF-5592

See merge request espressif/esp-idf!20283

components/lwip/apps/sntp/sntp.c

@@ -1,16 +1,8 @@
-// Copyright 2015-2019 Espressif Systems (Shanghai) PTE LTD
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-
-//     http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
+/*
+ * SPDX-FileCopyrightText: 2015-2022 Espressif Systems (Shanghai) CO LTD
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ */
 
 #include <stdlib.h>
 #include <time.h>
@@ -120,6 +112,17 @@ bool sntp_restart(void)
 
 void sntp_set_system_time(uint32_t sec, uint32_t us)
 {
+    // Note: SNTP/NTP timestamp is defined as 64-bit fixed point int
+    // in seconds from 1900 (integer part is the first 32 bits)
+    // which overflows in 2036.
+    // The lifetime of the NTP timestamps has been extended by convention
+    // of the MSB bit 0 to span between 1968 and 2104. This is implemented
+    // in lwip sntp module, so this API returns number of seconds/milliseconds
+    // representing dates range from 1968 to 2104.
+    // (see: RFC-4330#section-3 and https://github.com/lwip-tcpip/lwip/blob/239918cc/src/apps/sntp/sntp.c#L129-L134)
+    // Warning: Here, we convert the 32 bit NTP timestamp to 64 bit representation
+    // of system time (time_t). This won't work for timestamps in future
+    // after some time in 2104
     struct timeval tv = { .tv_sec = sec, .tv_usec = us };
     sntp_sync_time(&tv);
 }
@@ -128,6 +131,9 @@ void sntp_get_system_time(uint32_t *sec, uint32_t *us)
 {
     struct timeval tv = { .tv_sec = 0, .tv_usec = 0 };
     gettimeofday(&tv, NULL);
+    // Warning: Here, we convert 64 bit representation of system time (time_t) to
+    // 32 bit NTP timestamp. This won't work for future timestamps after some time in 2104
+    // (see: RFC-4330#section-3)
     *(sec) = tv.tv_sec;
     *(us) = tv.tv_usec;
     sntp_set_sync_status(SNTP_SYNC_STATUS_RESET);

components/lwip/test/CMakeLists.txt

@@ -1,3 +0,0 @@
-idf_component_register(SRC_DIRS "."
-                    PRIV_REQUIRES test_utils)
-target_compile_options(${COMPONENT_LIB} PRIVATE "-Wno-format")

components/lwip/test_apps/.build-test-rules.yml

@@ -0,0 +1,6 @@
+# Documentation: .gitlab/ci/README.md#manifest-file-to-control-the-buildtest-apps
+
+components/lwip/test_apps:
+  disable_test:
+    - if: IDF_TARGET != "esp32"
+      reason: running this test for one target only is enough to be sufficiently confident about no regression in lwip

components/lwip/test_apps/CMakeLists.txt

@@ -0,0 +1,7 @@
+# This is the project CMakeLists.txt file for the test subproject
+cmake_minimum_required(VERSION 3.16)
+
+set(EXTRA_COMPONENT_DIRS "$ENV{IDF_PATH}/tools/unit-test-app/components")
+
+include($ENV{IDF_PATH}/tools/cmake/project.cmake)
+project(lwip_test)

components/lwip/test_apps/README.md

@@ -0,0 +1,2 @@
+| Supported Targets | ESP32 | ESP32-C2 | ESP32-C3 | ESP32-S2 | ESP32-S3 |
+| ----------------- | ----- | -------- | -------- | -------- | -------- |

components/lwip/test_apps/main/CMakeLists.txt

@@ -0,0 +1,4 @@
+idf_component_register(SRCS "lwip_test.c"
+                       REQUIRES test_utils
+                       INCLUDE_DIRS "."
+                       PRIV_REQUIRES unity lwip test_utils)

components/lwip/test_apps/main/lwip_test.c

@@ -3,23 +3,41 @@
  *
  * SPDX-License-Identifier: Apache-2.0
  */
+#include <stdio.h>
+#include <string.h>
+#include <esp_types.h>
+
 #include "freertos/FreeRTOS.h"
 #include "freertos/event_groups.h"
 #include "test_utils.h"
 #include "unity.h"
+#include "unity_fixture.h"
+
+#include "soc/soc_caps.h"
+
 #include "lwip/inet.h"
 #include "lwip/netdb.h"
 #include "lwip/sockets.h"
 #include "ping/ping_sock.h"
 #include "dhcpserver/dhcpserver.h"
+#include "esp_sntp.h"
 
 #define ETH_PING_END_BIT BIT(1)
 #define ETH_PING_DURATION_MS (5000)
 #define ETH_PING_END_TIMEOUT_MS (ETH_PING_DURATION_MS * 2)
 #define TEST_ICMP_DESTINATION_DOMAIN_NAME "127.0.0.1"
 
-#if !TEMPORARY_DISABLED_FOR_TARGETS(ESP32C2)
-//IDF-5047
+
+TEST_GROUP(lwip);
+
+TEST_SETUP(lwip)
+{
+}
+
+TEST_TEAR_DOWN(lwip)
+{
+}
+
 static void test_on_ping_success(esp_ping_handle_t hdl, void *args)
 {
     uint8_t ttl;
@@ -31,7 +49,7 @@ static void test_on_ping_success(esp_ping_handle_t hdl, void *args)
     esp_ping_get_profile(hdl, ESP_PING_PROF_IPADDR, &target_addr, sizeof(target_addr));
     esp_ping_get_profile(hdl, ESP_PING_PROF_SIZE, &recv_len, sizeof(recv_len));
     esp_ping_get_profile(hdl, ESP_PING_PROF_TIMEGAP, &elapsed_time, sizeof(elapsed_time));
-    printf("%d bytes from %s icmp_seq=%d ttl=%d time=%d ms\n",
+    printf("%" PRId32 "bytes from %s icmp_seq=%d ttl=%d time=%" PRId32 " ms\n",
            recv_len, inet_ntoa(target_addr.u_addr.ip4), seqno, ttl, elapsed_time);
 }
 
@@ -54,13 +72,13 @@ static void test_on_ping_end(esp_ping_handle_t hdl, void *args)
     esp_ping_get_profile(hdl, ESP_PING_PROF_REQUEST, &transmitted, sizeof(transmitted));
     esp_ping_get_profile(hdl, ESP_PING_PROF_REPLY, &received, sizeof(received));
     esp_ping_get_profile(hdl, ESP_PING_PROF_DURATION, &total_time_ms, sizeof(total_time_ms));
-    printf("%d packets transmitted, %d received, time %dms\n", transmitted, received, total_time_ms);
+    printf("%" PRId32 " packets transmitted, %" PRId32 " received, time %" PRId32 "ms\n", transmitted, received, total_time_ms);
     if (transmitted == received) {
         xEventGroupSetBits(eth_event_group, ETH_PING_END_BIT);
     }
 }
 
-TEST_CASE("localhost ping test", "[lwip]")
+TEST(lwip, localhost_ping_test)
 {
     EventBits_t bits;
     EventGroupHandle_t eth_event_group = xEventGroupCreate();
@@ -112,9 +130,8 @@ TEST_CASE("localhost ping test", "[lwip]")
 
     vEventGroupDelete(eth_event_group);
 }
-#endif //!TEMPORARY_DISABLED_FOR_TARGETS(ESP32C2)
 
-TEST_CASE("dhcp server init/deinit", "[lwip][leaks=0]")
+TEST(lwip, dhcp_server_init_deinit)
 {
     dhcps_t *dhcps = dhcps_new();
     TEST_ASSERT_NOT_NULL(dhcps);
@@ -124,7 +141,7 @@ TEST_CASE("dhcp server init/deinit", "[lwip][leaks=0]")
     dhcps_delete(dhcps);
 }
 
-TEST_CASE("dhcp server start/stop on localhost", "[lwip]")
+TEST(lwip, dhcp_server_start_stop_localhost)
 {
     test_case_uses_tcpip();
     dhcps_t *dhcps = dhcps_new();
@@ -142,3 +159,109 @@ TEST_CASE("dhcp server start/stop on localhost", "[lwip]")
     TEST_ASSERT(dhcps_stop(dhcps, netif) == ERR_OK);
     dhcps_delete(dhcps);
 }
+
+
+int test_sntp_server_create(void)
+{
+    struct sockaddr_in dest_addr_ip4;
+    int sock = -1;
+    dest_addr_ip4.sin_addr.s_addr = htonl(INADDR_LOOPBACK);
+    dest_addr_ip4.sin_family = AF_INET;
+    dest_addr_ip4.sin_port = htons(LWIP_IANA_PORT_SNTP);
+    sock = socket(AF_INET, SOCK_DGRAM, IPPROTO_IP);
+    TEST_ASSERT_GREATER_OR_EQUAL(0, sock);
+    int reuse_en = 1;
+    TEST_ASSERT_GREATER_OR_EQUAL(0, setsockopt(sock, SOL_SOCKET, SO_REUSEADDR, &reuse_en, sizeof(reuse_en)));
+    TEST_ASSERT_GREATER_OR_EQUAL(0, bind(sock, (struct sockaddr*) &dest_addr_ip4, sizeof(dest_addr_ip4)));
+    return sock;
+}
+
+bool test_sntp_server_reply_with_time(int sock, int year, bool msb_flag)
+{
+    struct sntp_timestamp {
+        uint32_t seconds;
+        uint32_t fraction;
+    };
+    const int SNTP_MSG_LEN = 48;
+    const int SNTP_MODE_CLIENT = 0x03;
+    const int SNTP_MODE_SERVER = 0x04;
+    const int SNTP_MODE_MASK = 0x07;
+    const int SNTP_OFFSET_STRATUM = 1;
+
+    char rx_buffer[SNTP_MSG_LEN];
+    struct sockaddr_storage source_addr;
+    socklen_t socklen = sizeof(source_addr);
+
+    int len = recvfrom(sock, rx_buffer, SNTP_MSG_LEN, 0, (struct sockaddr *)&source_addr, &socklen);
+    if (len == SNTP_MSG_LEN && source_addr.ss_family == PF_INET && (SNTP_MODE_MASK & rx_buffer[0]) == SNTP_MODE_CLIENT) {
+        // modify the client's request to act as a server's response with the injected *xmit* timestamp
+        rx_buffer[0] &= ~SNTP_MODE_CLIENT;
+        rx_buffer[0] |= SNTP_MODE_SERVER;
+        rx_buffer[SNTP_OFFSET_STRATUM] = 0x1;
+        // set the desired timestamp
+        struct sntp_timestamp *timestamp = (struct sntp_timestamp *)(rx_buffer + SNTP_MSG_LEN - sizeof(struct sntp_timestamp)); // xmit is the last timestamp in the datagram
+        int64_t seconds_since_1900 = (365*24*60*60 /* seconds per year */ + 24*60*60/4 /* ~ seconds per leap year */ )*(year-1900);
+        // apply the MSB convention (set: 1968-2036, cleared: 2036-2104)
+        timestamp->seconds = htonl( (msb_flag ? 0x80000000 : 0) | (0xFFFFFFFF & seconds_since_1900) );
+        len = sendto(sock, rx_buffer, SNTP_MSG_LEN, 0, (struct sockaddr *)&source_addr, sizeof(source_addr));
+        if (len == SNTP_MSG_LEN) {
+            return true;
+        }
+    }
+    return false;
+}
+
+void test_sntp_timestamps(int year, bool msb_flag)
+{
+    int sock = test_sntp_server_create();
+
+    // setup lwip's SNTP in polling mode
+    sntp_setoperatingmode(SNTP_OPMODE_POLL);
+    sntp_setservername(0, "127.0.0.1");
+    sntp_init();
+
+    // wait until time sync
+    int retry = 0;
+    while (sntp_get_sync_status() == SNTP_SYNC_STATUS_RESET) {
+        TEST_ASSERT_TRUE(test_sntp_server_reply_with_time(sock, year, msb_flag)); // post the SNTP server's reply
+        retry++;
+        TEST_ASSERT_LESS_THAN(3, retry);
+    }
+
+    // check time and assert the year
+    time_t now;
+    struct tm timeinfo;
+    time(&now);
+    localtime_r(&now, &timeinfo);
+    TEST_ASSERT_EQUAL(year, 1900 + timeinfo.tm_year);
+
+    // close the SNTP and the fake server
+    sntp_stop();
+    close(sock);
+}
+
+TEST(lwip, sntp_client_time_2015)
+{
+    test_case_uses_tcpip();
+    test_sntp_timestamps(2015, true); // NTP timestamp MSB is set for time before 2036
+}
+
+TEST(lwip, sntp_client_time_2048)
+{
+    test_case_uses_tcpip();
+    test_sntp_timestamps(2048, false); // NTP timestamp MSB is cleared for time after 2036
+}
+
+TEST_GROUP_RUNNER(lwip)
+{
+    RUN_TEST_CASE(lwip, localhost_ping_test)
+    RUN_TEST_CASE(lwip, dhcp_server_init_deinit)
+    RUN_TEST_CASE(lwip, dhcp_server_start_stop_localhost)
+    RUN_TEST_CASE(lwip, sntp_client_time_2015)
+    RUN_TEST_CASE(lwip, sntp_client_time_2048)
+}
+
+void app_main(void)
+{
+    UNITY_MAIN(lwip);
+}

components/lwip/test_apps/pytest_lwip.py

@@ -0,0 +1,11 @@
+# SPDX-FileCopyrightText: 2022 Espressif Systems (Shanghai) CO LTD
+# SPDX-License-Identifier: Apache-2.0
+
+import pytest
+from pytest_embedded import Dut
+
+
+@pytest.mark.esp32
+@pytest.mark.generic
+def test_lwip(dut: Dut) -> None:
+    dut.expect_unity_test_output()

components/lwip/test_apps/sdkconfig.defaults

@@ -0,0 +1,2 @@
+CONFIG_UNITY_ENABLE_FIXTURE=y
+CONFIG_UNITY_ENABLE_IDF_TEST_RUNNER=n

tools/ci/check_copyright_ignore.txt

@@ -790,7 +790,6 @@ components/log/esp_log_private.h
 components/log/host_test/log_test/main/log_test.cpp
 components/log/log_linux.c
 components/lwip/apps/ping/ping.c
-components/lwip/apps/sntp/sntp.c
 components/lwip/include/apps/dhcpserver/dhcpserver_options.h
 components/lwip/include/apps/esp_ping.h
 components/lwip/include/apps/ping/ping.h