Re: [tcpm] review of 1323bis-15

"Scheffenegger, Richard" <rs@netapp.com> Tue, 12 November 2013 18:28 UTC

Return-Path: <rs@netapp.com>
X-Original-To: tcpm@ietfa.amsl.com
Delivered-To: tcpm@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id 0909F11E815C for <tcpm@ietfa.amsl.com>; Tue, 12 Nov 2013 10:28:34 -0800 (PST)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -5.451
X-Spam-Level:
X-Spam-Status: No, score=-5.451 tagged_above=-999 required=5 tests=[AWL=-3.452, BAYES_00=-2.599, J_CHICKENPOX_35=0.6]
Received: from mail.ietf.org ([12.22.58.30]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 0qb2p8BB8FNj for <tcpm@ietfa.amsl.com>; Tue, 12 Nov 2013 10:28:29 -0800 (PST)
Received: from mx11.netapp.com (mx11.netapp.com [216.240.18.76]) by ietfa.amsl.com (Postfix) with ESMTP id 06C0511E810B for <tcpm@ietf.org>; Tue, 12 Nov 2013 10:28:28 -0800 (PST)
X-IronPort-AV: E=Sophos;i="4.93,686,1378882800"; d="scan'208";a="74059914"
Received: from vmwexceht01-prd.hq.netapp.com ([10.106.76.239]) by mx11-out.netapp.com with ESMTP; 12 Nov 2013 10:28:26 -0800
Received: from SACEXCMBX02-PRD.hq.netapp.com ([169.254.1.86]) by vmwexceht01-prd.hq.netapp.com ([10.106.76.239]) with mapi id 14.03.0123.003; Tue, 12 Nov 2013 10:28:21 -0800
From: "Scheffenegger, Richard" <rs@netapp.com>
To: "mallman@icir.org" <mallman@icir.org>, "tcpm@ietf.org" <tcpm@ietf.org>
Thread-Topic: [tcpm] review of 1323bis-15
Thread-Index: AQHO2YRNtvv/BV/OuUuBwm532/vjlpohvhLQ
Date: Tue, 12 Nov 2013 18:28:13 +0000
Message-ID: <012C3117EDDB3C4781FD802A8C27DD4F25E8BB4B@SACEXCMBX02-PRD.hq.netapp.com>
References: <20131104172910.32F932639802@lawyers.icir.org>
In-Reply-To: <20131104172910.32F932639802@lawyers.icir.org>
Accept-Language: de-AT, en-US
Content-Language: en-US
X-MS-Has-Attach:
X-MS-TNEF-Correlator:
x-originating-ip: [10.106.53.53]
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
Subject: Re: [tcpm] review of 1323bis-15
X-BeenThere: tcpm@ietf.org
X-Mailman-Version: 2.1.12
Precedence: list
List-Id: TCP Maintenance and Minor Extensions Working Group <tcpm.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/tcpm>, <mailto:tcpm-request@ietf.org?subject=unsubscribe>
List-Archive: <http://www.ietf.org/mail-archive/web/tcpm>
List-Post: <mailto:tcpm@ietf.org>
List-Help: <mailto:tcpm-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/tcpm>, <mailto:tcpm-request@ietf.org?subject=subscribe>
X-List-Received-Date: Tue, 12 Nov 2013 18:28:34 -0000

Hi Mark,

thanks for your comments.


Richard Scheffenegger


 
>   - I'd remove the mention of RTTM in the abstract.  It is downplayed in
>     the text.

I replace the "are used" by "can be used", and reversed the order of PAWS and RTTM.


>   - Sec 1: "has resulted" --> "have resulted"

Check

>   - Sec 1: In the last paragraph before 1.1:
> 
>     - should "recommended" be "RECOMMENDED"?  (I am not sure)

This text is before the definition of RFC2119 style wording; I tend to keep this lower case...

>     - perhaps a mention here that the changes from 1323 are partially
>       due to our changed understanding of how things work (i.e., this is
>       not just a set of bug fixes)

Added this to the 2nd before last paragraph of section 1.

>   - Sec 1.1: I found this a bit strange.  The two "fundamental problems"
>     seem right to me.  But, this document will go on to develop
>     PAWS---which addresses neither problem directly.  You might develop
>     the notion that making the windows bigger causes *another*
>     fundamental problem and that is too quickly wrapping.  And,
>     therefore, that problem must be tackled and this document does so.

Hmm... sec 1.1 is about performance; PAWS is not a performance issue but a integrity (reliability) issue. As 1323bis also defines Window Scale, that appears to address the performance issue described in 1.1 (1); 

But I think you are correctly pointing out that the use WS will make it more likely for wrap around to occur, thus PAWS is necessary. I've added a sentence to point this out.


>   - Sec 1.2, point (1): This is not entirely correct.  You need both a
>     high transfer rate *and a whole bunch of data* to have a sequence
>     wrapping issue.  This point could be a lot better by noting that it
>     takes 4GB of data to wrap.

Sentence now reads: At a high enough transfer rate of large volume of data (at least 4 GiB in the same session), the 32-bit sequence space may be wrapped within the time that a sement is delayed in queues.

Do you think this is good enough?

>   - Sec 1.2, the paragraph starting "Duplicates from earlier ....": You
>     might also throw in there that randomizing ephemeral ports can also
>     help probabilistically reduce the chances of duplicates from
>     previous connections muddying the waters.

Check.

>   - Sec 1.3: "be crashed by" --> "crash on"

check

>   - Sec 1.3 (and other places): I'd like to see the TS option be
>     discussed as the 10 byte option that it is.  It is fine to note that
>     often it consumes 12 bytes because passing is needed.  However,
>     there is a disconnect in the document between the actual
>     specification (10 bytes) and the discussion (12 bytes) that could
>     lead to confusion.

Fixed this instance; i couldn't find other text that incorrectly states the option to be 12 bytes in size; only the Appendix A shows the 12-byte layout recommendation (but without much text).

 
>   - Sec 1.3: I'd remove the word "optimal".  I don't think anyone ever
>     claimed that any RTO scheme---TS based or non-TS based---ever
>     produced an "optimal" RTO.  The statement should be precise and say
>     that additional RTT samples per RTT is shown to have little effect
>     on the ultimate RTO when compared to one sample per RTT.

Check.

>   - Sec 2.2: "to do both" --> "to both"

Check

>   - Sec 2.2: "is never scaled." --> "MUST NOT be scaled."

Check

>   - Sec 2.3: The list at the beginning of the section should be
>     converted to standards language (MUST, SHOULD, etc.).

Added mostly MUSTs (only one SHOULD on the receiver, which may choose to limit the window for memory constrains etc).


>   - Sec 2.3: I find this notion in the first bullet that the window
>     should be stored as 32-bit quantities to be weird.  The maximum size
>     for advertised windows is 30 bits.  So, why specify the use of 32
>     bit memory locations in the control block?  I mean, that is the
>     natural thing to do, but it just adds a little confusion to things
>     without really helping the implementer at all.

Commented this point out in the xml. (I guess, current OS may want to store this all in 64bit quanta even).

>   - Sec 2.3: "This is safe as a sender can always choose to only
>     partially use any signaled receive window."  That is correct, but it
>     takes a leap from the previous discussion to figure out what you
>     mean.  I'd flesh this out a bit more so the reader doesn't have to
>     scratch their head.  I.e., if the receiver is scaling by X>14 and
>     the receiver is only scaling by 14 then the window will appear
>     smaller than it is in reality.

Check.

>   - Sec 3.1: The third paragraph doesn't make much sense, or at least
>     isn't precise.  You are trying to make a clear distinction with
>     imprecise words. :-)
>
>     I'd say you draw a distinction between the timestamp option that
>     *conveys* timestamp information and the *use* of that information
>     (e.g., by PAWS or by RTTM or by Eifel or whatnot).  In particular,
>     not all the mechanisms that use *timestamps* use "the RTT signal as
>     input".  E.g., PAWS does not.  So, you are trying to push a
>     distinction that doesn't make sense.

Reworded; I'm not sure if the new wording is satisfactory though (more/specific text donations gladly accepted):

        It is necessary to remember that there is a distinction between the 
        Timestamps option conveying timestamp information, and the use of that 
        information. In particular, the Round Trip Time Measurement (RTTM) 
        mechanism must be viewed independently from updating the Retransmission 
        Timeout (RTO) (see <xref target="sec331"/>). In this case, the sample 
        granularity also needs to be taken into account. Other mechanisms, such 
        as PAWS, or Eifel, are not built upon the timestamp information itself, 
        but are based on the intrinsic property of monotonically increasing values.
     

>   - Sec 3.1: "option is useful" --> "option may be useful" (i.e., it
>     depends on the balancing of costs and benefits and so I don't think
>     an absolute statement is warranted)

Check

>   - Sec 3.1: "Eifel ([cites]) and others" --> I'd add cites for "and
>     others".  Or, drop it.  This is a way to try to make an argument
>     that there are scads of these things and it isn't worth listing them
>     all, when in reality there are not.

Added three references that I know offhand, where the timestamps option is used, or at least improves the mechanisms.


>   - Sec 3.2: The first paragraph feels out of place.  Before you even
>     define timestamps you are telling us how they are used.  I'd move
>     this to later in the section.

I'm unsure; to me, this paragraph reads like a very short summary of the entire section. It has been carried forward in this form and position from 1323. (also, it provides the rationale why the rfc1072 options were superceded).


>     Sec 3.2: This whole discussion of valid and invalid TSecr is
>     confusing.  You note that when the value is invalid it should be set
>     to zero.  But, a TSecr of zero does not mean the TS is invalid.  So,
>     how should we tell?

This was my attempt at "liberal at what you accept, and conservative in what you send". There are implementations that assume TSecr=0 == invalid, with not strictly defined behavior in that case (e.g. drop that segment and stall until the sender timestamp increases to 1, or end up with strange RTO values...). When TSval is more or less the uptime of the sender, this roll-over hardly matters [which is probably the reason this was not fixed a long time ago in the affected stack], but when TSval is randomized, this corner case is more likely to occur and the implementer should properly treat this case. 

If anyone can think of better wording to express this, please donate text.

>   - Sec 3.2:
> 
>       "A TCP MAY send the Timestamps option (TSopt) in an initial <SYN>
>       segment (i.e., segment containing a SYN bit and no ACK bit), and
>       MAY send a TSopt in other segments only if it received a TSopt in
>       the initial <SYN> or <SYN,ACK> segment for the connection."
> 
>     That second MAY strikes me as totally wrong.  Shouldn't it be a
>     MUST?  Didn't the WG decide that once timestamps are turned on (in
>     the 3WHS) that they must always be used?  The next paragraph seems
>     to back up this notion of using a MUST here.  In fact, I am not
>     entirely sure why there are two paragraphs here.  They seem to at
>     least largely overlap.

Too much editing in this space since around -06... sorry.

That paragraph now describes just the handshake <SYN>, <SYN,ACK> case, where the receiver is still free to choose not to send TSopt. The next paragraph handles strictly the "established" case.

>   - Likewise there is a notion that segments without timestamps "SHOULD"
>     be dropped.  But, if we're going to enforce things maybe that should
>     also be a "MUST"?

We had a discussion around this; the consensus at the time was, to allow existing implementations claim compliance, to have a SHOULD here (most will accept segments, which all of a sudden don't have TSopt any more; in the case of FreeBSD, this was added as a feature - apparently some middleboxes ate TSopt during path changes, leading to consternation...

 
>   - Sec 3.3: It'd be nice to see references to this middlebox behavior
>     you sketch.  Even if just to a mailing list discussion it'd be more
>     concrete than the nebulous language in there now.

This comment seems to be around paragraph 9, sec 3.2; added a referecence to Andre's comment to that effect, why FreeBSD adheres to a SHOULD (again).


>   - I would suggest splitting 3.3 & 3.4 off into their own section.  You
>     say you are splitting the TS mechanism from its use, but then you
>     don't in the document.  I think a new section would make this
>     distinction more clear.  I.e., the new section could start with "One
>     use for the timestamp option is ...".  And, the PAWS section could
>     similarly start with "Another use for the timestamp option is ...".

Check.


>   - Sec 3.3: Since you have a section on updating the RTO you should
>     move all the discussion of updating the RTO there.  I.e., you should
>     not have it scattered.

check

> 
>   - Sec 3.4: Swap the first two paragraphs.

Check. 


>   - Sec 3.4: You say one way to deal with the problem of the gains is
>     addressed in the appendix.  Fine.  But, it might be useful to stake
>     out a principle here.  E.g., something like "in the face of multiple
>     RTT samples per RTT an implementation SHOULD try to adhere to the
>     spirit of the history specified in [RFC6298]".  Something like
>     that.  I.e., throw out at least a high level goal.

Check.


>   - Sec 4.2: "PAWS uses the same TCP Timestamps option as the RTTM
>     mechanism" --> PAWS uses the TCP Timestamps option"

Check

>   - Sec 4.2: "In both the PAWS and the RTTM mechanism," --> "In the PAWS
>     mechanism,"

Check

>   - Sec 4.2: The first few paragraphs are just a mess.  Get rid of the
>     references to RTTM.  PAWS does not and never did depend on RTTM.
>     Change this to the correct and more accurate "timestamp option" sort
>     of language.

Tried to fix this.


>   - Sec 4.2: What is 'snd.ts.ok'???  This is seemingly important, but
>     not defined.  (It is defined in an appendix, but that is not good
>     enough for a normative discussion.)

Added this to the Timestamps option section...

>   - Sec 4.2:
> 
>       When an <RST> segment is received, it MUST NOT be subjected to
>       PAWS checks, and information from the Timestamps option MUST NOT
>       be used to update connection state information.  SEG.TSecr MAY be
>       used to provide stricter <RST> acceptance checks.
> 
>     Don't these two sentences contradict each other?  They are at least
>     not clear.

PAWS works on Tsval; added wording to point this out.

>   - Sec 4.4: "to measure RTTs" .... PAWS does not measure RTTs.  Get rid
>     of this.

Updated wording

> 
>   - Sec 4.4: "internet" --> "Internet"

check

> Also, one thing to think about ... Might it be useful to recommend a
> random offset to the TS per connection?  People have shown it is possible
> to get signal from these things and perhaps that is not useful.  I know
> OpenBSD uses a random offset.  Perhaps just as a MAY or something.  I.e.,
> there is no harm in it and so let implementers chew on it.

Used Davids text suggestion at the appropriate section.

> I hope that is useful!

Very!

I'll update the formating (page flow) and post an update shortly.

Best regards,
   Richard Scheffenegger