Re: [Gen-art] Gen-art last call review of draft-ietf-pwe3-cbit-negotiation-04

[Lizhong Jin<[e-mail]>]

Hi Elwyn,
Thank you for the prompt reply. See inline below.

Lizhong
 

Elwyn Davies <[e-mail]> wrote 2012/06/19 23:16:08:

> Hi, Lizhong.
> 
> On Tue, 2012-06-19 at 21:48 +0800, Lizhong Jin wrote:
> > 
> > Hi Elwyn, 
> > Thank you for the review. It seems you are reviewing v-03, not v-04,
> > but some comments are for v-04. I am confused.
> Yes, I started reviewing v03 but had to stop because of other tasks and
> came back to v04.  I though I had revisited all my old commnets and
> updated them, but clearly not.
> >  See my reply inline below. 
> > 
> > Regards 
> > Lizhong 
> > 
> > 
> > Elwyn Davies <[e-mail]> wrote 2012/06/19 20:12:17:
> > 
> > > 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. 
> > [Lizhong] We have two editors, and both Raymond and I are editing this
> > document. Hope the below reply will make it clear.
> 
> > > 
> > > 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? 
> > [Lizhong] this draft is to solve the case to change a PW from c=0 to
> > c=1, that means one PE should change its use of control word from NOT
> > PREFERRED to PREFERRED. RFC4447 is already there and deployed, and the
> > PE will not always send its locally configured preferrence according
> > RFC4447, see PE1 behavior in section 2 step 1&2. That's why we could
> > not simply apply the mechanism at the end of section 3. Hope it is
> > clear. 
> There are two issues here:
> - Clarity: RFC 4447 does not have any discussion of what might happen if
> the configuration value changes.  The draft focusses on on transition
> direction but does not mention the other except for one paragaph right
> at the end of s3.  It is therefore reasonable that somebody looking at
> this draft would wonder 'what about the other transition direction?'.
> Whether or not anything needs to be done it would save people wondering
> if you put in a sentence in the introduction to explain that the other
> direction is not (or is) a problem.
[Lizhong] accept. Add following:
When PE changes the preference for the use of control word from PREFERRED 
to NOT PREFERRED, it should follow [RFC4447], and there is no problem.

> - Technical: The paragraph in s3 implies that the PREFERRED to NOT
> PREFERRED direction will result in some part of the RFC 4447 protocol
> being (re-)invoked.  What part is not made very clear.  As far as I can
> see sending more messages other than Label Release or Label Withdraw for
> this PW is not part of the RFC 4447 protocol and hence will cause an
> error.  Alternatively if it isn't reinvoked, the PW will continue to use
> control words.  Please explain what is going on here.  The fact that RFC
> 4447 has been deployed doesn't explain what is going on.
[Lizhong] now I understand you concern. The last sentance of s3 is not 
clear. How about the following:
In that case, local PE will always send Label Withdraw message if already 
sending Label Mapping message, and then send new Label Mapping message 
with C-bit value following the procedures defined in [RFC4447].

> > 
> > > 
> > > 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. 
> > [Lizhong] The message would be processed in sequence in TCP-based LDP
> > session. I do not see a problem here. But we do have a note in section
> > 3 for multi-segment PW for sequence processing. 
> There are several network round trips in the message exchanges.  The
> protocol and the configuration mechanism in a PE are potentially
> separate threads.  There is scope, depending on the implementation, for
> the user at the 'remote' PE to change the configuration during the
> message exchange making for a potential race condition. 
[Lizhong] we discussed the race condition on the PWE3 maillist from Spike, 
and for multi-segment PW, we add a note to ensure the sequence processing 
for implementation.

> > 
> > > 
> > > 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. 
> > [Lizhong] Adrian raise the same comment, and I change it as below: 
> >        -i.  PE MUST send a Label Release message to remote PE. If a
> > PE 
> >             has previously sent a Label Mapping message to a remote
> > PE, 
> >             it MUST also send a Label Withdraw message to the remote
> > PE, 
> >             and wait until it receives a Label Release message from
> > the
> >            remote PE. 
> What does it send first if both must be sent?  The text in the rest of
> the document implies withdraw then release.  This new text sort of
> implies release then withdraw but isn't really clear.
[Lizhong] both should be sent, and does not require the sending sequence. 
The two messages does not have dependence.

> > 
> > > 
> > > 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.) 
> > [Lizhong] we follow RFC6073, and passive role is only applied for the
> > PW FEC, other cases are out of scope. 
> Why? This needs an explanation. Also this doesn't cover the point about
> whether reconfig of an S-PE is allowed and how this squares with the
> passive role.
> > SHOULD means highly recommended. We do not meet any problem when
> > implementing RFC6073. Do you suggest to change this to MUST? 
> Whenever a specification includes a 'SHOULD' the question arises of what
> alternatives there might be and what is the reason for preferring the
> suggested solution.  In the original setup, it is fairly clear that life
> is easier letting setup progress from  one end to the other (although
> this is not spelt out - I would have argued for this had  I reviewed the
> doc).  For the configuration change this is less obvious.  Regarding the
> point about reconfig of the  S-PE, it is going to make life more
> complicated if the S-PE has to tell the T-PE to be active so it can be
> passive.  I would therefore argue that probably the notion of passivity
> is irrelevant to the transition use case.  Whether it should be SHOULD
> or MUST is therefore moot.
[Lizhong] how about the following? Because we just refer to RFC6073, and 
does not add anything new.
An initial passive role is defined in [RFC6073] for S-PE.

> 
> > > 
> > > 
> > > 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. 
> > [Lizhong] yes, and thank you and Adrian.
> > > 
> > > Abstract:  Should not contain references.  Best to give full title
> > of
> > > RFC and number in round brackets. 
> > [Lizhong] I removed brackets and do not intend to contain citations
> > here. It seems the citations are generated automatically by IETF.
> > Could the round brackets solve this? Or are you referring v-03? 
> (leftover from 03)
> > 
> > > Abstract: s/problem of control/ the problem with control/ (if there
> > is
> > > just one) 
> > [Lizhong] are you reviewing v-03 of the draft? In v-04, the abstract
> > has been revised in v-04.
> (leftover from 03)
> > 
> > 
> > > 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. 
> > [Lizhong] are you reviewing v-03 of the draft? In v-04, the abstract
> > has been revised in v-04.
> (leftover from 03)
> > 
> > 
> > > 
> > > 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/ 
> > [Lizhong] has been revised in v-04.
> > > 
> yes.
> > > s1: Expand acronym PW. 
> > [Lizhong] has been revised in v-04.
> Not as far as I can see.
[Lizhong] write "pseudowire" directly in the text, not PW.

> > > 
> > > s2: Expand acronym PE. 
> > [Lizhong] accept 
> > 
> > > s2, 2nd sentence: s/configurable/configured/ 
> > [Lizhong] configurable is right. 
> Leave that one to the RFc Editor.
> > 
> > > 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. 
> > [Lizhong] no, the case is that operator deploys PW with control word
> > used in the first phase. In the second phase, they want to upgrade
> > their PW service to use control word. Two different deployment
> > timeframes.
> That is what my sentence says.
[Lizhong] ok.

> > 
> > 
> > > 
> > > 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./ 
> > [Lizhong] accept
> > > 
> > > s2, bullet #4: s/send label withdraw/send a Label Withdraw/ 
> > [Lizhong] accept
> > > 
> > > 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/ 
> > [Lizhong] accept
> > > 
> > > 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. 
> > [Lizhong] PREFERRED to NOT PREFERRED does not introduce any new.
> > Multi-segment case is fully inherited from single-segment case. It
> > would be redundancy to have sub-sections here. 
> I disagree strongly.  The multi-segment case affects the other RFC and
> needs to be made to stand out so that implementors can see where the
> changes occur.  The PREFERRED to NOT PREFERRED case also ought to be
> separated whether or not I am right about this case. 
[Lizhong] how about only divide multi-segment? The case of PREFERRED to 
NOT PREFERRED case is only for reference, and the text is not many.
> > 
> > > 
> > > s3, Various acronyms need expanding: FEC, T-PE, S-PE 
> > [Lizhong] accept
> > > 
> > > s3, para 1: s/adding label request/adding a new label request/ 
> > [Lizhong] much difference here?
> Yes. How it reads in English.  There are many missing definite articles
> and indefinite articles that make the text read very badly for a native
> English speaker.  I understand that this is a very difficult aspect for
> someone who has a first language that does not employ articles in the
> way that English does.
[Lizhong] ok
> > > 
> > > 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). 
> > [Lizhong] accept, and I do not see "remote PR".
> should have been 'remote PE'.
[Lizhong] ok
> > > 
> > > 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:/ 
> > [Lizhong] accept
> > > 
> > > 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). 
> > [Lizhong] do not see much difference here. Of couse the "label
> > release" should be capitalized.
> Your form implies that that the action can occur as soon as the message
> starts to be received (as in 'the radio is receiving a programme').
> This is not the case - the action can occur only when the whole message
> has been received. 
[Lizhong] ok, thanks.

> > > 
> > > 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
> > isreceived 
> > >       containing the peer's current configured preference for usage
> > of the 
> > >       control word. 
> > [Lizhong] no, the procedure here is not right. The local PE MUST send
> > a label request message, not label mapping.
> Try:
> The local PE MUST send a Label Request message to the peer PE  and then
> wait until a Label Mapping message is received containing the peer's
> current configured preference for usage of the control word. 
[Lizhong] ok

> > > 
> > > s3, para sfter item '-iii': s/successfully/has successfully/ 
> > [Lizhong] accept 
> > 
> > > 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. 
> > [Lizhong] we got similar comments from Adrian, and will update this. 
> > 
> > > 
> > > s3, discussion of multi-segment PWs: s/case for T-PE is same./case
> > for a
> > > T-PE operates similarly./ 
> > [Lizhong] accept
> > > 
> > > s3, last para/Appendix A:  The diagram doesn't cover the PREFERRED
> > to
> > > NOT PREFERRED transition. 
> > [Lizhong] no, there is a "no" branch under "NOT Preferred to
> > Preferred" to cover "PREFERRED to NOT PREFERRED". Anyway, the diagram
> > discription should be "NOT Preferred to Preferred" which is wrong in
> > v-04. 
> > 
> > > 
> > > s3.1: The wording in this section is generally  rather 'loose'.  A
> > more
> > > formal style would be helpful. 
> > [Lizhong] this is a usecase to give an example, so we make it "loose".
> > > 
> > > 
> > > 
> 
>