Last Call Review of draft-ietf-pwe3-cbit-negotiation-
review-ietf-pwe3-cbit-negotiation-genart-lc-davies-2012-07-05-00

I am the assigned Gen-ART reviewer for this draft. For background on 
Gen-ART, please see the FAQ at 
<

http://wiki.tools.ietf.org/area/gen/trac/wiki/GenArtfaq>. 

Please resolve these comments along with any other Last Call comments 
you may receive. 

Document: draft-ietf-pwe3-cbit-negotiation-04.txt
Reviewer: Elwyn Davies
Review Date: 19 June 2012
IETF LC End Date: 15 June 2012
IESG Telechat date: 21 June 2012

Summary: 
Not ready. The proposal for the NOT PREFERRED to PREFERRED transition
case does not appear to be compatible with the existing RFC 4447
standard in the way stated and there are a number of other minor issues.
The draft is also in need of an editing pass by an author whose mother
tongue is English as there are parts where the syntax is misleading
rather than just clumsy.

Apologies for the late submission of the review.

Major issues: 
s3: The discussion of the NOT PREFERRED to PREFERRED transition case
(buried at the end of s3 - causing me to ask 'what about this case?'
during reading of s2 and most of s3)  implies that some existing RFC
4447 procedure applies. Clearly, if the PW is not using the control word
then there is nothing to do.  On the other hand, inspection of s6.2 of
RFC4447 indicates that once the two PEs have agreed on c = 1, 'setup is
complete' and Label Mapping messages would therefore be
'unexpected' (see item '-i' in second set of bullets in s6.2 of RFC
4447). So, what procedure is to be used? And what implications does this
have for backwards compatibility?  Wouldn't it be generally simpler to
apply the PREFERRED to NOT PREFERRED mechanism to all case?

Minor issues: 
s3: Has there been any discussion on possible race conditions?  Changing
the configuration value during the message exchange strikes me as
dangerous - it is probably sufficient to note that changes should be
suppressed during the Label Mapping message exchange but I am not
totally sure about this.

s3, bullet '-i':  I completely misparsed this section on first reading
and I am still not absolutely sure what message sequence is being
specified.  Working back from later sections I *think* that the
intention is:
IF Mapping sent THEN { send Withdraw; send Release;}
Wait to receive Release
The implication at present is that a Mapping might not have been sent
and then only a Release is needed: is this a possibility? Please
clarify.
The picture in Appendix A suffers from the same problem.

s3, discussion of multi-segment PWs: The statement that S-PE's SHOULD
assume an initial passive role seems to have several problems:
- Does this mean that changing the configuration of an S-PE would not
provoke the new mechanisms?
- The passive role situation is only specified for some sorts of linked
FECs in RFC 6073 - what about other cases?
- What are the consequences for ignoring the SHOULD in this case? (I
have to say I am unsure that RFC 6073 deals with this problem either.)


Nits/editorial comments: 
General: In RFC 4447, the label message names use title case (e.g.,
Label Mapping).  For consistency this should be followed throughout this
document.
 
Abstract:  Should not contain references.  Best to give full title of
RFC and number in round brackets.
Abstract: s/problem of control/ the problem with control/ (if there is
just one)
Abstract: The second two sentences say the same thing in different ways 
OLD
   Based on the problem analysis, a
   message exchanging mechanism is introduced to solve this control word
   negotiation issue.  This document is to update [RFC4447] control word
   negotiation mechanism.
NEW
Based on the problem analysis, this document introduces a modified message exchange 
sequence updating the control word negotiation mechanism in RFC 4447.

This issue also applies to s1.

s1: Subtle distinction - this is a practical problem with the mechanism defined in RFC 4447 rather 
than the abstract problem of how to do control word negotiation:
 s/the problem of control word negotiation/problems identified in the control word negotiation/

s1: Expand acronym PW.

s2: Expand acronym PE.
s2, 2nd sentence: s/configurable/configured/
s2, 3rd and 4th sentences: I *think* this text is trying to say: 
  The intention of the control word negotiation is that the control word
will be used when both endpoints are configured with control word usage
PREFERRED.  However if one endpoint is initially configured with control
word usage NOT PREFERRED but later changes to PREFERRED, a PW between
the endpoints will not transition to usage of the control word as
explained below. 

s2, bullet #2: s/PE1 send label mapping with C bit=0 finally./ultimately
PE1 sends a Label Mapping message with the C bit set to 0./

s2, bullet #4: s/send label withdraw/send a Label Withdraw/

s2, bullet #5: s/the received/the previously received/, 
s/indicates C bit=0/carried the C bit set to 0/, 
s/send label mapping with C bit=0/send a label mapping message with the
C bit set to 0/

s3: It would be much clearer if s3 was divided into 3 sub-sections
(possibly reordered):
- PREFERRED to NOT PREFERRED transition
- NOT PREFERRED to PREFERRED transition
- Multi-segment case (which should refer to both previous cases)
The pointer to the diagram in Appendix A could usefully occur in the
introduction to s3.  If this was adopted s3.1 could either be a fourth
sub-section or a sub-sub-section of the PREFERRED to NOT PREFERRED
section.

s3, Various acronyms need expanding: FEC, T-PE, S-PE

s3, para 1: s/adding label request/adding a new label request/

s3 bullets '-i' to '-iii': s/Local PE/The local PE/, s/remote PR/the
remote peer PE/.  Also a more usual labeling of the bullets would be
appropriate (e.g., just i, ii and iii).

s3, para 2: s/When Local/When the local/,
s/the remote label mapping message with C bit=0, additional procedure
will be added as follow:/a label mapping message from the PW peer with
the C bits set to 0, the following additional procedure will be acrried
out:/
 
s3, bullet '-i': s/wait until receiving a label release/wait until it
has received a Label Release message/ (but general clarification and
rewording needed - see issues section above).

s3, bullet '-ii':
OLD:
      Local PE MUST send a label request message to remote PE, and
      wait until receiving a label mapping message containing the remote
      PE locally configured preference for use of control word.
NEW:
      The local PE MUST send a Label Mapping message to the peer PE with the
      C bit set to 1, and then wait until a Label Mapping message is received 
      containing the peer's current configured preference for usage of the 
      control word.

s3, para sfter item '-iii': s/successfully/has successfully/
The following implies that there is memory of previous action although
the label binding has been destroyed. Better would be:
OLD:
>    ...and removed the remote label binding, it MUST reset
>    its use of control word with the locally configured preference, and
>    send label mapping as a response of label request with locally
>    configured preference for use of control word.
NEW:
...and removed the remote label binding, subsequent Label Requests will
naturally be treated as new requests and processed as described in
Section 6 of RFC 4447.

s3, discussion of multi-segment PWs: s/case for T-PE is same./case for a
T-PE operates similarly./

s3, last para/Appendix A:  The diagram doesn't cover the PREFERRED to
NOT PREFERRED transition.

s3.1: The wording in this section is generally  rather 'loose'.  A more
formal style would be helpful.