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 2020/11/27 02:39:38 UTC
[GitHub] [incubator-nuttx] anchao opened a new pull request #2414: net/tcp: implement the fast retransmit
anchao opened a new pull request #2414:
URL: https://github.com/apache/incubator-nuttx/pull/2414
## Summary
net/tcp: implement the fast retransmit
https://tools.ietf.org/html/rfc2001#section-3
```
RFC2001: TCP Slow Start, Congestion Avoidance, Fast Retransmit,
and Fast Recovery Algorithms
...
3. Fast Retransmit
Modifications to the congestion avoidance algorithm were proposed in
1990 [3]. Before describing the change, realize that TCP may
generate an immediate acknowledgment (a duplicate ACK) when an out-
of-order segment is received (Section 4.2.2.21 of [1], with a note
that one reason for doing so was for the experimental fast-
retransmit algorithm). This duplicate ACK should not be delayed.
The purpose of this duplicate ACK is to let the other end know that a
segment was received out of order, and to tell it what sequence
number is expected.
Since TCP does not know whether a duplicate ACK is caused by a lost
segment or just a reordering of segments, it waits for a small number
of duplicate ACKs to be received. It is assumed that if there is
just a reordering of the segments, there will be only one or two
duplicate ACKs before the reordered segment is processed, which will
then generate a new ACK. If three or more duplicate ACKs are
received in a row, it is a strong indication that a segment has been
lost. TCP then performs a retransmission of what appears to be the
missing segment, without waiting for a retransmission timer to
expire.
```
Change-Id: Ie2cbcecab507c3d831f74390a6a85e0c5c8e0652
Signed-off-by: chao.an <an...@xiaomi.com>
## Impact
TCP retransmit
## Testing
iperf test
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] acassis commented on pull request #2414: net/tcp: implement the fast retransmit
Posted by GitBox <gi...@apache.org>.
acassis commented on pull request #2414:
URL: https://github.com/apache/incubator-nuttx/pull/2414#issuecomment-735229396
@davids5 I don't know if @antmerlino is available, maybe @masayuki2009 could help us on it.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] yamt commented on a change in pull request #2414: net/tcp: implement the fast retransmit
Posted by GitBox <gi...@apache.org>.
yamt commented on a change in pull request #2414:
URL: https://github.com/apache/incubator-nuttx/pull/2414#discussion_r673617283
##########
File path: net/tcp/tcp_send_buffered.c
##########
@@ -484,6 +485,26 @@ static uint16_t psock_send_eventhandler(FAR struct net_driver_s *dev,
wrb, TCP_WBSEQNO(wrb), TCP_WBPKTLEN(wrb));
}
}
+ else if (ackno == TCP_WBSEQNO(wrb))
+ {
+ /* Duplicate ACK? Retransmit data if need */
+
+ if (++TCP_WBNACK(wrb) ==
+ CONFIG_NET_TCP_FAST_RETRANSMIT_WATERMARK)
+ {
+ /* Do fast retransmit */
+
+ rexmit = true;
+ }
+ else if ((TCP_WBNACK(wrb) >
+ CONFIG_NET_TCP_FAST_RETRANSMIT_WATERMARK) &&
+ TCP_WBNACK(wrb) == sq_count(&conn->unacked_q) - 1)
Review comment:
what does "TCP_WBNACK(wrb) == sq_count(&conn->unacked_q) - 1" mean?
--
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
[GitHub] [incubator-nuttx] xiaoxiang781216 merged pull request #2414: net/tcp: implement the fast retransmit
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 merged pull request #2414:
URL: https://github.com/apache/incubator-nuttx/pull/2414
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] yamt commented on a change in pull request #2414: net/tcp: implement the fast retransmit
Posted by GitBox <gi...@apache.org>.
yamt commented on a change in pull request #2414:
URL: https://github.com/apache/incubator-nuttx/pull/2414#discussion_r671039119
##########
File path: net/tcp/tcp_send_buffered.c
##########
@@ -484,6 +485,26 @@ static uint16_t psock_send_eventhandler(FAR struct net_driver_s *dev,
wrb, TCP_WBSEQNO(wrb), TCP_WBPKTLEN(wrb));
}
}
+ else if (ackno == TCP_WBSEQNO(wrb))
+ {
+ /* Duplicate ACK? Retransmit data if need */
+
+ if (++TCP_WBNACK(wrb) ==
+ CONFIG_NET_TCP_FAST_RETRANSMIT_WATERMARK)
+ {
+ /* Do fast retransmit */
+
+ rexmit = true;
+ }
+ else if ((TCP_WBNACK(wrb) >
+ CONFIG_NET_TCP_FAST_RETRANSMIT_WATERMARK) &&
+ TCP_WBNACK(wrb) == sq_count(&conn->unacked_q) - 1)
Review comment:
@anchao can you explain the intention of this condition?
--
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
[GitHub] [incubator-nuttx] davids5 commented on pull request #2414: net/tcp: implement the fast retransmit
Posted by GitBox <gi...@apache.org>.
davids5 commented on pull request #2414:
URL: https://github.com/apache/incubator-nuttx/pull/2414#issuecomment-734873449
@antmerlino - would you please have a look?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] anchao commented on a change in pull request #2414: net/tcp: implement the fast retransmit
Posted by GitBox <gi...@apache.org>.
anchao commented on a change in pull request #2414:
URL: https://github.com/apache/incubator-nuttx/pull/2414#discussion_r673055270
##########
File path: net/tcp/tcp_send_buffered.c
##########
@@ -484,6 +485,26 @@ static uint16_t psock_send_eventhandler(FAR struct net_driver_s *dev,
wrb, TCP_WBSEQNO(wrb), TCP_WBPKTLEN(wrb));
}
}
+ else if (ackno == TCP_WBSEQNO(wrb))
+ {
+ /* Duplicate ACK? Retransmit data if need */
+
+ if (++TCP_WBNACK(wrb) ==
+ CONFIG_NET_TCP_FAST_RETRANSMIT_WATERMARK)
+ {
+ /* Do fast retransmit */
+
+ rexmit = true;
+ }
+ else if ((TCP_WBNACK(wrb) >
+ CONFIG_NET_TCP_FAST_RETRANSMIT_WATERMARK) &&
+ TCP_WBNACK(wrb) == sq_count(&conn->unacked_q) - 1)
Review comment:
In some cases, retransmission packets may be lost again. Here we reset the retransmission count and prepare to resend
--
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
[GitHub] [incubator-nuttx] anchao commented on a change in pull request #2414: net/tcp: implement the fast retransmit
Posted by GitBox <gi...@apache.org>.
anchao commented on a change in pull request #2414:
URL: https://github.com/apache/incubator-nuttx/pull/2414#discussion_r674524054
##########
File path: net/tcp/tcp_send_buffered.c
##########
@@ -484,6 +485,26 @@ static uint16_t psock_send_eventhandler(FAR struct net_driver_s *dev,
wrb, TCP_WBSEQNO(wrb), TCP_WBPKTLEN(wrb));
}
}
+ else if (ackno == TCP_WBSEQNO(wrb))
+ {
+ /* Duplicate ACK? Retransmit data if need */
+
+ if (++TCP_WBNACK(wrb) ==
+ CONFIG_NET_TCP_FAST_RETRANSMIT_WATERMARK)
+ {
+ /* Do fast retransmit */
+
+ rexmit = true;
+ }
+ else if ((TCP_WBNACK(wrb) >
+ CONFIG_NET_TCP_FAST_RETRANSMIT_WATERMARK) &&
+ TCP_WBNACK(wrb) == sq_count(&conn->unacked_q) - 1)
Review comment:
If the received dupack count is equal to unacked_q. That means that the ack of all packets after this packet loss has been received, so the counting of DUP ack will be restarted.
--
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
[GitHub] [incubator-nuttx] yamt commented on a change in pull request #2414: net/tcp: implement the fast retransmit
Posted by GitBox <gi...@apache.org>.
yamt commented on a change in pull request #2414:
URL: https://github.com/apache/incubator-nuttx/pull/2414#discussion_r676424780
##########
File path: net/tcp/tcp_send_buffered.c
##########
@@ -484,6 +485,26 @@ static uint16_t psock_send_eventhandler(FAR struct net_driver_s *dev,
wrb, TCP_WBSEQNO(wrb), TCP_WBPKTLEN(wrb));
}
}
+ else if (ackno == TCP_WBSEQNO(wrb))
+ {
+ /* Duplicate ACK? Retransmit data if need */
+
+ if (++TCP_WBNACK(wrb) ==
+ CONFIG_NET_TCP_FAST_RETRANSMIT_WATERMARK)
+ {
+ /* Do fast retransmit */
+
+ rexmit = true;
+ }
+ else if ((TCP_WBNACK(wrb) >
+ CONFIG_NET_TCP_FAST_RETRANSMIT_WATERMARK) &&
+ TCP_WBNACK(wrb) == sq_count(&conn->unacked_q) - 1)
Review comment:
in that case, we have likely already started fast retransmit.
* do you mean the segment fast-retransmitted has lost again? such a case is simpler to leave to the usual "slow retransmit".
* this implementation of fast retransmit can retransmit multiple segments. (usually fast retransmit only retransmit one segment. but to me this implementation doesn't seem to have such a mechanism.) so the calculation here doesn't seem to make much sense to me.
--
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
[GitHub] [incubator-nuttx] anchao commented on a change in pull request #2414: net/tcp: implement the fast retransmit
Posted by GitBox <gi...@apache.org>.
anchao commented on a change in pull request #2414:
URL: https://github.com/apache/incubator-nuttx/pull/2414#discussion_r533384983
##########
File path: net/tcp/tcp_send_buffered.c
##########
@@ -484,6 +485,28 @@ static uint16_t psock_send_eventhandler(FAR struct net_driver_s *dev,
wrb, TCP_WBSEQNO(wrb), TCP_WBPKTLEN(wrb));
}
}
+ else if (ackno == TCP_WBSEQNO(wrb))
+ {
+ /* Duplicate ACK? Retransmit data if need */
+
+ TCP_WBNACK(wrb)++;
+
+ if (TCP_WBNACK(wrb) ==
Review comment:
Done
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #2414: net/tcp: implement the fast retransmit
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #2414:
URL: https://github.com/apache/incubator-nuttx/pull/2414#discussion_r532593041
##########
File path: net/tcp/tcp_send_buffered.c
##########
@@ -484,6 +485,28 @@ static uint16_t psock_send_eventhandler(FAR struct net_driver_s *dev,
wrb, TCP_WBSEQNO(wrb), TCP_WBPKTLEN(wrb));
}
}
+ else if (ackno == TCP_WBSEQNO(wrb))
+ {
+ /* Duplicate ACK? Retransmit data if need */
+
+ TCP_WBNACK(wrb)++;
+
+ if (TCP_WBNACK(wrb) ==
Review comment:
merge line 492 to here
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org