Re: [CCAMP] AD review of draft-ietf-ccamp-gmpls-vcat-lcas-11.txt

[Greg Bernstein <[e-mail]>]

Hi Adrian and all, implemented almost all suggested changes in revision 
12 which has just been posted. See disposition of your suggestions in 
line below.

Cheers
Greg B.

On 4/29/2011 11:53 AM, Adrian Farrel wrote:
> Hi,
>
> Don't panic!
>
> I have performed my AD review of your draft. The purpose of the review
> is to catch any nits or issues before the document goes forward to IETF
> last call and IESG review. By getting these issues out at this stage we
> can hope for a higher quality review and a smoother passage through the
> process.
>
> Thank you for a detailed and well-written draft. I have no technical
> issues with the content, but a number of small editorial comments and
> process-related questions below need to be thought about.
>
> All of my comments are up for discussion, and you should not feel rail-
> roaded into making changes. But I do think my comments need to be
> addressed before the draft moves forward, and I would like to see your
> answers to my points and/or a revised I-D.
>
> I have moved the draft into "AD-review:Revised-ID-needed" state in the
> datatracker, and I look forward to seeing the new revision which I can
> put forward for IETF last call.
>
> Thanks,
> Adrian
>
> ---
>
> The header correctly says "Updates: 4606", but the correct format of
> this statement is "Updates: 4606 (if approved)"
--> Fixed
> ---
>
> Since the document was first submitted before 10 November 2008, can
> the editor confirm that all authors have waived their pre-RFC5378
> rights?
>
> ---
>
> Abstract
> To be more clear about the update of 4606 and to remove the citation
> from the abstract, can we...
>
> OLD
>     This document updates the procedures for supporting virtual
>     concatenation in [RFC4606].
> NEW
>     This document updates RFC 4606 by making modifications to the
>     procedures for supporting virtual concatenation.
> END
>
--> Fixed with suggested text.
> ---
>
> In the Table of Contents, and in the section header...
>
> c/Author's Addresses/Authors' Addresses/
--> Fixed.
> ---
>
>
> Some acronyms need to be expanded on first use.
>
> TDM -->  fixed
> LSR -->  fixed
> LSP -->  fixed
> NVC -->  fixed
--> Fixed.
> ---
>
> Section 2.2
>
> Should you include a definition of "co-routed". This is not quite as
> obvious as it might seem since the meanings could be:
> - same routers in the same order (includes parallel links)
> - same logical links in the same order (includes bundles)
> - same data links in the same order (i.e. same physical interfaces)
>
> A bit of this comes out through careful reading of the different
> categories, so maybe it is not needed, but it might help.
>
--> Hmm, we only seem to use this term in the requirements section.  I 
meant to use it more in the
VCAT notion of signals going across the same nodes and similar links, 
i.e., not introducing huge differential delays.  The main point was to 
differentiate this from diversely routed (or partially diversely routed 
VCG members).
-->  Is there particular text you would like to see added?
> ---
>
> Section 2.2
>
> In "member sharing" are the members required to be co-routed or can
> they be diversely routed? I think either. Can you add a note?
--> Added the following text in the "fixed member sharing" definition: 
"Member signals need not be co-routed or be guaranteed to be diversely 
routed. "
> ---
>
> Section 2.3
>
>       .  GMPLS signaling for non-LCAS-capable interfaces MUST support
>          only the "fixed" scenarios of section 2.2.
>
> This appears to say:
>     MUST NOT support the "dynamic" scenarios.
> If you mean that, can you reword accordingly.
>
> On the other hand, it is also possible you mean:
>     MUST support the "fixed" scenarios and MAY also support the "dynamic"
>     scenarios.
--> Changed this to: ".    GMPLS signaling for non-LCAS-capable 
interfaces MUST support the "fixed" scenarios of section 2.2. "  The 
dynamic scenarios don't make sense without LCAS so the term "only" was 
confusing.
> ---
>
> Section 4
>
> I agree that it is right to include this section, but I would like you
> to make it clearer that 4606 is normative. How about...
>
> OLD
>     Note that this section is included for informational
>     purposes only.
> NEW
>     Note that this section is included for informational
>     purposes only and does not modify [RFC4606]. It is provided to
>     show how the existing GMPLS procedures may be used. [RFC4606]
>     provides the normative definition for GMPLS processing of VCGs
>     composed of a single member set, and in the event of any
>     conflict between this section and that document, [RFC4606]
>     takes precedence.
> END
>
-->  Used your suggested text.
> ---
>
> Section 4.3
>
>     Note if using LCAS, a VCG member can be temporary removed from the
>     VCG due to a failure of the component signal. The LCAS data plane
>     signaling will take appropriate actions to adjust the VCG as
>     described in [ITU-T-G.7042].
>
> s/temporary/temporarily/
--> Fixed
> ---
>
> Figure 1 caption
>
>      Figure 1 Figure 1. Conceptual containment relationship between VCG,
>          VCAT calls, control plane LSPs, and data plane connections.
>
> "Figure 1" appears twice
--> Fixed.
> ---
>
> Section 5.2
>
> Given that "Number of Members" is a two octet integer, you need to
> mention the byte order on the wire.
--> Hmm, RFC 4606 didn't specify the byte order of any of its 16 bit 
quantities.  Doesn't this flow from RSVP?  If not please suggest a 
reference for appropriate wording.
> ---
>
> Section 5.2
>
> I'm a bit surprised that you have used a whole 8 bits for "LCAS
> Required". Probably not important, but you could carve out some
> spare bits for future extensions without changing the format.
--> Fixed. Changed this to use only 2 bits, leaving 6 reserved.
> ---
>
> Section 5.2
>
> Should you also have a registry for the Elementary Signal Types listed
> in this section? it seems likely that there will be future additions to
> the list. You should say that unlisted values are reserved and you
> should say what the allocation policy is for future values.
>
> The same applies to "Action" since future actions might show up.
>
--> Sounds good.  Is there example text for this somewhere that I can 
leverage?
> ---
>
> Section 6 might need to note error cases for unknown "LCAS Required"
> setting (perhaps this can be noted as a malformed message and result in
> a standard result code?) and unknown or unsupported "Action" (I suggest
> that this is another instance for your list because some of the actions
> might not be supported by an implementation, and new actions may be
> defined.)
--> Sounds good. Added the following error codes:

Unknown LCR (LCAS required) value6

Unknown or unsupported ACTION7


> ---
>
> Section 7.1
>
> I suggest you remove the suggested value (2) from the document.
> Suggesting values is pretty risky for implementers and in this case
> your suggested value clashes with RFC 6004.
--> Removed.
> ---
>
> Section 8
>
> I suggest you add one additional paragraph to this section.
>
>     See [RFC5920] for additional information on GMPLS security.
>
> And (obviously) add an Informative Reference to 5920
--> Fixed.
> ---
>
> You can probably delete the final line...
>
> "Acknowledgment"
--> Fixed.
>


-- 
===================================================
Dr Greg Bernstein, Grotto Networking [phone]