Subject: [tsvwg] Review of draft-ietf-tsvwg-rfc4960-errata-04.txt

From: Gorry Fairhurst <[email-masked]>
Date: Wed, 13 Dec 2017 21:01:22 +0000

This email contains a shepherd review for the above draft. There are 
several issues, that seem straight forward to correct and where 
alternate wording may be needed.

I have a comment about the use of lower case RFC2119 wording: While I 
know that documents continue to be published with a mix of upper case 
normative keywords and lower case words, I'd like the authors to 
explicitly check each of the lower case uses. In some places, the 
correct thing appears to be to use a normative version.

I suggest we should be extremely careful in this usage when considering 
progressing a document to full standard.  I'd advise replacing all other 
non-normative uses of /should/ and /may/ with /ought to/ and /could/ to 
remove this ambiguity. This would make it really clear what the 
normative requirements are.

Gorry
(TSVWG Co-Chair).

----

Abstract
- English?
/time based/
- seems wrong, this could be hypenated, although maybe it would be 
better as /time ordered/?
---
Introduction
/the publishing of this/
- may be better as /the publication of this/.
---
3.1.2
/the endpoint should mark the corresponding/
- This seems like a protocol action, why is this not
/the endpoint SHOULD mark the corresponding/
---
/and may also/
- similarly /and MAY also/
- although I would understand why the latter was /and could also/ ... 
but I'd prefer to avoid the lower case MAY.
---

/After this, the endpoint should continue
    HEARTBEAT on this destination address but should stop increasing the
    counter./
- are these /SHOULD/ if not, why not? and if a reason, I suggest you 
replace by /ought/.
---
3.2.2
/and should discard subsequent ULP shutdown requests./

- are these /SHOULD/ if not, why not? and if a reason, I suggest you 
replace by /ought/.
---
3.2.3
/If it did the endpoints/
- please insert comma aftre /did/.
---
3.3.2
- RFC2434 needs to be replaced by RFC5226.
---
3.6.2
/An endpoint shall keep a counter on the total number of consecutive
    retransmissions to its peer/
- is this /SHOULD/ if not, I suggest you replace by /ought/.
---
/the path currently used for data transfer shall
    not increment the association error counter, /
- Could this lead to failure, is this /SHOULD NOT/ or actually /MUST NOT/?
---
3.9.2
/to the current Path MTU/
- Elsewhere this is called PMTU.
---
/for path MTU discovery./
- seems wrong, should this be:
/for Path MTU Discovery./ or /for PMTU Discovery./ ?
---
/constraint of the path MTU/
- Capital P, or PMTU?
---
3.11.2
/SCTP should perform slow/
- If SCTP is to follow TCP CC, then I think this needs to be:
  /SCTP SHOULD perform slow/
---
3.13.2
/When a
    HEARTBEAT ACK is received from the peer endpoint, the counter should
    also be reset./
- I think that is SHOULD be reset.
---
/The receiver of the HEARTBEAT ACK may choose not to
    clear the counter if there is outstanding data on the association.
- I think that is MAY or /could/
---
/
Upon the receipt of the HEARTBEAT ACK, the sender of the HEARTBEAT
    should clear the error counter/
- I think that is SHOULD be clear, since it sets what comes on the wire.
---
/The endpoint may optionally report/
- /could optionally report/ or /is permitted to optionally report/?
---
/The receiver of the HEARTBEAT ACK should also clear/
- I'd have thought that was SHOULD.
---
3.15.2
- PMTU or Path MTU.
---
3.16.2
- The following text seems odd, but perhaps you could refer to RFC5681 
where this exact phrase is used?:
/The initial value of ssthresh SHOULD be arbitrarily high (e.g.,
       to the size of the largest possible advertised window)./
---
3.20.1
/But receiving new packets from the peer does not guarantee peer's 
accessibility/
- Maybe better as:
/However, reception of new packets from the peer does not guarantee the 
peer's accessibility/
- Is reachability better than accessibility?
---
3.20.2
/should not increment the error counter/
- /ought not/ or /should not/.
---
/This is because the receiver MAY
    keep its window closed for an indefinite time./
- is this really /MAY/ - are we permitting something or saying it can be 
so?
If the latter, please use /could/ in place of MAY.
---
- This text seems odd:
/Refer to Section 6.2 on the receiver behavior when it advertises a zero 
window./
- would this be better as:

/Section 6.2 deescribes the receiver behavior when it advertises a zero 
window./

---
3.21.2 (multiple)
/When
       this flag is set, the stream id and Stream Sequence Number must
       accompany this receive./
- This seems like protocol action is it /MUST/?
---
/the endpoint shall
    clear the error counter of the destination transport address to which
    the DATA chunk was last sent (or HEARTBEAT was sent), and SHOULD/
- why is the first /shall/ lower case, and not an /ought to/ or a SHALL?
---
/should clear the error counter/
- why is the  /should/ lower case, and not an /ought to/ or a SHALL?

--- (multiple)
/ An endpoint should limit the number of retransmissions of the
    SHUTDOWN chunk to the protocol parameter 'Association.Max.Retrans'./
- why is the  /should/ lower case, and not an /ought to/ or a SHALL?
---
- A MUST states a requirement:
/Furthermore, we require
    that the receiver of an INIT chunk MUST enforce these rules by
    silently discarding the INIT chunk and all further chunks if the INIT
    chunk is bundled with other chunks or the packet has a non-zero
    verification tag. /
- needs to be rewritten as:
/The receiver of an INIT chunk MUST silently discard the INIT chunk
    and all further chunks if the INIT
    chunk is bundled with other chunks or the packet has a non-zero
    verification tag./
---
3.26.2
/
    When cwnd is greater than ssthresh, cwnd should be incremented by
    1*MTU per RTT if the sender has cwnd or more bytes of data
    outstanding for the corresponding transport address./
- /SHOULD/.
---
3.27.2
/
When the endpoint does not transmit data on a given transport
       address, the cwnd of the transport address should be adjusted to
       max(cwnd/2, 4*MTU) per RTO. At the first cwnd adjustment, the
       ssthresh of the transport address should be adjusted to the cwnd./
- /SHOULD be adjusted/ (twice)?
---
3.32.1
- This update is based on RFC6298.
I think the updated text should in some way refer
to this update as the basis of the method.
---
3.33.2.
/
The endpoint may bundle the ERROR chunk/
- /MAY/.
---
3.35.2
- This text doesn't seem very clear to me:
/
    SCTP implementations MAY allow an application to configure the
    Differentiated Services Code Point (DSCP) used for sending packets.
    If a DSCP change might result in outgoing packets being queued in
    different queues, the congestion control parameters for all affected
    destination addresses MUST be reset to their initial values.

/
- (1) Does this mean for each packet? or for the entire asscociation or 
what?
(I think it reads like an association?)
- What does /being queued in different queues/ mean? ... any change to the
DSCP could result in different results, but this is more likely if the 
change
modifies an AF class. It is probably safer to point to RFC7657 for 
specifics.
- Please update this.
---
3.40.2
- The update you note in the following two specific cases is not in the 
ID you cite, but in AccECN I think... where the change
from RTT-based reduction is mentioned. Please check and update.
/
[RFC3168] updated by [I-D.ietf-tsvwg-ecn-experimentation]
    details a specific bit for a receiver to send back in its
    TCP acknowledgements to notify the sender of the Congestion
    Experienced (CE) bit having arrived from the network./
-
/
    RFC3168] updated by [I-D.ietf-tsvwg-ecn-experimentation]
    details a specific bit for a sender to send in the header
    of its next outbound TCP segment to indicate to its peer that it has
    reduced its congestion window./
- Although maybe you could choose not to cite this?
---
3.44
- Section 3.44 clearly DOES have IANA implications. This represents changes
that were made to port assignment, and although these are captured in
a published RFC they are re-stated here and I think this NEEDS to be
called out in the IANA considerations. IANA would be expected to review
these changes (again), and this needs to be explained in section 4.
---
/
       If the name resolution is not successful, the endpoint MUST
       immediately send an ABORT with "Unresolvable Address" error cause
       to its peer.  The ABORT shall be sent to the source IP address
       from which the last peer packet was received./
- please replace/update the /shall/.
---
/Should be set to all '0's and ignored by the receiver./
- SHOULD.
---
3.4.62
- Can one of the authors give me assurance that the code quoted is still 
correct at this time?
---

---

From: Michael Tuexen <[email-masked]>
Date: Thu, 14 Dec 2017 00:05:51 +0100

> On 13. Dec 2017, at 22:01, Gorry Fairhurst <[email-masked]> wrote:
> 
> 
> This email contains a shepherd review for the above draft. There are several issues, that seem straight forward to correct and where alternate wording may be needed.
Thanks for the review. I'll go through you comments in detail in a couple of days.
> 
> I have a comment about the use of lower case RFC2119 wording: While I know that documents continue to be published with a mix of upper case normative keywords and lower case words, I'd like the authors to explicitly check each of the lower case uses. In some places, the correct thing appears to be to use a normative version.
> 
> I suggest we should be extremely careful in this usage when considering progressing a document to full standard.  I'd advise replacing all other non-normative uses of /should/ and /may/ with /ought to/ and /could/ to remove this ambiguity. This would make it really clear what the normative requirements are.
Clarification question: Are you asking to check each of the lowercase uses in RFC 4960 or in draft-ietf-tsvwg-rfc4960-errata-04.txt?

Best regards
Michael
> 
> Gorry
> (TSVWG Co-Chair).
> 
> ----
> 
> Abstract
> - English?
> /time based/
> - seems wrong, this could be hypenated, although maybe it would be better as /time ordered/?
> ---
> Introduction
> /the publishing of this/
> - although I would understand why the latter was /and could also/ ... but I'd prefer to avoid the lower case MAY.
> ---
> 
> - are these /SHOULD/ if not, why not? and if a reason, I suggest you replace by /ought/.
> ---
> 3.2.2
> - are these /SHOULD/ if not, why not? and if a reason, I suggest you replace by /ought/.
> ---
> 3.2.3
> - The following text seems odd, but perhaps you could refer to RFC5681 where this exact phrase is used?:
> /But receiving new packets from the peer does not guarantee peer's accessibility/
> - Maybe better as:
> /However, reception of new packets from the peer does not guarantee the peer's accessibility/
> - is this really /MAY/ - are we permitting something or saying it can be so?
> /Refer to Section 6.2 on the receiver behavior when it advertises a zero window./
> /Section 6.2 deescribes the receiver behavior when it advertises a zero window./
> 
> ---
> 3.21.2 (multiple)
> /When
> - (1) Does this mean for each packet? or for the entire asscociation or what?
> DSCP could result in different results, but this is more likely if the change
> modifies an AF class. It is probably safer to point to RFC7657 for specifics.
> - Please update this.
> ---
> 3.40.2
> - The update you note in the following two specific cases is not in the ID you cite, but in AccECN I think... where the change
> - Can one of the authors give me assurance that the code quoted is still correct at this time?
> ---
>