Subject: Gen-ART LC/TC Review of draft-ietf-ccamp-pc-spc-rsvpte-ext-06

From: Ben Campbell <[email-masked]>
Date: Mon, 1 Feb 2010 16:56:20 -0600

I have been selected as the General Area Review Team (Gen-ART)
reviewer for this draft (for background on Gen-ART, please see
http://www.alvestrand.no/ietf/gen/art/gen-art-FAQ.html).

Please wait for direction from your document shepherd
or AD before posting a new version of the draft.

Document: draft-ietf-ccamp-pc-spc-rsvpte-ext-06
Reviewer: Ben Campbell
Review Date: 1 Feb 2010
IESG Telechat date: 4 Feb 2010

Summary: This draft is almost ready for publication as a draft standard, but  I think there are a few issues that need clarification first, as well as some editorial issues.

Note: I was assigned this draft to review for IETF last call, but let it slip through the cracks. This review servers as both a last call and telechat review. Apologies for any inconvenience this might have caused.

Major issues:

-- This draft defines two different procedures for accomplishing the same goal. While it describes them as primary and alternative, it is not clear to me if an implementation is expected to implement both of them, or whether they are intended to interoperate. It would also be helpful to have some guidance on when one might choose one over the other.

Minor issues:

-- section 4, 4th paragraph:

ben: When might it make sense to not include the labels? What are the consequences of not doing so? (i.e. can you motivate why is this a SHOULD and not a MUST?)

-- section 4.1, 3rd paragraph from end (last bullet list entry):

I'm surprised the notification requirement is only a "MAY". Is that per RSVP-TE in general, or unique to this scenario?

-- section 4.1, 2nd to last paragraph:

Why only "SHOULD release"? If they don't release state at this time, does it cause a problem?

-- section 4.2.1.1, 2nd to last paragraph:

Why only a SHOULD? Does it cause problems if the flag is not set?

-- section 4.2.2.3, case III: 

The "MAY generate" requirement seems to mean that, even if the RECOVERY LABEL is present, LSR B may or may not send a PathErr. Is this the intent? Is there any negative consequence of not sending PathErr if the label is present?

-- section 5, first paragraph:

Do you mean to make a normative requirement that implementations MUST support both methods? Are the methods intended to be interoperable? (Please excuse my lack of RSVP-TE knowledge--is it a matter of the ingress node selecting a method, and downstream nodes just do the right thing, or does each node have to understand the selected method?) How would an ingress node decide which method to use? (see "major issue" above)

Nits/editorial comments:

-- Abstract, first paragraph, first sentence:

I can't parse first part of sentence. Should "… connections are controlled …" be "…connections controlled…"? 

-- Abstract, end of last sentence: Should "...Data plane." be "...Data plane connection."?

-- section 1, first paragraph:

s/"… within Management Plane…"/"… within the [or 'a'] Management Plane… " 

-- section 1, third paragraph:

Please expand LSP on first mention.

-- fourth paragraph:

Please expand RSVP-TE on first mention.

-- section 4.1, 2nd paragraph:

ben: What is the antecedent for "it" in "... it supplies the exact path…"

-- 5th paragraph:

Please expand LSR on first mention.

-- section 4.2.1.1 and later:

Figure numbers would be helpful.

-- section 4.2.2.1, 1st paragraph, last sentence:

I didn't find details about PathErr construction in 4.2.1.1 (other than a requirement to send it)

-- 2nd paragraph:

I don't see any mention of PathTear in 4.2.1.1.

-- last paragraph:

s/PahTear/PathTear

-- section 4.4, Case I

The passive voice in "…deletion procedure MUST NOT be followed" obscures which node(s) you mean to forbid from following the procedure. I _think_ you mean that any node in the path MUST NOT…, correct?

-- section 7.1:

Is this intended to recommend IANA choose 25?

-- section 7.2:

Do you mean to recommend a code of 35?

-- section 8:

Last two sentences appear to have missing words.

-- sections 11 and 12

It seems unusual to find Security and IANA considerations after the contributors section. I'm not sure if that violates any policy or not, but I am afraid some readers may never read all the way to section 11.

-- idnits returns the following:

> idnits 2.12.00 
> 
> tmp/draft-ietf-ccamp-pc-spc-rsvpte-ext-06.txt:
> 
>   Checking boilerplate required by RFC 5378 and the IETF Trust (see
>   http://trustee.ietf.org/license-info):
>   ----------------------------------------------------------------------------
> 
>   == You're using the IETF Trust Provisions' Section 6.b License Notice from
>      12 Sep 2009 rather than the newer Notice from 28 Dec 2009.  (See
>      http://trustee.ietf.org/license-info/)
> 
> 
>   Checking nits according to http://www.ietf.org/ietf/1id-guidelines.txt:
>   ----------------------------------------------------------------------------
> 
>      No issues found here.
> 
>   Checking nits according to http://www.ietf.org/ID-Checklist.html:
>   ----------------------------------------------------------------------------
> 
>   ** There is 1 instance of too long lines in the document, the longest one
>      being 1 character in excess of 72.
> 
> 
>   Miscellaneous warnings:
>   ----------------------------------------------------------------------------
> 
>   == The document seems to lack a disclaimer for pre-RFC5378 work, but was
>      first submitted before 10 November 2008.  Should you add the disclaimer?
>      (See the Legal Provisions document at
>      http://trustee.ietf.org/license-info for more information.) -- however,
>      there's a paragraph with a matching beginning. Boilerplate error?
> 
> 
>   Checking references for intended status: Proposed Standard
>   ----------------------------------------------------------------------------
> 
>      (See RFCs 3967 and 4897 for information about using normative references
>      to lower-maturity documents in RFCs)
> 
>   == Missing Reference: 'RFC 3473' is mentioned on line 637, but not defined
> 
>   == Unused Reference: 'RFC3471' is defined on line 852, but no explicit
>      reference was found in the text
> 
> 
>      Summary: 1 error (**), 4 warnings (==), 0 comments (--).
> 
>      Run idnits with the --verbose option for more detailed information about
>      the items above.
>

---

From: Ben Campbell <[email-masked]>
Date: Tue, 16 Feb 2010 16:39:54 -0600

This is an update of my review of draft-ietf-ccamp-pc-spc-rsvpte-ext-06, based on the newly released version 07.

Updated Summary: Ready for publication. (I said draft standard the first time-but this is intended as a proposed standard, right?)

Major Issues: None
Minor Issues: None

> I can't parse first part of sentence. Should "… connections are controlled …" be "…connections controlled…"? 

No change--I still can't parse the first part of the sentence. Maybe the phrase "may independently exist" is the problem?
> I don't see any mention of PathTear in 4.2.1.1.

I think I meant to say the first paragraph, not the second. But I still don't see the term "PathTear" mentioned in 4.2.1.1.

> Is this intended to recommend IANA choose 25?

No change. You say the bit number is (TBD by IANA) (25). Is the 25 a recommendation? (in which case, "(TBD by IANA, 25 preferred)" would be helpful.

> Do you mean to recommend a code of 35?

No change--same as above.

Thanks!

Ben.