Subject: [CCAMP] Review of draft-ietf-ccamp-flexible-grid-ospf-ext-07

From: Pete Resnick <[email-masked]>
Date: Fri, 03 Feb 2017 15:22:19 -0800

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

---

From: Jari Arkko <[email-masked]>
Date: Mon, 6 Feb 2017 13:32:29 +0200

Thanks for your review, Pete. Authors, any comments?

Jari

---

From: Daniele Ceccarelli <[email-masked]>
Date: Mon, 6 Feb 2017 13:36:46 +0000

Hi Jari, Pete,

First of all thanks for the accurate review.

All the nits/editorial comments are correct and will be fixed.

Regarding the minor issue: 

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

it was difficult to express in the figure the fact that only some of the priorities are advertised. As you said it could be one, two or any number up to 8. 
What about a single field (with ~ on the borders to indicate that it can have variable length) saying "Max Slot Width at Prio n" and then say that 16 bits are used for each prio and when an odd number of priorities is used the field is padded to line up with multiples of 32 bits?

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

It means that if the priority field is set to e.g. 10010010 in the following you would find 4 fields indicating respectively: Max Slot Width at Priority 0, Max Slot Width at Priority 3, Max Slot Width at Priority 6 and since they are 3 a 16 bits padding to 32 bits. 
It also means that at least one priority must be advertised (i.e. priority 00000000 is not allowed).

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

An example is provided in section 4.1.2, where the available range goes from -2 to +8 but the range supported by the link goes from -9 to +11. The sentence means that even if not available it could be possible to indicate also n=-9 to indicate the starting point of the range supported by the link. 

  " In this example, it is assumed that the lowest nominal central
   frequency supported is n= -9 and the highest is n=11. Note they
   cannot be used as a nominal central frequency for setting up a LSP,
   but merely as the way to express the supported frequency range."

I'm ok with dropping the sentence.

Thank you
Daniele 

> -----Original Message-----
> From: Jari Arkko [mailto:[email-masked]]
> Sent: lunedì 6 febbraio 2017 12:32
> To: Pete Resnick <[email-masked]>
> Cc: [email-masked]; [email-masked];
> [email-masked]; [email-masked]
> Subject: Re: [Gen-art] Review of draft-ietf-ccamp-flexible-grid-ospf-ext-07
>

---

From: Pete Resnick <[email-masked]>
Date: Mon, 6 Feb 2017 11:37:48 -0600

On 6 Feb 2017, at 7:36, Daniele Ceccarelli wrote:

>> Width at
> it was difficult to express in the figure the fact that only some of 
> the priorities are advertised. As you said it could be one, two or any 
> number up to 8.
> What about a single field (with ~ on the borders to indicate that it 
> can have variable length) saying "Max Slot Width at Prio n" and then 
> say that 16 bits are used for each prio and when an odd number of 
> priorities is used the field is padded to line up with multiples of 32 
> bits?

Yes, I think that would make more sense to me.

>> sentence, do
> It means that if the priority field is set to e.g. 10010010 in the 
> following you would find 4 fields indicating respectively: Max Slot 
> Width at Priority 0, Max Slot Width at Priority 3, Max Slot Width at 
> Priority 6 and since they are 3 a 16 bits padding to 32 bits.

OK, then I think my sentence is clearer for that: "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."

> priority 00000000 is not allowed).

I get that part ("At least one priority level MUST be advertised"). It's 
the end I don't understand: "that, unless overridden by local policy, 
SHALL be at priority level 0." What does that mean?

> An example is provided in section 4.1.2, where the available range 
> goes from -2 to +8 but the range supported by the link goes from -9 to 
> +11. The sentence means that even if not available it could be 
> possible to indicate also n=-9 to indicate the starting point of the 
> range supported by the link.
>

I think dropping the sentence would make the most sense.

> Daniele

Thanks for considering my suggested changes.

pr

>> Cc: [email-masked]; 
>> [email-masked];
>> Subject: Re: [Gen-art] Review of 
>> draft-ietf-ccamp-flexible-grid-ospf-ext-07
>>

-- 
Pete Resnick <http://www.qualcomm.com/~presnick/>
Qualcomm Technologies, Inc. - +1 (858)651-4478

---

From: Daniele Ceccarelli <[email-masked]>
Date: Tue, 7 Feb 2017 09:36:51 +0000

Hi Pete,

We’re getting there.
Please see in line for further explanation.

Thanks a lot
Daniele

From: Pete Resnick [mailto:[email-masked]]
Sent: lunedì 6 febbraio 2017 18:38
To: Daniele Ceccarelli <[email-masked]>
Cc: Jari Arkko <[email-masked]>; [email-masked]; [email-masked]; [email-masked]; [email-masked]
Subject: Re: [Gen-art] Review of draft-ietf-ccamp-flexible-grid-ospf-ext-07

Hi Jari, Pete,

4.1.1:

[DC] OK

OK, then I think my sentence is clearer for that: "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."

[DC] Agree. We’ll use your text.

I get that part ("At least one priority level MUST be advertised"). It's the end I don't understand: "that, unless overridden by local policy, SHALL be at priority level 0." What does that mean?

[DC] It means that if only one priority is supported it has to be priority 0. For any particular administrative purpose it could be possible to set it to a different value, but that shouldn’t be done.

[DC] Ok for me

Thank you
Daniele

pr

-----Original Message-----
From: Jari Arkko [mailto:[email-masked]]
Sent: lunedì 6 febbraio 2017 12:32
To: Pete Resnick <[email-masked]<mailto:[email-masked]>>
Cc: [email-masked]<mailto:[email-masked]>; [email-masked]<mailto:[email-masked]>;
[email-masked]<mailto:[email-masked]>; [email-masked]<mailto:[email-masked]>
Subject: Re: [Gen-art] Review of draft-ietf-ccamp-flexible-grid-ospf-ext-07

Jari

--
Pete Resnick http://www.qualcomm.com/~presnick/<http://www.qualcomm.com/%7Epresnick/>
Qualcomm Technologies, Inc. - +1 (858)651-4478

---

From: Pete Resnick <[email-masked]>
Date: Tue, 7 Feb 2017 11:04:56 -0600

Hi Daniele,

Thanks for addressing everything. There's only one issue left in section 
4.1.1 on Priority, below. I've trimmed out all the rest.

On 7 Feb 2017, at 3:36, Daniele Ceccarelli wrote:

>> I get that part ("At least one priority level MUST be advertised"). 
>> It's the end I don't understand: "that, unless overridden by local 
>> policy, SHALL be at priority level 0." What does that mean?
>
> [DC] It means that if only one priority is supported it has to be 
> priority 0.

So, let me see if I have this right: It's OK to have 01100000 but not 
01000000 or 00100000? If so, why is that?

> For any particular administrative purpose it could be possible to set 
> it to a different value, but that shouldn’t be done.

Well, it doesn't say that shouldn't be done, but it probably doesn't 
need to say anything about local configurations.

pr

---

From: Daniele Ceccarelli <[email-masked]>
Date: Wed, 8 Feb 2017 10:14:48 +0000

Hi Pete,

This is an “inheritance” from GMPLS, where supporting a single priority equals not supporting priorities. If you don’t want to support priorities you don’t want your traffic to be preempted…hence priority 0.

>Well, it doesn't say that shouldn't be done, but it probably doesn't need to say anything about local configurations.
For me it’s ok not to say anything on that.

Thanks
Daniele

Sent: martedì 7 febbraio 2017 18:05

Hi Daniele,

Thanks for addressing everything. There's only one issue left in section 4.1.1 on Priority, below. I've trimmed out all the rest.

[DC] It means that if only one priority is supported it has to be priority 0.

So, let me see if I have this right: It's OK to have 01100000 but not 01000000 or 00100000? If so, why is that?

For any particular administrative purpose it could be possible to set it to a different value, but that shouldn’t be done.

Well, it doesn't say that shouldn't be done, but it probably doesn't need to say anything about local configurations.

pr

---

From: Pete Resnick <[email-masked]>
Date: Wed, 8 Feb 2017 18:08:15 -0600

If that's what you mean, let me suggest simplifying:

OLD
                                                At least one priority
    level MUST be advertised that, unless overridden by local policy,
    SHALL be at priority level 0.
NEW
                                                At least one priority
    level MUST be advertised. If only one priority level is advertised,
    it MUST be at priority level 0.

Thanks for the extended discussion of this. It all looks fine.

pr

On 8 Feb 2017, at 4:14, Daniele Ceccarelli wrote:

> Hi Pete,
>
> This is an “inheritance” from GMPLS, where supporting a single 
> priority equals not supporting priorities. If you don’t want to 
> support priorities you don’t want your traffic to be 
> preempted…hence priority 0.
>
>
> Cc: Jari Arkko <[email-masked]>; [email-masked]; 
> [email-masked]; [email-masked]; 
> [email-masked]
> Thanks for addressing everything. There's only one issue left in 
> section 4.1.1 on Priority, below. I've trimmed out all the rest.
>
> Pete Resnick 
> http://www.qualcomm.com/~presnick/<http://www.qualcomm.com/%7Epresnick/>

---

From: Daniele Ceccarelli <[email-masked]>
Date: Thu, 9 Feb 2017 08:31:20 +0000

Gone! ☺

Thanks to you for the careful review.

BR
Daniele

From: Pete Resnick [mailto:[email-masked]]
Sent: giovedì 9 febbraio 2017 01:08
To: Daniele Ceccarelli <[email-masked]>
Cc: [email-masked]; [email-masked]; [email-masked]; [email-masked]
Subject: Re: [Gen-art] Review of draft-ietf-ccamp-flexible-grid-ospf-ext-07

OLD
At least one priority

pr

Hi Pete,

Thanks

Daniele

From: Pete Resnick [mailto:[email-masked]]
Sent: martedì 7 febbraio 2017 18:05
To: Daniele Ceccarelli <[email-masked]<mailto:[email-masked]>>
Cc: Jari Arkko <[email-masked]<mailto:[email-masked]>>; [email-masked]<mailto:[email-masked]>; [email-masked]<mailto:[email-masked]>; [email-masked]<mailto:[email-masked]>; [email-masked]<mailto:[email-masked]>
Subject: Re: [Gen-art] Review of draft-ietf-ccamp-flexible-grid-ospf-ext-07

Hi Daniele,

pr