aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorRichard Scheffenegger <rscheff@FreeBSD.org>2020-05-21 21:33:15 +0000
committerRichard Scheffenegger <rscheff@FreeBSD.org>2020-05-21 21:33:15 +0000
commitaf2fb894c90b874b565bba7049205d0a283ac140 (patch)
tree1e58f7a9a4cef77d0fc1b4def107584851310395
parent8e0511652b1023d14eb506b47e7bc983014d9a94 (diff)
downloadsrc-af2fb894c90b874b565bba7049205d0a283ac140.tar.gz
src-af2fb894c90b874b565bba7049205d0a283ac140.zip
With RFC3168 ECN, CWR SHOULD only be sent with new data
Overly conservative data receivers may ignore the CWR flag on other packets, and keep ECE latched. This can result in continous reduction of the congestion window, and very poor performance when ECN is enabled. Reviewed by: rgrimes (mentor), rrs Approved by: rgrimes (mentor), tuexen (mentor) MFC after: 3 days Sponsored by: NetApp, Inc. Differential Revision: https://reviews.freebsd.org/D23364
Notes
Notes: svn path=/head/; revision=361347
-rw-r--r--sys/netinet/tcp_input.c10
-rw-r--r--sys/netinet/tcp_output.c19
-rw-r--r--sys/netinet/tcp_stacks/rack.c25
3 files changed, 34 insertions, 20 deletions
diff --git a/sys/netinet/tcp_input.c b/sys/netinet/tcp_input.c
index 83c812bd579f..068f6b7e178a 100644
--- a/sys/netinet/tcp_input.c
+++ b/sys/netinet/tcp_input.c
@@ -447,9 +447,15 @@ cc_cong_signal(struct tcpcb *tp, struct tcphdr *th, uint32_t type)
}
break;
case CC_ECN:
- if (!IN_CONGRECOVERY(tp->t_flags)) {
+ if (!IN_CONGRECOVERY(tp->t_flags) ||
+ /*
+ * Allow ECN reaction on ACK to CWR, if
+ * that data segment was also CE marked.
+ */
+ SEQ_GEQ(th->th_ack, tp->snd_recover)) {
+ EXIT_CONGRECOVERY(tp->t_flags);
TCPSTAT_INC(tcps_ecn_rcwnd);
- tp->snd_recover = tp->snd_max;
+ tp->snd_recover = tp->snd_max + 1;
if (tp->t_flags2 & TF2_ECN_PERMIT)
tp->t_flags2 |= TF2_ECN_SND_CWR;
}
diff --git a/sys/netinet/tcp_output.c b/sys/netinet/tcp_output.c
index 7cb86a4d1f4e..a1e7117e186f 100644
--- a/sys/netinet/tcp_output.c
+++ b/sys/netinet/tcp_output.c
@@ -1170,7 +1170,8 @@ send:
*/
if (len > 0 && SEQ_GEQ(tp->snd_nxt, tp->snd_max) &&
(sack_rxmit == 0) &&
- !((tp->t_flags & TF_FORCEDATA) && len == 1)) {
+ !((tp->t_flags & TF_FORCEDATA) && len == 1 &&
+ SEQ_LT(tp->snd_una, tp->snd_max))) {
#ifdef INET6
if (isipv6)
ip6->ip6_flow |= htonl(IPTOS_ECN_ECT0 << 20);
@@ -1178,14 +1179,14 @@ send:
#endif
ip->ip_tos |= IPTOS_ECN_ECT0;
TCPSTAT_INC(tcps_ecn_ect0);
- }
-
- /*
- * Reply with proper ECN notifications.
- */
- if (tp->t_flags2 & TF2_ECN_SND_CWR) {
- flags |= TH_CWR;
- tp->t_flags2 &= ~TF2_ECN_SND_CWR;
+ /*
+ * Reply with proper ECN notifications.
+ * Only set CWR on new data segments.
+ */
+ if (tp->t_flags2 & TF2_ECN_SND_CWR) {
+ flags |= TH_CWR;
+ tp->t_flags2 &= ~TF2_ECN_SND_CWR;
+ }
}
if (tp->t_flags2 & TF2_ECN_SND_ECE)
flags |= TH_ECE;
diff --git a/sys/netinet/tcp_stacks/rack.c b/sys/netinet/tcp_stacks/rack.c
index 5103ead115db..afd74c623705 100644
--- a/sys/netinet/tcp_stacks/rack.c
+++ b/sys/netinet/tcp_stacks/rack.c
@@ -4095,9 +4095,15 @@ rack_cong_signal(struct tcpcb *tp, struct tcphdr *th, uint32_t type)
}
break;
case CC_ECN:
- if (!IN_CONGRECOVERY(tp->t_flags)) {
+ if (!IN_CONGRECOVERY(tp->t_flags) ||
+ /*
+ * Allow ECN reaction on ACK to CWR, if
+ * that data segment was also CE marked.
+ */
+ SEQ_GEQ(th->th_ack, tp->snd_recover)) {
+ EXIT_CONGRECOVERY(tp->t_flags);
KMOD_TCPSTAT_INC(tcps_ecn_rcwnd);
- tp->snd_recover = tp->snd_max;
+ tp->snd_recover = tp->snd_max + 1;
if (tp->t_flags2 & TF2_ECN_PERMIT)
tp->t_flags2 |= TF2_ECN_SND_CWR;
}
@@ -13556,13 +13562,14 @@ send:
#endif
ip->ip_tos |= IPTOS_ECN_ECT0;
KMOD_TCPSTAT_INC(tcps_ecn_ect0);
- }
- /*
- * Reply with proper ECN notifications.
- */
- if (tp->t_flags2 & TF2_ECN_SND_CWR) {
- flags |= TH_CWR;
- tp->t_flags2 &= ~TF2_ECN_SND_CWR;
+ /*
+ * Reply with proper ECN notifications.
+ * Only set CWR on new data segments.
+ */
+ if (tp->t_flags2 & TF2_ECN_SND_CWR) {
+ flags |= TH_CWR;
+ tp->t_flags2 &= ~TF2_ECN_SND_CWR;
+ }
}
if (tp->t_flags2 & TF2_ECN_SND_ECE)
flags |= TH_ECE;