Subject: Re: simplify microsecond rtt sampling

From: John Heffner <[email-masked]>
Date: Wed, 27 Sep 2006 16:13:29 +0100

Okay, this patch is junk (never trust compile-tested code).  Will send 
something better soon.

   -John

John Heffner wrote:
> About commit 2d2abbab63f6726a147ae61ada39bf2c9ee0db9a:
> 
> It looks like this patch bypassed the enforcement of Karn's algorithm in 
> tcp_ack_no_tstamp() for the purposes of usec RTT sampling used by 
> congestion control modules.  This will give them bad RTT data when there 
> are retransmits.  I haven't actually observed this, but it seems like it 
> would be the case. ;)  Please correct me if I'm wrong.
> 
> Here's a patch that should be a fix.
> 
> 
> 
> Signed-off-by: John Heffner <[email-masked]>
> 
> 
> ------------------------------------------------------------------------
> 
> diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
> index 159fa3f..725c868 100644
> --- a/net/ipv4/tcp_input.c
> +++ b/net/ipv4/tcp_input.c
> @@ -2306,8 +2306,6 @@ static int tcp_clean_rtx_queue(struct so
>  				seq_rtt = -1;
>  			} else if (seq_rtt < 0) {
>  				seq_rtt = now - scb->when;
> -				if (rtt_sample)
> -					(*rtt_sample)(sk, tcp_usrtt(skb));
>  			}
>  			if (sacked & TCPCB_SACKED_ACKED)
>  				tp->sacked_out -= tcp_skb_pcount(skb);
> @@ -2320,8 +2318,6 @@ static int tcp_clean_rtx_queue(struct so
>  			}
>  		} else if (seq_rtt < 0) {
>  			seq_rtt = now - scb->when;
> -			if (rtt_sample)
> -				(*rtt_sample)(sk, tcp_usrtt(skb));
>  		}
>  		tcp_dec_pcount_approx(&tp->fackets_out, skb);
>  		tcp_packets_out_dec(tp, skb);
> @@ -2333,6 +2329,8 @@ static int tcp_clean_rtx_queue(struct so
>  	if (acked&FLAG_ACKED) {
>  		tcp_ack_update_rtt(sk, acked, seq_rtt);
>  		tcp_ack_packets_out(sk, tp);
> +		if (rtt_sample && !(acked & FLAG_RETRANS_DATA_ACKED))
> +			(*rtt_sample)(sk, tcp_usrtt(skb));
>  
>  		if (icsk->icsk_ca_ops->pkts_acked)
>  			icsk->icsk_ca_ops->pkts_acked(sk, pkts_acked);