svn commit: r362586 - in stable/12/sys/netinet: . tcp_stacks

[Richard Scheffenegger]

Author: rscheff
Date: Wed Jun 24 16:17:58 2020
New Revision: 362586
URL: https://svnweb.freebsd.org/changeset/base/362586

Log:
  MFC r361347: 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.
  
  PR:		243590
  Reviewed by:	rgrimes (mentor), rrs
  Approved by:	rgrimes (mentor, blanket)
  MFC after:	3 weeks
  Sponsored by:	NetApp, Inc.
  Differential Revision:	https://reviews.freebsd.org/D23364

Modified:
  stable/12/sys/netinet/tcp_input.c
  stable/12/sys/netinet/tcp_output.c
  stable/12/sys/netinet/tcp_stacks/rack.c
Directory Properties:
  stable/12/   (props changed)

Modified: stable/12/sys/netinet/tcp_input.c
==============================================================================
--- stable/12/sys/netinet/tcp_input.c	Wed Jun 24 15:46:33 2020	(r362585)
+++ stable/12/sys/netinet/tcp_input.c	Wed Jun 24 16:17:58 2020	(r362586)
@@ -417,9 +417,15 @@ cc_cong_signal(struct tcpcb *tp, struct tcphdr *th, ui
 		}
 		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_flags & TF_ECN_PERMIT)
 				tp->t_flags |= TF_ECN_SND_CWR;
 		}

Modified: stable/12/sys/netinet/tcp_output.c
==============================================================================
--- stable/12/sys/netinet/tcp_output.c	Wed Jun 24 15:46:33 2020	(r362585)
+++ stable/12/sys/netinet/tcp_output.c	Wed Jun 24 16:17:58 2020	(r362586)
@@ -1132,7 +1132,8 @@ send:
 		 * Ignore pure ack packets, retransmissions and window probes.
 		 */
 		if (len > 0 && SEQ_GEQ(tp->snd_nxt, tp->snd_max) &&
-		    !((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);
@@ -1140,15 +1141,15 @@ send:
 #endif
 				ip->ip_tos |= IPTOS_ECN_ECT0;
 			TCPSTAT_INC(tcps_ecn_ect0);
+			/*
+			 * Reply with proper ECN notifications.
+			 * Only set CWR on new data segments.
+			 */
+			if (tp->t_flags & TF_ECN_SND_CWR) {
+				flags |= TH_CWR;
+				tp->t_flags &= ~TF_ECN_SND_CWR;
+			}
 		}
-		
-		/*
-		 * Reply with proper ECN notifications.
-		 */
-		if (tp->t_flags & TF_ECN_SND_CWR) {
-			flags |= TH_CWR;
-			tp->t_flags &= ~TF_ECN_SND_CWR;
-		} 
 		if (tp->t_flags & TF_ECN_SND_ECE)
 			flags |= TH_ECE;
 	}

Modified: stable/12/sys/netinet/tcp_stacks/rack.c
==============================================================================
--- stable/12/sys/netinet/tcp_stacks/rack.c	Wed Jun 24 15:46:33 2020	(r362585)
+++ stable/12/sys/netinet/tcp_stacks/rack.c	Wed Jun 24 16:17:58 2020	(r362586)
@@ -1415,9 +1415,15 @@ rack_cong_signal(struct tcpcb *tp, struct tcphdr *th, 
 		}
 		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_flags & TF_ECN_PERMIT)
 				tp->t_flags |= TF_ECN_SND_CWR;
 		}
@@ -8283,13 +8289,14 @@ send:
 #endif
 				ip->ip_tos |= IPTOS_ECN_ECT0;
 			TCPSTAT_INC(tcps_ecn_ect0);
-		}
-		/*
-		 * Reply with proper ECN notifications.
-		 */
-		if (tp->t_flags & TF_ECN_SND_CWR) {
-			flags |= TH_CWR;
-			tp->t_flags &= ~TF_ECN_SND_CWR;
+			/*
+			 * Reply with proper ECN notifications.
+			 * Only set CWR on new data segments.
+			 */
+			if (tp->t_flags & TF_ECN_SND_CWR) {
+				flags |= TH_CWR;
+				tp->t_flags &= ~TF_ECN_SND_CWR;
+			}
 		}
 		if (tp->t_flags & TF_ECN_SND_ECE)
 			flags |= TH_ECE;