From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-io1-xd2d.google.com (mail-io1-xd2d.google.com [IPv6:2607:f8b0:4864:20::d2d]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by lists.bufferbloat.net (Postfix) with ESMTPS id A1E123B29D for ; Thu, 25 Jun 2020 11:38:15 -0400 (EDT) Received: by mail-io1-xd2d.google.com with SMTP id f23so6540036iof.6 for ; Thu, 25 Jun 2020 08:38:15 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :content-transfer-encoding; bh=MNjzrQ1xvuxQPCdCMvWwB/VwqwF+fqsarbuByLIdcxg=; b=JmmfiY2ZhZ4YiF/+rVyXMATSDehMeWctwJXziqv/jou+t5t2sXHmFgaNK2fGpSGxD0 iUYyV51JH04iyO/NpDSSIzLnE2cyBYnKkmBs8aUYDTHPzt3d1YuwnPSJVr9QU4vuCoZX owLbIYtOwiJMMsIBgrav0yJluEyMLvCqiKTjDUqNdswZaY2j0XqP+JCzBMaJdSEF2Yws dHR/p0EAlizryFGIQrfVBwJzGguW9P95ZxOj17tJV4nZ4PWhlCg7FpWXHd9lU2IymgUF kiem5qCqwv0f9bBfp6k5KPWfNZ3ArVJXhjkW0PqWih7ydVnCHvglRk0I2t5fkTrQBHLe /M+A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:content-transfer-encoding; bh=MNjzrQ1xvuxQPCdCMvWwB/VwqwF+fqsarbuByLIdcxg=; b=I80bpf4zRC//q4GfKKW64XqZWcG2FH2W1OxVNu8x+XNPauHkZGjL5uDulnQOMSZF6s ToUTbThPG3PYm0MFc70gJaaikeg73pbmEBuyLFGB2ZV/zm8Hx+oO9Re/1Z5i4XNTAo2q kZEg2afg1C7yeVOqHslqd1NY2I216x3zh6Rkwu0ul2Fgw9LBkMuGkdeqLTNXyjjzir66 FDiJgt12XFCuUneMK4P+JewnVQA0pZNzSz0eAxbiSb1Pn4w97prr9O3IX097tfOyHA5L ctSjgsI51GXphNVxbnvEvi/WHdLY560jSSClb+e/Q92zZHH4sswCK6SHrH4FjyIeTBow xm/g== X-Gm-Message-State: AOAM532oq9lckrAH9mPiRJG8ebaWahxdX62U/TfxW+jDs7nlaO+6PIR0 DyETcMWcHWDwiruA+dWPEXYVaCVj5r7eoPy7D+QP9A== X-Google-Smtp-Source: ABdhPJycQD+Xxr0K/xH5H4IUF3ZEPAvOGg6SZD3NrvetMBjGWWjgZ1MOOi10fMAWAIfJKrzePxUskXnJCgOkPupC9Tw= X-Received: by 2002:a6b:f801:: with SMTP id o1mr33861822ioh.25.1593099494750; Thu, 25 Jun 2020 08:38:14 -0700 (PDT) MIME-Version: 1.0 References: <20200625115106.23370-1-denis.kirjanov@suse.com> In-Reply-To: <20200625115106.23370-1-denis.kirjanov@suse.com> From: Dave Taht Date: Thu, 25 Jun 2020 08:38:02 -0700 Message-ID: To: ECN-Sane Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Subject: [Ecn-sane] Fwd: [PATCH v3] tcp: don't ignore ECN CWR on pure ACK X-BeenThere: ecn-sane@lists.bufferbloat.net X-Mailman-Version: 2.1.20 Precedence: list List-Id: Discussion of explicit congestion notification's impact on the Internet List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 25 Jun 2020 15:38:15 -0000 ---------- Forwarded message --------- From: Denis Kirjanov Date: Thu, Jun 25, 2020 at 4:52 AM Subject: [PATCH v3] tcp: don't ignore ECN CWR on pure ACK To: Cc: , , , , there is a problem with the CWR flag set in an incoming ACK segment and it leads to the situation when the ECE flag is latched forever the following packetdrill script shows what happens: // Stack receives incoming segments with CE set +0.1 <[ect0] . 11001:12001(1000) ack 1001 win 65535 +0.0 <[ce] . 12001:13001(1000) ack 1001 win 65535 +0.0 <[ect0] P. 13001:14001(1000) ack 1001 win 65535 // Stack repsonds with ECN ECHO +0.0 >[noecn] . 1001:1001(0) ack 12001 +0.0 >[noecn] E. 1001:1001(0) ack 13001 +0.0 >[noecn] E. 1001:1001(0) ack 14001 // Write a packet +0.1 write(3, ..., 1000) =3D 1000 +0.0 >[ect0] PE. 1001:2001(1000) ack 14001 // Pure ACK received +0.01 <[noecn] W. 14001:14001(0) ack 2001 win 65535 // Since CWR was sent, this packet should NOT have ECE set +0.1 write(3, ..., 1000) =3D 1000 +0.0 >[ect0] P. 2001:3001(1000) ack 14001 // but Linux will still keep ECE latched here, with packetdrill // flagging a missing ECE flag, expecting // >[ect0] PE. 2001:3001(1000) ack 14001 // in the script In the situation above we will continue to send ECN ECHO packets and trigger the peer to reduce the congestion window. To avoid that we can check CWR on pure ACKs received. v3: - Add a sequence check to avoid sending an ACK to an ACK v2: - Adjusted the comment - move CWR check before checking for unacknowledged packets Signed-off-by: Denis Kirjanov --- net/ipv4/tcp_input.c | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c index 12fda8f27b08..f3a0eb139b76 100644 --- a/net/ipv4/tcp_input.c +++ b/net/ipv4/tcp_input.c @@ -261,7 +261,8 @@ static void tcp_ecn_accept_cwr(struct sock *sk, const struct sk_buff *skb) * cwnd may be very low (even just 1 packet), so we should = ACK * immediately. */ - inet_csk(sk)->icsk_ack.pending |=3D ICSK_ACK_NOW; + if (TCP_SKB_CB(skb)->seq !=3D TCP_SKB_CB(skb)->end_seq) + inet_csk(sk)->icsk_ack.pending |=3D ICSK_ACK_NOW; } } @@ -3665,6 +3666,15 @@ static int tcp_ack(struct sock *sk, const struct sk_buff *skb, int flag) tcp_in_ack_event(sk, ack_ev_flags); } + /* This is a deviation from RFC3168 since it states that: + * "When the TCP data sender is ready to set the CWR bit after redu= cing + * the congestion window, it SHOULD set the CWR bit only on the fir= st + * new data packet that it transmits." + * We accept CWR on pure ACKs to be more robust + * with widely-deployed TCP implementations that do this. + */ + tcp_ecn_accept_cwr(sk, skb); + /* We passed data and got it acked, remove any soft error * log. Something worked... */ @@ -4800,8 +4810,6 @@ static void tcp_data_queue(struct sock *sk, struct sk_buff *skb) skb_dst_drop(skb); __skb_pull(skb, tcp_hdr(skb)->doff * 4); - tcp_ecn_accept_cwr(sk, skb); - tp->rx_opt.dsack =3D 0; /* Queue data for delivery to the user. -- 2.27.0 --=20 "For a successful technology, reality must take precedence over public relations, for Mother Nature cannot be fooled" - Richard Feynman dave@taht.net CTO, TekLibre, LLC Tel: 1-831-435-0729