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

2018-05-22 Thread Neal Cardwell
On Tue, May 22, 2018 at 8:31 PM kbuild test robot  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


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

2018-05-22 Thread kbuild test robot
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)
^~~

vim +/tcp_enter_quickack_mode +4656 net//ipv4/tcp_input.c

292e8d8c8 Pavel Emelyanov  2012-05-10  4577  
^1da177e4 Linus Torvalds   2005-04-16  4578  static void 
tcp_data_queue(struct sock *sk, struct sk_buff *skb)
^1da177e4 Linus Torvalds   2005-04-16  4579  {
^1da177e4 Linus Torvalds   2005-04-16  4580 struct tcp_sock *tp = 
tcp_sk(sk);
5357f0bd4 Eric Dumazet 2017-08-01  4581 bool fragstolen;
5357f0bd4 Eric Dumazet 2017-08-01  4582 int eaten;
^1da177e4 Linus Torvalds   2005-04-16  4583  
532182cd6 Eric Dumazet 2016-04-01  4584 if 
(TCP_SKB_CB(skb)->seq == TCP_SKB_CB(skb)->end_seq) {
532182cd6 Eric Dumazet 2016-04-01  4585 
__kfree_skb(skb);
532182cd6 Eric Dumazet 2016-04-01  4586 return;
532182cd6 Eric Dumazet 2016-04-01  4587 }
f84af32cb Eric Dumazet 2010-04-28  4588 skb_dst_drop(skb);
155c6e1ad Peter Pan(潘卫平 2014-09-24  4589)   __skb_pull(skb, 
tcp_hdr(skb)->doff * 4);
^1da177e4 Linus Torvalds   2005-04-16  4590  
735d38311 Florian Westphal 2014-09-29  4591 tcp_ecn_accept_cwr(tp, 
skb);
^1da177e4 Linus Torvalds   2005-04-16  4592  
^1da177e4 Linus Torvalds   2005-04-16  4593 tp->rx_opt.dsack = 0;
^1da177e4 Linus Torvalds   2005-04-16  4594  
^1da177e4 Linus Torvalds   2005-04-16  4595 /*  Queue data for 
delivery to the user.
^1da177e4 Linus Torvalds   2005-04-16  4596  *  Packets in sequence 
go to the receive queue.
^1da177e4 Linus Torvalds   2005-04-16  4597  *  Out of sequence 
packets to the out_of_order_queue.
^1da177e4 Linus Torvalds   2005-04-16  4598  */
^1da177e4 Linus Torvalds   2005-04-16  4599 if 
(TCP_SKB_CB(skb)->seq == tp->rcv_nxt) {
^1da177e4 Linus Torvalds   2005-04-16  4600 if 
(tcp_receive_window(tp) == 0)
^1da177e4 Linus Torvalds   2005-04-16  4601 goto 
out_of_window;
^1da177e4 Linus Torvalds   2005-04-16  4602  
^1da177e4 Linus Torvalds   2005-04-16  4603 /* Ok. In 
sequence. In window. */
^1da177e4 Linus Torvalds   2005-04-16  4604  queue_and_out:
76dfa6082 Eric Dumazet 2015-05-15  4605 if 
(skb_queue_len(&sk->sk_receive_queue) == 0)
76dfa6082 Eric Dumazet 2015-05-15  4606 
sk_forced_mem_schedule(sk, skb->truesize);
76dfa6082 Eric Dumazet 2015-05-15  4607 else if 
(tcp_try_rmem_schedule(sk, skb, skb->truesize))
^1da177e4 Linus Torvalds   2005-04-16  4608 goto 
drop;
5357f0bd4 Eric Dumazet 2017-08-01  4609  
b081f85c2 Eric Dumazet 2012-05-02  4610 eaten = 
tcp_queue_rcv(sk, skb, 0, &fragstolen);
bdd1f9eda Eric Dumazet 2015-04-28  4611 
tcp_rcv_nxt_update(tp, TCP_SKB_CB(skb)->end_seq);
^1da177e4 Linus Torvalds   2005-04-16  4612 if (skb->len)
9e412ba76 Ilpo Järvinen2007-04-20  4613 
tcp_event_data_recv(sk, skb);
155c6e1ad Peter Pan(潘卫平 2014-09-24  4614)   if 
(TCP_SKB_CB(skb)->tcp_flags & TCPHDR_FIN)
20c4cb792 Eric Dumazet 2011-10-20  4615 
tcp_fin(sk);
^1da177e4 Linus Torvalds   2005-04-16  4616  
9f5afeae5 Yaogong Wang 2016-09-07  4617 if 
(!RB_EMPTY_ROOT(&tp->out_of_order_queue)) {
^1da177e4 Linus Torvalds   2005-04-16  4618 
tcp_ofo_queue(sk);
^1da177e4 Linus Torvalds   2005-04-16  4619  
^1da177e4 Linus Torvalds   2005-04-16  4620 /* 
RFC2581. 4.2. SHOULD send immediate ACK, when
^1da177e4 Linus Torvalds   2005-04-16  4621  * gap 
in queue is filled.
^1da177e4 Linus To

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

2018-05-22 Thread Neal Cardwell
On Mon, May 21, 2018 at 6:09 PM Eric Dumazet  wrote:

> We want to add finer control of the number of ACK packets sent after
> ECN events.

> This patch is not changing current behavior, it only enables following
> change.

> Signed-off-by: Eric Dumazet 
> ---

Acked-by: Neal Cardwell 

Thanks!

neal


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

2018-05-22 Thread Soheil Hassas Yeganeh
On Mon, May 21, 2018 at 6:08 PM, Eric Dumazet  wrote:
> We want to add finer control of the number of ACK packets sent after
> ECN events.
>
> This patch is not changing current behavior, it only enables following
> change.
>
> Signed-off-by: Eric Dumazet 

Acked-by: Soheil Hassas Yeganeh 

> ---
>  net/ipv4/tcp_input.c | 24 +---
>  1 file changed, 13 insertions(+), 11 deletions(-)
>
> diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
> index 
> aebb29ab2fdf2ceaa182cd11928f145a886149ff..2e970e9f4e09d966b703af2d14d521a4328eba7e
>  100644
> --- a/net/ipv4/tcp_input.c
> +++ b/net/ipv4/tcp_input.c
> @@ -203,21 +203,23 @@ static void tcp_measure_rcv_mss(struct sock *sk, const 
> struct sk_buff *skb)
> }
>  }
>
> -static void tcp_incr_quickack(struct sock *sk)
> +static void tcp_incr_quickack(struct sock *sk, unsigned int max_quickacks)
>  {
> struct inet_connection_sock *icsk = inet_csk(sk);
> unsigned int quickacks = tcp_sk(sk)->rcv_wnd / (2 * 
> icsk->icsk_ack.rcv_mss);
>
> if (quickacks == 0)
> quickacks = 2;
> +   quickacks = min(quickacks, max_quickacks);
> if (quickacks > icsk->icsk_ack.quick)
> -   icsk->icsk_ack.quick = min(quickacks, TCP_MAX_QUICKACKS);
> +   icsk->icsk_ack.quick = quickacks;
>  }
>
> -static void tcp_enter_quickack_mode(struct sock *sk)
> +static void tcp_enter_quickack_mode(struct sock *sk, unsigned int 
> max_quickacks)
>  {
> struct inet_connection_sock *icsk = inet_csk(sk);
> -   tcp_incr_quickack(sk);
> +
> +   tcp_incr_quickack(sk, max_quickacks);
> icsk->icsk_ack.pingpong = 0;
> icsk->icsk_ack.ato = TCP_ATO_MIN;
>  }
> @@ -261,7 +263,7 @@ static void __tcp_ecn_check_ce(struct tcp_sock *tp, const 
> struct sk_buff *skb)
>  * it is probably a retransmit.
>  */
> if (tp->ecn_flags & TCP_ECN_SEEN)
> -   tcp_enter_quickack_mode((struct sock *)tp);
> +   tcp_enter_quickack_mode((struct sock *)tp, 
> TCP_MAX_QUICKACKS);
> break;
> case INET_ECN_CE:
> if (tcp_ca_needs_ecn((struct sock *)tp))
> @@ -269,7 +271,7 @@ static void __tcp_ecn_check_ce(struct tcp_sock *tp, const 
> struct sk_buff *skb)
>
> if (!(tp->ecn_flags & TCP_ECN_DEMAND_CWR)) {
> /* Better not delay acks, sender can have a very low 
> cwnd */
> -   tcp_enter_quickack_mode((struct sock *)tp);
> +   tcp_enter_quickack_mode((struct sock *)tp, 
> TCP_MAX_QUICKACKS);
> tp->ecn_flags |= TCP_ECN_DEMAND_CWR;
> }
> tp->ecn_flags |= TCP_ECN_SEEN;
> @@ -686,7 +688,7 @@ static void tcp_event_data_recv(struct sock *sk, struct 
> sk_buff *skb)
> /* The _first_ data packet received, initialize
>  * delayed ACK engine.
>  */
> -   tcp_incr_quickack(sk);
> +   tcp_incr_quickack(sk, TCP_MAX_QUICKACKS);
> icsk->icsk_ack.ato = TCP_ATO_MIN;
> } else {
> int m = now - icsk->icsk_ack.lrcvtime;
> @@ -702,7 +704,7 @@ static void tcp_event_data_recv(struct sock *sk, struct 
> sk_buff *skb)
> /* Too long gap. Apparently sender failed to
>  * restart window, so that we send ACKs quickly.
>  */
> -   tcp_incr_quickack(sk);
> +   tcp_incr_quickack(sk, TCP_MAX_QUICKACKS);
> sk_mem_reclaim(sk);
> }
> }
> @@ -4179,7 +4181,7 @@ static void tcp_send_dupack(struct sock *sk, const 
> struct sk_buff *skb)
> if (TCP_SKB_CB(skb)->end_seq != TCP_SKB_CB(skb)->seq &&
> before(TCP_SKB_CB(skb)->seq, tp->rcv_nxt)) {
> NET_INC_STATS(sock_net(sk), LINUX_MIB_DELAYEDACKLOST);
> -   tcp_enter_quickack_mode(sk);
> +   tcp_enter_quickack_mode(sk, TCP_MAX_QUICKACKS);
>
> if (tcp_is_sack(tp) && sock_net(sk)->ipv4.sysctl_tcp_dsack) {
> u32 end_seq = TCP_SKB_CB(skb)->end_seq;
> @@ -4706,7 +4708,7 @@ static void tcp_data_queue(struct sock *sk, struct 
> sk_buff *skb)
> tcp_dsack_set(sk, TCP_SKB_CB(skb)->seq, 
> TCP_SKB_CB(skb)->end_seq);
>
>  out_of_window:
> -   tcp_enter_quickack_mode(sk);
> +   tcp_enter_quickack_mode(sk, TCP_MAX_QUICKACKS);
> inet_csk_schedule_ack(sk);
>  drop:
> tcp_drop(sk, skb);
> @@ -5790,7 +5792,7 @@ static int tcp_rcv_synsent_state_process(struct sock 
> *sk, struct sk_buff *skb,
>  * to stand against the temptation 8) --ANK
>  */
> inet_csk_schedule_ack(sk);
> -   tcp_enter_quickack_mode(sk);
> +   tcp_

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

2018-05-21 Thread Eric Dumazet
We want to add finer control of the number of ACK packets sent after
ECN events.

This patch is not changing current behavior, it only enables following
change.

Signed-off-by: Eric Dumazet 
---
 net/ipv4/tcp_input.c | 24 +---
 1 file changed, 13 insertions(+), 11 deletions(-)

diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 
aebb29ab2fdf2ceaa182cd11928f145a886149ff..2e970e9f4e09d966b703af2d14d521a4328eba7e
 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -203,21 +203,23 @@ static void tcp_measure_rcv_mss(struct sock *sk, const 
struct sk_buff *skb)
}
 }
 
-static void tcp_incr_quickack(struct sock *sk)
+static void tcp_incr_quickack(struct sock *sk, unsigned int max_quickacks)
 {
struct inet_connection_sock *icsk = inet_csk(sk);
unsigned int quickacks = tcp_sk(sk)->rcv_wnd / (2 * 
icsk->icsk_ack.rcv_mss);
 
if (quickacks == 0)
quickacks = 2;
+   quickacks = min(quickacks, max_quickacks);
if (quickacks > icsk->icsk_ack.quick)
-   icsk->icsk_ack.quick = min(quickacks, TCP_MAX_QUICKACKS);
+   icsk->icsk_ack.quick = quickacks;
 }
 
-static void tcp_enter_quickack_mode(struct sock *sk)
+static void tcp_enter_quickack_mode(struct sock *sk, unsigned int 
max_quickacks)
 {
struct inet_connection_sock *icsk = inet_csk(sk);
-   tcp_incr_quickack(sk);
+
+   tcp_incr_quickack(sk, max_quickacks);
icsk->icsk_ack.pingpong = 0;
icsk->icsk_ack.ato = TCP_ATO_MIN;
 }
@@ -261,7 +263,7 @@ static void __tcp_ecn_check_ce(struct tcp_sock *tp, const 
struct sk_buff *skb)
 * it is probably a retransmit.
 */
if (tp->ecn_flags & TCP_ECN_SEEN)
-   tcp_enter_quickack_mode((struct sock *)tp);
+   tcp_enter_quickack_mode((struct sock *)tp, 
TCP_MAX_QUICKACKS);
break;
case INET_ECN_CE:
if (tcp_ca_needs_ecn((struct sock *)tp))
@@ -269,7 +271,7 @@ static void __tcp_ecn_check_ce(struct tcp_sock *tp, const 
struct sk_buff *skb)
 
if (!(tp->ecn_flags & TCP_ECN_DEMAND_CWR)) {
/* Better not delay acks, sender can have a very low 
cwnd */
-   tcp_enter_quickack_mode((struct sock *)tp);
+   tcp_enter_quickack_mode((struct sock *)tp, 
TCP_MAX_QUICKACKS);
tp->ecn_flags |= TCP_ECN_DEMAND_CWR;
}
tp->ecn_flags |= TCP_ECN_SEEN;
@@ -686,7 +688,7 @@ static void tcp_event_data_recv(struct sock *sk, struct 
sk_buff *skb)
/* The _first_ data packet received, initialize
 * delayed ACK engine.
 */
-   tcp_incr_quickack(sk);
+   tcp_incr_quickack(sk, TCP_MAX_QUICKACKS);
icsk->icsk_ack.ato = TCP_ATO_MIN;
} else {
int m = now - icsk->icsk_ack.lrcvtime;
@@ -702,7 +704,7 @@ static void tcp_event_data_recv(struct sock *sk, struct 
sk_buff *skb)
/* Too long gap. Apparently sender failed to
 * restart window, so that we send ACKs quickly.
 */
-   tcp_incr_quickack(sk);
+   tcp_incr_quickack(sk, TCP_MAX_QUICKACKS);
sk_mem_reclaim(sk);
}
}
@@ -4179,7 +4181,7 @@ static void tcp_send_dupack(struct sock *sk, const struct 
sk_buff *skb)
if (TCP_SKB_CB(skb)->end_seq != TCP_SKB_CB(skb)->seq &&
before(TCP_SKB_CB(skb)->seq, tp->rcv_nxt)) {
NET_INC_STATS(sock_net(sk), LINUX_MIB_DELAYEDACKLOST);
-   tcp_enter_quickack_mode(sk);
+   tcp_enter_quickack_mode(sk, TCP_MAX_QUICKACKS);
 
if (tcp_is_sack(tp) && sock_net(sk)->ipv4.sysctl_tcp_dsack) {
u32 end_seq = TCP_SKB_CB(skb)->end_seq;
@@ -4706,7 +4708,7 @@ static void tcp_data_queue(struct sock *sk, struct 
sk_buff *skb)
tcp_dsack_set(sk, TCP_SKB_CB(skb)->seq, 
TCP_SKB_CB(skb)->end_seq);
 
 out_of_window:
-   tcp_enter_quickack_mode(sk);
+   tcp_enter_quickack_mode(sk, TCP_MAX_QUICKACKS);
inet_csk_schedule_ack(sk);
 drop:
tcp_drop(sk, skb);
@@ -5790,7 +5792,7 @@ static int tcp_rcv_synsent_state_process(struct sock *sk, 
struct sk_buff *skb,
 * to stand against the temptation 8) --ANK
 */
inet_csk_schedule_ack(sk);
-   tcp_enter_quickack_mode(sk);
+   tcp_enter_quickack_mode(sk, TCP_MAX_QUICKACKS);
inet_csk_reset_xmit_timer(sk, ICSK_TIME_DACK,
  TCP_DELACK_MAX, TCP_RTO_MAX);
 
-- 
2.17.0.441.gb46fe60e1d-goog