Re: [trill] AD review of draft-ietf-trill-rfc7180bis

[Donald Eastlake <[e-mail]>]

Hi Alia,

Just responding where you have any additional comment or suggestion:

On Mon, Oct 5, 2015 at 12:26 PM, Alia Atlas <[e-mail]> wrote:
> Hi Donald,
>
> On Sun, Sep 27, 2015 at 10:55 PM, Donald Eastlake <[e-mail]> wrote:
>>
>> Hi Alia,
>>
>> Thanks for the review. See below.
>>
>> On Fri, Sep 25, 2015 at 12:12 PM, Alia Atlas <[e-mail]>
>> wrote:
>> > As is customary, I have done my AD review of
>> > draft-ietf-trill-rfc7180bis-05.  First, I'd like to thank the
>> > authors Donald, Mingui, Radia, Ayan, Anoop, and Sujay for their work
>> > on this.
>> >
>> > Due to some oddity in the datatracker, I didn't learn that the WG
>> > had requested publication until I got a private email.  I apologize
>> > for the delay in review.
>> >
>> > Below are my comments.  The only one that is really blocking for
>> > starting IETF Last Call is about Appendix C.  If we can get the
>> > comments resolved and a new draft issued before next Thursday, then
>> > this draft should be able to make the Oct 22 telechat.
>> >
>> > 1) In Sec 8.2: " Control packets can be ordered into priority
>> >    classes as shown below.  Although few implementations will
>> >    actually treat all of these classes differently, lower numbered
>> >    classes SHOULD NOT be treated as lower priority than higher
>> >    numbered class."
>> > Can you clarify what the point of listing these different priority
>> >   classes, if the lower number classes aren't lower priority &
>> >   treated that way??
>>
>> With this draft, a new class of IS-IS PDUs, the Flooding Scoped PDUs
>> [RFC7356], is added to the repertoire used by TRILL. The TRILL base
>> protocol [RFC6325] just says that all IS-IS PDUs SHOULD be sent with
>> the maximum priority 7. But as additional data being sent is somewhat
>> more "application" and less core routing between TRILL switches
>> oriented, particularly APPsub-TLVs being sent in FS-LSPs, some
>> implementations might want to send some IS-IS PDUs at lower
>> priority. This section provides some guidance.
>>
>> If you don't like the direction of the priority numbering, which I
>> admit is inconsistent with priority field values in TRILL, I don't see
>> any problem in renumbering the classes so the highest priority has the
>> highest number. Or we could use Roman numeral or something :-)
>
> Roman numerals or alphabetic would help. My key point, though, is
> that it's confusing to say here's the prioritization but
> implementations SHOULD NOT treat them differently. That's not
> guidance on how they could be treated differently.

What it was trying to say is that you "SHOULD NOT" send lower priority
cateogry packets in preference to higher priority category packets.

> Perhaps a "this is the relative criticality of these types of
> messages, where the most critical relate to the core routing between
> TRILL switches and the less critical closer to "application"
> information. An implementation MAY choose to prioritize more
> critical messages over less critical. However, for .... an
> implementation SHOULD NOT ...."

Sure, we can go for wording like that.

>> > 2) Sec 8.4: I would really like to see an explanation for why the
>> > Nickname Flags APPSub-TLV is needed.  I see the description and
>> > efforts to clarify edge cases, which is great, but nothing that says
>> > "This is the problem a Nickname Flags APP sub-TLV is solving and the
>> > types of problems additional flags might solve in the future."
>>
>> There are a number of anticipated additional uses including specific
>> Nickname Flags bits defined in draft-ietf-trill-irb (currently in WG
>> Last Call) and draft-ietf-trill-centralized-replication.  These drafts
>> are normatively dependent on rfc7180bis. In my opinion, the IS-IS
>> Nickname sub-TLV (originally specified in RFC 6326, which has been
>> obsoleted by RFC 7176) would have been improved by having some flags
>> associated with each nickname. The use specified in this draft could
>> have been included in the base TRILL specification, RFC 6325, if we
>> had thought of it at the time.
>>
>> We could make a change such as:
>>
>> OLD
>>    Additional flags may be assigned for other purposes out of the RESV
>>    field in the future.
>>
>> NEW
>>    (Additional flags are assigned from those labeled RESV above and
>>    specified in draft-ietf-trill-irb and
>>    draft-ietf-trill-centralized-replication, works in progress.)
>
> Sounds good to me.  Make sure to add them to the Informative References.

Will do.

>> > 3) In Sec 10.2.1.3: " 3. If the Hop Count is zero, it is set to the
>> > maximum value of 63 and the extended Hop Count is decremented."
>> > What happens to the Critical Bit Summary when the extended Hop Count
>> > goes to 0?  I don't see any mention of it after the ingress RBridge
>> > setting it.
>>
>> Right. If the extended Hop Count goes to zero, the Critical Reserved
>> bit MUST also be set to zero. Arguably this is implied by RFC 7179 but
>> it would be good to state here. Item 3 should be extended something
>> like the following:
>>
>> OLD
>>    3. If the Hop Count is zero, it is set to the maximum value of 63 and
>>       the extended Hop Count is decremented.
>>
>> NEW
>>    3. If the Hop Count is zero, it is set to the maximum value of 63 and
>>       the extended Hop Count is decremented. If this results in the
>>       extended Hop Count being zero, the Critical Reserved bit in the
>>       Criticial Summary bits is set to zero.
>
> Are there cases where the Critical Summary bit might remain set for a
> reason other than the extended Hop Count?

Not for that particular Critial Summary bit.

> Otherwise, LGTM.

...

> Thanks,
> Alia

Thanks,
Donald
=============================
 Donald E. Eastlake 3rd   [phone] (cell)
 155 Beaver Street, Milford, MA 01757 USA
 [e-mail]