Hi Joe,
thanks for your review, please see inline (##PP):
On 26/10/18 21:42 , Joe Clarke wrote:
Reviewer: Joe Clarke
Review result: Has Nits
I have been assigned to review
draft-ietf-ospf-ospfv3-segment-routing-extensions on behalf of the ops
directorate. This document defines OSPFv3 extensions needed for segment
routing (SR). And therein lies my first nit. While the document begins to set
forth this overarching scope, a small paragraph in section 1 further limits it
to MPLS dataplanes only. I think perhaps the abstract should be updated to
clarify that.
##PP
Done
Other items I found are listed below.
Overall, there are a lot of terminology used like RSVP, LDP, LSP, SID, etc. I
think this document would benefit from a terminology section.
##PP
added
With respect to TLV types 8, 9, 14, and 15, they are defined in
draft-ietf-ospf-segment-routing-extensions, and it took me a while to figure
out where you were getting those values and why they weren't spelled out in the
IANA considerations. You have a normative reference to this, which is good,
but you only mention it with respect to the algorithm parameters. I think
another mention is required.
I'm going to be pedantic here. According to RFC7770, when a new OSPF Router
Information LSA TLV is defined, the spec needs to explicitly state if it's
applicable to OSPFv2, v3, or both. While you reference the TLVs from
draft-ietf-ospf-segment-routing-extensions, I didn't see that either document
_explicitly_ states that they are applicable to both.
##PP
added the following to each of the values:
Type: X as defined in [I-D.ietf-ospf-segment-routing-extensions] and
aplicable to OSPFv3.
===
Section 2.1
s/length is other then 3 or 4/length is other than 3 or 4/
##PP
fixed
===
Section 3.2
s/If more then one SID/Label/If more than one SID/label/
##PP
fixed
===
Section 3.2
"When a router receives multiple overlapping ranges, it MUST
conform to the procedures defined in
[I-D.ietf-spring-segment-routing-mpls]."
It would be useful to include a section pointer here. I think your referring
to Section 2.3 where the router ignores the range? Is it likely that will
change to something other than "ignore?" If not, maybe it's just worth
mentioning that here.
##PP
I don't think it is good to specify the behavior which is described
somewhere else. Regarding the section, the
ietf-spring-segment-routing-mpls is still being worked on and the
section may changes. We used the same text in OSPFv2 and ISIS SR drafts.
I would like to be consistent here.
===
Section 3.3
s/If more then one SID/Label/If more than one SID/Label/
##PP
fixed.
===
Section 3.3
"The originating router MUST NOT advertise overlapping ranges."
You specify what a router should do if it receives overlapping ranges above. I
think the same text should be used here, too.
##PP
Here we say that the originating router MUST NOT advertise overlapping
ranges. We can not specify what it should do when it breaks the MUST.
We specify what other routers should do when they receive overlapping
ranges and we refer it to spring-segment-routing-mpls draft. Again this
is the same as we used in OSPFv3 and ISIS SR extensions. I would like to
keep the consistency here.
===
Section 5
"Other bits: Reserved. These MUST be zero when sent and are
ignored when received."
The normative language changes. In other places you say the bits SHOULD be 0.
I suggest:
##PP
Whenever we refer to "other bits" in the flag fields we use the same
language.
Other bits: Reserved. These SHOULD be set to 0 when sent and MUST be ignored
when received.
##PP
this refers to Reserved fields in the TLV (not the bits in a flag field)
and again is used consistently across document.
===
Section 7.4.1
s/state lower then 2-Way/state lower than 2-Way/
##PP
fixed.
thanks,
Peter
===
.
