tcp: Change the way to advance rcvseq

* Move the code to advance rcvseq for user data from tcp_input
  to receive handlers.
  Motivation: allow partial ack.

* If we drop a segment, ignore FIN as well. Note than tcp FIN bit is
  logically after the user data in the same segment.
This commit is contained in:
YAMAMOTO Takashi 2021-06-17 13:27:18 +09:00 committed by Masayuki Ishikawa
parent 5fd3eca9c9
commit 022a2490d1
3 changed files with 53 additions and 58 deletions

View File

@ -91,19 +91,23 @@ tcp_data_event(FAR struct net_driver_s *dev, FAR struct tcp_conn_s *conn,
* read-ahead buffers to retain the data -- drop the packet. * read-ahead buffers to retain the data -- drop the packet.
*/ */
ninfo("Dropped %d bytes\n", dev->d_len); ninfo("Dropped %d/%d bytes\n", buflen - recvlen, buflen);
#ifdef CONFIG_NET_STATISTICS #ifdef CONFIG_NET_STATISTICS
g_netstats.tcp.drop++; g_netstats.tcp.drop++;
#endif #endif
/* Clear the TCP_SNDACK bit so that no ACK will be sent. /* Clear the TCP_SNDACK bit so that no ACK will be sent.
* Clear the TCP_CLOSE because we effectively dropped
* the FIN as well.
* *
* Revisit: It might make more sense to send a dup ack * Revisit: It might make more sense to send a dup ack
* to give a hint to the peer. * to give a hint to the peer.
*/ */
ret &= ~TCP_SNDACK; ret &= ~(TCP_SNDACK | TCP_CLOSE);
} }
net_incr32(conn->rcvseq, recvlen);
} }
/* In any event, the new data has now been handled */ /* In any event, the new data has now been handled */

View File

@ -238,7 +238,7 @@ static void tcp_input(FAR struct net_driver_s *dev, uint8_t domain,
goto drop; goto drop;
} }
net_incr32(conn->rcvseq, 1); net_incr32(conn->rcvseq, 1); /* ack SYN */
/* Parse the TCP MSS option, if present. */ /* Parse the TCP MSS option, if present. */
@ -618,7 +618,6 @@ found:
if (dev->d_len > 0) if (dev->d_len > 0)
{ {
flags |= TCP_NEWDATA; flags |= TCP_NEWDATA;
net_incr32(conn->rcvseq, dev->d_len);
} }
dev->d_sndlen = 0; dev->d_sndlen = 0;
@ -706,7 +705,7 @@ found:
memcpy(conn->rcvseq, tcp->seqno, 4); memcpy(conn->rcvseq, tcp->seqno, 4);
conn->rcv_adv = tcp_getsequence(conn->rcvseq); conn->rcv_adv = tcp_getsequence(conn->rcvseq);
net_incr32(conn->rcvseq, 1); net_incr32(conn->rcvseq, 1); /* ack SYN */
conn->tx_unacked = 0; conn->tx_unacked = 0;
#ifdef CONFIG_NET_TCP_WRITE_BUFFERS #ifdef CONFIG_NET_TCP_WRITE_BUFFERS
@ -774,7 +773,6 @@ found:
* has been closed. * has been closed.
*/ */
net_incr32(conn->rcvseq, dev->d_len + 1);
flags |= TCP_CLOSE; flags |= TCP_CLOSE;
if (dev->d_len > 0) if (dev->d_len > 0)
@ -782,17 +780,26 @@ found:
flags |= TCP_NEWDATA; flags |= TCP_NEWDATA;
} }
tcp_callback(dev, conn, flags); result = tcp_callback(dev, conn, flags);
conn->tcpstateflags = TCP_LAST_ACK; if ((result & TCP_CLOSE) != 0)
conn->tx_unacked = 1; {
conn->nrtx = 0; conn->tcpstateflags = TCP_LAST_ACK;
conn->tx_unacked = 1;
conn->nrtx = 0;
net_incr32(conn->rcvseq, 1); /* ack FIN */
#ifdef CONFIG_NET_TCP_WRITE_BUFFERS #ifdef CONFIG_NET_TCP_WRITE_BUFFERS
conn->sndseq_max = tcp_getsequence(conn->sndseq) + 1; conn->sndseq_max = tcp_getsequence(conn->sndseq) + 1;
#endif #endif
ninfo("TCP state: TCP_LAST_ACK\n"); ninfo("TCP state: TCP_LAST_ACK\n");
tcp_send(dev, conn, TCP_FIN | TCP_ACK, tcpiplen);
}
else
{
ninfo("TCP: Dropped a FIN\n");
tcp_appsend(dev, conn, result);
}
tcp_send(dev, conn, TCP_FIN | TCP_ACK, tcpiplen);
return; return;
} }
@ -914,31 +921,12 @@ found:
if ((flags & (TCP_NEWDATA | TCP_ACKDATA)) != 0) if ((flags & (TCP_NEWDATA | TCP_ACKDATA)) != 0)
{ {
/* Clear sndlen and remember the size in d_len. The application
* may modify d_len and we will need this value later when we
* update the sequence number.
*/
dev->d_sndlen = 0; dev->d_sndlen = 0;
len = dev->d_len;
/* Provide the packet to the application */ /* Provide the packet to the application */
result = tcp_callback(dev, conn, flags); result = tcp_callback(dev, conn, flags);
/* If the application successfully handled the incoming data,
* then TCP_SNDACK will be set in the result. In this case,
* we need to update the sequence number. The ACK will be
* send by tcp_appsend().
*/
if ((result & TCP_SNDACK) != 0)
{
/* Update the sequence number using the saved length */
net_incr32(conn->rcvseq, len);
}
/* Send the response, ACKing the data or not, as appropriate */ /* Send the response, ACKing the data or not, as appropriate */
tcp_appsend(dev, conn, result); tcp_appsend(dev, conn, result);
@ -986,7 +974,7 @@ found:
ninfo("TCP state: TCP_CLOSING\n"); ninfo("TCP state: TCP_CLOSING\n");
} }
net_incr32(conn->rcvseq, 1); net_incr32(conn->rcvseq, 1); /* ack FIN */
tcp_callback(dev, conn, TCP_CLOSE); tcp_callback(dev, conn, TCP_CLOSE);
tcp_send(dev, conn, TCP_ACK, tcpiplen); tcp_send(dev, conn, TCP_ACK, tcpiplen);
return; return;
@ -1018,7 +1006,7 @@ found:
conn->timer = 0; conn->timer = 0;
ninfo("TCP state: TCP_TIME_WAIT\n"); ninfo("TCP state: TCP_TIME_WAIT\n");
net_incr32(conn->rcvseq, 1); net_incr32(conn->rcvseq, 1); /* ack FIN */
tcp_callback(dev, conn, TCP_CLOSE); tcp_callback(dev, conn, TCP_CLOSE);
tcp_send(dev, conn, TCP_ACK, tcpiplen); tcp_send(dev, conn, TCP_ACK, tcpiplen);
return; return;

View File

@ -166,9 +166,13 @@ static size_t tcp_recvfrom_newdata(FAR struct net_driver_s *dev,
* *
****************************************************************************/ ****************************************************************************/
static inline void tcp_newdata(FAR struct net_driver_s *dev, static inline uint16_t tcp_newdata(FAR struct net_driver_s *dev,
FAR struct tcp_recvfrom_s *pstate) FAR struct tcp_recvfrom_s *pstate,
uint16_t flags)
{ {
FAR struct tcp_conn_s *conn = (FAR struct tcp_conn_s *)
pstate->ir_sock->s_conn;
/* Take as much data from the packet as we can */ /* Take as much data from the packet as we can */
size_t recvlen = tcp_recvfrom_newdata(dev, pstate); size_t recvlen = tcp_recvfrom_newdata(dev, pstate);
@ -179,40 +183,39 @@ static inline void tcp_newdata(FAR struct net_driver_s *dev,
if (recvlen < dev->d_len) if (recvlen < dev->d_len)
{ {
FAR struct tcp_conn_s *conn =
(FAR struct tcp_conn_s *)pstate->ir_sock->s_conn;
FAR uint8_t *buffer = (FAR uint8_t *)dev->d_appdata + recvlen; FAR uint8_t *buffer = (FAR uint8_t *)dev->d_appdata + recvlen;
uint16_t buflen = dev->d_len - recvlen; uint16_t buflen = dev->d_len - recvlen;
#ifdef CONFIG_DEBUG_NET
uint16_t nsaved; uint16_t nsaved;
nsaved = tcp_datahandler(conn, buffer, buflen); nsaved = tcp_datahandler(conn, buffer, buflen);
#else
tcp_datahandler(conn, buffer, buflen);
#endif
/* There are complicated buffering issues that are not addressed fully
* here. For example, what if up_datahandler() cannot buffer the
* remainder of the packet? In that case, the data will be dropped but
* still ACKed. Therefore it would not be resent.
*
* This is probably not an issue here because we only get here if the
* read-ahead buffers are empty and there would have to be something
* serioulsy wrong with the configuration not to be able to buffer a
* partial packet in this context.
*/
#ifdef CONFIG_DEBUG_NET
if (nsaved < buflen) if (nsaved < buflen)
{ {
nerr("ERROR: packet data not saved (%d bytes)\n", buflen - nsaved); nwarn("WARNING: packet data not fully saved "
"(%d/%u/%zu/%u bytes)\n",
buflen - nsaved,
(unsigned int)nsaved,
recvlen,
(unsigned int)dev->d_len);
} }
#endif
recvlen += nsaved;
} }
if (recvlen < dev->d_len)
{
/* Clear the TCP_CLOSE because we effectively dropped the FIN as well.
*/
flags &= ~TCP_CLOSE;
}
net_incr32(conn->rcvseq, recvlen);
/* Indicate no data in the buffer */ /* Indicate no data in the buffer */
dev->d_len = 0; dev->d_len = 0;
return flags;
} }
/**************************************************************************** /****************************************************************************
@ -418,7 +421,7 @@ static uint16_t tcp_recvhandler(FAR struct net_driver_s *dev,
* packet in the read-ahead buffer). * packet in the read-ahead buffer).
*/ */
tcp_newdata(dev, pstate); flags = tcp_newdata(dev, pstate, flags);
/* Save the sender's address in the caller's 'from' location */ /* Save the sender's address in the caller's 'from' location */