From 9e31c5077d849485edefd05833c0fe5e031cbdd5 Mon Sep 17 00:00:00 2001 From: David Cermak Date: Wed, 21 Sep 2022 17:38:37 +0200 Subject: [PATCH 1/3] lwip: Document 32 bit NTP timestamp converstion to 64b time_t --- components/lwip/apps/sntp/sntp.c | 32 +++++++++++++++++------------ tools/ci/check_copyright_ignore.txt | 1 - 2 files changed, 19 insertions(+), 14 deletions(-) diff --git a/components/lwip/apps/sntp/sntp.c b/components/lwip/apps/sntp/sntp.c index 677574d200..ef3bb7c82c 100644 --- a/components/lwip/apps/sntp/sntp.c +++ b/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 #include @@ -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); diff --git a/tools/ci/check_copyright_ignore.txt b/tools/ci/check_copyright_ignore.txt index 1a83f09b8f..0ebabda0d5 100644 --- a/tools/ci/check_copyright_ignore.txt +++ b/tools/ci/check_copyright_ignore.txt @@ -792,7 +792,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 From ad0da9cd2b7afd9519aa9a1d26ad8d20830ad43b Mon Sep 17 00:00:00 2001 From: David Cermak Date: Thu, 29 Sep 2022 09:17:13 +0200 Subject: [PATCH 2/3] lwip: Migrate unit tests to test_apps --- components/lwip/test/CMakeLists.txt | 3 -- .../lwip/test_apps/.build-test-rules.yml | 6 +++ components/lwip/test_apps/CMakeLists.txt | 7 +++ components/lwip/test_apps/README.md | 2 + components/lwip/test_apps/main/CMakeLists.txt | 4 ++ .../main/lwip_test.c} | 45 +++++++++++++++---- components/lwip/test_apps/pytest_lwip.py | 11 +++++ components/lwip/test_apps/sdkconfig.defaults | 2 + 8 files changed, 69 insertions(+), 11 deletions(-) delete mode 100644 components/lwip/test/CMakeLists.txt create mode 100644 components/lwip/test_apps/.build-test-rules.yml create mode 100644 components/lwip/test_apps/CMakeLists.txt create mode 100644 components/lwip/test_apps/README.md create mode 100644 components/lwip/test_apps/main/CMakeLists.txt rename components/lwip/{test/test_lwip_apps.c => test_apps/main/lwip_test.c} (86%) create mode 100644 components/lwip/test_apps/pytest_lwip.py create mode 100644 components/lwip/test_apps/sdkconfig.defaults diff --git a/components/lwip/test/CMakeLists.txt b/components/lwip/test/CMakeLists.txt deleted file mode 100644 index d1d0882c5e..0000000000 --- a/components/lwip/test/CMakeLists.txt +++ /dev/null @@ -1,3 +0,0 @@ -idf_component_register(SRC_DIRS "." - PRIV_REQUIRES test_utils) -target_compile_options(${COMPONENT_LIB} PRIVATE "-Wno-format") diff --git a/components/lwip/test_apps/.build-test-rules.yml b/components/lwip/test_apps/.build-test-rules.yml new file mode 100644 index 0000000000..42eff56234 --- /dev/null +++ b/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 diff --git a/components/lwip/test_apps/CMakeLists.txt b/components/lwip/test_apps/CMakeLists.txt new file mode 100644 index 0000000000..69a0d3445b --- /dev/null +++ b/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) diff --git a/components/lwip/test_apps/README.md b/components/lwip/test_apps/README.md new file mode 100644 index 0000000000..b5be4985c5 --- /dev/null +++ b/components/lwip/test_apps/README.md @@ -0,0 +1,2 @@ +| Supported Targets | ESP32 | ESP32-C2 | ESP32-C3 | ESP32-S2 | ESP32-S3 | +| ----------------- | ----- | -------- | -------- | -------- | -------- | diff --git a/components/lwip/test_apps/main/CMakeLists.txt b/components/lwip/test_apps/main/CMakeLists.txt new file mode 100644 index 0000000000..bfd19d1277 --- /dev/null +++ b/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) diff --git a/components/lwip/test/test_lwip_apps.c b/components/lwip/test_apps/main/lwip_test.c similarity index 86% rename from components/lwip/test/test_lwip_apps.c rename to components/lwip/test_apps/main/lwip_test.c index d9a9f9dacd..99519d3ccf 100644 --- a/components/lwip/test/test_lwip_apps.c +++ b/components/lwip/test_apps/main/lwip_test.c @@ -3,10 +3,19 @@ * * SPDX-License-Identifier: Apache-2.0 */ +#include +#include +#include + #include "freertos/FreeRTOS.h" #include "freertos/event_groups.h" #include "test_utils.h" #include "unity.h" +#include "unity_fixture.h" + +#include "unity_test_utils.h" +#include "soc/soc_caps.h" + #include "lwip/inet.h" #include "lwip/netdb.h" #include "lwip/sockets.h" @@ -18,8 +27,17 @@ #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,15 @@ TEST_CASE("dhcp server start/stop on localhost", "[lwip]") TEST_ASSERT(dhcps_stop(dhcps, netif) == ERR_OK); dhcps_delete(dhcps); } + +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) +} + +void app_main(void) +{ + UNITY_MAIN(lwip); +} diff --git a/components/lwip/test_apps/pytest_lwip.py b/components/lwip/test_apps/pytest_lwip.py new file mode 100644 index 0000000000..32de3ef610 --- /dev/null +++ b/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() diff --git a/components/lwip/test_apps/sdkconfig.defaults b/components/lwip/test_apps/sdkconfig.defaults new file mode 100644 index 0000000000..168e08d4cd --- /dev/null +++ b/components/lwip/test_apps/sdkconfig.defaults @@ -0,0 +1,2 @@ +CONFIG_UNITY_ENABLE_FIXTURE=y +CONFIG_UNITY_ENABLE_IDF_TEST_RUNNER=n From 1f2c16af97bff4c26332dd99ab4d6146777652b6 Mon Sep 17 00:00:00 2001 From: David Cermak Date: Thu, 29 Sep 2022 14:17:14 +0200 Subject: [PATCH 3/3] lwip/test: Add SNTP cases to check NTP timestamp overflow --- components/lwip/test_apps/main/lwip_test.c | 96 +++++++++++++++++++++- 1 file changed, 95 insertions(+), 1 deletion(-) diff --git a/components/lwip/test_apps/main/lwip_test.c b/components/lwip/test_apps/main/lwip_test.c index 99519d3ccf..5b8b178388 100644 --- a/components/lwip/test_apps/main/lwip_test.c +++ b/components/lwip/test_apps/main/lwip_test.c @@ -13,7 +13,6 @@ #include "unity.h" #include "unity_fixture.h" -#include "unity_test_utils.h" #include "soc/soc_caps.h" #include "lwip/inet.h" @@ -21,6 +20,7 @@ #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) @@ -160,11 +160,105 @@ TEST(lwip, dhcp_server_start_stop_localhost) 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)