Subject: [tsvwg] SCTP Failover ( draft-ietf-tsvwg-sctp-failover-09.txt) - post WGLC

From: Gorry Fairhurst <[email-masked]>
Date: Mon, 02 Feb 2015 17:38:02 +0000

The above document passed WGLC with comments.

My Top-level view is that this is nearly done, and the expected protocol 
seems agreed by the WG during the WGLC (which is great).

However, after a detailed review, I note there are a number of NiTs, but 
more importantly I see ambiguity in the standards language.
I'm not at all sure after this careful review that this version is ready 
for a detailed IESG review. The later decision to make this PS probably 
avoided some detailed review of the standards language, which I believe 
really needs to be fixed.

My question is: would one of the authors be willing to take this 
editorial role and make an updated draft? - I include comments below to 
  suggest how to finish this process.  I've tried to make suggestions to 
ease this process.

I'd encourage you to make a new revision.

Gorry

----

I also want to check following the WGLC: I did not see a sentence 
explaining the answer to this:

"- Perhaps the authors can give me a hint why it should be RECOMMENDED 
to set the PFMR value to 0 when Quick Failover is used.  I would expect a
value > 0 when it is used and 0 when it is not(!) used."

----

/by introducing of/by introducing/
/These modes offers for/These modes can allow/
/more performance optimal operation/improvements in the performance of 
the operation/
/With procedures specified /With the procedures specified/

Abstract format: (editorial):
- normally this does not include Reference citations with square 
brackets, usual recommendation is to remove the square brackets. Please 
update.

Abstract: (editorial):
- The abstract uses the words /document/ and /memo/ both are acceptable 
in RFCs, but only one term should be used for consistency. The remainder 
of the ID appears to use the term "document".

Decision on "updates" status.
- The abstract specifies that it "compliments".  The IESG will require 
us to decide if this updates the SCTP base specification. Is this a 
change we expect all SCTP implementations to support (even if the do not 
enable), if so then I think this is an update to RFC4960. (If this 
updates, please an UPDATES tag to the ID boilerplate.) Community input 
on determining this is appreciated.

Introduction: (editorial):
The text reads:
"Concurrent Multipath Transfer (CMT) [IYENGAR06] is an extension to SCTP"
- I'd prefer this to say "proposed extension", since the current 
document is being published as a PS, it should not endorse or otherwise 
a proposal that has not yet reach standardisation.

(editorial):
" Also the operation after"
- could we remove the "also" to avoid starting a paragraph with this word?

Section 3: (editorial):
"and start using this address to send data to."
- may be clearer written as:
"and starts using this as the destination address for sending data."
---
(editorial):
"If the primary path becomes active again, SCTP uses the primary 
destination"
- could this be written as:
"If the primary path becomes active again, SCTP reverts to using the 
primary destination"
- or something similar?
---
(editorial):
"One issue with this failover process "
- can we ensure "this" is understood. Is it possible to say:
"One issue with this failover process defined in [RFC4960]"
---
Section 4.1:
"SCTP-PF as defined"
- I don't think this is clear what you refer to. Is this "SCTP-PF 
defined: somewhere else? This is confusing, the definition should be 
this document. If this is design rationale, I'd prefer to see these 
paras moved to section 3 as a conclusion? or to a separate non-normative 
section.
---
Section 4.1:
The para starting "SCTP-PF defined in this document"
- This is clear, and I think defines a new variable.
---
(editorial):
"and marks the corresponding destination address as PF"
- but later the ID uses the concept of a PF state. Is it equivalent to say:
"and marks the corresponding destination address as in the PF state"
---
Title:
4.2.  SCTP-PF Algorithm in Detail:
- If I understand this, this section contains the specification of the 
mechanism. To avoid ambiguity in what is standardised, could the name 
for this section becomesomething like:
"Specification of the SCTP-PF Algorithm"
---
(editorial):
Section 4.2:
"error counter of the destination address" - is "of" the correct word, 
should this be "for"?
---
Section 4.2: (editorial):
"or at times where a HEARTBEAT sent to an idle, active address is not 
acknowledged within an RTO."
- Unclear - is this
"or each time a HEARTBEAT chunk is sent when idle and not acknowledged 
within an RTO."
---
Section 4.2, point 2: (editorial):
- uses the term "destination transport address" in this place only, 
please explain why, or use the term "destination address".
---
Section 4.2, point 2: (editorial):
" MUST mark the destination transport address as PF."
- but later the ID uses the concept of a PF state. Is it equivalent to say:
" MUST mark the destination transport address as in the PF state."
---
Section 4.2, point 3: (editorial):
"The sender SHOULD avoid data transmission to PF destination addresses."
- can we reword this as:
"The sender SHOULD avoid data transmission to a destination address are 
in the PF state."
----
(editorial):
"and some in inactive state"
- insert "the" before "inactive"
---
(editorial):
"The sender SHOULD choose the destination address in PF state"
- insert "the" before "PF"
---
Format hard to follow:
Section 4.2, point 3:
I think this section contains lots of conditions which are a set, would 
it be possible to format as a second-level list to make this clearer?
----
Section 4.2, point 3: (standards language):
- So this section makes lots of SHOULD recommendations without really 
saying why they are not MAY or MUST. In particular, there are no notes 
about what happens if the SHOULD is violated. This sort of text tends to 
raise questions in IETF/ISEG review. Can the authors please clarify?
 From reading, I could interpret this as:
  "The sender SHOULD avoid data transmission to a destination address 
are in the PF state. (To avoid sending to potentially failed destination 
addresses).

"The sender SHOULD choose the destination address in PF state. .... 
(This is to maximise the probability of delivery)"
---
Section 4.2, point 3: nOt sure what this says:
"A sender may choose to deploy other strategies than the above when 
choosing among
  multiple PF destinations with equal error count."
- Trying to unwrap the logic here, I want to be clear whether this is a 
MAY that contradicts the SHOULD in the preceding clauses? - I am not yet 
clear whether this overrides the SHOULD before - this needs clarified.
---
Section 4.2, point 3: (editorial):
"MUST NOT change the state of chosen destination address "
- what state does this refer to?
---
  (standards language):
"If a HEARTBEAT chunk is not acknowledged, the sender SHOULD increment 
the error counter"
- I do not understand the SHOULD, are there any conditions under which 
the count needs to not be implemented. Can I challenge whether this 
needs to be "MUST increment"?
---
  (standards language):
"and exponentially back off the RTO value."
- Again, I'd have thought for stability in congestion collapse this was 
a "MUST"?
---
(editorial):
"packet containing HEARTBEAT chunk immediately"
- insert "the" before "HEARTBEAT".
---
(editorial):
"When data is transmitted to a PF destination,"
- Is this a "destination marked in the PF state"?
---
(editorial):
"the transmission of HEARTBEAT chunk"
- insert "a" before "HEARTBEAT".
---
(editorial):
"as receipt of SACK chunks or a T3-rtx timer expiration"
- the word "as" is problematic, is this intended to be "when"? - this 
clause needs some additional work to clarify the condition.
---
(editorial):
"It is RECOMMENDED that HEARTBEAT chunks are send"
- "send" needs to be "sent".
- This is equivalent to a SHOULD, so please rephrase with this and 
include a clause in brackets to explain why this is not a MUST.
---
(editorial):
Point 6:
"SHOULD notify ULP"
- insert "the" before "ULP" and expand acronym "Upper Layer Protocol (ULP)".
---
(standards language):
Point 7:
"When all destinations are in inactive state (association dormant state) 
the sender MUST also choose one destination address to transmit data to."
- This point does not say the reason why it transmits.
---
(format):
see note on earlier section - it would be easier to read this as a 
second level indent/bullet list.
---
(standards language):
"thus changing the prescriptions of"
- "I think the correct term is "updates" - but see the question at the 
start of this review.
---
(standards language):
"A sender MAY choose to deploy other strategies than the above.  "
- as above, there are standards requirements above stated as "SHOULD" 
you can not then propose an alternative with a "MAY" that appears to 
contradict. Reformatting as a bullet list could resolve this issue!
---
(editorial):
"the Potentially Failed state"
- other places called the PF state, suggest you us the same term.
---
"destination address to which the chunk was first transmitted)
         SHOULD NOT clear the error count of an inactive destination
         address and SHOULD NOT transition a PF destination address back
         to Active state, since a sender cannot disambiguate whether the
         ACK was for the original transmission or the retransmission(s)."
- I do not understand why these are not required using a MUST - what are 
the safe cases for not following these rules? ... or at least this needs 
to be restructured, for example to explain a sender may have unequivocal 
information to know that a particular destination becomes available, in 
which case it can...; etc ; If this information is not available it MUST 
; etc.
- The present MAY clause at the end of the point is problematic and 
needs to resolved by restructuring the clause. (Formatting with indent 
would help).
---
(editorial):
" The implementation of such an alternative approach is left to 
implementations."
- perhaps should be
" The design of such an alternative approach is left to implementations."
---
(Standards language):
Point 10:
"SCTP stack SHOULD "
- insert "The" before SCTP"
- Is this a sender or receiver function? (sender?)
- This appears to be a requirements recommendation, so you could use the 
word "RECOMMENDS" rather than SHOULD?

----
Section 4.3
- Para 1 and 2 appear to provide informational rationale that could have 
appeared in an earlier informational section - at the moment there is 
quite a lot to read through before you get to the Spec.
---
Section 4.3 para 2 ends with a requirements statement that I do not 
understand.
"
However as [RFC4960] switchback
    behavior is suboptimal in certain situations, especially in scenarios
    where a number of equally good paths are available, it is recommended
    for SCTP to support also, as alternative behavior, the Permanent
    Failover switch over modes of operation."

- what is the intended standards compliance statement at the end?
- Is this an OPTIONAL part of the specification in the document?
- IF it is optional: Is it required to offer this optional to implement 
the spec. (or is it optional to implement? - I think the latter?
- Can this be re-written clearly... e.g. "A sender that implements the 
OPTIONAL
Permanent Failover function implements the functions below:..."
---
(editorial):
"
   A previously failed primary path MAY come in use as data transfer
        path as per normal path selection when the present data transfer
        path fails."
- is this really a "MAY come in use", I think this could be a "can be used".
- insert "a" or "the" before "data"
---
(standards language):
" operation as detailed above and MAY enable it based on network
    configurations or users' requests."
- This "MAY" is not a protocol specification. The permission to do it 
this way is give in the previous MAY. Now, can you check this MAY does 
not contradict the previous SHOULD - maybe a small text change can 
ensure this does not conflict?
---
(incomplete security considerations):
Section 6
- I'm sure the security area review will ask whether there are really no 
potential new security vulnerabilities. Consider carefully. I'd 
recommend stating here that this is a sender-side only change enabled by 
configuration. Received SCTP chunks used by this protocol are verified 
as specified in RFC 4960, and include protection from off-path security 
attacks. There are no new security considerations introduced in this 
document.