sergiotarxz/nuttx
1
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

net/tcp/sendfile: TCP retransmission could not start because of incorrect snd_ackcb callback handling:

Browse Source 
Both the snd_ackcb and snd_datacb callbacks were created and destroyed right after sending every packet.
Whenever TCP_REXMIT event occurred due to TCP send timeout, TCP_REXMIT was ignored because
snd_ackcb callback had been destroyed by the time.
The issue is fixed as follows:
- both the snd_ackcb and snd_datacb callbacks are combined into one snd_cb callback
  (the same way as in tcp_send_unbuffered.c).
- the snd_cb callback lives until all requested data (via sendfile) is sent,
  including all ACKs and possible retransmissions.

As a positive side effect of the code optimization / fix, sendfile TCP payload throughput is increased.
This commit is contained in:
Alexander Lunev 2022-01-16 19:21:19 +03:00 committed by Xiang Xiao
parent e140ba1a21
commit f61f276120
1 changed files with 160 additions and 205 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

365
net/tcp/tcp_sendfile.c
View File

@ -76,8 +76,7 @@
struct sendfile_s
{
FAR struct socket *snd_sock; /* Points to the parent socket structure */
FAR struct devif_callback_s *snd_datacb; /* Data callback */
FAR struct devif_callback_s *snd_ackcb; /* ACK callback */
FAR struct devif_callback_s *snd_cb; /* Reference to callback instance */
FAR struct file *snd_file; /* File structure of the input file */
sem_t snd_sem; /* Used to wake up the waiting thread */
off_t snd_foffset; /* Input file offset */
@ -91,13 +90,124 @@ struct sendfile_s
* Private Functions
****************************************************************************/
static uint16_t ack_eventhandler(FAR struct net_driver_s *dev,
FAR void *pvconn,
FAR void *pvpriv, uint16_t flags)
{
FAR struct sendfile_s *pstate = (FAR struct sendfile_s *)pvpriv;
/****************************************************************************
* Name: sendfile_txnotify
*
* Description:
* Notify the appropriate device driver that we are have data ready to
* be send (TCP)
*
* Input Parameters:
* psock - Socket state structure
* conn - The TCP connection structure
*
* Returned Value:
* None
*
****************************************************************************/
ninfo("flags: %04x\n", flags);
static inline void sendfile_txnotify(FAR struct socket *psock,
FAR struct tcp_conn_s *conn)
{
#ifdef CONFIG_NET_IPv4
#ifdef CONFIG_NET_IPv6
/* If both IPv4 and IPv6 support are enabled, then we will need to select
* the device driver using the appropriate IP domain.
*/
if (psock->s_domain == PF_INET)
#endif
{
/* Notify the device driver that send data is available */
netdev_ipv4_txnotify(conn->u.ipv4.laddr, conn->u.ipv4.raddr);
}
#endif /* CONFIG_NET_IPv4 */
#ifdef CONFIG_NET_IPv6
#ifdef CONFIG_NET_IPv4
else /* if (psock->s_domain == PF_INET6) */
#endif /* CONFIG_NET_IPv4 */
{
/* Notify the device driver that send data is available */
DEBUGASSERT(psock->s_domain == PF_INET6);
netdev_ipv6_txnotify(conn->u.ipv6.laddr, conn->u.ipv6.raddr);
}
#endif /* CONFIG_NET_IPv6 */
}
/****************************************************************************
* Name: sendfile_eventhandler
*
* Description:
* This function is called to perform the actual send operation when
* polled by the lower, device interfacing layer.
*
* Input Parameters:
* dev The structure of the network driver that caused the event
* conn The connection structure associated with the socket
* flags Set of events describing why the callback was invoked
*
* Returned Value:
* None
*
* Assumptions:
* The network is locked
*
****************************************************************************/
static uint16_t sendfile_eventhandler(FAR struct net_driver_s *dev,
FAR void *pvconn, FAR void *pvpriv,
uint16_t flags)
{
FAR struct tcp_conn_s *conn = (FAR struct tcp_conn_s *)pvconn;
FAR struct sendfile_s *pstate = (FAR struct sendfile_s *)pvpriv;
FAR struct socket *psock = pstate->snd_sock;
int ret;
/* Check for a loss of connection */
if ((flags & TCP_DISCONN_EVENTS) != 0)
{
nwarn("WARNING: Lost connection\n");
/* We could get here recursively through the callback actions of
* tcp_lost_connection(). So don't repeat that action if we have
* already been disconnected.
*/
DEBUGASSERT(psock != NULL);
if (_SS_ISCONNECTED(psock->s_flags))
{
/* Report not connected */
tcp_lost_connection(psock, pstate->snd_cb, flags);
}
/* Report not connected */
pstate->snd_sent = -ENOTCONN;
goto end_wait;
}
/* The TCP socket is connected and, hence, should be bound to a device.
* Make sure that the polling device is the own that we are bound to.
*/
DEBUGASSERT(conn);
DEBUGASSERT(conn->dev != NULL);
if (dev != conn->dev)
{
return flags;
}
ninfo("flags: %04x acked: %" PRId32 " sent: %zd\n",
flags, pstate->snd_acked, pstate->snd_sent);
/* If this packet contains an acknowledgement, then update the count of
* acknowledged bytes.
*/
if ((flags & TCP_ACKDATA) != 0)
{
@ -131,14 +241,27 @@ static uint16_t ack_eventhandler(FAR struct net_driver_s *dev,
* is the number of bytes to be acknowledged.
*/
pstate->snd_acked = tcp_getsequence(tcp->ackno) - pstate->snd_isn;
pstate->snd_acked = TCP_SEQ_SUB(tcp_getsequence(tcp->ackno),
pstate->snd_isn);
ninfo("ACK: acked=%" PRId32 " sent=%zd flen=%zu\n",
pstate->snd_acked, pstate->snd_sent, pstate->snd_flen);
dev->d_sndlen = 0;
/* Have all of the bytes in the buffer been sent and acknowledged? */
flags &= ~TCP_ACKDATA;
if (pstate->snd_acked >= pstate->snd_flen)
{
/* Yes. Then pstate->snd_flen should hold the number of bytes
* actually sent.
*/
goto end_wait;
}
/* No. Fall through to send more data if necessary */
}
/* Check if we are being asked to retransmit data */
else if ((flags & TCP_REXMIT) != 0)
{
nwarn("WARNING: TCP_REXMIT\n");
@ -148,116 +271,11 @@ static uint16_t ack_eventhandler(FAR struct net_driver_s *dev,
*/
pstate->snd_sent = pstate->snd_acked;
#ifndef CONFIG_NET_TCP_WRITE_BUFFERS
conn->rexmit_seq = pstate->snd_sent + pstate->snd_isn;
#endif
}
/* Check for a loss of connection */
else if ((flags & TCP_DISCONN_EVENTS) != 0)
{
FAR struct socket *psock = pstate->snd_sock;
nwarn("WARNING: Lost connection\n");
/* We could get here recursively through the callback actions of
* tcp_lost_connection(). So don't repeat that action if we have
* already been disconnected.
*/
DEBUGASSERT(psock != NULL);
if (_SS_ISCONNECTED(psock->s_flags))
{
/* Report not connected */
tcp_lost_connection(psock, pstate->snd_ackcb, flags);
}
/* Report not connected */
pstate->snd_sent = -ENOTCONN;
}
/* Prohibit further callbacks */
pstate->snd_ackcb->flags = 0;
pstate->snd_ackcb->priv = NULL;
pstate->snd_ackcb->event = NULL;
/* Wake up the waiting thread */
nxsem_post(&pstate->snd_sem);
return flags;
}
/****************************************************************************
* Name: sendfile_eventhandler
*
* Description:
* This function is called to perform the actual send operation when
* polled by the lower, device interfacing layer.
*
* Input Parameters:
* dev The structure of the network driver that caused the event
* conn The connection structure associated with the socket
* flags Set of events describing why the callback was invoked
*
* Returned Value:
* None
*
* Assumptions:
* The network is locked
*
****************************************************************************/
static uint16_t sendfile_eventhandler(FAR struct net_driver_s *dev,
FAR void *pvconn, FAR void *pvpriv,
uint16_t flags)
{
FAR struct tcp_conn_s *conn = (FAR struct tcp_conn_s *)pvconn;
FAR struct sendfile_s *pstate = (FAR struct sendfile_s *)pvpriv;
int ret;
/* Check for a loss of connection */
if ((flags & TCP_DISCONN_EVENTS) != 0)
{
FAR struct socket *psock = pstate->snd_sock;
nwarn("WARNING: Lost connection\n");
/* We could get here recursively through the callback actions of
* tcp_lost_connection(). So don't repeat that action if we have
* already been disconnected.
*/
DEBUGASSERT(psock != NULL);
if (_SS_ISCONNECTED(psock->s_flags))
{
/* Report not connected */
tcp_lost_connection(psock, pstate->snd_datacb, flags);
}
/* Report not connected */
pstate->snd_sent = -ENOTCONN;
goto end_wait;
}
/* The TCP socket is connected and, hence, should be bound to a device.
* Make sure that the polling device is the own that we are bound to.
*/
DEBUGASSERT(conn);
DEBUGASSERT(conn->dev != NULL);
if (dev != conn->dev)
{
return flags;
}
ninfo("flags: %04x acked: %" PRId32 " sent: %zd\n",
flags, pstate->snd_acked, pstate->snd_sent);
/* We get here if (1) not all of the data has been ACKed, (2) we have been
* asked to retransmit data, (3) the connection is still healthy, and (4)
* the outgoing packet is available for our use. In this case, we are
@ -320,6 +338,10 @@ static uint16_t sendfile_eventhandler(FAR struct net_driver_s *dev,
tcp_setsequence(conn->sndseq, seqno);
/* Notify the device driver of the availability of TX data */
sendfile_txnotify(psock, conn);
/* Update the amount of data sent (but not necessarily ACKed) */
pstate->snd_sent += sndlen;
@ -330,81 +352,31 @@ static uint16_t sendfile_eventhandler(FAR struct net_driver_s *dev,
else
{
nwarn("WARNING: Window full, wait for ack\n");
goto wait;
}
}
if (pstate->snd_sent >= pstate->snd_flen
&& pstate->snd_acked < pstate->snd_flen)
{
/* All data has been sent, but there are outstanding ACK's */
/* Continue waiting */
goto wait;
}
return flags;
end_wait:
/* Do not allow any further callbacks */
pstate->snd_datacb->flags = 0;
pstate->snd_datacb->priv = NULL;
pstate->snd_datacb->event = NULL;
pstate->snd_cb->flags = 0;
pstate->snd_cb->priv = NULL;
pstate->snd_cb->event = NULL;
/* There are no outstanding, unacknowledged bytes */
conn->tx_unacked = 0;
/* Wake up the waiting thread */
nxsem_post(&pstate->snd_sem);
wait:
return flags;
}
/****************************************************************************
* Name: sendfile_txnotify
*
* Description:
* Notify the appropriate device driver that we are have data ready to
* be send (TCP)
*
* Input Parameters:
* psock - Socket state structure
* conn - The TCP connection structure
*
* Returned Value:
* None
*
****************************************************************************/
static inline void sendfile_txnotify(FAR struct socket *psock,
FAR struct tcp_conn_s *conn)
{
#ifdef CONFIG_NET_IPv4
#ifdef CONFIG_NET_IPv6
/* If both IPv4 and IPv6 support are enabled, then we will need to select
* the device driver using the appropriate IP domain.
*/
if (psock->s_domain == PF_INET)
#endif
{
/* Notify the device driver that send data is available */
netdev_ipv4_txnotify(conn->u.ipv4.laddr, conn->u.ipv4.raddr);
}
#endif /* CONFIG_NET_IPv4 */
#ifdef CONFIG_NET_IPv6
#ifdef CONFIG_NET_IPv4
else /* if (psock->s_domain == PF_INET6) */
#endif /* CONFIG_NET_IPv4 */
{
/* Notify the device driver that send data is available */
DEBUGASSERT(psock->s_domain == PF_INET6);
netdev_ipv6_txnotify(conn->u.ipv6.laddr, conn->u.ipv6.raddr);
}
#endif /* CONFIG_NET_IPv6 */
}
/****************************************************************************
* Public Functions
****************************************************************************/
@ -514,24 +486,15 @@ ssize_t tcp_sendfile(FAR struct socket *psock, FAR struct file *infile,
/* Allocate resources to receive a callback */
state.snd_datacb = tcp_callback_alloc(conn);
state.snd_cb = tcp_callback_alloc(conn);
if (state.snd_datacb == NULL)
if (state.snd_cb == NULL)
{
nerr("ERROR: Failed to allocate data callback\n");
nerr("ERROR: Failed to allocate callback\n");
ret = -ENOMEM;
goto errout_locked;
}
state.snd_ackcb = tcp_callback_alloc(conn);
if (state.snd_ackcb == NULL)
{
nerr("ERROR: Failed to allocate ack callback\n");
ret = -ENOMEM;
goto errout_datacb;
}
/* Get the initial sequence number that will be used */
state.snd_isn = tcp_getsequence(conn->sndseq);
@ -542,17 +505,12 @@ ssize_t tcp_sendfile(FAR struct socket *psock, FAR struct file *infile,
conn->tx_unacked = 0;
/* Set up the ACK callback in the connection */
/* Set up the callback in the connection */
state.snd_ackcb->flags = (TCP_ACKDATA | TCP_REXMIT | TCP_DISCONN_EVENTS);
state.snd_ackcb->priv = (FAR void *)&state;
state.snd_ackcb->event = ack_eventhandler;
/* Perform the TCP send operation */
state.snd_datacb->flags = TCP_POLL;
state.snd_datacb->priv = (FAR void *)&state;
state.snd_datacb->event = sendfile_eventhandler;
state.snd_cb->flags = (TCP_ACKDATA | TCP_REXMIT | TCP_POLL |
TCP_DISCONN_EVENTS);
state.snd_cb->priv = (FAR void *)&state;
state.snd_cb->event = sendfile_eventhandler;
/* Notify the device driver of the availability of TX data */
@ -566,14 +524,11 @@ ssize_t tcp_sendfile(FAR struct socket *psock, FAR struct file *infile,
_SO_TIMEOUT(psock->s_sndtimeo));
if (ret != -ETIMEDOUT || acked == state.snd_acked)
{
break; /* Timeout without any progress */
break; /* Successful completion or timeout without any progress */
}
}
tcp_callback_free(conn, state.snd_ackcb);
errout_datacb:
tcp_callback_free(conn, state.snd_datacb);
tcp_callback_free(conn, state.snd_cb);
errout_locked:
nxsem_destroy(&state.snd_sem);