The Accumulated IGP Metric Attribute for BGP
draft-ietf-idr-aigp-18

In Section 7, it seems like improper configuration on both ends of an EBGP connection could result not only in unsound path selection, but also in the leakage of information outside of an administrative domain that was meant to be kept inside. Might be worth a mention.

- 
  When an AIGP attribute is created, it SHOULD contain no more than one
  AIGP TLV.  However, if it contains more than one AIGP TLV, only the
  first one is used as described in Section 3.4 and Section 4.  In the
  remainder of this document, we will use the term "value of the AIGP
  TLV" to mean the value of the first AIGP TLV in the AIGP attribute.
  Any other AIGP TLVs in the AIGP attribute MUST be passed along
  unchanged if the AIGP attribute is passed along.

Like Stefan in his OPS-DIR, I was confused by this.
Figure 1 only shows one TLV.
And the spec. says: SHOULD contain no more than one TLV
But if there is more than one, don't pay attention to it.
I saw Eric's answer:

    This allows for a future expansion in which additional TLVs can be appended
    without impacting path selection in nodes that don't understand the
    additional TLVs. 

It's in line with "Be conservative in what you send, be liberal in what you accept", but looks weird.
I read this as: I have some protocol extensions in mind, so let me prepare the spec for multiple TLVs, but I won't mention those in this spec. Or maybe I missed something (maybe this is common practice for BGP attributes?)
If not, 
	1. the writeup mentions implementations, which is a good thing
	2. this is only a comment
are two good reasons to ignore this, unless you feel like replying.

-
   This document only considers the use of the AIGP attribute in
   networks where each router uses tunneling of some sort to deliver a
   packet to its BGP next hop.  Use of the AIGP attribute in other
   scenarios is outside the scope of this document.

Don't you need stronger RFC 2119 wording here?
   The AIGP attribute MUST only be applied if each router uses tunneling 
   of some sort to deliver a packet to its BGP next hop.

- Editorial. First occurrence of AIGP:

   We will refer to the set of
   ASes in a common administrative domain as an "AIGP Administrative
   Domain".

A in AIGP = AS?
Well, no, if we pay attention the title, A = Accumulated

I agree with Alissa that there should be a mention of the possibility of leakage in the Security Considerations section.  

Although the scope is limited to an administrative domain boundary, a statement is made in Section 2, that says "The specified procedures prevent the AIGP attribute from "leaking
   out" past an AIGP administrative domain boundary into the Internet."  

Section 3.3 tells you what to do when you receive this value where it is not expected, so clearly there is a way to handle the possibility of leakage. - and -
Section 3.4.1 seems to cover the restrictions of sending this value, limiting it to an administrative domain.

It's a little confusing to have the document state that leakages are prevented and then having a way to handle the leakage.  A mention of the possibility appearing in the security considerations section could be helpful.

No objection to this document, but the shepherd write-up seems to be truncated at one position under Working Group Summary :
"The draft-ietf-idr-aigp-14.txt was" 

Not sure if this is a left-over or if any information is missing.

3.3/3.4.1: I really don't like MUSTs and SHOULDs for configuration items. Blech! Ptew! I'd much rather see an explanation of expected *behaviors* (with as many MUSTs and SHOULDs as you like) and leave the configuration items as "implementation advice". This one I find especially silly:

   It SHOULD be possible to set AIGP_ORIGINATE to "enabled for the
   routes of a particular IGP that are redistributed into BGP" (where "a
   particular IGP" might be "OSPF" or "ISIS").

But, now having tilted at that windmill, I will move on with my day.

3.4.1:

   The AIGP attribute may be added only to routes that satisfy one of
   the following conditions:

Should that be, "The AIGP attribute MUST NOT be added to routes unless they satisfy one of the following conditions"? Seems like a straightforward requirement to me, unless I'm misunderstanding what this is trying to say. Is this just a definition, or are you telling implementations that they ought not apply the attribute to other routes even though they might want to?

Aside from this relatively minor nit, which I express as a DISCUSS solely because of the tiny potential for interoperability issues, the document looks good and is clear and understandable, at least to the limits of my comprehension of routing documents. :)

Former DISCUSS text:
In section 3, isn't Accumulated IGP Metric a 64-bit (unsigned?) integer expressed in network byte order?   The current text suggests that IGP metrics might be added together to produce the AIGP metric, but without specifying a representation, I don't see how you can have interoperability.   The current text could as easily apply to an 8-octet ascii string, for instance.   I realize that this is a bit obvious, but it seems easy and harmless to fix.

This DISCUSS should be easy to address, either by specifying the data representation, or by pointing out to me why that's a bad idea.