netdev - [RFC PATCH] [TCP]: Move clearing of the prior

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 1 Mar 2007 19:55:41 +0200 (EET)
From:	"Ilpo Järvinen" <ilpo.jarvinen@...sinki.fi>
To:	netdev@...r.kernel.org
cc:	David Miller <davem@...emloft.net>
Subject: [RFC PATCH] [TCP]: Move clearing of the prior_ssthresh due to ECE
 earlier

Previously the prior_ssthresh was only used in the undo functions
that were called by fastretrans_alert. FRTO processing, however,
happens before that and one of the responses is doing undo too.

I think that after this patch, FRTO should be ECN-safe; only the
undo_cwr response is a suspect anyway. The possible cases:

    1)  Pre-RTO ECE: FRTO is not setting prior_ssthresh in
        CA_CWR at all, therefore it correctly remains cleared
        and ssthresh is the already reduced one

    2a) Post-RTO ECE, before deciding ACK: prior_ssthresh is
        cleared, so no undoing for ssthresh will occur

    2b) Post-RTO ECE, deciding ACK: this patch changes this case
        equal to 2a

    3)  Between two RTOs: prior_ssthresh is cleared like in 2a
        or the reduced value is used like in 1

Deciding ACK is the one after which FRTO declared RTO as
spurious.

Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@...sinki.fi>
---

This problem could be solved locally in the undo_cwr response of
FRTO by falling back to ratehalving response when FLAG_ECE is
enabled. If it's more preferrable way, just ping me to get a
patch. I'm not absolutely sure that this does not break anything
even though I couldn't find any using prior_ssthresh between
the new and the old place (seen more than once already within
years that things are not always as simple as they seem).


 net/ipv4/tcp_input.c |   19 +++++++++----------
 1 files changed, 9 insertions(+), 10 deletions(-)

diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index dc221a3..b0c9dfb 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -2080,15 +2080,11 @@ tcp_fastretrans_alert(struct sock *sk, u
 		tp->fackets_out = 0;
 
 	/* Now state machine starts.
-	 * A. ECE, hence prohibit cwnd undoing, the reduction is required. */
-	if (flag&FLAG_ECE)
-		tp->prior_ssthresh = 0;
-
-	/* B. In all the states check for reneging SACKs. */
+	 * A. In all the states check for reneging SACKs. */
 	if (tp->sacked_out && tcp_check_sack_reneging(sk))
 		return;
 
-	/* C. Process data loss notification, provided it is valid. */
+	/* B. Process data loss notification, provided it is valid. */
 	if ((flag&FLAG_DATA_LOST) &&
 	    before(tp->snd_una, tp->high_seq) &&
 	    icsk->icsk_ca_state != TCP_CA_Open &&
@@ -2097,10 +2093,10 @@ tcp_fastretrans_alert(struct sock *sk, u
 		NET_INC_STATS_BH(LINUX_MIB_TCPLOSS);
 	}
 
-	/* D. Synchronize left_out to current state. */
+	/* C. Synchronize left_out to current state. */
 	tcp_sync_left_out(tp);
 
-	/* E. Check state exit conditions. State can be terminated
+	/* D. Check state exit conditions. State can be terminated
 	 *    when high_seq is ACKed. */
 	if (icsk->icsk_ca_state == TCP_CA_Open) {
 		BUG_TRAP(tp->retrans_out == 0);
@@ -2143,7 +2139,7 @@ tcp_fastretrans_alert(struct sock *sk, u
 		}
 	}
 
-	/* F. Process state. */
+	/* E. Process state. */
 	switch (icsk->icsk_ca_state) {
 	case TCP_CA_Recovery:
 		if (prior_snd_una == tp->snd_una) {
@@ -2742,8 +2738,11 @@ static int tcp_ack(struct sock *sk, stru
 		if (TCP_SKB_CB(skb)->sacked)
 			flag |= tcp_sacktag_write_queue(sk, skb, prior_snd_una);
 
-		if (TCP_ECN_rcv_ecn_echo(tp, skb->h.th))
+		if (TCP_ECN_rcv_ecn_echo(tp, skb->h.th)) {
 			flag |= FLAG_ECE;
+			/* ECE, prohibit cwnd undoing, reduction is required */
+			tp->prior_ssthresh = 0;
+		}
 
 		tcp_ca_event(sk, CA_EVENT_SLOW_ACK);
 	}
-- 
1.4.2