All of lore.kernel.org
 help / color / mirror / Atom feed
From: William Allen Simpson <william.allen.simpson@gmail.com>
To: netdev@vger.kernel.org
Subject: Re: [PATCH v2 1/8] Only parse time stamp TCP option in time wait sock
Date: Wed, 21 Oct 2009 14:59:59 -0400	[thread overview]
Message-ID: <4ADF5A2F.9010309@gmail.com> (raw)
In-Reply-To: <4ADEDD65.6070802@codefidence.com>

Gilad Ben-Yossef wrote:
> William Allen Simpson wrote:
> 
>> Gilad Ben-Yossef wrote:
>>> A time wait socket is established - we already know if time stamp
>>> option is called for or not.
>>>
>> Not so sure about this.  A timewait sock isn't actually established,
>> and new/changed options could appear.  There's all sorts of edge cases.
> If you examine the specific context where tcp_parse_options is being 
> called here,
> the only TCP option which is of interest is the time stamp option, and 
> this code path
> is only being taken when we already know that the original socket  had
> used the time stamp option.
> 
> So while I agree that in general you are right, I do believe that in the 
> specific context
> of this patch we should call tcp_parse_options with the established flag 
> on and let it
> know we are expecting to see a time stamp option, which is what I was 
> referring to.
> 
No, a major reason for time-wait is rebooted systems.  We don't "know"
anything about them, and they certainly don't know anything about us.

As I mentioned, this is about edge cases.


>>
>> There's also some current work to note:
>>
>>  http://tools.ietf.org/html/draft-ietf-tcpm-1323bis
>>
>>  http://tools.ietf.org/html/draft-gont-tcpm-tcp-timestamps
> 
> Very interesting, thank you.
> 
> As I noted above, my comment about
> TIME WAIT sockets being "established" should really only be considered
> in the context of the specific call to tcp_parse_options() and the 
> "established"
> parameter of that function.
> 
My suggestion, as this patch is not essential to the other patches in the
series, is to separate it.  As I'm relatively new to this list, I don't
know the best practice.  But I'd like to support the others and delay
this for further consideration.

  reply	other threads:[~2009-10-21 19:07 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-10-21  8:56 [PATCH v2 0/8] Per route TCP options Gilad Ben-Yossef
2009-10-21  8:56 ` [PATCH v2 1/8] Only parse time stamp TCP option in time wait sock Gilad Ben-Yossef
2009-10-21  9:49   ` William Allen Simpson
2009-10-21 10:07     ` Gilad Ben-Yossef
2009-10-21 18:59       ` William Allen Simpson [this message]
2009-10-25  8:41         ` Gilad Ben-Yossef
2009-10-21  8:56 ` [PATCH v2 2/8] Allow tcp_parse_options to consult dst entry Gilad Ben-Yossef
2009-10-21 13:03   ` Ilpo Järvinen
2009-10-21 14:07     ` Gilad Ben-Yossef
2009-10-22  9:41       ` Ilpo Järvinen
2009-10-21  8:56 ` [PATCH v2 3/8] Add dst_feature to query route entry features Gilad Ben-Yossef
2009-10-21  8:56 ` [PATCH v2 4/8] Add the no SACK route option feature Gilad Ben-Yossef
2009-10-21 19:22   ` William Allen Simpson
2009-10-25  8:44     ` Gilad Ben-Yossef
2009-10-21  8:56 ` [PATCH v2 5/8] Allow disabling TCP timestamp options per route Gilad Ben-Yossef
2009-10-21 19:22   ` William Allen Simpson
2009-10-25  8:43     ` Gilad Ben-Yossef
2009-10-21  8:56 ` [PATCH v2 6/8] Allow to turn off TCP window scale opt " Gilad Ben-Yossef
2009-10-21  8:57 ` [PATCH v2 7/8] Allow disabling of DSACK TCP option " Gilad Ben-Yossef
2009-10-21  8:57 ` [PATCH v2 8/8] Document future removal of sysctl_tcp_* options Gilad Ben-Yossef
2009-10-21  9:40   ` William Allen Simpson
2009-10-21 10:23     ` Gilad Ben-Yossef
2009-10-21 19:30       ` William Allen Simpson
2009-10-22  4:32         ` Bill Fink
2009-10-22  4:57           ` Eric Dumazet
2009-10-22 10:53             ` William Allen Simpson
2009-10-25  9:09             ` Gilad Ben-Yossef
2009-10-26  0:21               ` Bill Fink
2009-10-26  5:03                 ` Eric Dumazet
2009-10-26  8:05                   ` Gilad Ben-Yossef
2009-10-26 15:08                     ` Bill Fink
2009-10-26 15:51                       ` Gilad Ben-Yossef
2009-10-27  5:09                         ` Bill Fink
2009-10-25  8:45         ` Gilad Ben-Yossef

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=4ADF5A2F.9010309@gmail.com \
    --to=william.allen.simpson@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.