Subject: [Isis-wg] AD review of draft-ietf-isis-pcr-03

From: Alia Atlas <[email-masked]>
Date: Mon, 7 Dec 2015 15:04:13 -0500

Hi,

As is customary, I have done my usual AD review of draft-ietf-isis-pcr-03
before requesting IETF Last Call.  First, I would like to thank the
authors, Janos, Nigel, Paul, Glenn, Peter, and Chris, as well as those who
provided helpful reviews for their hard work on this draft.

I do have a few comments from my review (given below) but I am comfortable
having this progress to IETF Last Call while the draft is updated.  I have
scheduled this draft for the IESG telechat on January 7.  Please  update
the draft as soon as practical.

Minor:

1)  In Section 6.2 h:  How to tell if a Delay Constraint is present isn't
clearly described.   I assume that one can tell by calculating the expected
length of the Hop sub-TLV without the Delay Constraint and then comparing
that value to the actual length (as reported in the TLV).  If the actual
length is 6 bytes more, then there is a Delay Constraint.   If that's the
mechanism, could you please clearly write it down.  As described, it sounds
like the way to tell is simply by trying to parse the last 6 bytes as a
TLV.  I am assuming that there's a reason you didn't simply use an
additional flag to indicate whether the Delay Constraint is present -
because the trick above only works once per TLV.

2) In Sec 6.4 f and 6.2 c7:  For reserved bits, generally it is good to
describe them as "MUST be set to 0 on sending and the value MUST be ignored
on reception".  This allows extensions to work in the future.

Nits:

a) In Figure 1, the size of the Res & Base VID is described as (0 or 2
bytes).  If present, based on the description for (d) and (e), I think it
is always 2 bytes.  The figure should reflect this.  The field is missing -
not 0 bytes - if the "Num Base VIDs" is 0.

b) I see the same issue in Figure 3.  If you want to show the variability,
perhaps indicate (n bytes if present) or such.

Thanks,
Alia

---

From: János Farkas <[email-masked]>
Date: Thu, 17 Dec 2015 15:19:52 +0100

Hi Alia,

Thank you very much for your review and comments!
A new revision has been posted 
(https://www.ietf.org/id/draft-ietf-isis-pcr-04.txt), which addresses 
your comments.
See some further notes in-line marked with [JF].

On 12/7/2015 9:04 PM, Alia Atlas wrote:
> Hi,
>
> As is customary, I have done my usual AD review of 
> draft-ietf-isis-pcr-03 before requesting IETF Last Call. First, I 
> would like to thank the authors, Janos, Nigel, Paul, Glenn, Peter, and 
> Chris, as well as those who provided helpful reviews for their hard 
> work on this draft.
>
> I do have a few comments from my review (given below) but I am 
> comfortable having this progress to IETF Last Call while the draft is 
> updated.  I have scheduled this draft for the IESG telechat on January 
> 7.  Please  update the draft as soon as practical.
>
> Minor:
>
> 1)  In Section 6.2 h:  How to tell if a Delay Constraint is present 
> isn't clearly described.   I assume that one can tell by calculating 
> the expected length of the Hop sub-TLV without the Delay Constraint 
> and then comparing that value to the actual length (as reported in the 
> TLV).  If the actual length is 6 bytes more, then there is a Delay 
> Constraint.   If that's the mechanism, could you please clearly write 
> it down.  As described, it sounds like the way to tell is simply by 
> trying to parse the last 6 bytes as a TLV.  I am assuming that there's 
> a reason you didn't simply use an additional flag to indicate whether 
> the Delay Constraint is present - because the trick above only works 
> once per TLV.
[JF] You've got it right. We have added explanation to the beginning of 
Section 6.2 h.

>
> 2) In Sec 6.4 f and 6.2 c7:  For reserved bits, generally it is good 
> to describe them as "MUST be set to 0 on sending and the value MUST be 
> ignored on reception".  This allows extensions to work in the future.
[JF] We have added the suggested text to each occurrence of reserved 
bits in the document.

> bytes).  If present, based on the description for (d) and (e), I think 
> it is always 2 bytes.
[JF] You are right.

> The figure should reflect this.  The field is missing - not 0 bytes - 
> if the "Num Base VIDs" is 0.
>
> b) I see the same issue in Figure 3.  If you want to show the 
> variability, perhaps indicate (n bytes if present) or such.
[JF] We have changed every occurrence to what you suggested: (n bytes if 
present). This also resolves comment a).

Thank you and regards,
Janos

---

From: Alia Atlas <[email-masked]>
Date: Thu, 17 Dec 2015 09:27:38 -0500

Hi Janos,

Thanks for the updated draft.

On Thu, Dec 17, 2015 at 9:19 AM, János Farkas <[email-masked]>
wrote:

> A new revision has been posted (
> https://www.ietf.org/id/draft-ietf-isis-pcr-04.txt), which addresses your
> comments.
> [JF] We have added the suggested text to each occurrence of reserved bits
> in the document.
>

I apologize, but I got it slightly wrong in my suggestion.  It should be
"SHOULD be set to 0 on sending" instead of MUST.
That way if the reserved bits are used later, they don't violate this base
draft.

Alia

> is always 2 bytes.
> The figure should reflect this.  The field is missing - not 0 bytes - if
> the "Num Base VIDs" is 0.
>

---

From: Alia Atlas <[email-masked]>
Date: Fri, 18 Dec 2015 10:09:05 -0500

After more discussion and thought (thanks Bruno), leaving it as a MUST set
to 0 and
MUST be ignored is fine. This means that any draft that uses a reserved bit
needs to be
specified as updating this document, but it also preserves the high
assurance that the
reserved bits aren't used for something proprietary.

Thanks,
Alia

On Thu, Dec 17, 2015 at 9:27 AM, Alia Atlas <[email-masked]> wrote:

>> https://www.ietf.org/id/draft-ietf-isis-pcr-04.txt), which addresses
>> your comments.
>> updated.  I have scheduled this draft for the IESG telechat on January 7.
>> Please  update the draft as soon as practical.
>>
>> Minor:
>>