Document: draft-ietf-ccamp-automesh-02.txt Reviewer: Miguel Garcia Review Date: 2006-09-25 IETF LC Date: 2006-10-04 Summary: This draft is on the right track but has open issues, described in the review. Comments: I think the draft is well written, although I have a few major issues regarding the clarity and intention of the draft, which should be clarified. The bulk of my comments are mostly editorial, though. Issue 1: -------- After carefully reading the document, it is not really clear to me whether the intention of the document is: a) to define two OSPF TE TLVs (one for IPv4, another for IPv6) and two IS-IS sub-TLVs (one for IPv4, another for IPv6); or b) to define one OSPF TE TLV, which is further structured depending on the Type field. I suspect it is a), which is endorsed by the IANA considerations section. If this is correct, I wonder why the two OSPF TLVs are going to share the same name, why not name them TE-MESH-GROUP-IPV4 and TE-MESH-GROUP-IPV6, respectively. The same should apply to the IS-IS sub-TLVs. For example, Section 3.2 now says "This document specifies a TE-MESH-GROUP TLV that indicate ..." when in fact the document specifies not one, but four TLVs. In any case, I would suggest to add some high-level description around Section 3. Issue 2: -------- IANA actions for OSPF: It is not clear to me which registry IANA has to modify. I believe the IANA sections should indicate the registry and subregistry (by name, not by reference) that IANA has to act upon. So, I guess you are looking at something similar to this (please check the registry and subregistry names are correct): This document defines two new types of OSPF TE TLVs. IANA should register them in the 'Top level Types in TE LSAs' subregistry of the 'Open Shortest Path First (OSPF) Traffic Engineering TLVs per [RFC3630]' registry with theh following registration data Value Top Level Types Reference ----------- ---------------------- --------- x TE mesh membership (IPv4) [RFCXXXX] y TE mesh membership (IPv6) [RFCXXXX] Note 1: RFCXXXX refers to this RFC Note 2: suggested x value: 5. Suggested y value: 6. Note that value 4 is already allocated by IANA, and thus, it cannot be re-allocated again as suggested in the draft. Issue 3: -------- IANA actions to IS-IS. Here is even less clear than with OSPF. First of all, I failed to understand what needs to be registered. The document says that two new ISIS TLV code-points. So this makes me think IANA has to act upon the 'TLV Codepoints Registry' of the 'IS-IS TLV Codepoints per [RFC3563]' registry. But this seems to be the registry of top level TLVs for IS-IS. However, this draft defines two sub-TLVs that are advertised in the IS-IS CAPABILITY TLV defined in draft-ietf-isis-caps. This made me doubt and think that this document is defining two sub-TLVs in the mentioned CAPABILITY TLV, in which case first the subregistry of the CAPABILITY TLV must be first created, and the filled up. If this is correct, I think draft-ietf-isis-caps and not this draft should establish the registry for sub-TLVs (which it doesn't), and then this draft should populate this registry with the two initial values. Please clarify this issue because it is very confusing and will be more confusing to IANA. Issue 4: -------- Is it true that there aren't any security issues? It is hard to believe these days about the lack of security issues. In most cases, absence of security considerations equals a lack of thought of those security issues. Perhaps the draft should 'import' and 'apply' the Security Considerations of the mother protocols (OSPF, IS-IS, OSPF and IS-IS Traffic Engineering, OSPF and IS-IS capabilities, etcc.). Perhaps the draft means that this document does not impose any security concern above those express in [list of mother protocols]. Still some thoughts... for example, what happens if a malicious router does a join operation to become part of a mesh-group it is not authorized or shouldn't be part of? Is this a potential attack or not? Editorial comments ------------------- 5) Some times the text uses the term "ISIS" and some times "IS-IS". Although existing RFCs (e.g., 4444 and 3358) use different acronym, I would suggest to use a single one throughout the text. 6) Expand IGP, ISIS, and OSPF in the Abstract and add them to the Terminology section. No need to expand MPLS, TE, and LSR in the Abstract since it is expanded already in the title. 7) Expand TLV and LSA in Section 2 8) Section 1, under Figure 1, the text reads: The TLV is padded to four-octet alignment; padding is not included in the length field (so a three octet value would have a length of three, but the total size of the TLV would be eight octets). I would expect that the four-octet alignment rule will make a three octet value encoded in four octets, three for the value and one more for padding. This differs from the eight octets indicated in the text. 9) Continuation of the previous point: the text then reads about a 32-bit alignment, implying that it is the same as the four-octet alignment already mentioned. I would suggest to always use the same unit of measure, either octets or bits, but not both. 10) At the beginning of Sections 4.1. and 4.2 the text first describes the structure of the TLV and then its purpose. I think it would be easier to read the other way round, so I would suggest. In Section 4.1, move the paragraph: The TE-MESH-GROUP TLV is used to advertise the desire of an LSR to join/leave a given TE mesh-group. No sub-TLV is currently defined for the TE-mesh-group TLV. above its preceding paragraph. In Section 4.2, extract from the first paragraph these sentences: The TE-MESH-GROUP sub-TLV is used to advertise the desire of an LSR to join/leave a given TE mesh-group. No sub-TLV is currently defined for the TE-MESH-GROUP sub-TLV. make them a standalone paragraph of its own, and make it the first paragraph in Section 4.2. 11) Figures 2, 3, 4 and 5 are missing a line of digits indicating the tens in bits. To be precise, replace: 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 with 0 1 2 3 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 at the top of Figures 2, 3, 4, and 5. And similarly, Figure 1 the line indicating the tens is misplaced. So in this Figure 1 replace: 0 1 2 3 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 with 0 1 2 3 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 12) I would suggest to add some text above Figures 2, 3, 4 and 5 introducing the figures, basically saying that the figures further break down the of the 'value' depicted in Figure 1. For example (for Figure 2): "The value of the TE-MESH-GROUP TLV is further structured, but the actual structure depends on the Type field. There are two types of TL-MESH-GROUP TLVs depending on whether they are taylored for IPv4 or IPv6. Figure 2 shows the format of the TE-MESH-GROUP TLV when the Type field indicates IPv4 addresses while Figure 3 shows the format of the same TLV when the Type field indicates IPv6 addresses." The same applies to Section 4.2. 13) More about Figures 2 and 3. The caption indicates that these figures are TLVs, however, ther Type and Length are not painted in the figures (I guess they are assumed from Figure 1). So I deduced (correct me if I am wrong), that these are not TLVs, but a portion of it. I would suggest to add the Type and Length boxes that you already have in Figure 1 to the top of Figures 2 and 3 to become real TLVs and increase readability. A side effect of the latter is that, when I read "Type: to be assinged by IANA" I don't need to come back to Figure 1 to see where the Type is represented. The same applies to Section 4.2. 14) In Figures 2 and 3 there is a box of undetermined length at the bottom of the figures that do not contain any name. I guess these boxes represent the continuation of the Tail-end name, but it is not clear at a first glance. Shouldn't the boxes be labeled to understand which contents are inside? 15) Then I would suggest to add new subheadings (e.g., 4.1.1 and 4.1.2) or some other types of delimiters to separate the two types of TLVs that correspond to Figures 2 and 3, respectively. The same applies to Section 4.2. 16) The draft uses the concept of a 'mesh-group entry', but it isn't explicitly defined. I deduced that a mesh-group entry is any of the objects represents in either Figures 2, 3, 4, or 5. I suggest to explicitly define it and perhaps refers to those figures in the definition. I believe this is an important concept to understand the join and leave operations. 17) Section 4.1, page 7 defines the Tail-end name as: - A Tail-end name: a variable length field used to facilitate the TE LSP identification. The Name length field indicates the length of the display string before padding, in bytes. I would suggest to start the description with what this object is rather than what is used for. I also suggest to separate the description of "Name length" into another paragraph, since it is separate field. Suggested rewording: - A Tail-end name: A display string that is allocated to the Tail-end. The field is of variable length field and is used to facilitate the TE LSP identification. - Name length field: An integer, expressed in octets, that indicates the length of the Tail-end name before padding. 18) Section 5.1 I would suggest to further indent the two paragraphs that describe Types 10 and 11, so they are clearly seen as children of the previous paragraph. So, the text could be formated as this: - entire OSPF routing domain (type 11). In this case, the flooding scope is equivalent to the Type 5 LSA flooding scope. A router may generate multiple OSPF Router Information LSAs with different flooding scopes. The TE-MESH-GROUP TLV may be advertised within a type 10 or 11 Router Information LSA depending on the MPLS TE mesh group profile: * If the MPLS TE mesh-group is contained within a single area (all the LSRs of the mesh-group are contained within a single area), the TE-MESH-GROUP TLV MUST be generated within a Type 10 Router Information LSA; * If the MPLS TE mesh-group spans multiple OSPF areas, the TE mesh-group TLV MUST be generated within a Type 11 router information LSA. 19) General comment applicable to all the text: I understand that the text sometimes reads about a TE mesh-group and sometimes about the TLV, which is named as "TE-MESH-GROUP TLV". According to this, the term "TE mesh-group TLV" shouldn't appear, because there is not such TLV. Suggestion: replace "TE mesh-group TLV" with "TE-MESH-GROUP TLV". At least I found one instance in Section 4.1 and another one in Section 5.1. Perhaps there are more... 20) I suspect that Section 5.2 goes beyond its scope by specifying procedures that are already specified elsewhere. For example, the text in the first paragraph reads: An IS-IS router MUST originate a new IS-IS LSP whenever the content of the any of the advertised sub-TLV changes or whenever required by regular IS-IS procedure (LSP refresh). While not being my field of expertise, I would expect that the IS-IS mother specification specifies when an IS-IS router originates a new LSP, independent of this specification. To avoid double specifying procedures, I would suggest to replace the above text and rewording along the following lines: According to the RFC xxxx [add ref], an IS-IS router originates a new IS-IS LSP whenever the content of the any of the advertised sub-TLV changes or whenever required by regular IS-IS procedure (LSP refresh). 21) Since this document provides IANA actions on two different registries, it might be a good idea to create Sections 7.1 and 7.2 devoted to OSPF and IS-IS, respectively. 22) Unused Reference: 'RFC3209' is defined on line 463, but not referenced