Last Call Review of draft-ietf-ccamp-flexible-grid-ospf-ext-07
review-ietf-ccamp-flexible-grid-ospf-ext-07-genart-lc-resnick-2017-02-03-00

Request Review of draft-ietf-ccamp-flexible-grid-ospf-ext
Requested rev. no specific revision (document currently at 09)
Type Last Call Review
Team General Area Review Team (Gen-ART) (genart)
Deadline 2017-01-31
Requested 2017-01-17
Authors Xian Zhang, Haomian Zheng, Ramon Casellas, Oscar de Dios, Daniele Ceccarelli
Draft last updated 2017-02-03
Completed reviews Rtgdir Telechat review of -07 by Ben Niven-Jenkins (diff)
Opsdir Telechat review of -07 by Bert Wijnen (diff)
Secdir Last Call review of -07 by Catherine Meadows (diff)
Genart Last Call review of -07 by Pete Resnick (diff)
Genart Telechat review of -08 by Pete Resnick (diff)
Assignment Reviewer Pete Resnick 
State Completed
Review review-ietf-ccamp-flexible-grid-ospf-ext-07-genart-lc-resnick-2017-02-03
Reviewed rev. 07 (document currently at 09)
Review result Ready with Issues
Review completed: 2017-02-03

Review
review-ietf-ccamp-flexible-grid-ospf-ext-07-genart-lc-resnick-2017-02-03

I am the assigned Gen-ART reviewer for this draft. The General Area
Review Team (Gen-ART) reviews all IETF documents being processed
by the IESG for the IETF Chair.  Please treat these comments just
like any other last call comments.

For more information, please see the FAQ at

<https://trac.ietf.org/trac/gen/wiki/GenArtfaq>.

Document: draft-ietf-ccamp-flexible-grid-ospf-ext-07
Reviewer: Pete Resnick
Review Date: 2017-02-03
IETF LC End Date: 2017-01-31
IESG Telechat date: 2017-02-16

Summary:

Looks good from a non-expert point of view. A few things I found ambiguous or confusing that should be easily clarified, but nothing that should stop the document from moving forward.

Major issues:

None.

Minor issues:

4.1.1:

The figure is a bit confusing: There might not exist a "Max Slot Width at Priority 7" if bit 7 is clear in the Priority field, correct? Perhaps it would be better to just show that as "..." or "Max Slot Width at Priority n". The thing is, it might only be a single value, followed by the padding, so having the second value in there might be misleading. (Perhaps similar constructs are used in other MPLS docs and people will understand. But it took me a while to figure it out.)

The discussion of Priority was very confusing for me. In the third sentence, do you mean, "A bit is set (1) corresponding to each priority represented in the sub-TLV, and clear (0) for each priority not represented in the sub-TLV"? I don't understand the MUST/MUST NOT as you had it. And I don't understand the last sentence at all. Are you trying to say, "The leftmost bit (priority level 0) MUST be set, and priority level 0 MUST be advertised in the sub-TLV."? Otherwise, I don't get it.

I don't understand the MAY in the last sentence. Does that mean that I MAY also set it to the highest possible nominal central frequency supported by the link? I don't understand what that sentence is trying to tell me.

Nits/editorial comments: 

3, last paragraph:

OLD
   That is, the additional information
NEW
   That is, this section defines the additional information
END

As it is, it is ungrammatical.

3.1:

   On a DWDM link, the allocated or in-use frequency slots must not 
   overlap with each other.

I think perhaps it's clearer to simply say "frequency slots do not overlap each other", assuming that's what you mean. I don't think you're trying to say something normative there; it's just a fact. But people often read "must" (whether it's uppercased or not) to be imposing a requirement. I don't think that's what you're doing here, so better to make it clear. (As you may know, I'm not a fan of overuse of MUSTs and SHOULDs, but I do like it when documents are clear.)

As an opposite example:

   Hence, in order to clearly show which LSPs can be supported and what 
   frequency slots are unavailable, the available frequency ranges MUST 
   be advertised by the routing protocol for the flexi-grid DWDM links.  
   A set of non-overlapping available frequency ranges MUST be 
   disseminated in order to allow efficient resource management of 
   flexi-grid DWDM links and RSA procedures which are described in 
   Section 4.8 of [RFC7698]. 

Those MUSTs look weird to me. I think instead of "MUST be" you mean "are", since it doesn't look like an implementation really has a choice here.

3.2:

   Hence, in order to support all possible applications and 
   implementations the following information should be advertised for a 
   flexi-grid DWDM link:
   
Is that "should" in there meant to be normative? That is, do bad things happen if I don't advertise one of those items? Or do you just mean "the following information is advertised..."? 

3.3:

   For this reason, the available frequency slot/ranges need to be 
   advertised for a flexi-grid DWDM link instead of the specific 
   "wavelengths" points that are sufficient for a fixed-grid link.  

Where you say "needs to be advertised", are you making a normative statement, or are you just describing, in which case "are advertised" would be clearer?

(By the way: Typo in the following sentence. Change "thus" to "this".)

4.1:

   When Switching Capability and Encoding fields are set to values as 
   stated above, the Interface Switching Capability Descriptor MUST be 
   interpreted as in [RFC4203] with the optional inclusion of one or 
   more Switching Capability Specific Information sub-TLVs.  

Same question as earlier about "MUST be" vs. "is".

4.1.1:

   The technology specific part of the Flexi-grid ISCD should include 

Same question as earlier about "should include" vs. "includes".

   Length (16 bits): The length of the value field of this sub-TLV. 

Perhaps obvious to an MPLS person, but is this the length in bits or bytes, and does it include the Type and Length fields?

   Max Slot Width
   
I think the first "MUST be" should be "is". In the rest, I think you mean that if bits 0, 3, and 6 are set in Priority, then there should be 3 Max Slot Width entries, and the first one is the max slot width for priority level 0, the second one is the width for priority level 3, and the last is the priority slot width for priority level 7. Do I understand that correctly? If so, it might be useful to say that. And the last sentence would be clearer if it said, "The number of Max Slot Width fields MUST be identical to the number of bits set in the Priority field." Saying it with the MUST NOT confused me.

   Unreserved Padding

Shouldn't this say, "MUST be set to 0 and MUST be ignored on receipt"?

   The Reserved field

Why is this a "SHOULD be ignored on receipt" whereas the Padding Bits "MUST be ignored"? (And I think it would be better to move this up in order so it's right after Priority.)

4.2:

In Switching Cap, you say "MUST be consistent with" and in Encoding, you say "must be consistent with". Either make them both "MUST be", or just strike the words and simply make it, "consistent with".