Skip to main content

Last Call Review of draft-ietf-lisp-lcaf-17
review-ietf-lisp-lcaf-17-genart-lc-yee-2016-10-13-00

Request Review of draft-ietf-lisp-lcaf
Requested revision No specific revision (document currently at 22)
Type Last Call Review
Team General Area Review Team (Gen-ART) (genart)
Deadline 2016-10-11
Requested 2016-10-06
Authors Dino Farinacci , David Meyer , Job Snijders
I-D last updated 2016-10-13
Completed reviews Genart Last Call review of -15 by Peter E. Yee (diff)
Genart Last Call review of -17 by Peter E. Yee (diff)
Secdir Last Call review of -15 by David Mandelberg (diff)
Opsdir Last Call review of -15 by Sarah Banks (diff)
Rtgdir Early review of -14 by Stig Venaas (diff)
Assignment Reviewer Peter E. Yee
State Completed
Request Last Call review on draft-ietf-lisp-lcaf by General Area Review Team (Gen-ART) Assigned
Reviewed revision 17 (document currently at 22)
Result Ready w/issues
Completed 2016-10-13
review-ietf-lisp-lcaf-17-genart-lc-yee-2016-10-13-00
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 background on Gen-ART,
please see the FAQ at
<

http://wiki.tools.ietf.org/area/gen/trac/wiki/GenArtfaq>

Document: draft-ietf-lisp-lcaf-17
Reviewer: Peter Yee
Review Date: October 12, 2016
IETF LC End Date: October 4, 2016
IESG Telechat date: October 13, 2016

Summary: This draft is basically ready for publication as an Experimental
specification, but has some issues that should be fixed before publication.
[Ready with issues]

Most of my issues and nits from the -16 draft have been addressed.  I list
below those that have not (and that I think still merit consideration) as
well as a few new nits.  Overall, the document looks good.

Major issues: None

Minor issues: 

Page 6, Rsvd2 definition: the definition both says "reserved for future use"
and then says some types actually use it.  That sounds like present use.
And to generically say that it should be sent as zero and ignored, but then
to give uses (such as Type 2)  for it  is confusing.  I suggest rethinking
the wording here.

Page 6, Length definition: there's mention of a "Reserved" field that's
included in the minimum length of 8 bytes that are not part of the length
value.  Since there are actually Rsvd1 and Rsvd2 fields in the generic
version of the LCAF and sometimes even Rsvd3 and Rsvd4 fields when using
specific Types, it might be better to spell out which reserved fields (Rsvd1
and Rsvd2) are meant here rather than giving the field a summary name that
doesn't actually appear in the format.  This is also important because any
Rsvd3 and Rsvd4 fields are included in the Length field, so using a generic
"Reserved" description is ambiguous at best.

Page 13, RTR RLOC Address definition, 4th sentence: The ability to determine
the number of RTRs encoded by looking at the value of the LCAF length
doesn't seem feasible.  3 IPv4 RTR RLOCs will produce the same LCAF Length
as 1 IPv6 RTR RLOC.

Page 13, RTR RLOC Address definition, 5th sentence: this sentence gives two
means to indicate that there RTR field values.  What's the point of having
two different means of doing so?  This just seems to introduce complexity
and increase the chance for implementations that may not handle both schemes
correctly.  One scheme ought to suffice.  I prefer the Length field method
since it requires fewer bytes transmitted, but either works.

Nits:

Page 5, Type definition: change the comma to a semicolon.

Page 8, Usage, 1st sentence: change the second "a" to "an".

Page 9, AS Number definition: insert "to" before "either".

Page 13, RTR RLOC Address definition, 3rd sentence: change "these" to
"this".

Page 14, section 4.5, 2nd paragraph: change "it's" to "its".

Page 15, Source/Subnet Address and Group Address definitions: delete an
extra space before "is" in each definition.

Page 16, Strict bit (S) definition, 1st sentence: change "Rencap" to
"Reencap".

Page 18, Key Count definition, 2nd sentence: change "the" to "of".

Page 18, AFI = x definition: insert two spaces before the second sentence.

Page 24, section 4.10.3, 1st paragraph: delete "where it is delimited by
length 'n' of the LCAF Length encoding"

Page 26, 1st paragraph after Length2 definition: change "AFI-encoded" to
"AFI encoded".  My apologies for suggesting a blind find-and-replace in the
previous review.  Obviously that was wrong here.

Page 27, section 5.1, Length value n definition: I'm not sure what qualifies
as "8-byte Application Data fields", but there appear to be 12 bytes before
the AFI and that's reflected in the Length field in the figure.  So the
local and remote port ranges are the 8-byte Application Data fields?  This
comes back to my minor issue with how Length values are described in the
text and in the figures.  Please clarify what's meant here.

Page 29, Key Field Num definition: change "the the" to "the".

Page 32, section 5.5, 1st paragraph, 2nd sentence: insert "the" before
"key".