Re: [OSPF] Alvaro Retana's Yes on draft-ietf-ospf-prefix-link-attr-10: (with COMMENT)

["Acee Lindem (acee)" <[e-mail]>]

Hi Alvaro, 

I’m glad you are continuing in Adrian’s tradition of fastidious document
reviews ;^) 

On 8/13/15, 2:55 PM, "Alvaro Retana (aretana)" <[e-mail]> wrote:

>Alvaro Retana has entered the following ballot position for
>draft-ietf-ospf-prefix-link-attr-10: Yes
>
>When responding, please keep the subject line intact and reply to all
>email addresses included in the To and CC lines. (Feel free to cut this
>introductory paragraph, however.)
>
>
>Please refer to https://www.ietf.org/iesg/statement/discuss-criteria.html
>for more information about IESG DISCUSS and COMMENT positions.
>
>
>The document, along with other ballot positions, can be found here:
>https://datatracker.ietf.org/doc/draft-ietf-ospf-prefix-link-attr/
>
>
>
>----------------------------------------------------------------------
>COMMENT:
>----------------------------------------------------------------------
>
>This is a good document, but I have some comments (and nits) that I would
>really like to see addressed before publication.  I don't think my
>comments raise to the level of a DISCUSS, but I am specially interested
>in the actions when errors are detected.
>
>
>1. Sections 2 and 3.
>
>a. RFC5250 used the name Opaque ID instead of Instance.  Please be
>consistent.  If there’s a really good reason to not use the “official”
>name, please at least write in the description of Instance that rfc5250
>calls it Opaque ID.

I agree - we can change this to use Opaque ID consistent with RFC 5250.


>
>b. “…the attributes from the Opaque LSA with the lowest Instance will be
>used.”  This sounds to me like a good place to be a little bit more
>prescriptive and s/will/MUST

We will make this a “SHOULD”.

>
>
>2. Section 2.1. (OSPFv2 Extended Prefix TLV)
>
>a. For the Route Type, are those the only allowed values?  What happens
>if something else is used?

Yes - these are the only route types allowed. If an invalid type is
specified, the attributes will not be correlated with a route.


>
>b. Flags
>- What are the defined flags used for?  Since (according to Section 5)
>they are not supported in all implementations then I’m assuming there’s a
>specific application in mind. [I know there are other ID’s in progress
>that use the TLVs defined here.  Is the use of these bits general to
>those applications?]

The “Flags” will be supported by all implementations. The reason they are
not implemented by all implementations is that they have been added and
changed as the draft evolved.


>
>- How should the rest of the flags be assigned?  The IANA section says
>nothing about that.

We will add a registry.


>
>c. "Address Prefix     The prefix itself encoded as a 32-bit value.”  It
>is variable, not fixed.  I know that only IPv4 is currently supported,
>but as explained in the AF section, there may be future extensions.

I already changed this once, since IPv4 is the only AF specified in this
document. 
“For Address Family IPv4, the prefix is encoded as a 32-bit value.
Encoding for 
 other address families is not defined by this specification.”



>
>
>3. Section 3.1. (OSPFv2 Extended Link TLV)  What should happen if an
>unknown/incorrect Link-Type/Link-ID/Link Data value is received?

The attributes will not be correlated to an OSPF Router-LSA link.


>
>
>4. Section 6. (Security Considerations)
>
>a. I think you should also point the readers at the considerations in
>rfc5250.
>
>b. “…implementations must assure that malformed TLV and Sub-TLV
>permutations do not result in errors that cause hard OSPF failures.”
>What does that mean?  That the software should be resilient?  Or are you
>pointing at just ignoring the information if there is a malformed
>TLV/sub-TLV (that is somehow salvageable)?  It would be vey nice is this
>draft set the example by specifying what to do in some cases (maybe not
>malformed, but incorrect/unknown as mentioned above).


This stems from prior attacks on routing protocols involved carefully
crafted malformed TLVs which could result in software crashes.


>
>
>5. Section 7.  (IANA Considerations)  Is there a significant reason why
>in all cases the assignment policy for the 33024-65535 range is not being
>defined upfront?

The assignments are deferred until needed in cases we have requirements to
crave them up differently. I believe this is a common practice.



>
>
>Nits:
>1. In Sections 2 and 3  s/differential/differentiate

Ok

>
>2. In Section 2. (OSPFv2 Extended Prefix Opaque LSA), the figure that
>shows the TLV format seems to show a Value field of only 32 bits.  The
>description (below the figure), which looks like an exact copy from
>rfc3630, describes the field correctly.  It would be nice to either copy
>over the whole text from rfc3630 (including the figure that shows that
>the Value may be longer) or just leave out that text completely (which is
>my preference).   The text saying that the TLV format is the same as in
>rfc3630 is enough.

Are you not familiar with “…”? I’ll fix this as this rather than omit it.
In this case, the description is brief and allows one to implement the
encoding w/o referring to RFC 3630. I can attest to the fact that
programmers appreciate this.



>- In Section 3 you write: "The format of the TLVs within the body of the
>OSPFv2 Extended Link Opaque LSA is the same as described in Section 2.”
>One more level of indirection..

As stated above, RFC 3630 need not be consulted.


>
>3. 2.1. (OSPFv2 Extended Prefix TLV)
>a. The description of “Flags” is misaligned (it looks like a
>sub-paragraph of AF).

Ok. 


>b. s/originated by the different/originated by different

Ok. 

Thanks,
Acee 

>