[Idr] Alvaro's AD Comments on draft-ietf-idr-ls-distribution
"Adrian Farrel" <adrian@olddog.co.uk> Sat, 03 October 2015 10:48 UTC
Return-Path: <adrian@olddog.co.uk>
X-Original-To: idr@ietfa.amsl.com
Delivered-To: idr@ietfa.amsl.com
Received: from localhost (ietfa.amsl.com [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id 4D6581ACF24; Sat, 3 Oct 2015 03:48:45 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -102
X-Spam-Level:
X-Spam-Status: No, score=-102 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, J_CHICKENPOX_29=0.6, RCVD_IN_DNSWL_LOW=-0.7, USER_IN_WHITELIST=-100] autolearn=ham
Received: from mail.ietf.org ([4.31.198.44]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id cPNb029lIk_e; Sat, 3 Oct 2015 03:48:41 -0700 (PDT)
Received: from asmtp2.iomartmail.com (asmtp2.iomartmail.com [62.128.201.249]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id 48BE31AD05B; Sat, 3 Oct 2015 03:48:41 -0700 (PDT)
Received: from asmtp2.iomartmail.com (localhost.localdomain [127.0.0.1]) by asmtp2.iomartmail.com (8.13.8/8.13.8) with ESMTP id t93Ama31017102; Sat, 3 Oct 2015 11:48:36 +0100
Received: from 950129200 (dsl-sp-81-140-15-32.in-addr.broadbandscope.com [81.140.15.32]) (authenticated bits=0) by asmtp2.iomartmail.com (8.13.8/8.13.8) with ESMTP id t93AmZSv017084 (version=TLSv1/SSLv3 cipher=AES256-SHA bits=256 verify=NO); Sat, 3 Oct 2015 11:48:35 +0100
From: Adrian Farrel <adrian@olddog.co.uk>
To: Alvaro Retana <aretana@cisco.com>
Date: Sat, 03 Oct 2015 11:48:36 +0100
Message-ID: <022901d0fdc9$0ea4ce80$2bee6b80$@olddog.co.uk>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
X-Mailer: Microsoft Outlook 14.0
Thread-Index: AdD9yQO60fpcSwLyRE2qgwPvotAKew==
Content-Language: en-gb
X-TM-AS-MML: disable
X-TM-AS-Product-Ver: IMSS-7.1.0.1679-8.0.0.1202-21854.006
X-TM-AS-Result: No--22.285-10.0-31-10
X-imss-scan-details: No--22.285-10.0-31-10
X-TMASE-MatchedRID: /bTTn+BoLfGI0KPyMNrNUvSG/+sPtZVk6Jj6zYvfFARcrXmN3JzdSyj5 3aEB5qDLXqyKGNlj+bBM/kLNgXkw4Lsl8Gv1eXkKXP5rFAucBUFyawdArtww599RlPzeVuQQbtO Jifv5keM1+ur+dPcuTfg6HAfVSMztzsxad8fKloZIcJTn2HkqsSOjxzQ2Rhgi+Cckfm+bb6Ba3+ jh2Kxjc2JTtZG4fJE1zG2gYPz0DrkTY/HRCFabdAhWgIsZuXlPGSqdEmeD/nUkqhTM/UZ9tii3h 4ReYbaX2Z3p8kMWERBz9l/9DJOekCVqTewNlt0eVo6mn+xXmdVAg1IWH8kbY1MnnsDzMI/04K0L m8vvPlVbVyEwCEUBHDctK0KUOQ1l+mXiTEe9zMUG7bNEJKsuNqX1XMd/SqvuxN4K0DXKS1hTGRT OR+jFyvi12qLq8EUdBxVVqSr+s6ZCzHFhiypoVsg6fo0rxLVrW0/6WPAv0J/JwfJEFqACwYxjQX Vn9/QiS4Qt/q7VVQt9LCCek00RFkm3PFovGG1bt1AhvyEKdj685/eNJbrs7GHrKQqddbU4dO/tk DTEgySFHenhvAeGneEccfDC+OkOiAu/LCYLhG/sEjYqO+DyaEeeKTVwRgrpzsQ8iRVyD46GkTn1 RxBcpZpy+Yx5yIznKFKUiJL1+WpIL8dxyyZLf+JmMZmLmwx5Q87jeIrU1+u8NrbzjPvzJ/Ay4SY TK0I8UgmpeIU8MMb0wDvpKqldXHZN97YB6zrdE0Q83A2vD+vvkIRmqjgVzE4ijQ77llXhCKyMLV HvlDCAwPgM1Jrd1XkPNF6Z/501XciWNQDc56+eAiCmPx4NwLTrdaH1ZWqCpvI8UZOf47jUZxEAl FPo846HM5rqDwqtlExlQIQeRG0=
Archived-At: <http://mailarchive.ietf.org/arch/msg/idr/wbPNQ-HM2NeR75gR2Or948J9o1I>
Cc: idr@ietf.org, draft-ietf-idr-ls-distribution@ietf.org
Subject: [Idr] Alvaro's AD Comments on draft-ietf-idr-ls-distribution
X-BeenThere: idr@ietf.org
X-Mailman-Version: 2.1.15
Precedence: list
Reply-To: adrian@olddog.co.uk
List-Id: Inter-Domain Routing <idr.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/idr>, <mailto:idr-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/idr/>
List-Post: <mailto:idr@ietf.org>
List-Help: <mailto:idr-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/idr>, <mailto:idr-request@ietf.org?subject=subscribe>
X-List-Received-Date: Sat, 03 Oct 2015 10:48:45 -0000
Thanks, Alvaro, for your review of (I think) version -10 of the draft. Here are some answers to your questions. > Major: > > Section 5 (IANA Considerations) > I didn't see this comment as part of IANA's review, but the text about the Designated Experts > should be clarified. There is some guidance in RFC5226. You're right. I think we should add a new subsection 5.1... 5.1 Guidance for Designated Experts In all cases of review by Designated Expert (DE) described here, the DE is expected to ascertain the existence of suitable documentation (a specification) as described in RFC5226, and to verify the permanent and publically readily available of the document. The DE is also expected to check the clarity of purpose and use of the requested code points. Lastly, the DE must verify that any specification produced in the IETF that requests one of these code points has been made available for review by the IDR working group, and that any specification produced outside the IETF does not conflict with work that is active or already published within the IETF. > "Note to RFC Editor: this section may be removed on publication as an RFC." The IANA > considerations section should remain because there are in fact considerations there. Yes, this text is a hang-over from the document template. > I would like to have Designated Experts in place soon. It seems to me that having 2 of the > authors volunteer (primary and backup) would be ideal. Please let me know if you want to do it. I think the absence of an answer speaks for itself. > 6.1.2 (Installation and Initial Setup): [default] "maximum rate at which Link-State NLRIs will > be advertised/withdrawn from neighbors is set to 200 updates per second." Where did the > number come from? It does look very big. Are you assuming just changes or startup as well? > One of my concerns is the interaction with other BGP stuff (like regular routing!). > In fact, in 6.1.5 (Impact on Network Operation) you wrote: "Frequency of Link-State NLRI > updates could interfere with regular BGP prefix distribution. A network operator MAY use > a dedicated Route-Reflector infrastructure to distribute Link-State NLRIs." > Why not use SHOULD? If there may be significant operational impact I think you should be > more direct with the guidance. There are several questions folded in here. Firstly, understand that the maximum rate described here is the default rate if not overridden by an operator or by an implementation that knows better. So we shouldn't get too hung up on it. Although, obviously, there is a reasonable expectation that if an implementation uses this default value the function should work. Why 200? Not sure where that came from. I have no evidence that 200 is a problem and given that IDR has done quite a bit of implementation and interop without anyone screaming, I assume this is OK. https://www.ietf.org/id/draft-ietf-idr-ls-distribution-impl-04.txt doesn't mention any problems. Are you aware of implementations where this is a problem? Could BGP-LS advertisements impact on "other BGP stuff"? Well, that really is an implementation and deployment issues? Will the BGP-LS speaker doing the advertisements also advertise "other BGP stuff" on the same session? Is the advertisement higher cost than the consolidation of IGP-TE information and the application of policy to determine what LS information to advertise. Could the advertisements be prioritised? This has been something of a recent thread on the IDR list and, although you are raising interesting questions, I don't believe we have any way to draw a line unless someone reports real problems that are not caused by implementation choices, yet we are required (by the IESG) to supply defaults for configurable parameters. Why "MAY" use a dedicated RR rather than "SHOULD"? Because there really doesn't appear to be any indication that this is a problem in all networks with all BGP implementations. > 6.2.2 (Fault Management). "If an implementation of BGP-LS detects a malformed attribute, then > it SHOULD use the 'Attribute Discard' action.." Doesn't this mean that the information may be > useless, completely missing, or in the best case incomplete? Aren't we better off just resetting > the session or at least requesting a route refresh? I don't think we are assuming corruption. So the malformed attribute comes as the result of an implementation difference or a bug at the sender. Requesting a refresh runs a significant risk of simply thrashing the advertisement since each time it is sent it will contain the same malformed attribute. This problem is considerably worsened if the session is reset because then we thrash the whole session. > Minor: > > 3.1 (TLV Format): "Unrecognized types are preserved and propagated." This statement sounds necessary > for interoperability and the correct propagation of the information. Should it have a "MUST" in it? Isn't this standard behavior for unknown TLVs in BGP? E.g., 5512 section 4 "Unknown types are to be ignored and skipped upon receipt." That said, 5512 section 4 also says "When the TLV is being processed by a BGP speaker that will be performing encapsulation, any unknown sub-TLVs MUST be ignored and skipped." Since the behavior described here is new in as much as it describes processing of a new NLRI, I think using "MUST" would be OK. > 3.1 (TLV Format): "If there are more TLVs of the same type, then the TLVs MUST be ordered > in ascending order of the TLV value within the TLVs with the same type." Ordered by value? > How is that done? Some TLVs contain sub-TLVs in the value field, but others don't. We could say "treating the entire value field an opaque hexadecimal string and comparing leftmost octets first regardless of the length of the string." > 3.2.1 (Node Descriptors): The BGP-LS Identifier is used, but it is defined later in 3.2.1.4. > Please add a reference so that it is not mistaken for the "Identifier" (in the NLRIs > themselves in Section 3.2). Per Alia's comment replacement text is... BGP-LS uses the Autonomous System (AS) Number and BGP-LS Identifier (see Section 3.2.1.4) to disambiguate the Router-IDs, as described in Section 3.2.1.1. > 3.3.2 (Link Attribute TLVs): The attributes used are defined for ISIS, some are easily > derived from OSPF, but other don't map to anything. For example, "administrative > group (color)". How are these types of undefined parameters in OSPF figured out? > I suspect the easy answer is to just say that it is out of scope, but the fact remains > that there can be information there that doesn't exist in the original protocol. It > should at least be clarified what is expected. Note that in 3.3.2.5 (Shared Risk Link > Group TLV) a different case of this shows up: the text seems to indicate that > because there is no SRLG TLV in OSPD-TE, then this information wouldn't be > available. Again, please clarify if there may be procedures outside the scope of > the document that could apply. I'm not sure the missing attributes should be "figured out". The job of BGP-LS is to collect and distribute information from the IGP. If the information is not there, it would not be invented by the BGP-LS speaker. The SRLG case has been fixed in -11: it was an error. > 3.3.2.2 (MPLS Protocol Mask TLV) > OLD> > Generation of the MPLS Protocol Mask TLV is only valid for > originators which have local link insight, like for example Protocol- > IDs 'Static' or 'Direct' as per Table 2. The 'MPLS Protocol Mask' > TLV MUST NOT be included in NLRIs with protocol-IDs 'IS-IS L1', 'IS- > IS L2', 'OSPFv2' or 'OSPFv3' as per Table 2. > NEW> > Generation of the MPLS Protocol Mask TLV is only valid for > originators which have local link insight, like for example Protocol- > IDs 'Static' or 'Direct' as per Table 2. The 'MPLS Protocol Mask' > TLV MUST NOT be included in NLRIs with other protocol-Ids. Why? Certainly your proposal appears right: there are only four protocols listed in Table 2, but there is the question of future IDs. So we could write... Generation of the MPLS Protocol Mask TLV is only valid for originators which have local link insight, like for example Protocol- IDs 'Static' or 'Direct' as per Table 2. The 'MPLS Protocol Mask' TLV MUST NOT be included in NLRIs with the other Protocol-IDs listed in Table 2. I don't object to this change, but struggle to see the value. > "LINK_STATE attribute" shows up in 3.3.2 and 3.3.3. What is it? Thanks for this catch. Old text. Should now read "BGP-LS attribute with a link NLRI" > In 3.3.1.1 (Node Flag Bits TLV) you included the ISIS Overload Bit, but didn't > say anything about OSPFv3's R-bit or v6-bit; and in 3.3.3.1 (IGP Flags TLV) you > included the IS-IS Up/Down Bit in this TLV, but I didn't see the OSPF DN-bit > anywhere. Are these opportunities to reuse the same bit with a more generic > definition while covering a similar option in OSPF? I know you can't cover all > the options, but these two are well-known and seem low hanging fruit. Although no-one was asking for this and no-one will have implemented it, I can see no reason not to change to read... 3.3.1.1. Node Flag Bits TLV The Node Flag Bits TLV carries a bit mask describing node attributes. The value is a variable length bit array of flags, where each bit represents a node capability. 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 +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ | Type | Length | +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ |O|T|E|B|R|V| Rsvd| +-+-+-+-+-+-+-+-+-+ Figure 15: Node Flag Bits TLV format The bits are defined as follows: +----------+-------------------------+-----------+ | Bit | Description | Reference | +----------+-------------------------+-----------+ | 'O' | Overload Bit | [RFC1195] | | 'T' | Attached Bit | [RFC1195] | | 'E' | External Bit | [RFC2328] | | 'B' | ABR Bit | [RFC2328] | | 'R' | Router Bit | [RFC5340] | | 'V' | V6 Bit | [RFC5340] | | Reserved | Reserved for future use | | +----------+-------------------------+-----------+ Table 8: Node Flag Bits Definitions And similarly no-one has asked for the DN-bit from 2328 or 5340 in section 3.3.3.1. I am less sanguine about adding this as I am not quite sure that it is useful in BGP-LS, but it would certainly be easy to add. With this bit, and with other potential additions to both bit fields, I suggest we wait for the need/desire and folk can write new drafts. > 6.2.5. (Performance Management) It might be nice to include updates/second as > one of the performance indicators. I think that we add to the bottom of 6.2.5 as follows: "These statistics should be recorded as absolute counts since system or session start time. An implementation MAY also enhance this information by also recording peak per-second counts in each case." > Section 8 (Security Considerations) "The principal attack a consumer may apply is to > attempt to start multiple sessions either sequentially or simultaneously." Isn't this > an attack that any other node can try? Why limit the discussion only to consumers? I think you have to be a recognised peer for this attack to have any significant value that is specific to *this* document. > Looking at the references, it seems to me that the following changes could > be made: > Make Informative: RFC1918 Sure > Make Normative: I-D.ietf-idr-error-handling Yes, and this is now RFC7606 > Curious question: When encoding a "normal" size node using BGP-LS, what is the > resulting size of the UPDATE? I'll leave this as an exercise for the reader. > Nits: > 3.2: s/'Static' protocol types/'Static configuration' protocol types Again in 3.3.2.2. > 3.2.1 and 6.2.3 : What does "Paragraph 2" refer to? > 3.2.1.2: "mandatory TLV in all three types of NLRIs" I assume you're referring to Node, > Link and Prefix NLRIs, right? However, it is confusing because there are in fact 4 types > defined. Please correct. > 3.2.1.4: "IGP Router ID.mandatory TLV" Should be sub-TLV. > 3.2.3.1: "The Route Type TLV allows to discrimination of these advertisements./The > Route Type TLV allows the discrimination of these advertisements. > 3.3.2.2: Please be consistent in the name. Is it "MPLS Protocol Mask TLV" or just > "MPLS Protocol TLV"? > The description of the example in 3.7 is easier to read (because of the formatting) than > the one in 3.6. It would be nice to be consistent. > 3.8 seems to be a better example of simply ships in the night than migration as the > emphasis is put on identifying the common links of the two processes. > 4.1 s/"see" the full topology of AS/"see" the full topology of AS2 I'll work through the nits as well. New revision "soon". Thanks, Adrian
- [Idr] Alvaro's AD Comments on draft-ietf-idr-ls-d… Adrian Farrel
- Re: [Idr] Alvaro's AD Comments on draft-ietf-idr-… Acee Lindem (acee)
- Re: [Idr] Alvaro's AD Comments on draft-ietf-idr-… Alvaro Retana (aretana)
- Re: [Idr] Alvaro's AD Comments on draft-ietf-idr-… Adrian Farrel
- Re: [Idr] Alvaro's AD Comments on draft-ietf-idr-… Adrian Farrel
- Re: [Idr] Alvaro's AD Comments on draft-ietf-idr-… Adrian Farrel
- Re: [Idr] Alvaro's AD Comments on draft-ietf-idr-… Alvaro Retana (aretana)
- Re: [Idr] Alvaro's AD Comments on draft-ietf-idr-… Jeffrey Haas
- Re: [Idr] Alvaro's AD Comments on draft-ietf-idr-… Alvaro Retana (aretana)
- Re: [Idr] Alvaro's AD Comments on draft-ietf-idr-… Susan Hares
- Re: [Idr] Alvaro's AD Comments on draft-ietf-idr-… Susan Hares
- Re: [Idr] Alvaro's AD Comments on draft-ietf-idr-… Adrian Farrel