Re: [Gen-art] Gen-ART Last Call review of draft-ietf-ospf-two-part-metric-05

[Brian E Carpenter <[e-mail]>]

Hi Acee,

Thanks, just a few points in line:

On 09/08/2016 05:47, Acee Lindem (acee) wrote:
> Hi Brian, 
> 
> Thanks much for the thorough review. Jeffrey and I discussed your comments
> this morning. See responses to your major/minor comments below. We will
> incorporate all the nits.
> 
> On 8/6/16, 9:38 PM, "Brian E Carpenter" <[e-mail]>
> wrote:
> 
>> I am the assigned Gen-ART reviewer for this draft. The General Area
>> Review Team (Gen-ART) reviews all IETF documents being processed
>> by the IESG for the IETF Chair.  Please treat these comments just
>> like any other last call comments.
>>
>> For more information, please see the FAQ at
>> <http://wiki.tools.ietf.org/area/gen/trac/wiki/GenArtfaq>.
>>
>> Document: draft-ietf-ospf-two-part-metric-05.txt
>> Reviewer: Brian Carpenter
>> Review Date: 2016-08-07
>> IETF LC End Date: 2016-08-15
>> IESG Telechat date:
>>
>> Summary: Almost ready
>> --------
>>
>> Major issues:
>> -------------
>>
>>> Updates: 2328, 5340 (if approved)
>>
>> If that is so, the text needs to explain what is changed in those two
>> RFCs. Since
>> this draft describes an "optional extension" to OSPF, it does not
>> obviously update
>> them. Is any text in those two RFCs made invalid by this draft?
> 
> This has been an ongoing debate as to whether an RFC the augments an
> existing draft updates it or whether it must actually change the existing
> behavior. In this case, the SPF calculation is modified as specified in
> section 3.6 but only when the new Network-to-Router metric is advertised.
> In RFC 2328 and RFC 5340, this cost is always zero (i.e., cost to reach
> all routers connected to the network is solely the outgoing metric metric
> or Router-to-Network metric).

OK, fair comment.

> 
> I, for one, would be very happy to have consensus of precisely what
> constitutes update to an existing RFC. 

So would many people, and since it affects all RFC streams, not just the
IETF stream, I happen to know that the RFC Editor is working on definitions
for both "updates" and "obsoletes".

> If we don’t update the existing
> RFCs, we would avoid the pre-2008 IPR language.

That doesn't seem right. You only need that language if you are updating
whole chunks of older text. If you take a paragraph from a pre-2008 document,
change a few words, and patch it into the new document, you need either
the agreement of the original authors or the pre-2008 disclaimer. But I
don't think you're doing that in this case, are you?

>>> 3.6.  SPF Calculation
>>>
>>>   During the first stage of shortest-path tree calculation for an area,
>>>   when a vertex V corresponding to a Network-LSA is added to the
>>>   shortest-path tree and its adjacent vertex W (joined by a link in V's
>>>   corresponding Network LSA), the cost from V to W, which is W's
>>>   network-to-router cost, is determined as follows:
>>
>> I can't parse that sentence. If we delete the subordinate clauses, we get
>>
>>  When a vertex V is added to the shortest-path tree and its adjacent
>> vertex W,
>>  the cost from V to W is determined as follows:
>>
>> What does that mean? What does "its" refer to? Is W adjacent to V, or is
>> W adjacent
>> to the existing tree? Is W added to the tree before V, or is V added
>> before W?
>> If I was coding this, I'd have no idea what to do.
> 
> You really do have to look at RFC 2328 to understand it. 

Yes, I did that in some detail when I was teaching routing a few years ago ;-)

> Does this
> modified text parse better?
> 
>     The first stage of the shortest-path tree calculation is described
>     in section 16.1 of [RFC 2328] and modified for OSPFv3 as described in
>     section 4.8.1 of [RFC 5340]. When a vertex V corresponding to a
> Network-LSA
>     has just been added to the Shortest Path Tree (SPT) and an adjacent
> vertex W 
>     (joined by a link in V’s corresponding Network-LSA) is being added to
>     the SPT, the cost from V to W (W’s network-to-router cost) is
> determined 
>     as follows:

MUCH better. It also clarifies the "Updates:" aspect.

Thanks
   Brian

> 
>>
>>> 3.7.  Backward Compatibility
>>
>> This calls for a Router Functional Capability Bit assignment under RFC
>> 7770.
>> The bit number should be given as (say) TBD1 not as 0.
>>
>>> 4.  IANA Considerations
>>
>> The IANA considerations ask for four assignments. These should be
>> specified as TBD1,
>> TBD2, TBD3, TBD4 and the TBDs elsewhere in the text should be updated
>> correspondingly.
>> Also, please reference the relevant RFCs (7770 and whatever defines the
>> Sub-TLV registries.)
> 
> 3.7 and 4 are both fixed in -06 based on comments from Alia.
> 
>>
>> Finally, to put this on the standards track, I would really expect to see
>> an Implementation Status section (RFC 7942). Has this been tested?
> 
> The implementation of this stalled. However, it is viewed by the WG as
> straight-forward enough to advance.
> 
> 
>>
>> Minor issues:
>> -------------
>>
>> Please check the three occurrences of lower-case "must" in Section 3.
>> Should they be "MUST"?
>>
>>> 5.  Security Considerations
>>>
>>>   This document does not introduce new security risks.
>>
>> That's easy to say but hard to prove. Shouldn't you at least refer to the
>> security
>> considerations of OSPFv2 and OSPFv3?
> 
> We will add the reference.
> 
>>
>> Also, does section 3.7 introduce a new risk whereby a rogue router could
>> flap its
>> Two-Part Metric bit on and off, causing all its OSPF peers to continually
>> recalculate
>> their routes?
> 
> This is no more of a risk than other intra-area metric change.
> 
> Thanks,
> Jeffrey and Acee 
> 
> 
> 
> 
>>
>> Nits:
>> -----
>>
>>> Requirements Language
>>
>> It's unusual to put this at the front. The normal place is after the
>> Introduction.
>>
>>>  This document may contain material from IETF Documents or IETF
>>>  Contributions published or made publicly available before November
>>>  10, 2008. ...
>>
>> Why is this needed? What did you copy from an old document?
>>
>>> 0 OSPF Two-part Metric [TPM]
>>
>> The abbreviation TPM is defined but not used, so why bother? Also,
>> s/[TPM]/(TPM)/ to
>> avoid confusion with a reference.
>>
>>> routes w/o considering any network-to-router costs.
>>
>> Just say "without".
>