Re: [pcp] Review of draft-ietf-pcp-upnp-igd-interworking

[<[e-mail]>]

Hi Xioahong,

Thank you for your review.

Please see inline.

Cheers
Med

________________________________
De : [e-mail] [mailto:[e-mail]] De la part de Xiaohong Deng
Envoyé : mardi 14 août 2012 21:48
À : [e-mail]; [e-mail]
Objet : [pcp] Review of draft-ietf-pcp-upnp-igd-interworking

Hello authors and WG,

Here is my review of draft-ietf-pcp-upnp-igd-interworking-02.

Overall I think the draft is in a good shape, neat, easy to follow, and has a good architecture for WGLC. Below are a couple of questions and thoughts on text. Two types of them: the ones begin with +, are ones that, in my point of view, are worth to be addressed, and the ones begin with *, are minor ones that may make no significant difference. In both cases if I miss something, please elaborate and clarify. Thanks.


  Interworking Function (IGD-PCP IWF) is required to be embedded in CP
+ CP -> Customer Premises (CP)
[Med] I expanded the acronym.


   Triggers for deactivating the UPnP IGD-PCP Interworking Function from
   the IGD and relying on a PCP-only mode are out of scope of this
+ I'm not sure I understand exactly "Triggers for deactivating the UPnP IGD-PCP Interworking Function". what does that exactly mean? Deactivating the whole IWF or IGD?
[Med] The sentence is about de-activating the IWF. It is out of scope of this document to elaborate on conditions which may justify to de-activate the IWF in the IGD.



   (1)  Section 4.1 provides the mapping between WANIPConnection State
        Variables and PCP parameters;
+ would suggest using'correspondence' than 'mapping', as no mappings required to store in IGD for that, and also for lowering confusion with NAT mappings
[Med] Done.


      on a per UPnP CP basis.
+ UPnP CP -> UPnP Control Point?
[Med] Yes. I added an entry to Section 2.
 There are two 'CP' exit in the document, which I suppose stand for different things respectively.
[Med] The core text does not use CP but IGD. It appears only in the abstract.



   PortMappingEnabled:
      PCP does not support deactivating the dynamic NAT mapping since
      the initial goal of PCP is to ease the traversal of Carrier Grade
      NAT.  Supporting such per-subscriber function may overload the
      Carrier Grade NAT.
+ What if the customer wants to deactivate a static NAT mappings on CGN? it is not stated clearly that IWF should support it or not. My reading here is that for the same reason: not to overload the carrier Grade NAT, it should not support deactivate static mappings either. IMO,it's worth to state clearer that it applys to both static and dynamic mappings, if that is what text here means.
[Med] IGD spec says "PortMappingEnabled: This variable allows security conscious users to disable and enable dynamic NAT port mappings on the IGD.". PCP does not provide such feature.


      On reading the value is 1, writing a value different from 1 is not
      supported.
+ what if on reading the value is 0, which means deactivating the mapping?
[Med] see above. Only "1" is supported.


      Point, it must be resolved to an IP address by the Interworking
      Function when relying the message to the PCP Server.
+ Must? Does this mean IWF must have a DNS resolving function also?
[Med] No. This can be passed to a name resolution library.


   6 MALFORMED_OPTION:  501 "ActionFailed"
      Should not happen.
*It may happen when IWF is not correctly implemented, mistached version of implementation with PCP server, and extra.
[Med] Yes; but there is no need to list when it can occur.


   7 NETWORK_FAILURE:  Not applicable
      Should not happen after communication was successfully established
     with a PCP Server.
*What if IWF/PCP Client fails to establish communication with a PCP server? In that case, I see that "ActionFailed" looks like the semantical correspondence.
[Med] If the IWF fails to establish a communication with the PCP Server, then no error is received. An error should be generated locally by the IWF and sent back to the UPnP CP. I updated section 5.7.1 and section 5.7.2 to indicate "501" error code is returned when the IWF fails to contact its PCP Server.



      Note: According to some experiments, some UPnP 1.0
      implementations, e.g., uTorrent, simply try the same external port
      X times (usually 4 times) and then fail.  Also note that some
      applications uses GetSpecificPortMapping() to check whether a
      mapping exists.
*Behavior of uTorrent is more like following: it tries GetSpecificPortMapping X times (usually 4 times),if it finds an external port not being used before X times, it will call AddPortMapping
[Med] I updated the text to indicate the behaviour when a port is not in use. Thanks.



   Upon receipt of GetSpecificPortMappingEntry() from a UPnP Control
   Point, the IGD-PCP IWF MUST check first if the external port number
   is used by the requesting UPnP Control Point.  If the external port
   is already in use by the requesting UPnP Control Point, the IGD-PCP
   IWF MUST send back a positive answer.  If not, the IGD-PCP IWF MUST

*or more precisely: IWF MUST send back mapping entires specified by the unique tuple of ExternalPort and PortMappingProtocol.
[Med] I prefer the initial wording without detailing the content of the response. The exact content of the response is part of the UPnP spec.


   relay to the PCP Server a MAP request, with short lifetime (e.g.,
   60s), including a PREFER_FAILURE Option.  If the requested external
   port is in use, a PCP error message will be sent by the PCP Server to
   the IGD-PCP IWF indicating CANNOT_PROVIDE_EXTERNAL as the error
   cause.  Then, the IGD-PCP IWF relays a negative message to the UPnP
   Control Point.  If the port is not in use, the mapping will be

+ if CANNOT-PROVIDE_EXTERNAL is returned from PCP server, it means the enquired external port is being used: for example, enquired external port is being used by another customer sharing the same external IP. Since one of major usage of GetSpecificPortMappingEntry() is for UPnP control point to find out if an external port is  available, how about using 718 "ConflictInMappingEntry"? which, to my eyes, provides a better semantic in this context.
[Med] Are you sure 718 error code is allowed for GetSpecificPortMappingEntry?


   created by the PCP Server and a positive response will be sent back
   to the IGD-PCP IWF.  Once received by the IGD-PCP IWF, it MUST relay
   a negative message to the UPnP Control Point indicating
   NoSuchEntryInArray as error code.

*Since this is a somehow complicated to be understood case, add something like below may make it slightly clearer:

it MUST relay a negative message to the UPnP Control Point indicating NoSuchEntryInArray as error code so that the UPnP control point knows the enquired mapping doesn't exist.
[Med] Updated. Thanks.

Cheers,
-Xiaohong
<http://fr.linkedin.com/pub/xiaohong-deng/28/577/599>