From 01539332412a5108927af9935e36c4b555f6b062 Mon Sep 17 00:00:00 2001 From: Oleg Hahm Date: Fri, 20 Nov 2015 22:04:35 +0100 Subject: [PATCH 1/5] posix sockets: store src_port in socket struct --- sys/posix/sockets/posix_sockets.c | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/sys/posix/sockets/posix_sockets.c b/sys/posix/sockets/posix_sockets.c index 5da5795872..a54e3260e4 100644 --- a/sys/posix/sockets/posix_sockets.c +++ b/sys/posix/sockets/posix_sockets.c @@ -56,6 +56,7 @@ typedef struct { int protocol; bool bound; socket_conn_t conn; + uint16_t src_port; } socket_t; socket_t _pool[SOCKET_POOL_SIZE]; @@ -225,6 +226,7 @@ static int socket_close(int socket) } } s->domain = AF_UNSPEC; + s->src_port = 0; mutex_unlock(&_pool_mutex); return res; } @@ -278,6 +280,7 @@ int socket(int domain, int type, int protocol) } } s->bound = false; + s->src_port = 0; mutex_unlock(&_pool_mutex); return res; } @@ -433,6 +436,7 @@ int bind(int socket, const struct sockaddr *address, socklen_t address_len) errno = EOPNOTSUPP; return -1; } + s->src_port = byteorder_ntohs(port); s->bound = true; return 0; } @@ -729,6 +733,7 @@ ssize_t recvfrom(int socket, void *restrict buffer, size_t length, int flags, errno = -res; return -1; } + break; #endif #ifdef MODULE_CONN_IP @@ -860,9 +865,18 @@ ssize_t sendto(int socket, const void *buffer, size_t length, int flags, sport, port); } else if (address != NULL) { - uint16_t sport = (uint16_t)genrand_uint32_range(1LU << 10U, 1LU << 16U); + ipv6_addr_t local; + s->src_port = (uint16_t)genrand_uint32_range(1LU << 10U, 1LU << 16U); + /* implicitly bind the socket here */ + ipv6_addr_set_unspecified(&local); + if ((res = conn_udp_create(&s->conn.udp, &local, sizeof(local), + s->domain, s->src_port)) < 0) { + errno = -res; + return -1; + } + s->bound = true; res = conn_udp_sendto(buffer, length, NULL, 0, addr, addr_len, s->domain, - sport, port); + s->src_port, port); } else { errno = ENOTCONN; From c366f2bbcdd0dd744a3cccbc8af74c46e7d3e497 Mon Sep 17 00:00:00 2001 From: Oleg Hahm Date: Fri, 20 Nov 2015 22:07:56 +0100 Subject: [PATCH 2/5] sockets: perform implicit bind during sendto() A client should not require to explicitly call bind() to receive packets, but is expected to receive replies sent to the ephemeral port that was selected as a source port by the UDP implementation. --- sys/posix/sockets/posix_sockets.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/sys/posix/sockets/posix_sockets.c b/sys/posix/sockets/posix_sockets.c index a54e3260e4..356dc5707e 100644 --- a/sys/posix/sockets/posix_sockets.c +++ b/sys/posix/sockets/posix_sockets.c @@ -728,12 +728,11 @@ ssize_t recvfrom(int socket, void *restrict buffer, size_t length, int flags, switch (s->type) { #ifdef MODULE_CONN_UDP case SOCK_DGRAM: - if ((res = conn_udp_recvfrom(&s->conn.udp, buffer, length, addr, &addr_len, - port)) < 0) { + if ((res = conn_udp_recvfrom(&s->conn.udp, buffer, length, addr, + &addr_len, port)) < 0) { errno = -res; return -1; } - break; #endif #ifdef MODULE_CONN_IP From 5f663826c75743c231f42474ab719ab2b66ed4b7 Mon Sep 17 00:00:00 2001 From: Oleg Hahm Date: Fri, 20 Nov 2015 22:12:59 +0100 Subject: [PATCH 3/5] posix sockets: remove pointless inline function --- sys/posix/sockets/posix_sockets.c | 21 ++++++++------------- 1 file changed, 8 insertions(+), 13 deletions(-) diff --git a/sys/posix/sockets/posix_sockets.c b/sys/posix/sockets/posix_sockets.c index 356dc5707e..a5481fe796 100644 --- a/sys/posix/sockets/posix_sockets.c +++ b/sys/posix/sockets/posix_sockets.c @@ -120,11 +120,6 @@ static inline int _choose_ipproto(int type, int protocol) return -1; } -static inline void _htons_port(uint16_t *port) -{ - *port = htons(*port); -} - static inline ipv4_addr_t *_in_addr_ptr(struct sockaddr_storage *addr) { return (ipv4_addr_t *)(&((struct sockaddr_in *)addr)->sin_addr); @@ -366,8 +361,8 @@ int accept(int socket, struct sockaddr *restrict address, res = -1; break; } - _htons_port(port); /* XXX: sin(6)_port is supposed to be network byte - * order */ + *port = htons(*port); /* XXX: sin(6)_port is supposed to be + network byte order */ *address_len = _addr_truncate(address, *address_len, &tmp, tmp_len); } break; @@ -542,8 +537,8 @@ int getpeername(int socket, struct sockaddr *__restrict address, return -1; } tmp.ss_family = s->domain; - _htons_port(port); /* XXX: sin(6)_port is supposed to be network byte - * order */ + *port = htons(*port); /* XXX: sin(6)_port is supposed to be network byte + order */ *address_len = _addr_truncate(address, *address_len, &tmp, tmp_len); return 0; } @@ -626,8 +621,8 @@ int getsockname(int socket, struct sockaddr *__restrict address, return -1; } tmp.ss_family = AF_INET; - _htons_port(port); /* XXX: sin(6)_port is supposed to be network byte - * order */ + *port = htons(*port); /* XXX: sin(6)_port is supposed to be network byte + order */ *address_len = _addr_truncate(address, *address_len, &tmp, tmp_len); return 0; } @@ -762,8 +757,8 @@ ssize_t recvfrom(int socket, void *restrict buffer, size_t length, int flags, } if ((address != NULL) && (address_len != NULL)) { tmp.ss_family = s->domain; - _htons_port(port); /* XXX: sin_port is supposed to be network byte - * order */ + *port = htons(*port); /* XXX: sin(6)_port is supposed to be network + byte order */ *address_len = _addr_truncate(address, *address_len, &tmp, tmp_len); } return res; From 860321c3e952948d23726b7a176b8b2b751608b1 Mon Sep 17 00:00:00 2001 From: Oleg Hahm Date: Sat, 21 Nov 2015 22:24:28 +0100 Subject: [PATCH 4/5] posix sockets: use network byteorder for port --- sys/posix/sockets/posix_sockets.c | 31 +++++++++++++++++-------------- 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/sys/posix/sockets/posix_sockets.c b/sys/posix/sockets/posix_sockets.c index a5481fe796..623283e636 100644 --- a/sys/posix/sockets/posix_sockets.c +++ b/sys/posix/sockets/posix_sockets.c @@ -149,7 +149,7 @@ static inline socklen_t _addr_truncate(struct sockaddr *out, socklen_t out_len, } static inline int _get_data_from_sockaddr(const struct sockaddr *address, size_t address_len, - void **addr, size_t *addr_len, uint16_t *port) + void **addr, size_t *addr_len, network_uint16_t *port) { switch (address->sa_family) { case AF_INET: @@ -160,8 +160,7 @@ static inline int _get_data_from_sockaddr(const struct sockaddr *address, size_t struct sockaddr_in *in_addr = (struct sockaddr_in *)address; *addr = &in_addr->sin_addr; *addr_len = sizeof(ipv4_addr_t); - /* XXX sin_port is in network byteorder */ - *port = ntohs(in_addr->sin_port); + port->u16 = in_addr->sin_port; break; case AF_INET6: if (address_len < sizeof(struct sockaddr_in6)) { @@ -171,8 +170,7 @@ static inline int _get_data_from_sockaddr(const struct sockaddr *address, size_t struct sockaddr_in6 *in6_addr = (struct sockaddr_in6 *)address; *addr = &in6_addr->sin6_addr; *addr_len = sizeof(ipv6_addr_t); - /* XXX sin6_port is in network byteorder */ - *port = ntohs(in6_addr->sin6_port); + port->u16 = in6_addr->sin6_port; break; default: errno = EAFNOSUPPORT; @@ -382,7 +380,7 @@ int bind(int socket, const struct sockaddr *address, socklen_t address_len) int res = 0; void *addr; size_t addr_len; - uint16_t port; + network_uint16_t port = { 0 }; mutex_lock(&_pool_mutex); s = _get_socket(socket); mutex_unlock(&_pool_mutex); @@ -409,7 +407,8 @@ int bind(int socket, const struct sockaddr *address, socklen_t address_len) #endif #ifdef MODULE_CONN_TCP case SOCK_STREAM: - if ((res = conn_tcp_create(&s->conn.tcp, addr, addr_len, s->domain, port)) < 0) { + if ((res = conn_tcp_create(&s->conn.tcp, addr, addr_len, s->domain, + byteorder_ntohs(port))) < 0) { errno = -res; return -1; } @@ -417,7 +416,9 @@ int bind(int socket, const struct sockaddr *address, socklen_t address_len) #endif #ifdef MODULE_CONN_UDP case SOCK_DGRAM: - if ((res = conn_udp_create(&s->conn.udp, addr, addr_len, s->domain, port)) < 0) { + if ((res = conn_udp_create(&s->conn.udp, addr, addr_len, s->domain, + byteorder_ntohs(port))) < 0) { + errno = -res; return -1; } @@ -442,7 +443,7 @@ int connect(int socket, const struct sockaddr *address, socklen_t address_len) int res = 0; void *addr; size_t addr_len; - uint16_t port; + network_uint16_t port; mutex_lock(&_pool_mutex); s = _get_socket(socket); mutex_unlock(&_pool_mutex); @@ -464,8 +465,9 @@ int connect(int socket, const struct sockaddr *address, socklen_t address_len) switch (s->type) { #ifdef MODULE_CONN_TCP case SOCK_STREAM: - /* XXX sin_port is in network byteorder */ - if ((res = conn_tcp_connect(&s->conn.tcp, addr, addr_len, port)) < 0) { + if ((res = conn_tcp_connect(&s->conn.tcp, addr, addr_len, + byteorder_ntohs(port))) < 0) { + errno = -res; return -1; } @@ -776,7 +778,8 @@ ssize_t sendto(int socket, const void *buffer, size_t length, int flags, int res = 0; void *addr = NULL; size_t addr_len = 0; - uint16_t port = 0; + network_uint16_t port; + port.u16 = 0; (void)flags; mutex_lock(&_pool_mutex); s = _get_socket(socket); @@ -856,7 +859,7 @@ ssize_t sendto(int socket, const void *buffer, size_t length, int flags, /* cppcheck bug? res is read below in l824 */ /* cppcheck-suppress unreadVariable */ res = conn_udp_sendto(buffer, length, src_addr, src_len, addr, addr_len, s->domain, - sport, port); + sport, byteorder_ntohs(port)); } else if (address != NULL) { ipv6_addr_t local; @@ -870,7 +873,7 @@ ssize_t sendto(int socket, const void *buffer, size_t length, int flags, } s->bound = true; res = conn_udp_sendto(buffer, length, NULL, 0, addr, addr_len, s->domain, - s->src_port, port); + s->src_port, byteorder_ntohs(port)); } else { errno = ENOTCONN; From fdf6da07eb4165baaad6f3326a1a8779034d649e Mon Sep 17 00:00:00 2001 From: Oleg Hahm Date: Fri, 27 Nov 2015 18:04:42 +0100 Subject: [PATCH 5/5] examples: every socket thread needs a msg queue --- examples/posix_sockets/main.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/examples/posix_sockets/main.c b/examples/posix_sockets/main.c index 995019ad8c..de60a3bb13 100644 --- a/examples/posix_sockets/main.c +++ b/examples/posix_sockets/main.c @@ -20,9 +20,11 @@ #include +#include "msg.h" #include "shell.h" -#define MAIN_MSG_QUEUE_SIZE (1) +#define MAIN_MSG_QUEUE_SIZE (4) +static msg_t main_msg_queue[MAIN_MSG_QUEUE_SIZE]; extern int udp_cmd(int argc, char **argv); @@ -33,6 +35,9 @@ static const shell_command_t shell_commands[] = { int main(void) { + /* a sendto() call performs an implicit bind(), hence, a message queue is + * required for the thread executing the shell */ + msg_init_queue(main_msg_queue, MAIN_MSG_QUEUE_SIZE); puts("RIOT socket example application"); /* start shell */ puts("All up, running the shell now");