Skip to content

lwip: enable LWIP_SO_RCVTIMEO if sock layer is used#17779

Merged
benpicco merged 2 commits intoRIOT-OS:masterfrom
benpicco:LWIP_SO_RCVTIMEO
Mar 10, 2022
Merged

lwip: enable LWIP_SO_RCVTIMEO if sock layer is used#17779
benpicco merged 2 commits intoRIOT-OS:masterfrom
benpicco:LWIP_SO_RCVTIMEO

Conversation

@benpicco
Copy link
Copy Markdown
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 kfessel and yarrick March 9, 2022 16:06
@benpicco benpicco mentioned this pull request Mar 9, 2022
Merged
@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
Copy Markdown
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
Copy Markdown
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
miri64
miri64 reviewed Mar 10, 2022
View reviewed changes
pkg/lwip/Makefile.dep

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

@miri64 miri64 Mar 10, 2022

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

Reviewers

@miri64 miri64 miri64 left review comments

@yarrick yarrick yarrick approved these changes

@kfessel kfessel Awaiting requested review from kfessel

Assignees

No one assigned

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

Milestone

Release 2022.04

Development

Successfully merging this pull request may close these issues.

5 participants

@benpicco @kfessel @miri64 @yarrick @OlegHahm