Date: Thu, 09 Jul 2015 14:16:18 -0700 (PDT)
Message-Id: <20150709.141618.71467123946708533.davem@davemloft.net>
To: jmaxwell37@gmail.com
Cc: kuznet@ms2.inr.ac.ru, jmorris@namei.org, yoshfuji@linux-ipv6.org,
        kaber@trash.net, edumazet@google.com, ncardwell@google.com,
        ycheng@google.com, netdev@vger.kernel.org,
        linux-kernel@vger.kernel.org, jmaxwell@redhat.com
Subject: Re: [PATCH net-next] tcp: v1 always send a quick ack when
 quickacks are enabled
From: David Miller <davem@davemloft.net>
In-Reply-To: <1436314348-25577-1-git-send-email-jmaxwell37@gmail.com>
References: <1436314348-25577-1-git-send-email-jmaxwell37@gmail.com>
Mime-Version: 1.0
Content-Type: Text/Plain; charset=us-ascii
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 2316
Lines: 46

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.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/