Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

lwip: enable LWIP_SO_RCVTIMEO if sock layer is used #17779

Merged
merged 2 commits into from
Mar 10, 2022

Conversation

benpicco
Copy link
Contributor

@benpicco benpicco commented Mar 9, 2022

Contribution description

It is extremely counter-intuitive that LWIP will by default ignore timeouts unless LWIP_SO_RCVTIMEO is set.
When using the sock API, the application should not have to care what backend is used and timeouts are part of the sock API - so always enable timeouts in LWIP when the sock API is in use.

Testing procedure

lwip tests should still pass without manually setting LWIP_SO_RCVTIMEO

tests/lwip_sock_udp

main(): This is RIOT! (Version: 2022.04-devel-779-gca8c8-LWIP_SO_RCVTIMEO)
code 0x41
Calling test_sock_udp_create6__EADDRINUSE()
Calling test_sock_udp_create6__EAFNOSUPPORT()
Calling test_sock_udp_create6__EINVAL_addr()
Calling test_sock_udp_create6__EINVAL_netif()
Calling test_sock_udp_create6__no_endpoints()
Calling test_sock_udp_create6__only_local()
Calling test_sock_udp_create6__only_local_port0()
Calling test_sock_udp_create6__only_local_reuse_ep()
Calling test_sock_udp_create6__only_remote()
Calling test_sock_udp_create6__full()
Calling test_sock_udp_recv6__EADDRNOTAVAIL()
Calling test_sock_udp_recv6__EAGAIN()
Calling test_sock_udp_recv6__ENOBUFS()
Calling test_sock_udp_recv6__ETIMEDOUT()
 * Calling sock_udp_recv()
 * (timed out with timeout 1000000)
Calling test_sock_udp_recv6__socketed()
Calling test_sock_udp_recv6__socketed_with_remote()
Calling test_sock_udp_recv6__socketed_with_port0()
Calling test_sock_udp_recv6__unsocketed()
Calling test_sock_udp_recv6__unsocketed_with_remote()
Calling test_sock_udp_recv6__with_timeout()
Calling test_sock_udp_recv6__non_blocking()
Calling test_sock_udp_recv6__aux()
Calling test_sock_udp_recv_buf6__success()
Calling test_sock_udp_send6__EAFNOSUPPORT()
Calling test_sock_udp_send6__EINVAL_addr()
Calling test_sock_udp_send6__EINVAL_netif()
Calling test_sock_udp_send6__EINVAL_port()
Calling test_sock_udp_send6__EHOSTUNREACH()
Calling test_sock_udp_send6__ENOTCONN()
Calling test_sock_udp_send6__socketed_no_local_no_netif()
Calling test_sock_udp_send6__socketed_no_netif()
Calling test_sock_udp_send6__socketed_no_local()
Calling test_sock_udp_send6__socketed()
Calling test_sock_udp_sendv6__socketed()
Calling test_sock_udp_send6__socketed_other_remote()
Calling test_sock_udp_send6__unsocketed_no_local_no_netif()
Calling test_sock_udp_send6__unsocketed_no_netif()
Calling test_sock_udp_send6__unsocketed_no_local()
Calling test_sock_udp_send6__unsocketed()
Calling test_sock_udp_send6__no_sock_no_netif()
Calling test_sock_udp_send6__no_sock()
ALL TESTS SUCCESSFUL

Issues/PRs references

@benpicco benpicco requested a review from miri64 as a code owner March 9, 2022 16:06
@github-actions github-actions bot added Area: network Area: Networking Area: pkg Area: External package ports Area: tests Area: tests and testing framework labels Mar 9, 2022
@benpicco benpicco requested review from yarrick and kfessel March 9, 2022 16:06
@benpicco benpicco added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Mar 9, 2022
@kfessel
Copy link
Contributor

kfessel commented Mar 9, 2022

Did you check weather lwip sock_* depends on any timer (on the first glance it did not) - on the second sock_* -> lwip_sock_* -> lwip_* -> lwip_contrib -> ztimer_msec

@benpicco
Copy link
Contributor Author

benpicco commented Mar 9, 2022

lwip will pull in ztimer_msec through lwip_contrib.

@benpicco benpicco merged commit ddf8f67 into RIOT-OS:master Mar 10, 2022
@benpicco benpicco deleted the LWIP_SO_RCVTIMEO branch March 10, 2022 07:34
@@ -42,6 +42,7 @@ endif

ifneq (,$(filter lwip_sock_%,$(USEMODULE)))
USEMODULE += lwip_sock
CFLAGS += -DLWIP_SO_RCVTIMEO
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would preferred this switch to be in pkg/lwip/include/lwipopts.h. Could this maybe be done as a follow-up?

@OlegHahm OlegHahm added this to the Release 2022.04 milestone Apr 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: network Area: Networking Area: pkg Area: External package ports Area: tests Area: tests and testing framework CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants