Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <160765171921.6905.7897898635812579754.stgit@localhost.localdomain>
 <CANn89iJ5HnJYv6eWb1jm6rK173DFkp2GRnfvi9vnYwXZPzE4LQ@mail.gmail.com> <CAKgT0Uf_q=FgMHd9_wq5Bx8rCC-kS0Qz563rE9dL2hpQ6Evppg@mail.gmail.com>
In-Reply-To: <CAKgT0Uf_q=FgMHd9_wq5Bx8rCC-kS0Qz563rE9dL2hpQ6Evppg@mail.gmail.com>
From:   Eric Dumazet <edumazet@google.com>
Date:   Fri, 11 Dec 2020 17:22:26 +0100
Message-ID: <CANn89iJUT6aWm75ZpU_Ggmuqbb+cbLSGj0Bxysu9_wXRgNS8MQ@mail.gmail.com>
Subject: Re: [net PATCH] tcp: Mark fastopen SYN packet as lost when receiving ICMP_TOOBIG/ICMP_FRAG_NEEDED
To:     Alexander Duyck <alexander.duyck@gmail.com>
Cc:     Yuchung Cheng <ycheng@google.com>,
        David Miller <davem@davemloft.net>,
        Jakub Kicinski <kuba@kernel.org>,
        Alexey Kuznetsov <kuznet@ms2.inr.ac.ru>,
        Hideaki YOSHIFUJI <yoshfuji@linux-ipv6.org>,
        netdev <netdev@vger.kernel.org>,
        LKML <linux-kernel@vger.kernel.org>,
        Martin KaFai Lau <kafai@fb.com>,
        kernel-team <kernel-team@fb.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Fri, Dec 11, 2020 at 5:03 PM Alexander Duyck
<alexander.duyck@gmail.com> wrote:

> That's fine. I can target this for net-next. I had just selected net
> since I had considered it a fix, but I suppose it could be considered
> a behavioral change.

We are very late in the 5.10 cycle, and we never handled ICMP in this
state, so net-next is definitely better.

Note that RFC 7413 states in 4.1.3 :

 The client MUST cache cookies from servers for later Fast Open
   connections.  For a multihomed client, the cookies are dependent on
   the client and server IP addresses.  Hence, the client should cache
   at most one (most recently received) cookie per client and server IP
   address pair.

   When caching cookies, we recommend that the client also cache the
   Maximum Segment Size (MSS) advertised by the server.  The client can
   cache the MSS advertised by the server in order to determine the
   maximum amount of data that the client can fit in the SYN packet in
   subsequent TFO connections.  Caching the server MSS is useful
   because, with Fast Open, a client sends data in the SYN packet before
   the server announces its MSS in the SYN-ACK packet.  If the client
   sends more data in the SYN packet than the server will accept, this
   will likely require the client to retransmit some or all of the data.
   Hence, caching the server MSS can enhance performance.

   Without a cached server MSS, the amount of data in the SYN packet is
   limited to the default MSS of 536 bytes for IPv4 [RFC1122] and 1220
   bytes for IPv6 [RFC2460].  Even if the client complies with this
   limit when sending the SYN, it is known that an IPv4 receiver
   advertising an MSS less than 536 bytes can receive a segment larger
   than it is expecting.

   If the cached MSS is larger than the typical size (1460 bytes for
   IPv4 or 1440 bytes for IPv6), then the excess data in the SYN packet
   may cause problems that offset the performance benefit of Fast Open.
   For example, the unusually large SYN may trigger IP fragmentation and
   may confuse firewalls or middleboxes, causing SYN retransmission and
   other side effects.  Therefore, the client MAY limit the cached MSS
   to 1460 bytes for IPv4 or 1440 for IPv6.


Relying on ICMP is fragile, since they can be filtered in some way.