From 873d1fe378e4c71741de822999d697271c39f317 Mon Sep 17 00:00:00 2001 From: Simon Brummer Date: Sat, 22 May 2021 13:31:33 +0200 Subject: [PATCH] gnrc_tcp: align with sock_tcp --- sys/include/net/gnrc/tcp.h | 51 ++++++++ sys/net/gnrc/transport_layer/tcp/gnrc_tcp.c | 130 +++++++++++++++++++- tests/gnrc_tcp/main.c | 82 ++++++++++++ tests/gnrc_tcp/tests-as-root/01-run.py | 114 ++++++++++++++++- tests/gnrc_tcp/tests-as-root/helpers.py | 26 +++- 5 files changed, 395 insertions(+), 8 deletions(-) diff --git a/sys/include/net/gnrc/tcp.h b/sys/include/net/gnrc/tcp.h index bedef622b9..b5b9f524ca 100644 --- a/sys/include/net/gnrc/tcp.h +++ b/sys/include/net/gnrc/tcp.h @@ -101,6 +101,14 @@ int gnrc_tcp_init(void); */ void gnrc_tcp_tcb_init(gnrc_tcp_tcb_t *tcb); +/** + * @brief Initialize Transmission Control Block (TCB) queue + * @pre @p queue must not be NULL. + * + * @param[in,out] queue TCB queue to initialize. + */ +void gnrc_tcp_tcb_queue_init(gnrc_tcp_tcb_queue_t *queue); + /** * @brief Opens a connection. * @@ -168,6 +176,7 @@ int gnrc_tcp_listen(gnrc_tcp_tcb_queue_t *queue, gnrc_tcp_tcb_t *tcbs, size_t tc * * @return 0 on success. * @return -ENOMEM if all connection in @p queue were already accepted. + * @return -EINVAL if listen was never called on queue. * @return -EAGAIN if @p user_timeout_duration_ms was 0 and no connection is ready to accept. * @return -ETIMEDOUT if @p user_timeout_duration_ms was not 0 and no connection * could be established. @@ -263,6 +272,48 @@ void gnrc_tcp_abort(gnrc_tcp_tcb_t *tcb); */ void gnrc_tcp_stop_listen(gnrc_tcp_tcb_queue_t *queue); +/** + * @brief Get the local end point of a connected TCB + * + * @pre tcb must not be NULL + * @pre ep must not be NULL + * + * @param[in] tcb TCB holding the connection information. + * @param[out] ep The local end point. + * + * @return 0 on success. + * @return -EADDRNOTAVAIL, when @p tcb in not in a connected state. + */ +int gnrc_tcp_get_local(gnrc_tcp_tcb_t *tcb, gnrc_tcp_ep_t *ep); + +/** + * @brief Get the remote end point of a connected TCB + * + * @pre tcb must not be NULL + * @pre ep must not be NULL + * + * @param[in] tcb TCB holding the connection information. + * @param[out] ep The remote end point. + * + * @return 0 on success. + * @return -ENOTCONN, when @p tcb in not in a connected state. + */ +int gnrc_tcp_get_remote(gnrc_tcp_tcb_t *tcb, gnrc_tcp_ep_t *ep); + +/** + * @brief Gets the local end point of a TCB queue + * + * @pre queue must not be NULL + * @pre ep must not be NULL + * + * @param[in] queue TCB queue to stop listening + * @param[out] ep The local end point. + * + * @return 0 on success. + * @return -EADDRNOTAVAIL, when @p queue has no local end point. + */ +int gnrc_tcp_queue_get_local(gnrc_tcp_tcb_queue_t *queue, gnrc_tcp_ep_t *ep); + /** * @brief Calculate and set checksum in TCP header. * diff --git a/sys/net/gnrc/transport_layer/tcp/gnrc_tcp.c b/sys/net/gnrc/transport_layer/tcp/gnrc_tcp.c index 5fe06af5bb..674ce9db9c 100644 --- a/sys/net/gnrc/transport_layer/tcp/gnrc_tcp.c +++ b/sys/net/gnrc/transport_layer/tcp/gnrc_tcp.c @@ -310,6 +310,8 @@ int gnrc_tcp_init(void) void gnrc_tcp_tcb_init(gnrc_tcp_tcb_t *tcb) { TCP_DEBUG_ENTER; + assert(tcb != NULL); + memset(tcb, 0, sizeof(gnrc_tcp_tcb_t)); #ifdef MODULE_GNRC_IPV6 tcb->address_family = AF_INET6; @@ -324,6 +326,16 @@ void gnrc_tcp_tcb_init(gnrc_tcp_tcb_t *tcb) TCP_DEBUG_LEAVE; } +void gnrc_tcp_tcb_queue_init(gnrc_tcp_tcb_queue_t *queue) +{ + TCP_DEBUG_ENTER; + assert(queue != NULL); + + mutex_init(&queue->lock); + queue->tcbs = NULL; + queue->tcbs_len = 0; + TCP_DEBUG_LEAVE; +} int gnrc_tcp_open(gnrc_tcp_tcb_t *tcb, const gnrc_tcp_ep_t *remote, uint16_t local_port) { @@ -561,14 +573,19 @@ int gnrc_tcp_accept(gnrc_tcp_tcb_queue_t *queue, gnrc_tcp_tcb_t **tcb, ++avail_tcbs; } - /* Return if a connection was found, accept was called as non-blocking or all - * TCBs were already accepted. + /* Return if a connection was found, queue is not listening, accept was called as non-blocking + * or all TCBs were already accepted. */ - if ((*tcb) || (user_timeout_duration_ms == 0) || (avail_tcbs == 0)) { + if ((*tcb) || (queue->tcbs == NULL) || (user_timeout_duration_ms == 0) || + (avail_tcbs == 0)) { if (*tcb) { TCP_DEBUG_INFO("Accepting connection."); ret = 0; } + else if (queue->tcbs == NULL) { + TCP_DEBUG_ERROR("-EINVAL: Queue is not listening."); + ret = -EINVAL; + } else if (avail_tcbs == 0) { TCP_DEBUG_ERROR("-ENOMEM: All TCBs are currently accepted."); ret = -ENOMEM; @@ -933,6 +950,113 @@ void gnrc_tcp_stop_listen(gnrc_tcp_tcb_queue_t *queue) TCP_DEBUG_LEAVE; } +int gnrc_tcp_get_local(gnrc_tcp_tcb_t *tcb, gnrc_tcp_ep_t *ep) +{ + TCP_DEBUG_ENTER; + assert(tcb != NULL); + assert(ep != NULL); + + int ret = 0; + + /* Lock the TCB for this function call */ + mutex_lock(&(tcb->function_lock)); + _gnrc_tcp_fsm_state_t state =_gnrc_tcp_fsm_get_state(tcb); + + /* Check if connection is established */ + if ((state == FSM_STATE_ESTABLISHED) || (state == FSM_STATE_CLOSE_WAIT)) { + /* Construct endpoint from connection parameters */ + ep->family = tcb->address_family; + ep->port = tcb->local_port; + ep->netif = tcb->ll_iface; +#ifdef MODULE_GNRC_IPV6 + if (ep->family == AF_INET6) { + memcpy(ep->addr.ipv6, tcb->local_addr, sizeof(ep->addr.ipv6)); + } +#endif + } else { + TCP_DEBUG_ERROR("-EADDRNOTAVAIL: TCB is not connected."); + ret = -EADDRNOTAVAIL; + } + + mutex_unlock(&(tcb->function_lock)); + TCP_DEBUG_LEAVE; + return ret; +} + +int gnrc_tcp_get_remote(gnrc_tcp_tcb_t *tcb, gnrc_tcp_ep_t *ep) +{ + TCP_DEBUG_ENTER; + assert(tcb != NULL); + assert(ep != NULL); + + int ret = 0; + + /* Lock the TCB for this function call */ + mutex_lock(&(tcb->function_lock)); + _gnrc_tcp_fsm_state_t state =_gnrc_tcp_fsm_get_state(tcb); + + /* Check if connection is established */ + if ((state == FSM_STATE_ESTABLISHED) || (state == FSM_STATE_CLOSE_WAIT)) { + /* Construct endpoint from connection parameters */ + ep->family = tcb->address_family; + ep->port = tcb->peer_port; + ep->netif = 0; +#ifdef MODULE_GNRC_IPV6 + if (ep->family == AF_INET6) { + memcpy(ep->addr.ipv6, tcb->peer_addr, sizeof(ep->addr.ipv6)); + } +#endif + } else { + TCP_DEBUG_ERROR("-ENOTCONN: TCB is not connected."); + ret = -ENOTCONN; + } + + mutex_unlock(&(tcb->function_lock)); + TCP_DEBUG_LEAVE; + return ret; +} + +int gnrc_tcp_queue_get_local(gnrc_tcp_tcb_queue_t *queue, gnrc_tcp_ep_t *ep) +{ + TCP_DEBUG_ENTER; + assert(queue != NULL); + assert(ep != NULL); + + int ret = 0; + + /* Lock the TCB queue for this function call */ + mutex_lock(&(queue->lock)); + + /* Check if queue has associated TCBs */ + if (queue->tcbs) { + /* There are listening TCBs: Construct ep from first TCB. */ + gnrc_tcp_tcb_t *tcb = queue->tcbs; + mutex_lock(&(tcb->function_lock)); + + /* Construct endpoint from tcbs connection parameters */ + ep->family = tcb->address_family; + ep->port = tcb->local_port; + ep->netif = 0; +#ifdef MODULE_GNRC_IPV6 + if (ep->family == AF_INET6) { + if (tcb->status & STATUS_ALLOW_ANY_ADDR) { + ipv6_addr_set_unspecified((ipv6_addr_t *) ep->addr.ipv6); + } else { + memcpy(ep->addr.ipv6, tcb->local_addr, sizeof(ep->addr.ipv6)); + } + } +#endif + mutex_unlock(&(tcb->function_lock)); + } else { + TCP_DEBUG_ERROR("-EADDRNOTAVAIL: queue was never listening."); + ret = -EADDRNOTAVAIL; + } + + mutex_unlock(&(queue->lock)); + TCP_DEBUG_LEAVE; + return ret; +} + int gnrc_tcp_calc_csum(const gnrc_pktsnip_t *hdr, const gnrc_pktsnip_t *pseudo_hdr) { TCP_DEBUG_ENTER; diff --git a/tests/gnrc_tcp/main.c b/tests/gnrc_tcp/main.c index 9b8abc0714..67f8c06e46 100644 --- a/tests/gnrc_tcp/main.c +++ b/tests/gnrc_tcp/main.c @@ -217,6 +217,10 @@ int gnrc_tcp_accept_cmd(int argc, char **argv) int timeout = atol(argv[1]); int err = gnrc_tcp_accept(&queue, &tmp, timeout); switch (err) { + case -EINVAL: + printf("%s: returns -EINVAL\n", argv[0]); + break; + case -EAGAIN: printf("%s: returns -EAGAIN\n", argv[0]); break; @@ -341,6 +345,78 @@ int gnrc_tcp_stop_listen_cmd(int argc, char **argv) return 0; } +int gnrc_tcp_get_local_cmd(int argc, char **argv) +{ + dump_args(argc, argv); + gnrc_tcp_ep_t ep; + + int err = gnrc_tcp_get_local(tcb, &ep); + switch (err) { + case 0: + printf("%s: returns 0\n", argv[0]); + printf("Endpoint: addr.ipv6="); + ipv6_addr_print((ipv6_addr_t *) ep.addr.ipv6); + printf(" netif=%u port=%u\n", ep.netif, ep.port); + break; + + case -EADDRNOTAVAIL: + printf("%s: returns -EADDRNOTAVAIL\n", argv[0]); + break; + + default: + printf("%s: returns %d\n", argv[0], err); + } + return 0; +} + +int gnrc_tcp_get_remote_cmd(int argc, char **argv) +{ + dump_args(argc, argv); + gnrc_tcp_ep_t ep; + + int err = gnrc_tcp_get_remote(tcb, &ep); + switch (err) { + case 0: + printf("%s: returns 0\n", argv[0]); + printf("Endpoint: addr.ipv6="); + ipv6_addr_print((ipv6_addr_t *) ep.addr.ipv6); + printf(" netif=%u port=%u\n", ep.netif, ep.port); + break; + + case -ENOTCONN: + printf("%s: returns -ENOTCONN\n", argv[0]); + break; + + default: + printf("%s: returns %d\n", argv[0], err); + } + return 0; +} + +int gnrc_tcp_queue_get_local_cmd(int argc, char **argv) +{ + dump_args(argc, argv); + gnrc_tcp_ep_t ep; + + int err = gnrc_tcp_queue_get_local(&queue, &ep); + switch (err) { + case 0: + printf("%s: returns 0\n", argv[0]); + printf("Endpoint: addr.ipv6="); + ipv6_addr_print((ipv6_addr_t *) ep.addr.ipv6); + printf(" netif=%u port=%u\n", ep.netif, ep.port); + break; + + case -EADDRNOTAVAIL: + printf("%s: returns -EADDRNOTAVAIL\n", argv[0]); + break; + + default: + printf("%s: returns %d\n", argv[0], err); + } + return 0; +} + /* Exporting GNRC TCP Api to for shell usage */ static const shell_command_t shell_commands[] = { { "gnrc_tcp_ep_from_str", "Build endpoint from string", @@ -363,6 +439,12 @@ static const shell_command_t shell_commands[] = { gnrc_tcp_abort_cmd }, { "gnrc_tcp_stop_listen", "gnrc_tcp: stop listening", gnrc_tcp_stop_listen_cmd }, + { "gnrc_tcp_get_local", "gnrc_tcp: get local", + gnrc_tcp_get_local_cmd }, + { "gnrc_tcp_get_remote", "gnrc_tcp: get remote", + gnrc_tcp_get_remote_cmd }, + { "gnrc_tcp_queue_get_local", "gnrc_tcp: get queue local", + gnrc_tcp_queue_get_local_cmd }, { "buffer_init", "init internal buffer", buffer_init_cmd }, { "buffer_get_max_size", "get max size of internal buffer", diff --git a/tests/gnrc_tcp/tests-as-root/01-run.py b/tests/gnrc_tcp/tests-as-root/01-run.py index 5f10a67a8c..0b4eeb6a4a 100755 --- a/tests/gnrc_tcp/tests-as-root/01-run.py +++ b/tests/gnrc_tcp/tests-as-root/01-run.py @@ -183,7 +183,7 @@ def test_gnrc_tcp_garbage_packets_ack_instead_of_sym(child): riot_srv.close() -@Runner(timeout=5, skip=False) +@Runner(timeout=5) def test_gnrc_tcp_garbage_packets_option_parsing(child): """ This test verfies that malformed option don't break TCP doesn't break GNRC_TCP. See: https://github.com/RIOT-OS/RIOT/issues/12086 @@ -394,6 +394,118 @@ def test_gnrc_tcp_accept_returns_ENOMEM(child): child.expect_exact('gnrc_tcp_accept: returns -ENOMEM') +@Runner(timeout=0.5) +def test_gnrc_tcp_accept_returns_EINVAL(child): + """ gnrc_tcp_accept must return with -EINVAL + if listen was not called before. + """ + child.sendline('gnrc_tcp_accept 0') + child.expect_exact('gnrc_tcp_accept: returns -EINVAL') + + +@Runner(timeout=5) +def test_gnrc_tcp_get_local_returns_0(child): + """ This test verifies that get_local returns 0 in a connected state + and the used endpoint contains the expected connection parameters + """ + local_port = 30423 + + # Setup Host as server + with HostTcpServer(generate_port_number()) as host_srv: + # Setup Riot as client + with RiotTcpClient(child, host_srv, local_port) as riot_cli: + + # Accept connection + host_srv.accept() + + # Get and verify local endpoint + riot_cli.get_local() + child.expect_exact('Endpoint: addr.ipv6={} netif={} port={}'.format( + riot_cli.address, riot_cli.interface, riot_cli.port) + ) + + # Close connection + host_srv.close() + + +@Runner(timeout=1) +def test_gnrc_tcp_get_local_returns_EADDRNOTAVAIL(child): + """ This Test verifies that get_local returns -EADDRNOTAVAIL + then not connected + """ + child.sendline('gnrc_tcp_get_local') + child.expect_exact('gnrc_tcp_get_local: returns -EADDRNOTAVAIL') + + +@Runner(timeout=5) +def test_gnrc_tcp_get_remote_returns_0(child): + """ This test verifies that get_remote returns 0 in a connected state + and the used endpoint contains the expected connection parameters + """ + # Setup Host as server + with HostTcpServer(generate_port_number()) as host_srv: + # Setup Riot as client + with RiotTcpClient(child, host_srv) as riot_cli: + + # Accept connection + host_srv.accept() + + # Get and verify local endpoint + riot_cli.get_remote() + child.expect_exact('Endpoint: addr.ipv6={} netif=0 port={}'.format( + host_srv.address, host_srv.listen_port) + ) + + # Close connection + host_srv.close() + + +@Runner(timeout=1) +def test_gnrc_tcp_get_remote_returns_ENOTCONN(child): + """ This test verifies that get_remote returns -ENOTCONN + then not connected + """ + child.sendline('gnrc_tcp_get_remote') + child.expect_exact('gnrc_tcp_get_remote: returns -ENOTCONN') + + +@Runner(timeout=1) +def test_gnrc_tcp_queue_get_local_returns_0(child): + """ This test verifies that queue_get_local returns 0 then put after listen. + And the endpoint content is as are as expected. + """ + # Enter listen with accept all address + listen_addr = '::' + listen_port = generate_port_number() + riot_srv = RiotTcpServer(child, listen_port) + riot_srv.listen() + riot_srv.queue_get_local() + child.expect_exact('Endpoint: addr.ipv6=:: netif=0 port={}'.format( + listen_port) + ) + riot_srv.stop_listen() + + # Enter listen with specified address + listen_addr = 'fe80::4c49:c7ff:fecd:34a3' + listen_port = generate_port_number() + riot_srv = RiotTcpServer(child, listen_port, listen_addr) + riot_srv.listen() + riot_srv.queue_get_local() + child.expect_exact('Endpoint: addr.ipv6={} netif=0 port={}'.format( + listen_addr, listen_port) + ) + riot_srv.stop_listen() + + +@Runner(timeout=1) +def test_gnrc_tcp_queue_get_local_returns_EADDRNOTAVAIL(child): + """ This Test verifies that queue_get_local returns -EADDRNOTAVAIL + then not listening + """ + child.sendline('gnrc_tcp_queue_get_local') + child.expect_exact('gnrc_tcp_queue_get_local: returns -EADDRNOTAVAIL') + + @Runner(timeout=10) def test_connection_listen_accept_cycle(child, iterations=10): """ This test verifies gnrc_tcp in a typical server role by diff --git a/tests/gnrc_tcp/tests-as-root/helpers.py b/tests/gnrc_tcp/tests-as-root/helpers.py index c8fff71909..2dc55f492d 100644 --- a/tests/gnrc_tcp/tests-as-root/helpers.py +++ b/tests/gnrc_tcp/tests-as-root/helpers.py @@ -181,6 +181,14 @@ class _RiotTcpNode: self._verify_pktbuf_empty() self.opened = False + def get_local(self): + self.child.sendline('gnrc_tcp_get_local') + self.child.expect_exact('gnrc_tcp_get_local: returns 0') + + def get_remote(self): + self.child.sendline('gnrc_tcp_get_remote') + self.child.expect_exact('gnrc_tcp_get_remote: returns 0') + def _get_interface(self): self.child.sendline('ifconfig') self.child.expect(r'Iface\s+(\d+)\s') @@ -230,10 +238,11 @@ class _RiotTcpNode: class RiotTcpServer(_RiotTcpNode): - def __init__(self, child, listen_port): + def __init__(self, child, listen_port, listen_addr='::'): super().__init__(child) self.listening = False self.listen_port = str(listen_port) + self.listen_addr = str(listen_addr) self.tcb_init() def __enter__(self): @@ -246,7 +255,9 @@ class RiotTcpServer(_RiotTcpNode): self.stop_listen() def listen(self): - self.child.sendline('gnrc_tcp_listen [::]:{}'.format(self.listen_port)) + self.child.sendline('gnrc_tcp_listen [{}]:{}'.format( + self.listen_addr, self.listen_port) + ) self.child.expect_exact('gnrc_tcp_listen: returns 0') self.listening = True @@ -261,12 +272,17 @@ class RiotTcpServer(_RiotTcpNode): self._verify_pktbuf_empty() self.listening = False + def queue_get_local(self): + self.child.sendline('gnrc_tcp_queue_get_local') + self.child.expect_exact('gnrc_tcp_queue_get_local: returns 0') + class RiotTcpClient(_RiotTcpNode): - def __init__(self, child, target): + def __init__(self, child, target, port=0): super().__init__(child) self.target_addr = target.address + '%' + self.interface self.target_port = str(target.listen_port) + self.port = port self.tcb_init() def __enter__(self): @@ -279,7 +295,9 @@ class RiotTcpClient(_RiotTcpNode): self.close() def open(self): - self.child.sendline('gnrc_tcp_open [{}]:{} 0'.format(self.target_addr, self.target_port)) + self.child.sendline('gnrc_tcp_open [{}]:{} {}'.format( + self.target_addr, self.target_port, self.port) + ) self.child.expect_exact('gnrc_tcp_open: returns 0') self.opened = True