Re: [PATCH net-next] tcp: v1 always send a quick ack when quickacks are enabled

[David Miller <davem@davemloft.net>]

From: Jon Maxwell <jmaxwell37@gmail.com>
Date: Wed,  8 Jul 2015 10:12:28 +1000

> V1 of this patch contains Eric Dumazet's suggestion to move the per
> dst RTAX_QUICKACK check into tcp_in_quickack_mode(). Thanks Eric.
> 
> I ran some tests and after setting the "ip route change quickack 1"
> knob there were still many delayed ACKs sent. This occured
> because when icsk_ack.quick=0 the !icsk_ack.pingpong value is
> subsequently ignored as tcp_in_quickack_mode() checks both these
> values. The condition for a quick ack to trigger requires
> that both icsk_ack.quick != 0 and icsk_ack.pingpong=0. Currently
> only icsk_ack.pingpong is controlled by the knob. But the
> icsk_ack.quick value changes dynamically depending on heuristics.
> The crux of the matter is that delayed acks still cannot be entirely
> disabled even with the RTAX_QUICKACK per dst knob enabled. This
> patch ensures that a quick ack is always sent when the RTAX_QUICKACK
> per dst knob is turned on.
> 
> The "ip route change quickack 1" knob was recently added to enable
> quickacks. It was modeled around the TCP_QUICKACK setsockopt() option.
> This issue is that even with "ip route change quickack 1" enabled
> we still see delayed ACKs under some conditions. It would be nice
> to be able to completely disable delayed ACKs.
 ...
> The issue is that both the "ip route change quickack 1" knob and
> the TCP_QUICKACK option set the icsk_ack.pingpong variable to 0.
> However at the business end in the __tcp_ack_snd_check() routine,
> tcp_in_quickack_mode() checks that both icsk_ack.quick != 0
> and icsk_ack.pingpong=0 in order to trigger a quickack. As
> icsk_ack.quick is determined by heuristics it can be 0. When
> that occurs the icsk_ack.pingpong value is ignored and a delayed
> ACK is sent regardless.
> 
> This patch moves the RTAX_QUICKACK per dst check into the
> tcp_in_quickack_mode() routine which ensures that a quickack is
> always sent when the quickack knob is enabled for that dst.
> 
> Signed-off-by: Jon Maxwell <jmaxwell37@gmail.com>

Looks good, applied, thanks Jon.