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
prev parent 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.