Dear Peter,
Thanks for the review.
Please see inline.
Cheers,
Med
-----Message d'origine-----
De : Peter Yee [mailto:peter(_at_)akayla(_dot_)com]
Envoyé : mardi 9 avril 2013 10:13
À :
draft-ietf-pcp-upnp-igd-interworking(_dot_)all(_at_)tools(_dot_)ietf(_dot_)org
Cc : gen-art(_at_)ietf(_dot_)org; ietf(_at_)ietf(_dot_)org
Objet : Gen-ART review of draft-ietf-pcp-upnp-igd-interworking-07
I am the assigned Gen-ART reviewer for this draft. For background on
Gen-ART, please see the FAQ at
<http://wiki.tools.ietf.org/area/gen/trac/wiki/GenArtfaq>
Please resolve these comments along with any other Last Call comments you
may receive.
Document: draft-ietf-pcp-upnp-igd-interworking-07
Reviewer: Peter Yee
Review Date: Apr-08-2013
IETF LC End Date: Apr-08-2013
IESG Telechat date: Unknown
Summary: This draft is on the right track but has open issues, described in
the review. [Ready with issues]
This is a well-written draft describing how Universal Plug-and-Play
Internet
Gateway Devices interwork with NAT devices that use the Port Control
Protocol. My review is mostly nits with otherwise minor issues. I have no
problems with the general concept and enjoyed reading a clearly articulated
concept.
[Med] Thanks.
Major Issues:
None.
Minor Issues:
Section 4.1: is the mapping of the state variables between the UPnP IGD
function and the PCP Client function really straightforward such that it
does not need further description? I'm asking if an implementor would
naturally gravitate to the correct use of the mapping without further
instructions. Similar questions arise for Sections 4.2 and 4.3, although I
believe those are more straightforward mappings.
[Med] I don't think further explanation is needed. Pointers to the appropriate
sections is included in Section 4.2 and Section 4.3. FWIW, there are already
existing implementations of the IWF and no "complaint" was received from these
implementers.
Page 13, 1st paragraph, 3rd sentence: what's meant here is if any PCP error
other than a short-lifetime error, or in the case of a failed resend, any
PCP error at all. The wording makes it seem like the short-lifetime errors
are somehow not PCP errors and is therefore confusing. It also doesn't
explicitly deal with how many repeats should be done on a resend.
[Med] The basic behavior is to relay the received error to the UPnP CP. For the
short-lifetime errors, the IWF may decide to resend the request and not relay
those errors immediately to the UPnP CP. The number of repeats is not specified
here as it can be implementation-specific.
Nits:
[Med] Fixed.
General: replace "re-send" with "resend".
Page 3, 1st paragraph, 2nd sentence: insert "a" before each occurrence of
UPnP.
Page 4, section 3, 4th bullet: consider changing "in" to "on" or "over".
Page 4, 1st paragraph after the bullet items: bracket "for instance" with
commas.
Page 5, Figure 3: perhaps the "LAN Side" label could move a bit to the left
to give it better delineation from the "External Side" label?
Page 5, 1st paragraph after Figure 4, 2nd sentence: add a comma after "
[I-D.ietf-pcp-base]".
Page 5, 2nd paragraph after Figure 4: change "can not" to "cannot".
Page 9, section 5, 3rd paragraph: insert "the same way" or "the same"
before
"as any PCP Client".
Page 9, section 5.1, 2nd paragraph: insert "whether" before "it should".
Page 9, section 5.1, 3rd paragraph: insert "the" before "requesting UPnP
Control Point".
Page 10, Section 5.3, 2nd paragraph, 1st sentence: consider changing
"retrieve" to "extract".
Page 11, 1st paragraph after bullet items, 2nd sentence: change "to" to
"than".
Page 11, Section 5.6: swap the order of "AddPortMapping()" and
"AddAnyPortMapping()" to match the order of the subsequent subsections.
Page 11, Section 5.6.1, 2nd paragraph, 2nd sentence: change "30s" to "30
seconds".
Page 13, 1st paragraph, 4th sentence: change "re-issue" to "issue"; change
"new requested" to "different requested".
Page 14, last paragraph: delete the comma after "4 times)".
Page 15, Section 5.7, 1st paragraph: append a comma after
"GetSpecificPortMappingEntry()".
Page 15, Section 5.7, 3rd paragraph, 3rd sentence: replace "60s" with "60
seconds".
Page 15, Section 5.7, 3rd paragraph, last sentence: insert "the" before
"error code"; change "enquried" to "queried".
Page 15, Section 5.8, 1st paragraph: insert ", respectively" before the
period.
Page 15, Section 5.8, 2nd paragraph, 2nd sentence: insert "the" before
'"606
Action Not'.
Page 16, last paragraph, 2nd sentence: insert "the" before'"731
PortMappingNotFound'.
Page 19, 1st continuation paragraph, 1st sentence fragment: change "of" to
"in".
Page 19, 1st continuation paragraph, 1st full sentence: consider swapping
the positions of "renew" and "periodically" for readability.
Page 19, Section 5.10, 1st paragraph, 2nd sentence: I prefer "In
particular"
to "Particularly" here.
Page 19, Section 5.10, 3rd paragraph, 3rd sentence: replace "signalled"
with
"signaled".