Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 31ba0c10 authored by Yuchung Cheng's avatar Yuchung Cheng Committed by David S. Miller
Browse files

tcp: move cwnd reduction after recovery state procesing 



Currently the cwnd is reduced and increased in various different
places. The reduction happens in various places in the recovery
state processing (tcp_fastretrans_alert) while the increase
happens afterward.

A better sequence is to identify lost packets and update
the congestion control state (icsk_ca_state) first. Then base
on the new state, up/down the cwnd in one central place. It's
more clear to reason cwnd changes.

Signed-off-by: default avatarYuchung Cheng <ycheng@google.com>
Signed-off-by: default avatarNeal Cardwell <ncardwell@google.com>
Signed-off-by: default avatarEric Dumazet <ncardwell@google.com>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent e662ca40

net/ipv4/tcp_input.c

+28 −32
Original line number Original line Diff line number Diff line
@@ -2471,14 +2471,12 @@ static void tcp_init_cwnd_reduction(struct sock *sk) 
	tcp_ecn_queue_cwr(tp);

	tcp_ecn_queue_cwr(tp);

}

}





static void tcp_cwnd_reduction(struct sock *sk, const int prior_unsacked,

static void tcp_cwnd_reduction(struct sock *sk, int newly_acked_sacked,

			       int fast_rexmit, int flag)

			       int flag)

{

{

	struct tcp_sock *tp = tcp_sk(sk);

	struct tcp_sock *tp = tcp_sk(sk);

	int sndcnt = 0;

	int sndcnt = 0;

	int delta = tp->snd_ssthresh - tcp_packets_in_flight(tp);

	int delta = tp->snd_ssthresh - tcp_packets_in_flight(tp);

	int newly_acked_sacked = prior_unsacked -

				 (tp->packets_out - tp->sacked_out);





	if (newly_acked_sacked <= 0 || WARN_ON_ONCE(!tp->prior_cwnd))

	if (newly_acked_sacked <= 0 || WARN_ON_ONCE(!tp->prior_cwnd))

		return;

		return;
@@ -2496,7 +2494,8 @@ static void tcp_cwnd_reduction(struct sock *sk, const int prior_unsacked, 
	} else {

	} else {

		sndcnt = min(delta, newly_acked_sacked);

		sndcnt = min(delta, newly_acked_sacked);

	}

	}

	sndcnt = max(sndcnt, (fast_rexmit ? 1 : 0));

	/* Force a fast retransmit upon entering fast recovery */

	sndcnt = max(sndcnt, (tp->prr_out ? 0 : 1));

	tp->snd_cwnd = tcp_packets_in_flight(tp) + sndcnt;

	tp->snd_cwnd = tcp_packets_in_flight(tp) + sndcnt;

}

}
@@ -2541,7 +2540,7 @@ static void tcp_try_keep_open(struct sock *sk) 
	}

	}

}

}





static void tcp_try_to_open(struct sock *sk, int flag, const int prior_unsacked)

static void tcp_try_to_open(struct sock *sk, int flag)

{

{

	struct tcp_sock *tp = tcp_sk(sk);

	struct tcp_sock *tp = tcp_sk(sk);
@@ -2555,8 +2554,6 @@ static void tcp_try_to_open(struct sock *sk, int flag, const int prior_unsacked) 




	if (inet_csk(sk)->icsk_ca_state != TCP_CA_CWR) {

	if (inet_csk(sk)->icsk_ca_state != TCP_CA_CWR) {

		tcp_try_keep_open(sk);

		tcp_try_keep_open(sk);

	} else {

		tcp_cwnd_reduction(sk, prior_unsacked, 0, flag);

	}

	}

}

}
@@ -2720,8 +2717,7 @@ static void tcp_process_loss(struct sock *sk, int flag, bool is_dupack, 
}

}





/* Undo during fast recovery after partial ACK. */

/* Undo during fast recovery after partial ACK. */

static bool tcp_try_undo_partial(struct sock *sk, const int acked,

static bool tcp_try_undo_partial(struct sock *sk, const int acked)

				 const int prior_unsacked, int flag)

{

{

	struct tcp_sock *tp = tcp_sk(sk);

	struct tcp_sock *tp = tcp_sk(sk);
@@ -2736,10 +2732,8 @@ static bool tcp_try_undo_partial(struct sock *sk, const int acked, 
		 * can undo. Otherwise we clock out new packets but do not

		 * can undo. Otherwise we clock out new packets but do not

		 * mark more packets lost or retransmit more.

		 * mark more packets lost or retransmit more.

		 */

		 */

		if (tp->retrans_out) {

		if (tp->retrans_out)

			tcp_cwnd_reduction(sk, prior_unsacked, 0, flag);

			return true;

			return true;

		}





		if (!tcp_any_retrans_done(sk))

		if (!tcp_any_retrans_done(sk))

			tp->retrans_stamp = 0;

			tp->retrans_stamp = 0;
@@ -2758,21 +2752,21 @@ static bool tcp_try_undo_partial(struct sock *sk, const int acked, 
 * taking into account both packets sitting in receiver's buffer and

 * taking into account both packets sitting in receiver's buffer and

 * packets lost by network.

 * packets lost by network.

 *

 *

 * Besides that it does CWND reduction, when packet loss is detected

 * Besides that it updates the congestion state when packet loss or ECN

 * and changes state of machine.

 * is detected. But it does not reduce the cwnd, it is done by the

 * congestion control later.

 *

 *

 * It does _not_ decide what to send, it is made in function

 * It does _not_ decide what to send, it is made in function

 * tcp_xmit_retransmit_queue().

 * tcp_xmit_retransmit_queue().

 */

 */

static void tcp_fastretrans_alert(struct sock *sk, const int acked,

static void tcp_fastretrans_alert(struct sock *sk, const int acked,

				  const int prior_unsacked,

				  bool is_dupack, int *ack_flag, int *rexmit)

				  bool is_dupack, int flag, int *rexmit)

{

{

	struct inet_connection_sock *icsk = inet_csk(sk);

	struct inet_connection_sock *icsk = inet_csk(sk);

	struct tcp_sock *tp = tcp_sk(sk);

	struct tcp_sock *tp = tcp_sk(sk);

	int fast_rexmit = 0, flag = *ack_flag;

	bool do_lost = is_dupack || ((flag & FLAG_DATA_SACKED) &&

	bool do_lost = is_dupack || ((flag & FLAG_DATA_SACKED) &&

				    (tcp_fackets_out(tp) > tp->reordering));

				    (tcp_fackets_out(tp) > tp->reordering));

	int fast_rexmit = 0;





	if (WARN_ON(!tp->packets_out && tp->sacked_out))

	if (WARN_ON(!tp->packets_out && tp->sacked_out))

		tp->sacked_out = 0;

		tp->sacked_out = 0;
@@ -2819,8 +2813,10 @@ static void tcp_fastretrans_alert(struct sock *sk, const int acked, 




	/* Use RACK to detect loss */

	/* Use RACK to detect loss */

	if (sysctl_tcp_recovery & TCP_RACK_LOST_RETRANS &&

	if (sysctl_tcp_recovery & TCP_RACK_LOST_RETRANS &&

	    tcp_rack_mark_lost(sk))

	    tcp_rack_mark_lost(sk)) {

		flag |= FLAG_LOST_RETRANS;

		flag |= FLAG_LOST_RETRANS;

		*ack_flag |= FLAG_LOST_RETRANS;

	}





	/* E. Process state. */

	/* E. Process state. */

	switch (icsk->icsk_ca_state) {

	switch (icsk->icsk_ca_state) {
@@ -2829,7 +2825,7 @@ static void tcp_fastretrans_alert(struct sock *sk, const int acked, 
			if (tcp_is_reno(tp) && is_dupack)

			if (tcp_is_reno(tp) && is_dupack)

				tcp_add_reno_sack(sk);

				tcp_add_reno_sack(sk);

		} else {

		} else {

			if (tcp_try_undo_partial(sk, acked, prior_unsacked, flag))

			if (tcp_try_undo_partial(sk, acked))

				return;

				return;

			/* Partial ACK arrived. Force fast retransmit. */

			/* Partial ACK arrived. Force fast retransmit. */

			do_lost = tcp_is_reno(tp) ||

			do_lost = tcp_is_reno(tp) ||
@@ -2858,7 +2854,7 @@ static void tcp_fastretrans_alert(struct sock *sk, const int acked, 
			tcp_try_undo_dsack(sk);

			tcp_try_undo_dsack(sk);





		if (!tcp_time_to_recover(sk, flag)) {

		if (!tcp_time_to_recover(sk, flag)) {

			tcp_try_to_open(sk, flag, prior_unsacked);

			tcp_try_to_open(sk, flag);

			return;

			return;

		}

		}
@@ -2880,7 +2876,6 @@ static void tcp_fastretrans_alert(struct sock *sk, const int acked, 




	if (do_lost)

	if (do_lost)

		tcp_update_scoreboard(sk, fast_rexmit);

		tcp_update_scoreboard(sk, fast_rexmit);

	tcp_cwnd_reduction(sk, prior_unsacked, fast_rexmit, flag);

	*rexmit = REXMIT_LOST;

	*rexmit = REXMIT_LOST;

}

}
@@ -3306,9 +3301,6 @@ static inline bool tcp_ack_is_dubious(const struct sock *sk, const int flag) 
/* Decide wheather to run the increase function of congestion control. */

/* Decide wheather to run the increase function of congestion control. */

static inline bool tcp_may_raise_cwnd(const struct sock *sk, const int flag)

static inline bool tcp_may_raise_cwnd(const struct sock *sk, const int flag)

{

{

	if (tcp_in_cwnd_reduction(sk))

		return false;



	/* If reordering is high then always grow cwnd whenever data is

	/* If reordering is high then always grow cwnd whenever data is

	 * delivered regardless of its ordering. Otherwise stay conservative

	 * delivered regardless of its ordering. Otherwise stay conservative

	 * and only grow cwnd on in-order delivery (RFC5681). A stretched ACK w/

	 * and only grow cwnd on in-order delivery (RFC5681). A stretched ACK w/
@@ -3551,6 +3543,7 @@ static int tcp_ack(struct sock *sk, const struct sk_buff *skb, int flag) 
	int prior_packets = tp->packets_out;

	int prior_packets = tp->packets_out;

	const int prior_unsacked = tp->packets_out - tp->sacked_out;

	const int prior_unsacked = tp->packets_out - tp->sacked_out;

	int acked = 0; /* Number of packets newly acked */

	int acked = 0; /* Number of packets newly acked */

	int acked_sacked; /* Number of packets newly acked or sacked */

	int rexmit = REXMIT_NONE; /* Flag to (re)transmit to recover losses */

	int rexmit = REXMIT_NONE; /* Flag to (re)transmit to recover losses */





	sack_state.first_sackt.v64 = 0;

	sack_state.first_sackt.v64 = 0;
@@ -3647,15 +3640,20 @@ static int tcp_ack(struct sock *sk, const struct sk_buff *skb, int flag) 




	if (tcp_ack_is_dubious(sk, flag)) {

	if (tcp_ack_is_dubious(sk, flag)) {

		is_dupack = !(flag & (FLAG_SND_UNA_ADVANCED | FLAG_NOT_DUP));

		is_dupack = !(flag & (FLAG_SND_UNA_ADVANCED | FLAG_NOT_DUP));

		tcp_fastretrans_alert(sk, acked, prior_unsacked,

		tcp_fastretrans_alert(sk, acked, is_dupack, &flag, &rexmit);

				      is_dupack, flag, &rexmit);

	}

	}

	if (tp->tlp_high_seq)

	if (tp->tlp_high_seq)

		tcp_process_tlp_ack(sk, ack, flag);

		tcp_process_tlp_ack(sk, ack, flag);





	acked_sacked = prior_unsacked - (tp->packets_out - tp->sacked_out);

	/* Advance cwnd if state allows */

	if (tcp_in_cwnd_reduction(sk)) {

		/* Reduce cwnd if state mandates */

		tcp_cwnd_reduction(sk, acked_sacked, flag);

	} else if (tcp_may_raise_cwnd(sk, flag)) {

		/* Advance cwnd if state allows */

		/* Advance cwnd if state allows */

	if (tcp_may_raise_cwnd(sk, flag))

		tcp_cong_avoid(sk, ack, acked);

		tcp_cong_avoid(sk, ack, acked);

	}





	if ((flag & FLAG_FORWARD_PROGRESS) || !(flag & FLAG_NOT_DUP)) {

	if ((flag & FLAG_FORWARD_PROGRESS) || !(flag & FLAG_NOT_DUP)) {

		struct dst_entry *dst = __sk_dst_get(sk);

		struct dst_entry *dst = __sk_dst_get(sk);
@@ -3672,8 +3670,7 @@ static int tcp_ack(struct sock *sk, const struct sk_buff *skb, int flag) 
no_queue:

no_queue:

	/* If data was DSACKed, see if we can undo a cwnd reduction. */

	/* If data was DSACKed, see if we can undo a cwnd reduction. */

	if (flag & FLAG_DSACKING_ACK)

	if (flag & FLAG_DSACKING_ACK)

		tcp_fastretrans_alert(sk, acked, prior_unsacked,

		tcp_fastretrans_alert(sk, acked, is_dupack, &flag, &rexmit);

				      is_dupack, flag, &rexmit);

	/* If this ack opens up a zero window, clear backoff.  It was

	/* If this ack opens up a zero window, clear backoff.  It was

	 * being used to time the probes, and is probably far higher than

	 * being used to time the probes, and is probably far higher than

	 * it needs to be for normal retransmission.

	 * it needs to be for normal retransmission.
@@ -3696,8 +3693,7 @@ static int tcp_ack(struct sock *sk, const struct sk_buff *skb, int flag) 
	if (TCP_SKB_CB(skb)->sacked) {

	if (TCP_SKB_CB(skb)->sacked) {

		flag |= tcp_sacktag_write_queue(sk, skb, prior_snd_una,

		flag |= tcp_sacktag_write_queue(sk, skb, prior_snd_una,

						&sack_state);

						&sack_state);

		tcp_fastretrans_alert(sk, acked, prior_unsacked,

		tcp_fastretrans_alert(sk, acked, is_dupack, &flag, &rexmit);

				      is_dupack, flag, &rexmit);

		tcp_xmit_recovery(sk, rexmit);

		tcp_xmit_recovery(sk, rexmit);

	}

	}