Re: [Isis-wg] draft-eastlake-isis-trill-00.txt

[Donald Eastlake <[e-mail]>]

Hi Mike,

Thanks for these detailed, thorough, and helpful comments,

Responses below:

On Fri, Jun 25, 2010 at 5:36 AM, mike shand <[e-mail]> wrote:
> Donald,
>
> A few comments, mostly relatively minor, but with some more substantial ones
> about PDU types towards the end.
>
>
> General... The terms TRILL IS, IS, Intermediate System and RBridge seem to
> be used interchangeably. Is there in fact any distinction being made by the
> different terms, or should they all be replaced by a common term?

In my opinion, this document should generally use IS-IS nomenclature.
There should be minimal references to RBridge. But TRILL will occur
more often as the document is about TRILL use of IS-IS. I'll do a pass
over the document with this in mind.

> Some TLV descriptions specify explicitly that multiple instances and or zero
> instances are permitted, but others don't. It would be helpful either to
> explicitly state each case, or announce a convention such as that only a
> single instance is allowed unless explicitly permitted.
>
> Aha! I now see that the tables at the end specify the number of instances
> permitted. Perhaps it would actually be better to say nothing about the
> number of times TLVs and sub-TLVs may occur in their textual descriptions
> and leave it to the tables at the end?

That's probably better, to avoid possible inconsistencies. I'll remove
this from the textural descriptions and add a note at the beginning so
people will know to look at the tables.

> --------------------------------------------------------------
>
> 2.2 Multi-Topology aware Port Capability TLV
>
>   The Multi Topology aware Port Capability (MT-PORT-CAP) TLV is IS-IS
>   TLV type 143 [TBD] and has the format show below. The sub-TLVs that
>   it carries are a new series of sub-TLVs.
>
>
> TYPO s/show/shown/

OK.

> Topology Identifier
>
> I'm a little puzzled why this appears to be different from the corresponding
> field in the GMAC-ADDR sub-TLV. One is called a topology ID and the other a
> topology identifier, and in one case the top 4 bits are reserved, and in the
> other they are not.

I'll change it to uniformly be "topology ID" and be 12 bits.

> ---------------------------------------------------------------
>
> 2.3 Sub-TLVs for the Router Capability TLV
>
>   The Router Capability TLV is specified in [RFC4971] and may be
>   generated by the originating IS. All of the sub-sections below of
>
> Stating that this may be generated by the originating IS is somewhat
> superfluous (and perhaps  implies that other TLVs may be generated by
> systems other than the originating IS, which, with the notable exception of
> the newly proposed purge ID TLV, is not the case)

OK.

> ----------------------------------------------------------------
>
> 2.3.4 The Tree Identifiers Sub-TLV
>
> In the event a tree identifier can be
>      computed from two such sub-TLVs and are different,
>
> TYPO presumably this should read "and they are different".

OK.

> 2.3.5 The Trees Used Identifiers Sub-TLV
>
> and the trees listed are those that the originating intermediate
>   systems wishes
>
> TYPO? Presumably "that the originating system wishes". But see also the
> general comment about consistency of nomenclature

Right.

> -----------------------------------------------------------
>
> 2.3.6 Interested VLANs and Spanning Tree Roots sub-TLV
>
> In the LSPs originated by an IS,
>
>
> Perhaps better, "In the set of LSPs....."

Right.

> The largest appoint forwarder
>
> TYPO? s/appoint/appointed/

Right.

> R, RESV: These reserved bits MUST be set as zero
>
> TYPO: s/set/sent/

"set" sort of makes sense, but I agree "sent" is better.

> Appointed Forwarder Status Lost Counter:
>
> I assume this is a 4 byte counter to avoid the possibility that it might
> wrap. I also assume that it is initialised to zero when the RBridge is
> started. I can't see see any specific information about when and how it is
> initialised in the TRILL spec reference quoted.

It is initialized when that is done for the LSP sequence numbers.
Detecting that it has changed is used as a criteria for trimming the
expiration time of some cached MAC addresses to be sure that behavior
is similar to similarly configured learning bridges. Neither false
positives nor false negatives would be particularly bad. I'll add a
note.

> ----------------------------------------------------------------
>
> 2.4 MTU sub-TLV of the Extended Reachability TLV
>
> It occurs nested as within
>
> Strange turn of phrase (copied from the original common layer 2 spec I
> think). Just "It occurs within.." would suffice.

OK.

> -----------------------------------------------------------------
>
> 2.5 TRILL Neighbor TLV
>
> The TRILL Neighbor TLV is used in TRILL-Hellos (see Section 4.1
>   below) in place of the IS Neighbor TLV, as specified in Section
>   4.4.2.1 of [RFCtrill].
>
> This is somewhat confusing. I assume it is saying that the TRILL neighbor
> TLV is used in a TRILL Hello in place of the IS Neighbor TLV in an IIH??

Yes.

> While the requirements for there to be no gaps, and the possibility of
> overlaps is called out in the referenced section of the TRILL spec, the
> necessity for an ID to appear in multiple "fragments" to ensure that
> condition is not obvious, and I wonder if some clarification is required,
> either here, or in the TRILL spec itself.

I think the last paragraph of 4.4.2.1 is adequately clear but I'll try
to add something more here.

> F: failed.  This bit is a one if MTU testing to their neighbor
>
> should that be "to this neighbor"?

Right.

> -------------------------------------------------------------
> 3. The MTU PDUs
>
> Are two seperate PDUs really required. Would not a single PDU type with a
> flag to indicate probe or ACK suffice?

Yes, you could use a single PDU type but I think they way they are
currently specified fits somewhat better into the pattern of PDU
definitions for IS-IS... Does anyone else out there have an opinion on
this?

> The MTU PDUs have the standard IS-IS PDU common header
>
> It's not very clear what is meant by "the standard IS-IS PDU common header".
> I assume this is up to and including the "Maximum Area Addresses" field, but
> this needs to be explicitly stated.

It means the header that occurs in common across every existing
defined IS-IS PDU. Most every reference I look at about IS-IS says
that an IS-IS PDU has the IS-IS PDU "common" area of fixed fields
followed by a PDU specific area of fixed fields, followed by TLVs.
I'll add a clarifying note.

> The Ack Source ID is set to zero in MTU-probe PDUs.  An Intermediate
>   System issuing an MTU-ack set this field
>
> TYPO: s/set this field/sets this field/
>
> and MUST be padded to the size being tested
>   with the Padding TLV.
>
>
> HOW must it be padded? Does it use the pad TLV (8) defined for IIHs, and if
> so does the padding have to be exact? (See clause 8.2.3 of ISO/IEC 10589).
> Or is the length simply extended with no "filling" with TLVs?

I think maybe you mean the footnote to clause 8.4.2? Padding should be
exact using the pad TLV. The substantive content of the MTU PDUs is
quite small and the minimum length much longer so this should not be a
problem. I'll add a note about this.

> ----------------------------------------------------------------
>
> 4.1 TRILL-Hello PDUs
>
>
> Instead, TRILL-Hellos uses the TRILL
>
> TYPO: s/uses/use/

Right.

> 4.2 Area Address
>
>   The TRILL uses
>
> TYPO: s/TRILL/TRILL-Hello.

This is at the same level as 4.1. I think it is better to just drop
the "The ". (Perhaps was originally intended to be "The TRILL
Protocols uses ...")

> ---------------------------------------------------------
>
> 6.1 Allocations From Existing Registries
>
> The final "NUMBER" column
>   indicates the permitted number of occurrences of the sub-TLV in their
>   TLV as follows:
>
> Since a router Capability TLV is permitted to occur multiple times within
> the set of LSPs generated by a particular system, there seems to be the
> potential for a sub-TLV with NUMBER 0-1 to occur exactly once within
> multiple router capability TLVs, which would appear to be in conformance
> with these restrictions, but in fact should still have unspecified results.
> I think this wording needs tightening up.

An excellent point. I'll fix the wording.

> -------------------------------------------------------------
>
> 6.2 New Sub-Registries Created and Their Initial Contents
>
> The final "NUMBER" column
>   indicates the permitted number of occurrences of the sub-TLV
>   cumulatively within all the occurrences of their TLV in a particular
>   PDU as follows:
>
> While I don't think this is an issue in the spec as currently defined (but
> see below), there is a potential ambiguity if this text were used to define
> the number of permissible occurrences within a PDU, and the PDU type was LSP
> and the restriction was exactly 1. This arises from the confusion between
> PDUs, LSPs and so called LSP fragments. As written, the text would appear to
> permit exactly one instance of the TLV in each individual LSP (i.e. PDU,
> with type LSP) generated by a particular system. This, I think, is not the
> intent. The actual requirement is that there MUST be exactly one instance
> within the entire set of LSPs (sometimes, in my opinion, erroneously
> referred to as LSP fragments) generated by a particular system.
>
> Since the required action on violation of this constraint is for the PDU to
> be ignored, it is critical that implementations perform this check
> consistently, since it could otherwise result in some systems ignoring an
> entire erroneous PDU (which may contain other significant routing data), and
> other systems making use of the PDU, and hence potentially generating
> routing loops.
>
> Clearly, such a situation would only occur when the generating system was
> non-conformant, but we have plenty of instances of non-conformant IS-IS
> implementations in the wild, and it would seem wise to be as conservative
> here as possible.
>
> I repeat, that this is not an issue in the current spec, but I am concerned
> either that at some stage the spec would be extended to include the
> ambiguous case, or that some other spec would take this text as the
> definitive way of documenting such restrictions and would fall into this
> trap.
>
> I think it could be fixed simply by saying
>
> The final "NUMBER" column
>   indicates the permitted number of occurrences of the sub-TLV
>   cumulatively within all the occurrences of their TLV in  a particular
>   PDU, or in the case of LSPs the entire set of LSPs generated by the
> systrem as follows:
>
> However, on reflection, since TRILL HELLOS (aka IIHs) are now permitted to
> be "fragmented", do we indeed have this problem? Presumably it is permitted
> for sequential IIHs containing different ranges of neighbor records to all
> contain an MT port capability TLV containing VLAN and Flags, provided that
> it doesn't occur multiple times within the same actual PDU?

Yes. Although the data in TRILL IIHs can be "serially fragmented" in
successive PDUs, this really isn't the same as the "parallel
fragmentation" of LSPs.

> In any case I think some clarification is required.

Indeed. I'll try to fix the wording.

> ---------------------------------------------------------------------------
>
>
> 8.1 Normative References
>
>   [IS-IS] - ISO/IEC 10589, "Intermediate System to Intermediate System
>         Intra-Domain Routing Exchange Protocol for use in Conjunction
>         with the Protocol for Providing the Connectionless-mode Network
>         Service (ISO 8473)", 2002.
>
> Just to be absolutely clear that we are referencing the second edition, it
> is usual to quote this reference as
>
> ISO/IEC 10589:2002, Second Edition

OK.

> ----------------------------------------------------------
>
> END of comments
>
>    Mike

Thanks,
Donald

> On 11/06/2010 18:53, Donald Eastlake wrote:
>>
>> Hi,
>>
>> I've submitted a draft proposing IS-IS support for TRILL:
>> http://tools.ietf.org/html/draft-eastlake-isis-trill-00
>>
>> Thanks,
>> Donald
>> =============================
>> Donald E. Eastlake 3rd   [phone] (cell)
>> 155 Beaver Street
>> Milford, MA 01757 USA
>> [e-mail] <mailto:[e-mail]>