From 0e2b6929b4d588511f6bbbd436b8936981734f54 Mon Sep 17 00:00:00 2001 From: Gregory Nutt Date: Thu, 28 May 2015 07:26:03 -0600 Subject: [PATCH 1/3] Add function headers to prototypes in net/socket/socket.h --- net/socket/net_sockets.c | 6 +- net/socket/socket.h | 245 +++++++++++++++++++++++++++++++++++++-- 2 files changed, 237 insertions(+), 14 deletions(-) diff --git a/net/socket/net_sockets.c b/net/socket/net_sockets.c index a98d67fc16..6c7e1d1128 100644 --- a/net/socket/net_sockets.c +++ b/net/socket/net_sockets.c @@ -157,10 +157,10 @@ void net_releaselist(FAR struct socketlist *list) * Allocate a socket descriptor * * Input Parameters: - * Lowest socket descripor index to be used. + * Lowest socket descriptor index to be used. * * Returned Value: - * On success, a socket desrciptor >= minsd is returned. A negater errno + * On success, a socket descriptor >= minsd is returned. A negated errno * value is returned on failure. * ****************************************************************************/ @@ -208,8 +208,10 @@ int sockfd_allocate(int minsd) * Free a socket. * * Input Parameters: + * psock - A reference to the socket instance to be freed. * * Returned Value: + * None * ****************************************************************************/ diff --git a/net/socket/socket.h b/net/socket/socket.h index 314206b6fc..ef16afc752 100644 --- a/net/socket/socket.h +++ b/net/socket/socket.h @@ -151,34 +151,255 @@ extern "C" * Public Function Prototypes ****************************************************************************/ -/* net_sockets.c *************************************************************/ - -int sockfd_allocate(int minsd); -void sock_release(FAR struct socket *psock); -void sockfd_release(int sockfd); -FAR struct socket *sockfd_socket(int sockfd); - -/* net_connect.c *************************************************************/ - #ifdef CONFIG_NET_TCP struct tcp_conn_s; /* Forward reference */ +#endif +/**************************************************************************** + * Name: sockfd_allocate + * + * Description: + * Allocate a socket descriptor + * + * Input Parameters: + * Lowest socket descriptor index to be used. + * + * Returned Value: + * On success, a socket descriptor >= minsd is returned. A negated errno + * value is returned on failure. + * + ****************************************************************************/ + +int sockfd_allocate(int minsd); + +/**************************************************************************** + * Name: sock_release + * + * Description: + * Free a socket. + * + * Input Parameters: + * psock - A reference to the socket instance to be freed. + * + * Returned Value: + * None + * + ****************************************************************************/ + +void sock_release(FAR struct socket *psock); + +/**************************************************************************** + * Name: sockfd_release + * + * Description: + * Free the socket by its socket descriptor. + * + * Input Parameters: + * sockfd - Socket descriptor identifies the socket to be released. + * + * Returned Value: + * None + * + ****************************************************************************/ + +void sockfd_release(int sockfd); + +/**************************************************************************** + * Name: sockfd_socket + * + * Description: + * Given a socket descriptor, return the underlying socket structure. + * + * Input Parameters: + * sockfd - The socket descriptor index o use. + * + * Returned Value: + * On success, a reference to the socket structure associated with the + * the socket descriptor is returned. NULL is returned on any failure. + * + ****************************************************************************/ + +FAR struct socket *sockfd_socket(int sockfd); + +/**************************************************************************** + * Name: net_startmonitor + * + * Description: + * Set up to receive TCP connection state changes for a given socket + * + * Input Parameters: + * psock - The socket of interest + * + * Returned Value: + * For now, this function always returns OK. + * + ****************************************************************************/ + +#ifdef CONFIG_NET_TCP int net_startmonitor(FAR struct socket *psock); +#endif + +/**************************************************************************** + * Name: net_stopmonitor + * + * Description: + * Stop monitoring TCP connection changes for a given socket + * + * Input Parameters: + * conn - The TCP connection of interest + * + * Returned Value: + * None + * + ****************************************************************************/ + +#ifdef CONFIG_NET_TCP void net_stopmonitor(FAR struct tcp_conn_s *conn); +#endif + +/**************************************************************************** + * Name: net_lostconnection + * + * Description: + * Called when a loss-of-connection event has occurred. + * + * Parameters: + * psock The TCP socket structure associated. + * flags Set of connection events events + * + * Returned Value: + * None + * + * Assumptions: + * Running at the interrupt level + * + ****************************************************************************/ + +#ifdef CONFIG_NET_TCP void net_lostconnection(FAR struct socket *psock, uint16_t flags); #endif -/* net_close.c ***************************************************************/ +/**************************************************************************** + * Function: psock_close + * + * Description: + * Performs the close operation on a socket instance + * + * Parameters: + * psock Socket instance + * + * Returned Value: + * 0 on success; -1 on error with errno set appropriately. + * + * Assumptions: + * + ****************************************************************************/ int psock_close(FAR struct socket *psock); -/* sockopt support ***********************************************************/ +/**************************************************************************** + * Function: net_close + * + * Description: + * Performs the close operation on socket descriptors + * + * Parameters: + * sockfd Socket descriptor of socket + * + * Returned Value: + * 0 on success; -1 on error with errno set appropriately. + * + * Assumptions: + * + ****************************************************************************/ + +int net_close(int sockfd); + +/**************************************************************************** + * Function: net_timeo + * + * Description: + * Check if a timeout has elapsed. This can be called from a socket poll + * function to determine if a timeout has occurred. + * + * Parameters: + * start_time Timeout start time in system clock ticks + * timeout Timeout value in deciseconds. + * + * Returned Value: + * 0 (FALSE) if not timeout; 1 (TRUE) if timeout + * + * Assumptions: + * + ****************************************************************************/ #ifdef CONFIG_NET_SOCKOPTS int net_timeo(uint32_t start_time, socktimeo_t timeo); #endif -/* send.c ********************************************************************/ +/**************************************************************************** + * Function: psock_send + * + * Description: + * The send() call may be used only when the socket is in a connected state + * (so that the intended recipient is known). The only difference between + * send() and write() is the presence of flags. With zero flags parameter, + * send() is equivalent to write(). Also, send(sockfd,buf,len,flags) is + * equivalent to sendto(sockfd,buf,len,flags,NULL,0). + * + * Parameters: + * psock An instance of the internal socket structure. + * buf Data to send + * len Length of data to send + * flags Send flags + * + * Returned Value: + * On success, returns the number of characters sent. On error, + * -1 is returned, and errno is set appropriately: + * + * EAGAIN or EWOULDBLOCK + * The socket is marked non-blocking and the requested operation + * would block. + * EBADF + * An invalid descriptor was specified. + * ECONNRESET + * Connection reset by peer. + * EDESTADDRREQ + * The socket is not connection-mode, and no peer address is set. + * EFAULT + * An invalid user space address was specified for a parameter. + * EINTR + * A signal occurred before any data was transmitted. + * EINVAL + * Invalid argument passed. + * EISCONN + * The connection-mode socket was connected already but a recipient + * was specified. (Now either this error is returned, or the recipient + * specification is ignored.) + * EMSGSIZE + * The socket type requires that message be sent atomically, and the + * size of the message to be sent made this impossible. + * ENOBUFS + * The output queue for a network interface was full. This generally + * indicates that the interface has stopped sending, but may be + * caused by transient congestion. + * ENOMEM + * No memory available. + * ENOTCONN + * The socket is not connected, and no target has been given. + * ENOTSOCK + * The argument s is not a socket. + * EOPNOTSUPP + * Some bit in the flags argument is inappropriate for the socket + * type. + * EPIPE + * The local end has been shut down on a connection oriented socket. + * In this case the process will also receive a SIGPIPE unless + * MSG_NOSIGNAL is set. + * + * Assumptions: + * + ****************************************************************************/ ssize_t psock_send(FAR struct socket *psock, FAR const void *buf, size_t len, int flags); From 01d176af764294da0d91101517d3594743e05fcc Mon Sep 17 00:00:00 2001 From: Gregory Nutt Date: Thu, 28 May 2015 08:23:51 -0600 Subject: [PATCH 2/3] net_startmonitor.c always returned zero. In the case where a socket has already been closed, it correctly handled the disconnetion event but still returned OK. Returning OK causes the callers of net_startmonitor to assume that the connection was okay, undoing the good things that net_startmonitor did and causing the socket to be marked as connected. This behavior was noted by Pelle Windestam. --- net/socket/accept.c | 17 ++++++++++++++++- net/socket/connect.c | 22 +++++++++++++++------- net/socket/net_close.c | 5 +++-- net/socket/net_monitor.c | 37 ++++++++++++++++++++++++++++--------- net/socket/socket.h | 8 +++++++- net/tcp/tcp_accept.c | 9 +++++---- 6 files changed, 74 insertions(+), 24 deletions(-) diff --git a/net/socket/accept.c b/net/socket/accept.c index 390776d330..83cd9602cc 100644 --- a/net/socket/accept.c +++ b/net/socket/accept.c @@ -299,7 +299,19 @@ int accept(int sockfd, FAR struct sockaddr *addr, FAR socklen_t *addrlen) * socket */ - net_startmonitor(pnewsock); + ret = net_startmonitor(pnewsock); + if (ret < 0) + { + /* net_startmonitor() can only fail on certain race conditions + * where the connection was lost just before this function was + * called. Undo everything we have done and return a failure. + */ + + net_unlock(state); + err = -ret; + goto errout_after_accept; + } + net_unlock(state); } #endif /* CONFIG_NET_TCP */ @@ -310,6 +322,9 @@ int accept(int sockfd, FAR struct sockaddr *addr, FAR socklen_t *addrlen) pnewsock->s_flags &= ~_SF_CLOSED; return newfd; +errout_after_accept: + psock_close(pnewsock); + errout_with_socket: sockfd_release(newfd); diff --git a/net/socket/connect.c b/net/socket/connect.c index 88db09a2e6..5775213f10 100644 --- a/net/socket/connect.c +++ b/net/socket/connect.c @@ -81,8 +81,8 @@ struct tcp_connect_s #ifdef CONFIG_NET_TCP static inline int psock_setup_callbacks(FAR struct socket *psock, FAR struct tcp_connect_s *pstate); -static inline void psock_teardown_callbacks(FAR struct tcp_connect_s *pstate, - int status); +static void psock_teardown_callbacks(FAR struct tcp_connect_s *pstate, + int status); static uint16_t psock_connect_interrupt(FAR struct net_driver_s *dev, FAR void *pvconn, FAR void *pvpriv, uint16_t flags); @@ -124,9 +124,18 @@ static inline int psock_setup_callbacks(FAR struct socket *psock, /* Set up the connection event monitor */ - net_startmonitor(psock); - ret = OK; + ret = net_startmonitor(psock); + if (ret < 0) + { + /* net_startmonitor() can only fail on certain race conditions + * where the connection was lost just before this function was + * called. Undo everything we have done and return a failure. + */ + + psock_teardown_callbacks(pstate, ret); + } } + return ret; } #endif /* CONFIG_NET_TCP */ @@ -136,15 +145,14 @@ static inline int psock_setup_callbacks(FAR struct socket *psock, ****************************************************************************/ #ifdef CONFIG_NET_TCP -static inline void psock_teardown_callbacks(FAR struct tcp_connect_s *pstate, - int status) +static void psock_teardown_callbacks(FAR struct tcp_connect_s *pstate, + int status) { FAR struct tcp_conn_s *conn = pstate->tc_conn; /* Make sure that no further interrupts are processed */ tcp_callback_free(conn, pstate->tc_cb); - pstate->tc_cb = NULL; /* If we successfully connected, we will continue to monitor the connection diff --git a/net/socket/net_close.c b/net/socket/net_close.c index 2ae8e606d5..c81a9b4bb6 100644 --- a/net/socket/net_close.c +++ b/net/socket/net_close.c @@ -523,8 +523,9 @@ int psock_close(FAR struct socket *psock) goto errout; } - /* We perform the uIP close operation only if this is the last count on the socket. - * (actually, I think the socket crefs only takes the values 0 and 1 right now). + /* We perform the uIP close operation only if this is the last count on + * the socket. (actually, I think the socket crefs only takes the values + * 0 and 1 right now). */ if (psock->s_crefs <= 1) diff --git a/net/socket/net_monitor.c b/net/socket/net_monitor.c index a9f0de3c15..99049737ed 100644 --- a/net/socket/net_monitor.c +++ b/net/socket/net_monitor.c @@ -121,7 +121,13 @@ static void connection_event(FAR struct tcp_conn_s *conn, uint16_t flags) * psock - The socket of interest * * Returned Value: - * For now, this function always returns OK. + * On success, net_startmonitor returns OK; On any failure, + * net_startmonitor will return a negated errno value. The only failure + * that can occur is if the socket has already been closed and, in this + * case, -ENOTCONN is returned. + * + * Assumptions: + * The caller holds the network lock. * ****************************************************************************/ @@ -131,22 +137,35 @@ int net_startmonitor(FAR struct socket *psock) DEBUGASSERT(psock && conn); - /* Set up to receive callbacks on connection-related events */ - - conn->connection_private = (void*)psock; - conn->connection_event = connection_event; - - /* Check if the connection has already been closed before any callbacks have - * been registered. (Maybe the connection is lost before accept has registered - * the monitoring callback.) + /* Check if the connection has already been closed before any callbacks + * have been registered. (Maybe the connection is lost before accept has + * registered the monitoring callback.) */ if (!(conn->tcpstateflags == TCP_ESTABLISHED || conn->tcpstateflags == TCP_SYN_RCVD)) { + /* Invoke the TCP_CLOSE connection event now */ + connection_event(conn, TCP_CLOSE); + + /* Make sure that the monitor is stopped */ + + conn->connection_private = NULL; + conn->connection_event = NULL; + + /* And return -ENOTCONN to indicate the the monitor was not started + * because the socket was already disconnected. + */ + + return -ENOTCONN; } + /* Set up to receive callbacks on connection-related events */ + + conn->connection_private = (void*)psock; + conn->connection_event = connection_event; + return OK; } diff --git a/net/socket/socket.h b/net/socket/socket.h index ef16afc752..8c0cae0d3a 100644 --- a/net/socket/socket.h +++ b/net/socket/socket.h @@ -231,7 +231,13 @@ FAR struct socket *sockfd_socket(int sockfd); * psock - The socket of interest * * Returned Value: - * For now, this function always returns OK. + * On success, net_startmonitor returns OK; On any failure, + * net_startmonitor will return a negated errno value. The only failure + * that can occur is if the socket has already been closed and, in this + * case, -ENOTCONN is returned. + * + * Assumptions: + * The caller holds the network lock. * ****************************************************************************/ diff --git a/net/tcp/tcp_accept.c b/net/tcp/tcp_accept.c index 96c6d8b062..9597b6b36b 100644 --- a/net/tcp/tcp_accept.c +++ b/net/tcp/tcp_accept.c @@ -219,7 +219,8 @@ static int accept_interrupt(FAR struct tcp_conn_s *listener, * Parameters: * psock The listening TCP socket structure * addr Receives the address of the connecting client - * addrlen Input: allocated size of 'addr', Return: returned size of 'addr' + * addrlen Input: allocated size of 'addr', Return: returned size of + * 'addr' * newconn The new, accepted TCP connection structure * * Returned Value: @@ -259,9 +260,9 @@ int psock_tcp_accept(FAR struct socket *psock, FAR struct sockaddr *addr, } /* In general, this uIP-based implementation will not support non-blocking - * socket operations... except in a few cases: Here for TCP accept with backlog - * enabled. If this socket is configured as non-blocking then return EAGAIN - * if there is no pending connection in the backlog. + * socket operations... except in a few cases: Here for TCP accept with + * backlog enabled. If this socket is configured as non-blocking then + * return EAGAIN if there is no pending connection in the backlog. */ else if (_SS_ISNONBLOCK(psock->s_flags)) From b450226296418c04b432ddba49d06bfe4c636950 Mon Sep 17 00:00:00 2001 From: Gregory Nutt Date: Thu, 28 May 2015 08:28:45 -0600 Subject: [PATCH 3/3] Update ChangeLog --- ChangeLog | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/ChangeLog b/ChangeLog index 55cf28c256..2575240c36 100755 --- a/ChangeLog +++ b/ChangeLog @@ -10449,3 +10449,10 @@ * net/tcp: Fix an important TCP networking bug: 16-bit flags was being converted to 8-bits in a few locations, causing loss of status indications (2015-05-27). + * net/socket and net/tcp: net_startmonitor.c always returned OK. In + the case where a socket has already been closed, it correctly handled + the disconnetion event but still returned OK. Returning OK caused + the callers of net_startmonitor to assume that the connection was + okay, undoing the good things that net_startmonitor did and causing + the socket to be marked as connected. This behavior was noted by + Pelle Windestam (2015-05-28).