netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jakub Sitnicki <jakub@cloudflare.com>
To: Jason Xing <kerneljasonxing@gmail.com>
Cc: netdev@vger.kernel.org,  Eric Dumazet <edumazet@google.com>,
	kernel-team@cloudflare.com
Subject: Re: [PATCH RFC net-next] tcp: Allow TIME-WAIT reuse after 1 millisecond
Date: Mon, 19 Aug 2024 15:54:50 +0200	[thread overview]
Message-ID: <87ed6kr51x.fsf@cloudflare.com> (raw)
In-Reply-To: <CAL+tcoD9BA_Y26dSz+rkvi2_ZEc6D29zVEBhSQ5++OtOqJ3Xvw@mail.gmail.com> (Jason Xing's message of "Mon, 19 Aug 2024 20:27:11 +0800")

Hi Jason,

On Mon, Aug 19, 2024 at 08:27 PM +08, Jason Xing wrote:
> On Mon, Aug 19, 2024 at 7:31 PM Jakub Sitnicki <jakub@cloudflare.com> wrote:

[...]

>> --- a/net/ipv4/tcp_ipv4.c
>> +++ b/net/ipv4/tcp_ipv4.c
>> @@ -116,7 +116,7 @@ int tcp_twsk_unique(struct sock *sk, struct sock *sktw, void *twp)
>>         const struct inet_timewait_sock *tw = inet_twsk(sktw);
>>         const struct tcp_timewait_sock *tcptw = tcp_twsk(sktw);
>>         struct tcp_sock *tp = tcp_sk(sk);
>> -       int ts_recent_stamp;
>> +       u32 ts_recent_stamp;
>>
>>         if (reuse == 2) {
>>                 /* Still does not detect *everything* that goes through
>> @@ -157,8 +157,7 @@ int tcp_twsk_unique(struct sock *sk, struct sock *sktw, void *twp)
>>          */
>>         ts_recent_stamp = READ_ONCE(tcptw->tw_ts_recent_stamp);
>>         if (ts_recent_stamp &&
>> -           (!twp || (reuse && time_after32(ktime_get_seconds(),
>> -                                           ts_recent_stamp)))) {
>> +           (!twp || (reuse && (u32)tcp_clock_ms() != ts_recent_stamp))) {
>
> At first glance, I wonder whether 1 ms is really too short, especially
> for most cases? If the rtt is 2-3 ms which is quite often seen in
> production, we may lose our opportunity to change the sub-state of
> timewait socket and finish the work that should be done as expected.

Good point about TW state management. Haven't thought of that.

> One second is safe for most cases, of course, since I obscurely
> remember I've read one paper (tuning the initial window to 10) saying
> in Google the cases exceeding 100ms rtt is rare but exists. So I still
> feel a fixed short value is not that appropriate...
>
> Like you said, how about converting the fixed value into a tunable one
> and keeping 1 second as the default value?

I'm also leaning toward a tunable. The adoption could then be based on
an opt-in model. We don't want to break any existing setups, naturally.

> After you submit the next version, I think I can try it and test it
> locally :) It's interesting!

Thanks for feedback,
(the other) Jakub

      reply	other threads:[~2024-08-19 13:54 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-08-19 11:31 [PATCH RFC net-next] tcp: Allow TIME-WAIT reuse after 1 millisecond Jakub Sitnicki
2024-08-19 11:59 ` Eric Dumazet
2024-08-19 13:44   ` Jakub Sitnicki
2024-08-19 12:27 ` Jason Xing
2024-08-19 13:54   ` Jakub Sitnicki [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87ed6kr51x.fsf@cloudflare.com \
    --to=jakub@cloudflare.com \
    --cc=edumazet@google.com \
    --cc=kernel-team@cloudflare.com \
    --cc=kerneljasonxing@gmail.com \
    --cc=netdev@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).