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

[Alia Atlas <[e-mail]>]

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 <[e-mail]> wrote:

> Hi Janos,
>
> Thanks for the updated draft.
>
> On Thu, Dec 17, 2015 at 9:19 AM, János Farkas <[e-mail]>
> wrote:
>
>> 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.
>>
>
> 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
>
>>
>> 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.
>>
>> [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
>>
>>
>