diff --git a/sys/include/net/gnrc/tcp.h b/sys/include/net/gnrc/tcp.h index b5b9f524ca..0e4c330ac2 100644 --- a/sys/include/net/gnrc/tcp.h +++ b/sys/include/net/gnrc/tcp.h @@ -34,6 +34,16 @@ extern "C" { #endif +/* Note: This value if configurable for test purposes. Do not override it. + * Changing this value may lead to errors that are hard to track down. + */ +#ifndef GNRC_TCP_NO_TIMEOUT +/** + * @brief Special timeout value representing no timeout. + */ +#define GNRC_TCP_NO_TIMEOUT (UINT32_MAX) +#endif + /** * @brief Address information for a single TCP connection endpoint. * @extends sock_tcp_ep_t @@ -172,7 +182,9 @@ int gnrc_tcp_listen(gnrc_tcp_tcb_queue_t *queue, gnrc_tcp_tcb_t *tcbs, size_t tc * * @param[in] queue Listening queue to accept connection from. * @param[out] tcb Pointer to TCB associated with a established connection. - * @param[in] user_timeout_duration_ms User specified timeout in milliseconds. + * @param[in] user_timeout_duration_ms User specified timeout in milliseconds. If + * GNRC_TCP_NO_TIMEOUT the function blocks until a + * connection was established or an error occurred. * * @return 0 on success. * @return -ENOMEM if all connection in @p queue were already accepted. @@ -182,7 +194,7 @@ int gnrc_tcp_listen(gnrc_tcp_tcb_queue_t *queue, gnrc_tcp_tcb_t *tcbs, size_t tc * could be established. */ int gnrc_tcp_accept(gnrc_tcp_tcb_queue_t *queue, gnrc_tcp_tcb_t **tcb, - uint32_t user_timeout_duration_ms); + const uint32_t user_timeout_duration_ms); /** * @brief Transmit data to connected peer. @@ -199,6 +211,9 @@ int gnrc_tcp_accept(gnrc_tcp_tcb_queue_t *queue, gnrc_tcp_tcb_t **tcb, * @param[in] user_timeout_duration_ms If not zero and there was not data transmitted * the function returns after user_timeout_duration_ms. * If zero, no timeout will be triggered. + * If GNRC_TCP_NO_TIMEOUT the timeout is disabled + * causing the function to block until some data was + * transmitted or and error occurred. * * @return The number of successfully transmitted bytes. * @return -ENOTCONN if connection is not established. @@ -228,6 +243,9 @@ ssize_t gnrc_tcp_send(gnrc_tcp_tcb_t *tcb, const void *data, const size_t len, * returns immediately. If not zero the function * blocks until data is available or * @p user_timeout_duration_ms milliseconds passed. + * If GNRC_TCP_NO_TIMEOUT, causing the function to + * block until some data was available or an error + * occurred. * * @return The number of bytes read into @p data. * @return 0, if the connection is closing and no further data can be read. diff --git a/sys/net/gnrc/transport_layer/tcp/gnrc_tcp.c b/sys/net/gnrc/transport_layer/tcp/gnrc_tcp.c index 551fd454c7..9385d0701c 100644 --- a/sys/net/gnrc/transport_layer/tcp/gnrc_tcp.c +++ b/sys/net/gnrc/transport_layer/tcp/gnrc_tcp.c @@ -538,7 +538,7 @@ int gnrc_tcp_listen(gnrc_tcp_tcb_queue_t *queue, gnrc_tcp_tcb_t *tcbs, size_t tc } int gnrc_tcp_accept(gnrc_tcp_tcb_queue_t *queue, gnrc_tcp_tcb_t **tcb, - uint32_t user_timeout_duration_ms) + const uint32_t user_timeout_duration_ms) { TCP_DEBUG_ENTER; assert(queue != NULL); @@ -611,8 +611,10 @@ int gnrc_tcp_accept(gnrc_tcp_tcb_queue_t *queue, gnrc_tcp_tcb_t **tcb, } /* Setup User specified Timeout */ - _sched_mbox(&event_user_timeout, user_timeout_duration_ms, - MSG_TYPE_USER_SPEC_TIMEOUT, &mbox); + if (user_timeout_duration_ms != GNRC_TCP_NO_TIMEOUT) { + _sched_mbox(&event_user_timeout, user_timeout_duration_ms, + MSG_TYPE_USER_SPEC_TIMEOUT, &mbox); + } /* Wait until a connection was established */ while (ret >= 0 && *tcb == NULL) { @@ -691,7 +693,7 @@ ssize_t gnrc_tcp_send(gnrc_tcp_tcb_t *tcb, const void *data, const size_t len, /* Setup connection timeout */ _sched_connection_timeout(&tcb->event_misc, &mbox); - if (timeout_duration_ms > 0) { + if ((0 < timeout_duration_ms) && (timeout_duration_ms != GNRC_TCP_NO_TIMEOUT)) { _sched_mbox(&event_user_timeout, timeout_duration_ms, MSG_TYPE_USER_SPEC_TIMEOUT, &mbox); } @@ -840,7 +842,7 @@ ssize_t gnrc_tcp_recv(gnrc_tcp_tcb_t *tcb, void *data, const size_t max_len, /* Setup connection timeout */ _sched_connection_timeout(&tcb->event_misc, &mbox); - if (timeout_duration_ms > 0) { + if (timeout_duration_ms != GNRC_TCP_NO_TIMEOUT) { _sched_mbox(&event_user_timeout, timeout_duration_ms, MSG_TYPE_USER_SPEC_TIMEOUT, &mbox); } diff --git a/tests/gnrc_tcp/Makefile b/tests/gnrc_tcp/Makefile index fa5e80def0..cd0555ad32 100644 --- a/tests/gnrc_tcp/Makefile +++ b/tests/gnrc_tcp/Makefile @@ -8,6 +8,9 @@ TAP ?= tap0 MSL_MS ?= 1000 TIMEOUT_MS ?= 3000 +# Set custom GNRC_TCP_NO_TIMEOUT constant for testing purposes +CUSTOM_GNRC_TCP_NO_TIMEOUT ?= 1 + # This test depends on tap device setup (only allowed by root) # Suppress test execution to avoid CI errors TEST_ON_CI_BLACKLIST += all @@ -57,3 +60,8 @@ endif ifndef CONFIG_KCONFIG_USEMODULE_SHELL CFLAGS += -DCONFIG_SHELL_NO_ECHO endif + +# Set GNRC_TCP_NO_TIMEOUT via CFLAGS +ifndef GNRC_TCP_NO_TIMEOUT + CFLAGS += -DGNRC_TCP_NO_TIMEOUT=$(CUSTOM_GNRC_TCP_NO_TIMEOUT) +endif diff --git a/tests/gnrc_tcp/tests-as-root/01-run.py b/tests/gnrc_tcp/tests-as-root/01-run.py index 0b4eeb6a4a..8f7efc701a 100755 --- a/tests/gnrc_tcp/tests-as-root/01-run.py +++ b/tests/gnrc_tcp/tests-as-root/01-run.py @@ -17,6 +17,10 @@ from scapy.all import Ether, IPv6, TCP, raw, sendp from helpers import Runner, RiotTcpServer, RiotTcpClient, HostTcpServer, HostTcpClient, \ generate_port_number, sudo_guard +# Custom NO_TIMEOUT constant. Note: the value must match +# with CUSTOM_GNRC_TCP_NO_TIMEOUT from the makefile +_GNRC_TCP_NO_TIMEOUT = 1 + @Runner(timeout=5) def test_connection_lifecycle_as_client(child): @@ -506,6 +510,71 @@ def test_gnrc_tcp_queue_get_local_returns_EADDRNOTAVAIL(child): child.expect_exact('gnrc_tcp_queue_get_local: returns -EADDRNOTAVAIL') +@Runner(timeout=5) +def test_gnrc_tcp_accept_respects_GNRC_TCP_NO_TIMEOUT(child): + """ gnrc_tcp_accept timeout mechanism must be disabled if GNRC_TCP_NO_TIMEOUT + is set as timeout value. + """ + + pexpect_timeout_sec = _GNRC_TCP_NO_TIMEOUT + 1 + + riot_srv = RiotTcpServer(child, generate_port_number()) + riot_srv.listen() + + child.sendline('gnrc_tcp_accept {}'.format(_GNRC_TCP_NO_TIMEOUT)) + + # The test is successful if gnrc_tcp_accept does not return before pexpects timeout. + # Afterwards the connection must be still able to accept a connection. + returned = True + try: + child.expect_exact('gnrc_tcp_accept: returns', timeout=pexpect_timeout_sec) + except pexpect.TIMEOUT: + returned = False + finally: + if returned: + raise RuntimeError('gnrc_tcp_accept returned') + + # Ensure that gnrc_tcp_accept returns after the test host connects + with HostTcpClient(riot_srv): + child.expect_exact('gnrc_tcp_accept: returns 0') + riot_srv.close() + + +@Runner(timeout=5) +def test_gnrc_tcp_recv_respects_GNRC_TCP_NO_TIMEOUT(child): + """ gnrc_tcp_recv timeout mechanism must be disabled if GNRC_TCP_NO_TIMEOUT + is set as timeout value. + """ + + pexpect_timeout_sec = _GNRC_TCP_NO_TIMEOUT + 1 + payload = "12345" + + # Setup Host as server + with HostTcpServer(generate_port_number()) as host_srv: + # Setup Riot as client + with RiotTcpClient(child, host_srv): + + # Accept connection + host_srv.accept() + + # Wait for incoming data blocking. + # This gnrc_tcp_recv must not return until some Data was sent. + child.sendline('gnrc_tcp_recv {} {}'.format(_GNRC_TCP_NO_TIMEOUT, len(payload))) + returned = True + try: + child.expect_exact('gnrc_tcp_recv: re', timeout=pexpect_timeout_sec) + except pexpect.TIMEOUT: + returned = False + finally: + if returned: + raise RuntimeError('gnrc_tcp_recv returned') + + # Send data to unblock gnrc_tcp_recv and teardown the connection + host_srv.send(payload) + child.expect_exact('gnrc_tcp_recv: received {}'.format(len(payload))) + host_srv.close() + + @Runner(timeout=10) def test_connection_listen_accept_cycle(child, iterations=10): """ This test verifies gnrc_tcp in a typical server role by @@ -531,7 +600,7 @@ def test_connection_listen_accept_cycle(child, iterations=10): host_cli.receive(sent_payload=data) # Randomize connection teardown: The connections - # can't be either closed or aborted from either + # can be either closed or aborted from either # side. Regardless the type of the connection teardown # riot_srv must be able to accept the next connection # Note: python sockets don't offer abort...