I have reviewed this document as part of the security directorate's ongoing effort to review all IETF documents being processed by the IESG. These comments were written primarily for the benefit of the security area directors. Document editors and WG chairs should treat these comments just like any other last call comments. The summary of the review is: almost ready Note: this is an early review, per request In general, the document reads fairly well but could use some improvements still. General notes: - Many acronyms are not expanded on first use like they generally should be except for the rare ones that likely everyone understands (BGP). Though MPLS and LSR, for example, are probably in the grey-area for those I'd suggest expanding them. I found it humerus, actually, to find the expansion for NLRI on the 7th page (section 3) even though it had been used a bunch before that. - There are more than 5 authors/editors on the draft, where 5 is the current recommended maximum. - In general I think the introduction could use a bit more clarity about the motivation and background of the document. Maybe hint at the example in section 2.2 about the purpose of needing this new option, etc, or even moving the example into the intro and referring back to it. - Appendix A seems important enough to warrant a pointer to it from somewhere else in the document (eg, section 3). I found it surprising to be at the end but I hadn't heard of it yet. Security specific thoughts: - the security considerations sections rightly discusses privacy but then indicates that leaks will only happen when a router doesn't implement the specification, which is going to be very common in the beginning. Thus, it seems that strong advice in the top of the document to operators that indicates RCAs should only be set up when all border routers that might propagate them also implement the RCA specification. And in the security considerations or somewhere it should state that "implementations MUST use default configuration settings with RCA disabled". Specific issues: - Section 1: "the RCA identifies itself with the next hop of the originator". I'm not sure "identifies" is the right term there. Maybe "the RCA is associated with..." or... - The second to last paragraph in section 1 ("An RCA carried...") could use a bit of clarity/rewriting to cover all the potential cases and how to handle them. One particular concern that is almost security-section worthy, but I didn't finish thinking through (but you're the experts) is what to do when new routes arrive that don't have a RCA where a previous route did. I don't think this case is clearly covered on what should be done in this case. The reason for the potential security concern is whether future route advertisements can affect or rely on previous ones with the RCA was attached. - 2.1 If you want to ensure maximum interoperability (always a goal, IMHO), I would delete the MAY for handling things in any order. I'd [IMHO] be very prescriptive and either require everything to be in order and routers should drop any RCA that doesn't match this requirement, or simply always process in order (or, I suppose, force routers to order them after receipt but then you still run into ordering of the same-code RCAs). Doing something other than strict processing requirements means different routers may behave differently. Similarly, in 2.4 there is a malformed-is-dropped and processing continues. This may or may not be safe and it might be better to not process the entire RCA if any section of it can't be understood. - 2.2 "In effect, this means that a given capability TLV will only be propagated along a path where it is suported by every BGP next-hop along the path" -- this might be true when constructing new RCAs, but isn't true when a router doesn't support RCA at all and passes it along as a binary blob. In fact, this is stated two paragraphs later but could be better tied together / explained so it's clear which type of router (supporting or not) passes on RCAs/values. Nits: - The sentence in the introduction in ()s that states the name is somewhat regrettable could probably be removed. It doesn't really add to the document and simply saying "note that this specification should not be confused with the BGP capabilities document [RFC5492]" - 2.1: "In turn, each Capability is a triple". Two comments: 1. Here and likely other places, you may want to use "Router Capability Attribute" or "RCA" instead of "Capability", since earlier you made the point of ensuring you're not talking about BGP capabilities -- thus it makes sense to use consistently different/unique terminology throughout. 2. I'd probably change "triple" to "Tag/Length/Value set" or something to be super clear, otherwise TLV is never really spelled out. - 2.1 the capability length is supposed to be the length of *just the value* but in the ()s after "triple" it's listed as "Capability Length" which could confuse readers that it should include the code/length length header too (IE, an extra 16). - 2.1 "... with different Capability Value and either ... Length." -- I'd put "and either the same or different Capability Length" into ()s as it's less important in the uniqueness requirement than the value, which deserves greater emphasis (how nit is this comment, I know). - 2.2 "If S is originating R" ... "MUST set the header" -- you might describe when this should be done and when it shouldn't. Certainly if the intention was to include an RCA, then it's a MUST. But it may be that one *shouldn't* be included at times (eg, when the trying to avoid leaking the upstream N, like discussed later) - section 3 could use a short sentence simply helping with the context switch between describing RCAs generically and now switching to the only document-defined code-point. Something like "ELCv3 is a capability code defined in this document to advertise the next hop..." - 3.2: "MUST NOT include the ELCv3 capability except" - it might be easier to break this into two parts to avoid confusion. maybe "SHOULD include the ELCv3 capability when it knows that the egress of the associated LSP L is EL-capable, otherwise it MUST NOT include the ELCv3 capability." - 3.3: the last paragraph of 3.3 could also be broken into two sentences -- it's rather long and hard to read as is -- Wes Hardaker USC/ISI