Last Call Review of draft-ietf-ospf-segment-routing-msd-17
review-ietf-ospf-segment-routing-msd-17-genart-lc-kyzivat-2018-08-27-00

Request Review of draft-ietf-ospf-segment-routing-msd
Requested rev. no specific revision (document currently at 25)
Type Last Call Review
Team General Area Review Team (Gen-ART) (genart)
Deadline 2018-08-29
Requested 2018-08-15
Draft last updated 2018-08-27
Completed reviews Rtgdir Early review of -10 by Tal Mizrahi (diff)
Rtgdir Last Call review of -15 by Tal Mizrahi (diff)
Secdir Last Call review of -18 by Vincent Roca (diff)
Genart Last Call review of -17 by Paul Kyzivat (diff)
Genart Telechat review of -20 by Paul Kyzivat (diff)
Assignment Reviewer Paul Kyzivat
State Completed
Review review-ietf-ospf-segment-routing-msd-17-genart-lc-kyzivat-2018-08-27
Reviewed rev. 17 (document currently at 25)
Review result Ready with Issues
Review completed: 2018-08-27

Review
review-ietf-ospf-segment-routing-msd-17-genart-lc-kyzivat-2018-08-27

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-segment-routing-msd-17
Reviewer: Paul Kyzivat
Review Date: 2018-08-27
IETF LC End Date: 2018-08-29
IESG Telechat date: ?

Summary:

This draft is on the right track but has open issues, described in the 
review.

Issues:

Major: 0
Minor: 3
Nits:  1

1) MINOR:

The abstract says:

    This
    document defines only one type of MSD, but defines an encoding that
    can support other MSD types.

and later section 1 says:

    It also defines
    the Base MPLS Imposition MSD type.

I can find nothing in the document that does this definition. It seems 
that this definition is actually done by 
draft-ietf-isis-segment-routing-msd, which is referenced. The text needs 
to agree with the structuring of the documents.

2) MINOR:

Section 1 also says:

    Although MSD
    advertisements are associated with Segment Routing, the
    advertisements MAY be present even if Segment Routing itself is not
    enabled.

I found nothing else in the document that elaborated on this. Further 
explanation is needed, in a section other than the introduction.

3) MINOR:

The first paragraph of section 4 says:

    When Link MSD is present for a given MSD type, the value of the Link
    MSD MUST take preference over the Node MSD.  When a Link MSD type is
    not signalled but the Node MSD type is, then the value of that *Link*
    MSD type MUST be considered as the corresponding *Node* MSD type
    value.

This appears to have scrambled the use of Link and Node a bit. I think 
it is intended to say:

    When Link MSD is present for a given MSD type, the value of the Link
    MSD MUST take preference over the Node MSD.  When a Link MSD type is
    not signalled but the Node MSD type is, then the value of that Node
    MSD type MUST be considered as the corresponding Link MSD type value.

4) NIT:

Section 2 uses: 'The Type:' when documenting the "Type" field of the 
TLV,  while sections 3 uses: 'Type:'. These ought to be consistent.