[Gen-art] Review of draft-ietf-ccamp-flexible-grid-ospf-ext-07

Pete Resnick <presnick@qti.qualcomm.com> Fri, 03 February 2017 23:22 UTC

Return-Path: <presnick@qti.qualcomm.com>
X-Original-To: gen-art@ietf.org
Delivered-To: gen-art@ietfa.amsl.com
Received: from ietfa.amsl.com (localhost [IPv6:::1]) by ietfa.amsl.com (Postfix) with ESMTP id F0EB3129980; Fri, 3 Feb 2017 15:22:19 -0800 (PST)
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit
From: Pete Resnick <presnick@qti.qualcomm.com>
To: gen-art@ietf.org
X-Test-IDTracker: no
X-IETF-IDTracker: 6.42.0
Auto-Submitted: auto-generated
Precedence: bulk
Message-ID: <148616413998.4157.2652633501511666882.idtracker@ietfa.amsl.com>
Date: Fri, 03 Feb 2017 15:22:19 -0800
Archived-At: <https://mailarchive.ietf.org/arch/msg/gen-art/INeB1_e3pB4_yVY9k5OUKK_NIBQ>
Cc: draft-ietf-ccamp-flexible-grid-ospf-ext.all@ietf.org, ccamp@ietf.org, ietf@ietf.org
Subject: [Gen-art] Review of draft-ietf-ccamp-flexible-grid-ospf-ext-07
X-BeenThere: gen-art@ietf.org
X-Mailman-Version: 2.1.17
List-Id: "GEN-ART: General Area Review Team" <gen-art.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/gen-art>, <mailto:gen-art-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/gen-art/>
List-Post: <mailto:gen-art@ietf.org>
List-Help: <mailto:gen-art-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/gen-art>, <mailto:gen-art-request@ietf.org?subject=subscribe>
X-List-Received-Date: Fri, 03 Feb 2017 23:22:20 -0000

Reviewer: Pete Resnick
Review result: Ready with Issues

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".