Message-ID: <CANn89iKB4GFd8sVzCbRttqw_96o3i2wDhX-3DraQtsceNGYwug@mail.gmail.com>
Date: Mon, 19 Aug 2024 13:59:40 +0200
From: Eric Dumazet <edumazet@...gle.com>
To: Jakub Sitnicki <jakub@...udflare.com>
Cc: netdev@...r.kernel.org, kernel-team@...udflare.com
Subject: Re: [PATCH RFC net-next] tcp: Allow TIME-WAIT reuse after 1 millisecond

On Mon, Aug 19, 2024 at 1:31 PM Jakub Sitnicki <jakub@...udflare.com> wrote:
>
> [This patch needs a description. Please see the RFC cover letter below.]
>
> Signed-off-by: Jakub Sitnicki <jakub@...udflare.com>
> ---
> Can we shorten the TCP connection reincarnation period?
>
> Situation
> =========
>
> Currently, we can reuse a TCP 4-tuple (source IP + port, destination IP + port)
> in the TIME-WAIT state to establish a new outgoing TCP connection after a period
> of 1 second. This period, during which the 4-tuple remains blocked from reuse,
> is determined by the granularity of the ts_recent_stamp / tw_ts_recent_stamp
> timestamp, which presently uses a 1 Hz clock (ktime_get_seconds).
>
> The TIME-WAIT block is enforced by __{inet,inet6}_check_established ->
> tcp_twsk_unique, where we check if the timestamp clock has ticked since the last
> ts_recent_stamp update before allowing the 4-tuple to be reused.
>
> This mechanism, introduced in 2002 by commit b8439924316d ("Allow to bind to an
> already in use local port during connect") [1], protects the TCP receiver
> against segments from an earlier incarnation of the same connection (FIN
> retransmits), which could potentially corrupt the TCP stream, as described by
> RFC 7323 [2, 3].
>
> Problem
> =======
>
> The one-second reincarnation period has not posed a problem when we had a
> sufficiently large pool of ephemeral ports (tens of thousands per host).


We now have network namespaces, and still ~30,000 ephemeral ports per netns :)

> However, as we began sharing egress IPv4 addresses between hosts by partitioning
> the available port range [4], the ephemeral port pool size has shrunk
> significantly—down to hundreds of ports per host.
>
> This reduction in port pool size has made it clear that a one-second connection
> reincarnation period can lead to ephemeral port exhaustion. Short-lived TCP
> connections, such as DNS queries, can complete in milliseconds, yet the TCP
> 4-tuple remains blocked for a period of time that is orders of magnitude longer.
>
> Solution
> ========
>
> We would like to propose to shorten the period during which the 4-tuple is tied
> up. The intention is to enable TIME-WAIT reuse at least as quickly as it takes
> nowadays to perform of a short-lived TCP connection, from setup to teardown.
>
> The ts_recent_stamp protection is based on the same principle as PAWS but
> extends it across TCP connections. As RFC 7323 outlines in Appendix B.2, point
> (b):
>
>     An additional mechanism could be added to the TCP, a per-host
>     cache of the last timestamp received from any connection.  This
>     value could then be used in the PAWS mechanism to reject old
>     duplicate segments from earlier incarnations of the connection,
>     if the timestamp clock can be guaranteed to have ticked at least
>     once since the old connection was open.  This would require that
>     the TIME-WAIT delay plus the RTT together must be at least one
>     tick of the sender's timestamp clock.  Such an extension is not
>     part of the proposal of this RFC.

Note the RTT part here. I do not see this implemented in your patch.

>
> Due to that, we would want to follow the same guidelines as the for TSval
> timestamp clock, for which RFC 7323 recommends a frequency in the range of 1 ms
> to 1 sec per tick [5], when reconsidering the default setting.
>
> (Note that the Linux TCP stack has recently introduced even finer granularity
> with microsecond TSval resolution in commit 614e8316aa4c "tcp: add support for
> usec resolution in TCP TS values" [6] for use in private networks.)
>
> A simple implementation could be to switch from a second to a millisecond clock,
> as demonstrated by the following patch. However, this could also be a tunable
> option to allow administrators to adjust it based on their specific needs and
> risk tolerance.
>
> A tunable also opens the door to letting users set the TIME-WAIT reuse period
> beyond the RFC 7323 recommended range at their own risk.
>
> Workaround
> ==========
>
> Today, when an application has only a small ephemeral port pool available, we
> work around the 1-second reincarnation period by manually selecting the local
> port with an explicit bind().
>
> This has been possible since the introduction of the ts_recent_stamp protection
> mechanism [1]. However, it is unclear why this is allowed for egress
> connections.
>
> To guide readers to the relevant code: if the local port is selected by the
> user, we do not pass a TIME-WAIT socket to the check_established helper from
> __inet_hash_connect. This way we circumvent the timestamp check in
> tcp_twsk_unique [7] (as twp is NULL).
>
> However, relying on this workaround conflicts with our goal of delegating TCP
> local port selection to the network stack, using the IP_BIND_ADDRESS_NO_PORT [8]
> and IP_LOCAL_PORT_RANGE [9] socket options.
>
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=b8439924316d5bcb266d165b93d632a4b4b859af
> [2] https://datatracker.ietf.org/doc/html/rfc7323#section-5.8
> [3] https://datatracker.ietf.org/doc/html/rfc7323#appendix-B
> [4] https://lpc.events/event/16/contributions/1349/
> [5] https://datatracker.ietf.org/doc/html/rfc7323#section-5.4
> [6] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=614e8316aa4cafba3e204cb8ee48bd12b92f3d93
> [7] https://elixir.bootlin.com/linux/v6.10/source/net/ipv4/tcp_ipv4.c#L156
> [8] https://manpages.debian.org/unstable/manpages/ip.7.en.html#IP_BIND_ADDRESS_NO_PORT
> [9] https://manpages.debian.org/unstable/manpages/ip.7.en.html#IP_LOCAL_PORT_RANGE
> ---
>
> ---
>  drivers/net/ethernet/chelsio/inline_crypto/chtls/chtls_cm.c | 2 +-
>  include/linux/tcp.h                                         | 4 ++--
>  net/ipv4/tcp_input.c                                        | 2 +-
>  net/ipv4/tcp_ipv4.c                                         | 5 ++---
>  net/ipv4/tcp_minisocks.c                                    | 9 ++++++---
>  5 files changed, 12 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/net/ethernet/chelsio/inline_crypto/chtls/chtls_cm.c b/drivers/net/ethernet/chelsio/inline_crypto/chtls/chtls_cm.c
> index 6f6525983130..b15b26db8902 100644
> --- a/drivers/net/ethernet/chelsio/inline_crypto/chtls/chtls_cm.c
> +++ b/drivers/net/ethernet/chelsio/inline_crypto/chtls/chtls_cm.c
> @@ -1866,7 +1866,7 @@ static void chtls_timewait(struct sock *sk)
>         struct tcp_sock *tp = tcp_sk(sk);
>
>         tp->rcv_nxt++;
> -       tp->rx_opt.ts_recent_stamp = ktime_get_seconds();
> +       tp->rx_opt.ts_recent_stamp = tcp_clock_ms();
>         tp->srtt_us = 0;
>         tcp_time_wait(sk, TCP_TIME_WAIT, 0);
>  }
> diff --git a/include/linux/tcp.h b/include/linux/tcp.h
> index 6a5e08b937b3..174257114ee4 100644
> --- a/include/linux/tcp.h
> +++ b/include/linux/tcp.h
> @@ -110,7 +110,7 @@ struct tcp_sack_block {
>
>  struct tcp_options_received {
>  /*     PAWS/RTTM data  */
> -       int     ts_recent_stamp;/* Time we stored ts_recent (for aging) */
> +       u32     ts_recent_stamp;/* Time we stored ts_recent (for aging) */
>         u32     ts_recent;      /* Time stamp to echo next              */
>         u32     rcv_tsval;      /* Time stamp value                     */
>         u32     rcv_tsecr;      /* Time stamp echo reply                */
> @@ -543,7 +543,7 @@ struct tcp_timewait_sock {
>         /* The time we sent the last out-of-window ACK: */
>         u32                       tw_last_oow_ack_time;
>
> -       int                       tw_ts_recent_stamp;
> +       u32                       tw_ts_recent_stamp;
>         u32                       tw_tx_delay;
>  #ifdef CONFIG_TCP_MD5SIG
>         struct tcp_md5sig_key     *tw_md5_key;
> diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
> index e37488d3453f..873a1cbd6d14 100644
> --- a/net/ipv4/tcp_input.c
> +++ b/net/ipv4/tcp_input.c
> @@ -3778,7 +3778,7 @@ static void tcp_send_challenge_ack(struct sock *sk)
>  static void tcp_store_ts_recent(struct tcp_sock *tp)
>  {
>         tp->rx_opt.ts_recent = tp->rx_opt.rcv_tsval;
> -       tp->rx_opt.ts_recent_stamp = ktime_get_seconds();
> +       tp->rx_opt.ts_recent_stamp = tcp_clock_ms();

Please do not abuse tcp_clock_ms().

Instead use tcp_time_stamp_ms(tp)

Same remark for other parts of the patch, try to reuse tp->tcp_mstamp
if available.

Also, (tcp_clock_ms() != ts_recent_stamp) can be true even after one
usec has elapsed, due to rounding.

The 'one second delay' was really: 'An average of 0.5 second delay'

Solution : no longer use jiffies, but usec based timestamps, since we
already have this infrastructure in TCP stack.

Powered by blists - more mailing lists