Re: [trill] QA review of draft-ietf-trill-multi-topology-01

[Donald Eastlake <[e-mail]>]

Hi Martin,

On Fri, Jun 3, 2016 at 6:15 AM, Martin Vigoureux
<[e-mail]> wrote:
> Hi Donald,
>
> thank you.
> please see in-line for the pending points.
>
> -m
>
> Le 31/05/2016 20:11, Donald Eastlake a écrit :
>>
>> Hi Martin,
>>
>> Thanks for your review. See response below.
>>
>> On Mon, May 30, 2016 at 6:55 AM, Martin Vigoureux
>> <[e-mail]> wrote:
>>>
>>> Hi,
>>>
>>> I have been selected as the Routing Directorate QA reviewer for this
>>> draft.
>>>
>>> Document: draft-ietf-trill-multi-topology-01
>>> Reviewer: Martin Vigoureux
>>> Review Date: May 20, 2016
>>> Intended Status: Proposed Standard
>>>
>>> The draft is both quite well written and well structured such that I did
>>> not
>>> have to go back and forth in the doc.
>>> As a result also, I have only very few editorial comments and questions.
>>
>> Thanks.
>>
>>> Section 1
>>>     If routers in the network do not agree on the topology
>>>     classification of packets or links, persistent routing loops can
>>>     occur.
>>> It is not clear if that could happen in mt-trill or if mt-trill solves
>>> that.
>>
>> Multi-topology TRILL doesn't specify what kind of traffic should be
>> classified as being in what topology. Indeed, the traffic classified
>> as being in topology T can be arbitrarily different in different parts
>> of the TRILL campus if there are disjoint instances of a topology T.
>> This classification needs to be decided and configuration by network
>> management. This is consistent with how IS-IS multi-topology is used
>> in other applications. So, yes, routing loops can be caused by
>> misconfiguring IS-IS mutli-topology is TRILL or IP.
>
> Maybe it is worth clarifying that point then.

OK.

>>> Section 1.1 goes beyond defining acronyms but specifies some pieces of
>>> technology:
>>>     By implication, an "FGL TRILL switch" does not support MT.
>>>     An MT TRILL switch MUST support FGL in the sense that it MUST be FGL
>>>     safe [RFC7172].
>>> Is this the right place to do this? By the way, this requirement is
>>> stated
>>> further down in the doc.
>>
>> There is a similar sentence at the end of the entry for "VL". The idea
>> is that the capabilities of an "MT TRILL switch" are a superset of the
>> capabilities of an "FGL TRILL switch" which are in turn a superset of
>> the capabilities of a "VL TRILL switch". This is intended to simplify
>> things by having, at least to some extent, a linear sequence of added
>> capabilities rather than the cross product of the presence/absence of
>> each added capability. Sort of MT > FLG > VL. I don't see anything
>> wrong with having these statements here as well as further down in the
>> document.
>
> I did not say it was wrong. I find surprising to specify technology elements
> in a section the objective of which is to define acronyms.
> But I can live with it.

OK.

>>> Section 2.2
>>> s/and received/and receive/
>>
>> OK.
>>
>>> Section 2.4
>>>     Commonly, the topology of a TRILL Data packet is commonly
>>> One superfluous occurrence of "commonly"
>>
>> OK. I think deleting the initial "Commonly, " would be a good
>> solution. So the sentence would start "The topology of a TRILL Data
>> packet is commonly ..."
>>
>>> Section 2.4.1
>>> It would be better to write "2/3" as "2 and 3"
>>
>> OK.
>>
>>>     A TRILL switch advertising in a Hello on Port P support for topology
>>>     T but not advertising in those Hellos that it requires explicit
>>>     topology labeling is assumed to have the ability and configuration to
>>>     correctly classify TRILL Data packets into topology T by examination
>>>     of those TRILL Data packets and/or by using the fact that they are
>>>     arriving at port P.
>>> Does this mean that Value 1 is default behaviour?
>>
>>
>> The first paragraph of Section 2.4.1 makes it clear that the default
>> value of the two bit field in the Port Capabilities sub-TLV is zero
>> and this is also the value assumed if that sub-TLV is not present. I
>> can clarify the statement you quote above but it means exactly what it
>> says. "not advertising in those Hellos that it requires explicit
>> topology labeling" means it is not advertising a value of 2 or 3 in
>> the Explicit Topology capability field.
>>
>> The sentence you quote is already effectively included in the entry
>> text for Explicit Topology capability field value 1. Probably the
>> sentence you quote should be deleted and the applicable portion of
>> should also merged into the text for Explicit Topology capability
>> field value 0. That seems like the best way to reduce confusion.
>
> yes, I think this is where the misunderstanding came from.

OK, I delete that sentence and move the relevant part of it to the
entry to field value 0.

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

>>> Section 3.4.1
>>> s/are determine/are determined/
>>
>>
>> OK.
>>
>>> Section 7
>>> s/some links was more/some links were more/
>>
>>
>> OK.
>>
>> Thanks,
>> Donald
>> ===============================
>>   Donald E. Eastlake 3rd   [phone] (cell)
>>   155 Beaver Street, Milford, MA 01757 USA
>>   [e-mail]