You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2022/02/23 17:40:59 UTC
[GitHub] [incubator-nuttx] anchao commented on a change in pull request #5589: net/tcp(buffered): retransmit only one the earliest not acknowledged segment
anchao commented on a change in pull request #5589:
URL: https://github.com/apache/incubator-nuttx/pull/5589#discussion_r813150170
##########
File path: net/tcp/tcp_send_buffered.c
##########
@@ -604,170 +603,116 @@ static uint16_t psock_send_eventhandler(FAR struct net_driver_s *dev,
else if ((flags & TCP_REXMIT) != 0)
{
- rexmit = true;
- }
-
- if (rexmit)
- {
+ size_t sndlen;
FAR struct tcp_wrbuffer_s *wrb;
- FAR sq_entry_t *entry;
+ bool rexmit_from_write_q = false;
- ninfo("REXMIT: %04x\n", flags);
+ /* According to RFC 6298 (5.4), retransmit the earliest segment
+ * that has not been acknowledged by the TCP receiver.
+ */
- /* If there is a partially sent write buffer at the head of the
- * write_q? Has anything been sent from that write buffer?
+ /* If the unacked_q is not empty, retrasmit the eariest segment
+ * that is in the head of the unacked_q.
*/
- wrb = (FAR struct tcp_wrbuffer_s *)sq_peek(&conn->write_q);
- ninfo("REXMIT: wrb=%p sent=%u\n", wrb, wrb ? TCP_WBSENT(wrb) : 0);
+ wrb = (FAR struct tcp_wrbuffer_s *)sq_peek(&conn->unacked_q);
- if (wrb != NULL && TCP_WBSENT(wrb) > 0)
+ if (wrb == NULL || TCP_WBSENT(wrb) == 0)
{
- FAR struct tcp_wrbuffer_s *tmp;
- uint16_t sent;
-
- /* Yes.. Reset the number of bytes sent sent from
- * the write buffer
+ /* If the unacked_q is empty, then the eariest segment is in
+ * the head of the write_q (the head of the write_q may be
+ * partially sent and may still have un-ACKed segment).
*/
- sent = TCP_WBSENT(wrb);
- if (conn->tx_unacked > sent)
- {
- conn->tx_unacked -= sent;
- }
- else
- {
- conn->tx_unacked = 0;
- }
+ wrb = (FAR struct tcp_wrbuffer_s *)sq_peek(&conn->write_q);
+ rexmit_from_write_q = true;
+ }
- if (conn->sent > sent)
- {
- conn->sent -= sent;
- }
- else
- {
- conn->sent = 0;
- }
+ DEBUGASSERT(wrb != NULL && TCP_WBSENT(wrb) > 0);
- TCP_WBSENT(wrb) = 0;
- ninfo("REXMIT: wrb=%p sent=%u, "
- "conn tx_unacked=%" PRId32 " sent=%" PRId32 "\n",
- wrb, TCP_WBSENT(wrb), conn->tx_unacked, conn->sent);
+ ninfo("REXMIT: flags=%04x wrb=%p sent=%u seq=%" PRIu32 ", "
+ "conn tx_unacked=%" PRId32 " sent=%" PRId32 "\n",
+ flags, wrb, TCP_WBSENT(wrb), TCP_WBSEQNO(wrb),
+ conn->tx_unacked, conn->sent);
- /* Increment the retransmit count on this write buffer. */
+ /* Increment the retransmit count on this write buffer. */
- if (++TCP_WBNRTX(wrb) >= TCP_MAXRTX)
- {
- nwarn("WARNING: Expiring wrb=%p nrtx=%u\n",
- wrb, TCP_WBNRTX(wrb));
+ if (++TCP_WBNRTX(wrb) >= TCP_MAXRTX)
+ {
+ nwarn("WARNING: Expiring wrb=%p nrtx=%u\n",
+ wrb, TCP_WBNRTX(wrb));
- /* The maximum retry count as been exhausted. Remove the write
- * buffer at the head of the queue.
- */
+ /* The maximum retry count has been exhausted. Remove the write
+ * buffer at the head of the queue.
+ */
+ if (rexmit_from_write_q)
+ {
+ FAR struct tcp_wrbuffer_s *tmp;
tmp = (FAR struct tcp_wrbuffer_s *)sq_remfirst(&conn->write_q);
DEBUGASSERT(tmp == wrb);
UNUSED(tmp);
+ }
- /* And return the write buffer to the free list */
+ /* And return the write buffer to the free list */
- tcp_wrbuffer_release(wrb);
+ tcp_wrbuffer_release(wrb);
- /* Notify any waiters if the write buffers have been
- * drained.
- */
-
- psock_writebuffer_notify(conn);
+ /* Notify any waiters if the write buffers have been
+ * drained.
+ */
- /* NOTE expired is different from un-ACKed, it is designed to
- * represent the number of segments that have been sent,
- * retransmitted, and un-ACKed, if expired is not zero, the
- * connection will be closed.
- *
- * field expired can only be updated at TCP_ESTABLISHED state
- */
+ psock_writebuffer_notify(conn);
- conn->expired++;
- }
- }
-
- /* Move all segments that have been sent but not ACKed to the write
- * queue again note, the un-ACKed segments are put at the head of the
- * write_q so they can be resent as soon as possible.
- */
+ /* NOTE expired is different from un-ACKed, it is designed to
+ * represent the number of segments that have been sent,
+ * retransmitted, and un-ACKed, if expired is not zero, the
+ * connection will be closed.
+ *
+ * conn->expired can only be updated in TCP_ESTABLISHED state.
+ */
- while ((entry = sq_remlast(&conn->unacked_q)) != NULL)
- {
- wrb = (FAR struct tcp_wrbuffer_s *)entry;
- uint16_t sent;
+ conn->expired++;
- /* Reset the number of bytes sent sent from the write buffer */
+ /* Continue waiting */
- sent = TCP_WBSENT(wrb);
- if (conn->tx_unacked > sent)
- {
- conn->tx_unacked -= sent;
- }
- else
- {
- conn->tx_unacked = 0;
- }
-
- if (conn->sent > sent)
- {
- conn->sent -= sent;
- }
- else
- {
- conn->sent = 0;
- }
+ return flags;
Review comment:
remove
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org