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 <elw...@folly.org.uk> 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.
- 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.
>
> >
> > 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.
>
> >
> > 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.
>
> >
> > 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.
> >
> >
> > 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.
> >
> > 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.
>
>
> >
> > 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.
>
> >
> > 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.
> >
> > 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'.
> >
> > 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.
> >
> > 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.
> >
> > 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".
> >
> >
> >
_______________________________________________
Gen-art mailing list
Gen-art@ietf.org
https://www.ietf.org/mailman/listinfo/gen-art
