On Wed, Oct 11, 2017 at 6:02 AM, Abdul Haleem
<abdhalee@xxxxxxxxxxxxxxxxxx> 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@xxxxxxxxxx>
> 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@xxxxxxxxxx>
> Signed-off-by: Neal Cardwell <ncardwell@xxxxxxxxxx>
> Signed-off-by: Yuchung Cheng <ycheng@xxxxxxxxxx>
> Signed-off-by: Eric Dumazet <edumazet@xxxxxxxxxx>
> Signed-off-by: David S. Miller <davem@xxxxxxxxxxxxx>
>
> 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@xxxxxxxxxx>
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@xxxxxxxxxx>
---
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
--
To unsubscribe from this list: send the line "unsubscribe linux-next" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
