Telechat Review of draft-ietf-ospf-ospfv3-segment-routing-extensions-19
review-ietf-ospf-ospfv3-segment-routing-extensions-19-genart-telechat-resnick-2018-12-03-00

Request Review of draft-ietf-ospf-ospfv3-segment-routing-extensions
Requested rev. no specific revision (document currently at 21)
Type Telechat Review
Team General Area Review Team (Gen-ART) (genart)
Deadline 2018-12-04
Requested 2018-11-16
Other Reviews Genart Last Call review of -18 by Pete Resnick (diff)
Secdir Last Call review of -16 by Yaron Sheffer (diff)
Rtgdir Last Call review of -17 by Tomonori Takeda (diff)
Opsdir Last Call review of -16 by Joe Clarke (diff)
Review State Completed
Reviewer Pete Resnick
Review review-ietf-ospf-ospfv3-segment-routing-extensions-19-genart-telechat-resnick-2018-12-03
Posted at https://mailarchive.ietf.org/arch/msg/gen-art/nmHw9lTHlLQCnDnKAru_WkhLxMo
Reviewed rev. 19 (document currently at 21)
Review result Ready with Issues
Draft last updated 2018-12-03
Review completed: 2018-12-03

Review
review-ietf-ospf-ospfv3-segment-routing-extensions-19-genart-telechat-resnick-2018-12-03

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 wait for direction from your
document shepherd or AD before posting a new version of the draft.

For more information, please see the FAQ at

<https://trac.ietf.org/trac/gen/wiki/GenArtfaq>.

Document: draft-ietf-ospf-ospfv3-segment-routing-extensions-19
Reviewer: Pete Resnick
Review Date: 2018-12-03
IETF LC End Date: 2018-11-16
IESG Telechat date: 2018-12-06

Summary: I think this document is ready, and I certainly don't want to stand in the way of it moving forward, but I do want to note the following issues I mentioned in my previous review. The document editor notes that similar sorts of things have been done in previous OSPF document without problems, but they still make me nervous. Thanks to the editor for quickly addressing all of the issues in my previous review.

Major/minor issues:

In 3.1:

      Length: Either 3 or 4 octets

      SID/Label: If length is set to 3, then the 20 rightmost bits
      represent a label.  If length is set to 4, then the value
      represents a 32-bit SID.

This sort of mechanism worries me. The Length is not a length, but rather a flag. This means you can't have a general parsing implementation, as it would treat the field as a length and get the left-most 24 bits when the value is 3. Even if the implementation chooses the right-most 24 bits, it's only supposed to take the right-most 20 bits and mask off the extra 4 bits, which are not required to be zeroed. I understand that similar things have been done before without problems, but this seems like an implementation accident waiting to happen.

In 7.1 and 7.2:

When the V-flag is set (making SID/Index/Label is a label), the value is in the low 20 bits of the first 3 bytes of the field (i.e., bits 4-23). As with the comment regarding 3.1, this seems like it has the potential for implementation problems. You could explicitly say to mask of bits 0-3 and 24-31 (since there is no requirement for producing implementations to clear those bits) and shift the value 8 bits to the right, but this just seems like a bad way to design this. That said, I again understand that similar things have been done before without problems.

Nits/editorial comments:

None.