Re: [tsvwg] Publication has been requested for draft-ietf-tsvwg-rfc4960-errata-05

[Michael Tuexen <[e-mail]>]

> On 11. May 2018, at 22:12, Spencer Dawkins at IETF <[e-mail]> wrote:
> 
> Dear Authors,
> 
> On Wed, Apr 18, 2018 at 1:50 AM, Gorry Fairhurst <[e-mail]> wrote:
> Gorry Fairhurst has requested publication of draft-ietf-tsvwg-rfc4960-errata-05 as Informational on behalf of the TSVWG working group.
> 
> Please verify the document's state at https://datatracker.ietf.org/doc/draft-ietf-tsvwg-rfc4960-errata/
> 
> Thanks for doing this work (and even more so, for being willing to provide an updated RFC to implementers, at some point in the future). 
> 
> I've completed AD evaluation for this draft, and have comments, but almost all of them are requests for clarifications. 
> 
> I'd like to work through them with you before requesting Last Call. Please let me know if you have questions.
Hi Spencer,

thanks for the review. I'll provide feedback inline.

Please note that I was also notified recently about a paragraph which should
have been removed when moving from RFC 2960 to RFC 4960, but the removal was
missed and the text is still there. I have received this not via any mailing
list but via direct e-mail conversation. The fix is included as
https://github.com/sctplab/rfc4960bis/commit/27c8609a1d57d990fc439ea5f283ab5e82d4fc9c

You can see the current version in the git repository of the document in
http://xml2rfc.ietf.org/cgi-bin/xml2rfc.cgi?input=&url=https%3A%2F%2Fraw.githubusercontent.com%2Fsctplab%2Frfc4960bis%2Fmaster%2Fdraft-ietf-tsvwg-rfc4960-errata.xml&modeAsFormat=html%2Fascii&type=towindow&Submit=Submit

If you want additional changes, please let me know. If you are fine with the current
version in the git repo, I can submit it anytime...

Best regards
Michael
> 
> Thanks!
> 
> Spencer 
> 
> A high-order bit here ... 
> 
> I'm not sure why this draft isn't standards-track, and I wonder if there's a reason it doesn't UPDATE RFC 4960, unless that's a side effect of being an Informational draft that would update a standards-track RFC. 
> 
> I'm thinking that this draft has achieved WG consensus, and if it's published after Last Call, it would have IETF consensus, and it's been reviewed by implementers. We've certainly published Proposed Standards that didn't measure up to that level of document quality.
> 
> I'm not objecting strongly to publishing as Informational, but I am saying that I expect other ADs to ask that question during IESG Evaluation, and I'd like to understand the thinking before someone asks …
That is actually a good question and it would be a possible way forward.

When being in the process updating RFC 2960 to RFC 4960 we were in the same situation. At that time we wrote
RFC 4460. We thought it is a good idea to not only publish RFC 4960 and let people figure what changed and
why, but document this. That is why we have RFC 4460. We also thought that it is a good idea to just have a
single specification for implementing the base protocol. If we would have made RFC 4460 a PS updating RFC 2960,
and implementer would have to read two RFCs. That is why we decided that it is simpler to publish RFC 4460 as
an informational RFC, and then publish RFC 4960 as a PS updating RFC 2960. As you see from the numbers, working
on RFC 4460 took a while, but once that was there, publishing RFC 4960 was straight forward. Just do the editorial
work described in RFC 4460 and some polishing of text we missed when working in the diff way...

Since this plan worked well, the implementers of SCTP (which were also at that time) are used to it, we though
it is a good way to handle the errata in RFC 4960.
> 
> I note without suggesting a change that if the material in Section 3 was presented in this order
> 
> 3.n.1   Description of the Problem
> 
> 
> 3.n.3.  Solution Description
> 
> 3.n.2.  Text Changes to the Document
> 
>    ---------
>    Old text:
>    ---------
> 
>    ---------
>    New text:
>    ---------
> 
> to allow readers to see the solution description before reading the detailed text changes, that would likely be easier to parse ...
That is definitely doable and I'm willing to do that change. However, this document right now uses the
same structure as RFC 4460 and I would prefer to keep the consistency.
> 
> I'm reading this text from the Abstract 
> 
>    Because some text is changed several times the last delta in the
>    text is the one which should be applied.  In addition to the delta a
>    description of the problem and the details of the solution are also
>    provided.
> 
> as saying that the deltas are cumulative, so you should apply the last change to specific text, but this text from Section 1 
> 
>   Note that when reading this document one must use care to assure that
>    a field or item is not updated further on within the document.  Each
>    section should be applied in sequence to the original [RFC4960] since
>    this document is a historical record of the sequential changes that
>    have been found necessary at various inter-op events and through
>    discussion on the list.
> 
> seems to say that you should apply multiple deltas to the same text sequentially. IIUC, the text actually does provide cumulative text deltas, so using the phrasing from the Abstract in both places would be helpful to the reader.
OK. Changed to

   Note that when reading this document one must use care to assure that
   a field or item is not updated further on within the document.  Since
   this document is a historical record of the sequential changes that
   have been found necessary at various inter-op events and through
   discussion on the list, the last delta in the text is the one which
   should be applied.

in https://github.com/sctplab/rfc4960bis/commit/b713055804f0ff26da9bb07bf9ac382ef3f6b434

> 
> This is a nit, but why is "Inconsistent" capitalized in this text?
> 
>   The handling of the 'Path.Max.Retrans' parameter is described in
>    Section 8.2 and Section 8.3 of [RFC4960] in an Inconsistent way.
Because I (or some autocorrection function of my editor) made a mistake.
Fixed in: https://github.com/sctplab/rfc4960bis/commit/12525d23e2ad49f84443e1a4d52a7ed1f90502a9
> 
> https://tools.ietf.org/idnits?url=https://tools.ietf.org/id/draft-ietf-tsvwg-rfc4960-errata-05.txt reports a few problems with references. I THINK this is a side effect of updating (for example) [RFC2434] to be [RFC5226], and then updating [RFC5226] to be [RFC8126]. If this was my draft, I'd put a note to the RFC Editor that obsoleted RFCs are used in "OLD TEXT" sections, and should have the obsoleting RFCs in the "NEW TEXT" sections, so multiple reviewers won't ask you about the IDNIT reporting multiple times.
After updating the document to reduce IDNITs warnings regarding too long lines in
https://github.com/sctplab/rfc4960bis/commit/8c2afb32018d7886c7540fb804a32ecdd00626d5
and cleaning up the references in
https://github.com/sctplab/rfc4960bis/commit/84700f46033b5f0a833a13398dfc47766256606e
I added the now true Note to the RFC Editor in
https://github.com/sctplab/rfc4960bis/commit/a8e60e5b230ad24cd79bb807e732ec8a5208de57
> 
> I'm probably due for an eye exam, but I'm not seeing any difference between 
> 
>   ---------
>    Old text: (Section 1.6)
>    ---------
> 
>    Transmission Sequence Numbers wrap around when they reach 2**32 - 1.
>    That is, the next TSN a DATA chunk MUST use after transmitting TSN =
>    2*32 - 1 is TSN = 0.
> 
> and
> 
>    ---------
>    New text: (Section 1.6)
>    ---------
> 
>    Transmission Sequence Numbers wrap around when they reach 2**32 - 1.
>    That is, the next TSN a DATA chunk MUST use after transmitting TSN =
>    2**32 - 1 is TSN = 0.
> 
> What am I missing?
The difference between "2*32" and "2**32" (the second occurrence in the OLD and NEW text)...
Whether this should trigger an eye exam or not is left to the reader.
> 
> In this text, 
> 
>   Move the sample code related to CRC32c computation and its
>    explanation from the end of Appendix C to the end of Appendix B.
> 
> I'm pretty sure I can figure out what text/code actually moves from Appendix C to Appendix B, but I am figuring that out myself. Perhaps it would be clearer if you said something like "all of Appendix C starting with this sentence to the end of Appendix B".
> 
>    The following non-normative sample code is taken from an open-source
>    CRC generator [WILLIAMS93], using the "mirroring" technique and
>    yielding a lookup table for SCTP CRC32c with 256 entries, each 32
>    bits wide.
> 
> (If I guessed wrong, that's likely because the fix says "code and its explanation", which sounds like "everything following the beginning of the code", but the explanation comes before the code)
You guessed perfectly right and I added the suggested text:
https://github.com/sctplab/rfc4960bis/commit/62674932f28950cf249cdfb2c51534ede667111d
> 
> If I'm following closely, the errata described as 
> 
>   Section 7.2.2 of [RFC4960] is unclear about the order of adjustments
>    applied to partial_bytes_acked and cwnd in the congestion avoidance
>    phase.
> 
> is actually clear about the order - assuming that you read left to right, it's just clearly wrong :-)
I see. But there is no explicit statement that you have to do it in that order. 

What about:

Section 7.2.2 of [RFC4960] likely implies the wrong order of adjustments
applied to partial_bytes_acked and cwnd in the congestion avoidance
phase.
which is committed in
https://github.com/sctplab/rfc4960bis/commit/9fa55976e242ff1e5125637336321887a2f57e80
> 
> At the risk of asking a technical question ;-)
> 
>   The counter SHOULD be reset each time a DATA chunk sent to that peer
>    endpoint is acknowledged (by the reception of a SACK). When a
>    HEARTBEAT ACK is received from the peer endpoint, the counter SHOULD
>    also be reset. The receiver of the HEARTBEAT ACK MAY choose not to
>    clear the counter if there is outstanding data on the association.
>    This allows for handling the possible difference in reachability
>    based on DATA chunks and HEARTBEAT chunks.
> 
> why is not clearing the counter if there is outstanding data on the association a good idea? (this was "shall", but is now "SHOULD")
You are correct. It should be a MUST.
Changed in: https://github.com/sctplab/rfc4960bis/commit/8ad403dd7e8102d1180b7d0363e3539637f9dafe
> 
> I have a similar (MUST vs. SHOULD) question about 
> 
>   Upon the receipt of the HEARTBEAT ACK, the sender of the HEARTBEAT
>    SHOULD clear the error counter of the destination transport address
>    to which the HEARTBEAT was sent, and mark the destination transport
>    address as active if it is not so marked.
> 
> Why would not clearing the error counter be a good idea?
You are right. The "should" should be changed to a "MUST", not a "SHOULD. Done in:
https://github.com/sctplab/rfc4960bis/commit/89446608b675e4d22cb4fe12a12c5a67a1b6faeb
> 
> Section 3.21 has multiple occurrences of something like 
> 
>   o  partial flag - if this returned flag is set to 1, then this
>       Receive contains a partial delivery of the whole message.  When
>       ^ this "Receive" is capitalized
> 
>       this flag is set, the stream id and Stream Sequence Number MUST
>       accompany this receive.  When this flag is set to 0, it indicates
>                      ^ this "receive" is not capitalized
> 
>       that no more deliveries will be received for this Stream Sequence
>       Number.
> 
> Is that intentional?
No. This is even not changed by this documents, but also inconsistent in RFC 4960.
I also improved the consistency of the spelling:
However, I improved the text:
https://github.com/sctplab/rfc4960bis/commit/2e8adee110f3527bc3372c2f0dc6fae72eaf1751