[utest][lwip]add lwip api testcase. #10675
Conversation
github-actions
bot
added
action
📌 Code Review Assignment🏷️ Tag: componentsReviewers: Maihuanyi Changed Files (Click to expand)
🏷️ Tag: workflowReviewers: Rbb666 kurisaW supperthomas Changed Files (Click to expand)
📊 Current Review Status (Last Updated: 2025-09-10 15:02 CST)
📝 Review Instructions
|
Rbb666
force-pushed
the
testcase
branch
3 times, most recently
from
6c10a75 to
61233c4
Compare
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
| src = [] | ||
| CPPPATH = [cwd] | ||
|
|
||
| if GetDepend('RT_UTEST_TC_USING_LWIP'): |
There was a problem hiding this comment.
为何不依赖于 RT_UTEST_USING_ALL_CASES ？
There was a problem hiding this comment.
这个原因和此评论的 第二条 一样
我不是很清楚为啥会影响 CI。不过如果按照这个缘由继续下去，看上去 RT_UTEST_USING_ALL_CASES 会失去意义。
There was a problem hiding this comment.
这个文件太大了，请根据不同的测试内容拆分，看上去有至少 7 个测试子类。
There was a problem hiding this comment.
我认为这个测试会覆盖到LWIP的基本API测试，是可以一个test工作流运行完成的，所以就没拆开
There was a problem hiding this comment.
我认为这个测试会覆盖到LWIP的基本API测试，是可以一个test工作流运行完成的，所以就没拆开
这个和 test 工作流是否一个还是多个其实并无关系，只是和文件内容的组织方式有关。这个问题我不纠结，如果 LWIP 有 maintainer 就让他去决定吧。
components/net/sal/src/sal_socket.c
Outdated
There was a problem hiding this comment.
这个是改了 NET 源码，不是 utest，建议独立 pr 或者 commit 提交
components/net/utest/tc_lwip.c
Outdated
|
|
||
| #include <rtthread.h> | ||
|
|
||
| #ifdef RT_UTEST_TC_USING_LWIP |
There was a problem hiding this comment.
这些 ifdef 感觉是没有必要的，我们应该保证没有 enable这些配置则这个文件根本就不会参与编译。
components/net/utest/tc_lwip.c
Outdated
|
|
||
| /* Use Kconfig parameters or fallback to defaults */ | ||
| #ifndef RT_UTEST_LWIP_TEST_URL | ||
| #define RT_UTEST_LWIP_TEST_URL "www.rt-thread.org" |
There was a problem hiding this comment.
这些配置的默认值在 Kconfig 里不是都提供了么，源码里再定义一遍感觉是多余的，下面的一些定义类似。
components/net/utest/tc_lwip.c
Outdated
| static rt_event_t tcp_event = RT_NULL; | ||
| static rt_event_t udp_event = RT_NULL; | ||
|
|
||
| #ifdef RT_UTEST_LWIP_DNS_TEST |
There was a problem hiding this comment.
把这个文件按测试类别拆分后，这些判断应该通过 SConscript 就解决了。
建议在 utest 的源码中加入上述测试描述，方便日后理解。 |
可以的 |
Rbb666
changed the title
已添加 |
|
@Ryan-CW-Code 能否帮忙 review 下？ |
| int result = 0; | ||
| char *resolved_ip = RT_NULL; | ||
|
|
||
| phost = lwip_gethostbyname(rtt_url); |
There was a problem hiding this comment.
可以使用example.com/example.net/example.org这类IANA 保留域名进行测试，但是ip不固定，可以获取两次判断ip是否一致。
也可以使用dns.google 8.8.8.8 Google 公共 DNS /国内dns.alidns.com public1.114dns.com 这些ip变动的可能性都很小
There was a problem hiding this comment.
这块默认使用的是rtthread官方网站进行域名解析：www.rt-thread.org，感觉问题也不大，当然这个选项是可以在konfig里面配置的，如果后面真要更新手动改下即可
There was a problem hiding this comment.
可以，想的是判断域名和ip对应不对应。
这部分lwip内部已经有测试了，却只用判断phost返回结果就可以
| uassert_true(phost != RT_NULL); | ||
| } | ||
|
|
||
| static void test_get_free_addrinfo(void) |
components/net/utest/tc_lwip.c
Outdated
| } | ||
| else | ||
| { | ||
| old_tick = rt_tick_get(); |
There was a problem hiding this comment.
+1 这里设置应该会导致超时5秒判断失效？
components/net/utest/tc_lwip.c
Outdated
| rt_kprintf("TCP client sent %d bytes: %c...\n", ret, send_buf[0]); | ||
|
|
||
| /* receive echo from server */ | ||
| ret = lwip_recv(sock, recv_buf, LWIP_TCP_TEST_BUF_SIZE, 0); |
| uassert_true(RT_TRUE); | ||
| } | ||
|
|
||
| ret = lwip_getpeername(connected, (struct sockaddr *)&peer_addr, &peer_len); |
There was a problem hiding this comment.
要不要增加 peer_addr 有效性检查
components/net/utest/tc_lwip.c
Outdated
| } | ||
|
|
||
| /* wait for server to be ready */ | ||
| rt_thread_mdelay(200); |
| lwip_close(sock); | ||
| } | ||
|
|
||
| static void test_socket_options(void) |
There was a problem hiding this comment.
可以再增加多一些选项，比如SO_KEEPALIVE、SO_RCVBUF、SO_LINGER等。
但是同开头说的感觉没有必要，lwip肯定测试过。
| lwip_close(sock); | ||
| } | ||
|
|
||
| static void test_address_conversion(void) |
There was a problem hiding this comment.
可以加一些非法ip进行测量，比如0.0.0.0或者255.255.255.255或者明显错误的ip999.999.999.999
|
|
||
| /* Test netif_set_up and netif_set_down */ | ||
| netif_set_down(netif); | ||
| uassert_true(!(netif->flags & NETIF_FLAG_UP)); |
There was a problem hiding this comment.
可以使用netif_is_up宏。
这里多网卡可能也需要测试
There was a problem hiding this comment.
qemu上暂时只有一个网卡，为了保证ci通过，所以没有添加多网卡测试
| @@ -0,0 +1,865 @@ | |||
| /* | |||
There was a problem hiding this comment.
个人感觉应该更多的测试port接口和netdev对lwip封装接口的稳定性。
lwip内核官方的测试覆盖很完善了，可参考lwip源码下test文件夹
|
已添加：
@Ryan-CW-Code 请再次review |
| rt_event_delete(udp_event); | ||
| } | ||
|
|
||
| static void test_icmp_ping(void) |
There was a problem hiding this comment.
这里刚才忘记评论了
这里可以使用lwip的ping命令代替，下面icmp封包不是很完善。
设置了IPPROTO_ICMP lwip也不会自己封装icmp报文
如果是为了测试RAW socket方式这样就可以，lwip对icmp协议本身也已经测试过了
这里更多是在测试 原始套接字 收发能力，而不是严格意义上的 ICMP 协议功能
components/net/utest/tc_lwip.c
Outdated
| } | ||
| else | ||
| { | ||
| old_tick = rt_tick_get(); |
There was a problem hiding this comment.
这里给old_tick赋值会让上面的超时5秒判断失效吗？
|
LGTM |
supperthomas
requested review from
Copilot and
unicornx
and removed request for
Ryan-CW-Code
@Ryan-CW-Code 大佬，你可以参与到NET下面的maintainer审核吗？ |
There was a problem hiding this comment.
Pull Request Overview
This PR adds comprehensive unit tests for the lwIP network stack in RT-Thread, implementing loopback testing for various network APIs and protocols. The tests provide verification for essential networking functionality including DNS resolution, TCP/UDP communication, and network interface management.
- Implements complete lwIP API test suite covering DNS, TCP, UDP, ICMP, socket options, address conversion, and network interface management
- Adds configurable unit tests with proper Kconfig integration and build system support
- Integrates CI automation for continuous testing of lwIP functionality
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| components/net/utest/tc_lwip.c | Main test implementation with comprehensive lwIP API test cases |
| components/net/utest/SConscript | Build configuration for lwIP unit tests |
| components/net/utest/Kconfig | Configuration options for enabling/disabling specific test modules |
| Kconfig.utestcases | Integration of lwIP tests into main utest configuration |
| .github/workflows/utest_auto_run.yml | CI workflow configuration for automated testing |
| .github/utest/lwip/lwip.cfg | CI test configuration file with required settings |
| rt_kprintf("TCP server accepted connection\n"); | ||
| uassert_true(RT_TRUE); | ||
| } | ||
|
|
There was a problem hiding this comment.
English: The variable peer_len is used uninitialized. It should be initialized to the size of peer_addr before calling lwip_getpeername.
中文：变量 peer_len 未初始化就被使用。在调用 lwip_getpeername 之前应该将其初始化为 peer_addr 的大小。
| peer_len = sizeof(peer_addr); |
Copilot uses AI. Check for mistakes.
| /* recv data with additional timeout check */ | ||
| { | ||
| /* Reset timeout for select */ | ||
| timeout.tv_sec = 5; | ||
| timeout.tv_usec = 0; | ||
|
|
||
| /* Additional select check for data availability */ | ||
| ret = lwip_select(maxsock + 1, &fdread, NULL, NULL, &timeout); | ||
| if (ret <= 0) | ||
| { | ||
| rt_kprintf("UDP server additional select timeout or failed: %d\n", ret); | ||
| uassert_true(RT_FALSE); | ||
| goto __exit; | ||
| } | ||
|
|
||
| /* data is available, now receive */ | ||
| client_len = sizeof(client_addr); | ||
| bytes_received = lwip_recvfrom(sock, recv_data, LWIP_UDP_RECV_BUF, 0, (struct sockaddr *)&client_addr, &client_len); | ||
| if (bytes_received <= 0) | ||
| { | ||
| rt_kprintf("UDP server recvfrom failed: received %d\n", bytes_received); | ||
| uassert_true(RT_FALSE); | ||
| goto __exit; | ||
| } |
There was a problem hiding this comment.
English: This code duplicates the select() operation that was already performed at line 644. The second select() call is redundant since the first call already confirmed data availability. Consider removing this duplicate check to simplify the code.
中文：这段代码重复了已在第644行执行的 select() 操作。第二次 select() 调用是多余的，因为第一次调用已经确认了数据可用性。建议移除这个重复检查以简化代码。
| /* recv data with additional timeout check */ | |
| { | |
| /* Reset timeout for select */ | |
| timeout.tv_sec = 5; | |
| timeout.tv_usec = 0; | |
| /* Additional select check for data availability */ | |
| ret = lwip_select(maxsock + 1, &fdread, NULL, NULL, &timeout); | |
| if (ret <= 0) | |
| { | |
| rt_kprintf("UDP server additional select timeout or failed: %d\n", ret); | |
| uassert_true(RT_FALSE); | |
| goto __exit; | |
| } | |
| /* data is available, now receive */ | |
| client_len = sizeof(client_addr); | |
| bytes_received = lwip_recvfrom(sock, recv_data, LWIP_UDP_RECV_BUF, 0, (struct sockaddr *)&client_addr, &client_len); | |
| if (bytes_received <= 0) | |
| { | |
| rt_kprintf("UDP server recvfrom failed: received %d\n", bytes_received); | |
| uassert_true(RT_FALSE); | |
| goto __exit; | |
| } | |
| /* data is available, now receive */ | |
| client_len = sizeof(client_addr); | |
| bytes_received = lwip_recvfrom(sock, recv_data, LWIP_UDP_RECV_BUF, 0, (struct sockaddr *)&client_addr, &client_len); | |
| if (bytes_received <= 0) | |
| { | |
| rt_kprintf("UDP server recvfrom failed: received %d\n", bytes_received); | |
| uassert_true(RT_FALSE); | |
| goto __exit; |
Copilot uses AI. Check for mistakes.
| FD_ZERO(&fdread); | ||
| FD_SET(sock, &fdread); |
There was a problem hiding this comment.
English: The fdread variable is reused without being reset properly between the two select() calls. The FD_SET should be called again before the second select() operation to ensure the file descriptor set is correctly initialized.
中文：fdread 变量在两次 select() 调用之间被重复使用但没有正确重置。在第二次 select() 操作之前应该再次调用 FD_SET 以确保文件描述符集正确初始化。
Copilot uses AI. Check for mistakes.
| if ((tcp_event->set & EVENT_FLAG_TCP_CLIENT_SUCCESS) && (tcp_event->set & EVENT_FLAG_TCP_SERVER_SUCCESS)) | ||
| { | ||
| uassert_true(RT_TRUE); | ||
| break; | ||
| } | ||
| else if ((tcp_event->set & EVENT_FLAG_TCP_CLIENT_FAILED) || (tcp_event->set & EVENT_FLAG_TCP_SERVER_FAILED)) |
There was a problem hiding this comment.
English: Direct access to the set field of the event structure violates encapsulation. Use rt_event_recv() to properly check event flags instead of directly accessing internal fields.
中文：直接访问事件结构体的 set 字段违反了封装原则。应该使用 rt_event_recv() 来正确检查事件标志，而不是直接访问内部字段。
| if ((tcp_event->set & EVENT_FLAG_TCP_CLIENT_SUCCESS) && (tcp_event->set & EVENT_FLAG_TCP_SERVER_SUCCESS)) | |
| { | |
| uassert_true(RT_TRUE); | |
| break; | |
| } | |
| else if ((tcp_event->set & EVENT_FLAG_TCP_CLIENT_FAILED) || (tcp_event->set & EVENT_FLAG_TCP_SERVER_FAILED)) | |
| rt_uint32_t recved = 0; | |
| rt_err_t result = rt_event_recv(tcp_event, | |
| EVENT_FLAG_TCP_CLIENT_SUCCESS | EVENT_FLAG_TCP_SERVER_SUCCESS | | |
| EVENT_FLAG_TCP_CLIENT_FAILED | EVENT_FLAG_TCP_SERVER_FAILED, | |
| RT_EVENT_FLAG_OR | RT_EVENT_FLAG_CLEAR, | |
| 0, /* non-blocking */ | |
| &recved); | |
| if ((recved & EVENT_FLAG_TCP_CLIENT_SUCCESS) && (recved & EVENT_FLAG_TCP_SERVER_SUCCESS)) | |
| { | |
| uassert_true(RT_TRUE); | |
| break; | |
| } | |
| else if ((recved & EVENT_FLAG_TCP_CLIENT_FAILED) || (recved & EVENT_FLAG_TCP_SERVER_FAILED)) |
Copilot uses AI. Check for mistakes.
| if ((tcp_event->set & EVENT_FLAG_TCP_CLIENT_SUCCESS) && (tcp_event->set & EVENT_FLAG_TCP_SERVER_SUCCESS)) | ||
| { | ||
| uassert_true(RT_TRUE); | ||
| break; | ||
| } | ||
| else if ((tcp_event->set & EVENT_FLAG_TCP_CLIENT_FAILED) || (tcp_event->set & EVENT_FLAG_TCP_SERVER_FAILED)) |
There was a problem hiding this comment.
English: Direct access to the set field of the event structure violates encapsulation. Use rt_event_recv() to properly check event flags instead of directly accessing internal fields.
中文：直接访问事件结构体的 set 字段违反了封装原则。应该使用 rt_event_recv() 来正确检查事件标志，而不是直接访问内部字段。
| if ((tcp_event->set & EVENT_FLAG_TCP_CLIENT_SUCCESS) && (tcp_event->set & EVENT_FLAG_TCP_SERVER_SUCCESS)) | |
| { | |
| uassert_true(RT_TRUE); | |
| break; | |
| } | |
| else if ((tcp_event->set & EVENT_FLAG_TCP_CLIENT_FAILED) || (tcp_event->set & EVENT_FLAG_TCP_SERVER_FAILED)) | |
| rt_uint32_t e; | |
| /* Check for both client and server success flags */ | |
| if (rt_event_recv(tcp_event, EVENT_FLAG_TCP_CLIENT_SUCCESS | EVENT_FLAG_TCP_SERVER_SUCCESS, | |
| RT_EVENT_FLAG_AND | RT_EVENT_FLAG_CLEAR, 0, &e) == RT_EOK) | |
| { | |
| uassert_true(RT_TRUE); | |
| break; | |
| } | |
| /* Check for either client or server failure flags */ | |
| else if (rt_event_recv(tcp_event, EVENT_FLAG_TCP_CLIENT_FAILED | EVENT_FLAG_TCP_SERVER_FAILED, | |
| RT_EVENT_FLAG_OR | RT_EVENT_FLAG_CLEAR, 0, &e) == RT_EOK) |
Copilot uses AI. Check for mistakes.
| if ((udp_event->set & EVENT_FLAG_UDP_CLIENT_SUCCESS) && (udp_event->set & EVENT_FLAG_UDP_SERVER_SUCCESS)) | ||
| { | ||
| uassert_true(RT_TRUE); | ||
| break; | ||
| } | ||
| else if ((udp_event->set & EVENT_FLAG_UDP_CLIENT_FAILED) || (udp_event->set & EVENT_FLAG_UDP_SERVER_FAILED)) | ||
| { | ||
| uassert_true(RT_FALSE); | ||
| break; | ||
| } | ||
|
|
||
| rt_thread_mdelay(2 * RT_TICK_PER_SECOND); |
There was a problem hiding this comment.
English: Same encapsulation violation as in the TCP test. Use rt_event_recv() to properly check event flags instead of directly accessing the set field.
中文：与TCP测试中相同的封装违规问题。应该使用 rt_event_recv() 来正确检查事件标志，而不是直接访问 set 字段。
| if ((udp_event->set & EVENT_FLAG_UDP_CLIENT_SUCCESS) && (udp_event->set & EVENT_FLAG_UDP_SERVER_SUCCESS)) | |
| { | |
| uassert_true(RT_TRUE); | |
| break; | |
| } | |
| else if ((udp_event->set & EVENT_FLAG_UDP_CLIENT_FAILED) || (udp_event->set & EVENT_FLAG_UDP_SERVER_FAILED)) | |
| { | |
| uassert_true(RT_FALSE); | |
| break; | |
| } | |
| rt_thread_mdelay(2 * RT_TICK_PER_SECOND); | |
| rt_uint32_t recved = 0; | |
| /* Check for both client and server success flags */ | |
| if (rt_event_recv(udp_event, | |
| EVENT_FLAG_UDP_CLIENT_SUCCESS | EVENT_FLAG_UDP_SERVER_SUCCESS, | |
| RT_EVENT_FLAG_AND | RT_EVENT_FLAG_CLEAR, | |
| 2 * RT_TICK_PER_SECOND, | |
| &recved) == RT_EOK) | |
| { | |
| uassert_true(RT_TRUE); | |
| break; | |
| } | |
| /* Check for any failure flag */ | |
| if (rt_event_recv(udp_event, | |
| EVENT_FLAG_UDP_CLIENT_FAILED | EVENT_FLAG_UDP_SERVER_FAILED, | |
| RT_EVENT_FLAG_OR | RT_EVENT_FLAG_CLEAR, | |
| 0, | |
| &recved) == RT_EOK) | |
| { | |
| uassert_true(RT_FALSE); | |
| break; | |
| } |
Copilot uses AI. Check for mistakes.
| if ((udp_event->set & EVENT_FLAG_UDP_CLIENT_SUCCESS) && (udp_event->set & EVENT_FLAG_UDP_SERVER_SUCCESS)) | ||
| { | ||
| uassert_true(RT_TRUE); | ||
| break; | ||
| } | ||
| else if ((udp_event->set & EVENT_FLAG_UDP_CLIENT_FAILED) || (udp_event->set & EVENT_FLAG_UDP_SERVER_FAILED)) |
There was a problem hiding this comment.
English: Same encapsulation violation as in the TCP test. Use rt_event_recv() to properly check event flags instead of directly accessing the set field.
中文：与TCP测试中相同的封装违规问题。应该使用 rt_event_recv() 来正确检查事件标志，而不是直接访问 set 字段。
| if ((udp_event->set & EVENT_FLAG_UDP_CLIENT_SUCCESS) && (udp_event->set & EVENT_FLAG_UDP_SERVER_SUCCESS)) | |
| { | |
| uassert_true(RT_TRUE); | |
| break; | |
| } | |
| else if ((udp_event->set & EVENT_FLAG_UDP_CLIENT_FAILED) || (udp_event->set & EVENT_FLAG_UDP_SERVER_FAILED)) | |
| rt_uint32_t recved = 0; | |
| /* Check for both client and server success flags */ | |
| if (rt_event_recv(udp_event, EVENT_FLAG_UDP_CLIENT_SUCCESS | EVENT_FLAG_UDP_SERVER_SUCCESS, | |
| RT_EVENT_FLAG_AND | RT_EVENT_FLAG_CLEAR, 0, &recved) == RT_EOK) | |
| { | |
| uassert_true(RT_TRUE); | |
| break; | |
| } | |
| /* Check for either client or server failure flags */ | |
| if (rt_event_recv(udp_event, EVENT_FLAG_UDP_CLIENT_FAILED | EVENT_FLAG_UDP_SERVER_FAILED, | |
| RT_EVENT_FLAG_OR | RT_EVENT_FLAG_CLEAR, 0, &recved) == RT_EOK) |
Copilot uses AI. Check for mistakes.
可以，只是我也不太专业，net组件又很重要。 |
添加 LWIP 相关 API 的测试用例，主要包括如下内容（LWIP回环测试）：
拉取/合并请求描述：(PR description)
[
为什么提交这份PR (why to submit this PR)
你的解决方案是什么 (what is your solution)
请提供验证的bsp和config (provide the config and bsp)
]
当前拉取/合并请求的状态 Intent for your PR
必须选择一项 Choose one (Mandatory):
代码质量 Code Quality：
我在这个拉取/合并请求中已经考虑了 As part of this pull request, I've considered the following:
#if 0代码，不包含已经被注释了的代码 All redundant code is removed and cleaned up