Re: [PATCH net-next 1/2] tcp: add max_quickacks param to tcp_incr_quickack and tcp_enter_quickack_mode

From: Neal Cardwell <ncardwell@google.com>
To: kbuild test robot <lkp@intel.com>
Cc: Eric Dumazet <edumazet@google.com>,
	kbuild-all@01.org, David Miller <davem@davemloft.net>,
	Netdev <netdev@vger.kernel.org>, Van Jacobson <vanj@google.com>,
	Yuchung Cheng <ycheng@google.com>,
	Soheil Hassas Yeganeh <soheil@google.com>,
	Eric Dumazet <eric.dumazet@gmail.com>
Subject: Re: [PATCH net-next 1/2] tcp: add max_quickacks param to tcp_incr_quickack and tcp_enter_quickack_mode
Date: Tue, 22 May 2018 21:37:00 -0400	[thread overview]
Message-ID: <CADVnQym+PaLtrNEdYNpLeajGjpMToddKNw1y15m86y2JEvKHAQ@mail.gmail.com> (raw)
In-Reply-To: <201805230804.KP8LODgK%fengguang.wu@intel.com>

On Tue, May 22, 2018 at 8:31 PM kbuild test robot <lkp@intel.com> wrote:

> Hi Eric,

> Thank you for the patch! Yet something to improve:

> [auto build test ERROR on net/master]
> [also build test ERROR on v4.17-rc6 next-20180517]
> [cannot apply to net-next/master]
> [if your patch is applied to the wrong git tree, please drop us a note to
help improve the system]

> url:
https://github.com/0day-ci/linux/commits/Eric-Dumazet/tcp-add-max_quickacks-param-to-tcp_incr_quickack-and-tcp_enter_quickack_mode/20180523-075103
> config: i386-randconfig-x012-201820 (attached as .config)
> compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
> reproduce:
>          # save the attached .config to linux build tree
>          make ARCH=i386

> All errors (new ones prefixed by >>):

>     net//ipv4/tcp_input.c: In function 'tcp_data_queue':
> >> net//ipv4/tcp_input.c:4656:2: error: too few arguments to function
'tcp_enter_quickack_mode'
>       tcp_enter_quickack_mode(sk);
>       ^~~~~~~~~~~~~~~~~~~~~~~
>     net//ipv4/tcp_input.c:199:13: note: declared here
>      static void tcp_enter_quickack_mode(struct sock *sk, unsigned int
max_quickacks)
>                  ^~~~~~~~~~~~~~~~~~~~~~~
...

For the record, this is an error in the tool, rather than the patch. The
tool seems to be using a stale net-next tree for building this patch.

The compile error is here in line 4656:

> ^1da177e4 Linus Torvalds           2005-04-16  4652     /* Out of window.
F.e. zero window probe. */
> ^1da177e4 Linus Torvalds           2005-04-16  4653     if
(!before(TCP_SKB_CB(skb)->seq, tp->rcv_nxt + tcp_receive_window(tp)))
> ^1da177e4 Linus Torvalds           2005-04-16  4654             goto
out_of_window;
> ^1da177e4 Linus Torvalds           2005-04-16  4655
> 463c84b97 Arnaldo Carvalho de Melo 2005-08-09 @4656
tcp_enter_quickack_mode(sk);
> ^1da177e4 Linus Torvalds           2005-04-16  4657
> ^1da177e4 Linus Torvalds           2005-04-16  4658     if
(before(TCP_SKB_CB(skb)->seq, tp->rcv_nxt)) {
> ^1da177e4 Linus Torvalds           2005-04-16  4659             /*
Partial packet, seq < rcv_next < end_seq */
...

But that line is not in net-next any more, after Eric's recent net-next
commit:

a3893637e1eb0e ("tcp: do not force quickack when receiving out-of-order
packets")

https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git/commit/net/ipv4/tcp_input.c?id=a3893637e1eb0ef5eb1bbc52b3a8d2dfa317a35d

That commit removed that line:

diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 0bf032839548f..f5622b2506651 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -4715,8 +4715,6 @@ drop:
         if (!before(TCP_SKB_CB(skb)->seq, tp->rcv_nxt +
tcp_receive_window(tp)))
                 goto out_of_window;

-       tcp_enter_quickack_mode(sk);
-
         if (before(TCP_SKB_CB(skb)->seq, tp->rcv_nxt)) {
                 /* Partial packet, seq < rcv_next < end_seq */
                 SOCK_DEBUG(sk, "partial packet: rcv_next %X seq %X - %X\n",

cheers,
neal
