lkml.org 
[lkml]   [2017]   [Oct]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
From
Date
SubjectRe: [linux-next] [6f5b24e] WARNING: CPU: 0 PID: 10288 at net/ipv4/tcp_input.c:889 tcp_update_reordering+0x1c4/0x1e0
On Wed, Oct 11, 2017 at 6:02 AM, Abdul Haleem
<abdhalee@linux.vnet.ibm.com> wrote:
> Hi,
>
> CPU OFF & ON in a loop on next kernel results in WARNING in dmesg.
>
> Machine Type: Power 8 PowerVM LPAR
> Kernel : 4.14.0-rc4-next-20171009
> Gcc : 6.3.1
> config : attached
> Test: CPU toggle
>
> WARN_ON was first introduced with the commit:
>
> commit 6f5b24eed0278136c29c27f2a7b3a2b6a202ac68
> Author: Soheil Hassas Yeganeh <soheil@google.com>
> Date: Tue May 16 17:39:02 2017 -0400
>
> tcp: warn on negative reordering values
>
> Commit bafbb9c73241 ("tcp: eliminate negative reordering
> in tcp_clean_rtx_queue") fixes an issue for negative
> reordering metrics.
>
> To be resilient to such errors, warn and return
> when a negative metric is passed to tcp_update_reordering().
>
> Signed-off-by: Soheil Hassas Yeganeh <soheil@google.com>
> Signed-off-by: Neal Cardwell <ncardwell@google.com>
> Signed-off-by: Yuchung Cheng <ycheng@google.com>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Signed-off-by: David S. Miller <davem@davemloft.net>
>
> diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
> index bbadd79..2fa55f5 100644
> --- a/net/ipv4/tcp_input.c
> +++ b/net/ipv4/tcp_input.c
> @@ -887,6 +887,9 @@ static void tcp_update_reordering(struct sock *sk,
> const int metric,
> struct tcp_sock *tp = tcp_sk(sk);
> int mib_idx;
>
> + if (WARN_ON_ONCE(metric < 0))
> + return;
> +

Thank you for your report, Abdul! This warning was added to find the
root cause of insanely high reordering in TCP, which luckily I believe
you have reproduced.

Yuchung had pointed out offline that we are using inconsistent types
in using in tcp_sacktag_state vs tcp_sock: int for reord and
fack_count in tcp_sacktag_state and u32 for their related fields in
tcp_sock. This is likely to be the culprit here.

Would you mind giving the following patch a try please? If that fixes
the issue for you, we will mail that as a fix. Thanks!

From: Soheil Hassas Yeganeh <soheil@google.com>
Date: Wed, 11 Oct 2017 09:52:49 -0400
Subject: [PATCH net] tcp: fix type of fack_count and reord in sack_tag_state

This bug is found by Yuchung.

Signed-off-by: Soheil Hassas Yeganeh <soheil@google.com>
---
net/ipv4/tcp_input.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index d0682ce2a5d6..2fada7acbdc5 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -1132,8 +1132,8 @@ static bool tcp_check_dsack(struct sock *sk,
const struct sk_buff *ack_skb,
}

struct tcp_sacktag_state {
- int reord;
- int fack_count;
+ u32 reord;
+ u32 fack_count;
/* Timestamps for earliest and latest never-retransmitted segment
* that was SACKed. RTO needs the earliest RTT to stay conservative,
* but congestion control should still get an accurate delay signal.
@@ -1209,7 +1209,7 @@ static u8 tcp_sacktag_one(struct sock *sk,
u64 xmit_time)
{
struct tcp_sock *tp = tcp_sk(sk);
- int fack_count = state->fack_count;
+ u32 fack_count = state->fack_count;

/* Account D-SACK for retransmitted packet. */
if (dup_sack && (sacked & TCPCB_RETRANS)) {
--
2.15.0.rc0.271.g36b669edcc-goog
\
 
 \ /
  Last update: 2017-10-11 16:10    [W:1.087 / U:0.132 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site