> On 08 Dec 2015, at 14:47, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> 
> On Tue, 2015-12-08 at 10:19 +0100, Per Hurtig wrote:
> 
>> +static u32 tcp_unsent_pkts(const struct sock *sk, u32 ulimit)
>> +{
>> +	struct sk_buff *skb = tcp_send_head(sk);
>> +	u32 pkts = 0;
>> +
>> +	if (skb)
>> +		tcp_for_write_queue_from(skb, sk) {
>> +			pkts += tcp_skb_pcount(skb);
>> +
>> +			if (ulimit && pkts >= ulimit)
>> +				return ulimit;
>> +		}
>> +
>> +	return pkts;
>> +}
> 
> 
> Considering Yuchung feedback, have you looked at using an approximation
> instead ?
> 
> (ie using tp->write_seq - tp->snd_nxt)
> 
> 

Well, an approximation is rather “dangerous” as missing a single packet
could inhibit the desired behaviour. If looping is undesired, I think a
better solution is to actually *not* do this check at all and instead rely
solely on the

tp->packets_out < TCP_RTORESTART_THRESH

check instead. The reason why the number of unsent packets was
included was only to fix a corner case where it should be possible
to use the modified restart, but impossible due to the conditioning.

However, this corner case is likely to not occur very often and we
may be better off with the simpler check.

The corner case (if I remember this correctly) is that the restart is
not triggered when you have 2 segments in flight and (i) have a
congestion window of exactly 3; or (ii) get a packet written to the socket
just between previous data transmission and the arrival of the
acknowledgment that triggers the restart.


— Per