Dear Peter, Thanks for the review. Please see inline. Cheers, Med >-----Message d'origine----- >De : Peter Yee [mailto:peter@xxxxxxxxxx] >Envoyé : mardi 9 avril 2013 10:13 >À : draft-ietf-pcp-upnp-igd-interworking.all@xxxxxxxxxxxxxx >Cc : gen-art@xxxxxxxx; ietf@xxxxxxxx >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". > >