Document: draft-ietf-dime-nat-control-07.txt Reviewer: Miguel Garcia Review Date: 2011-03-04 IETF LC End Date: 2011-03-08 IESG Telechat date: Summary: This draft is on the right track but has open issues, described in the review. Major issues: none Minor issues: - The draft relies on the roles of a DNCA Manager and a DNCA Agent. I don't understand why these two new roles need to be introduce, when RFC 3588 already provide some roles. In particular I don't see the difference between what you call the DNCA Manager and what RFC 3588 calls a Diameter Server; and I don't see the difference between your DNCA Agent and RFC 3588 Diameter Client. Furthermore, I haven't seen a proper definition of DNCA Manager and DNCA Agent, so, I have my own idea, which basically points to the Diameter Sever and Client I just mentioned. The suggestion is to not invent the wheel twice and refer to the terminology in RFC 3588, unless you can prove that this is insufficient. - When I was reading Section 4, which discusses the procedures, and I was asking myself whether this application would use the ASR/ASA commands. I finished reading Section 4, and I found no reference to these commands, and I thought there was an issue there. But then I found the first reference to ASR/ASA in Section 5.3, and elsewhere in the document. So, in order to avoid that the reader is as confused as me, I would suggest to add a new section, which would be 4.6, and discusses "Session Abort" like the rest of the procedures of the application. - On Section 6.2, the NCA command, I noticed that the NC-Request-Type AVP is required in the message, because it is enclosed in curly brackets {}. It is not clear to me why this AVP should be mandatory in the answer (I agree it should be mandatory in the request). - Also on Section 6.2, the NCA command, I don't see why the Result-Code AVP is optional in an answer. I thought an answer should always report a result of any kind. - On Section 7, page 23, I have a question on the state machine marked as "Open Access Session end detected Send STR Discon". I don't understand what is "Access Session end detected". It is not obvious from the rest of the document, so, I guess it should be explained. - On Section 7, page 23, there are two instances of the term "client". Now I am not sure if this is the Diameter client (that I think you should be using throughout the document) or it is something else. - On Section 7, the DNCA Agent state machine (page 24). There is a line that reads: "Open NCR request received, and unable to provide requested NAT control service Send failed NCA, Cleanup Idle". So, if I understand correctly, the case is that under an Open state, if the DNCA Agent receives an NCR request, but it is not able to provide the NAT control service, so it sends back an NCA with a failed result. The question is whether the NCR created a session or not. From the state machine I understand it didn't create a session. So, I can conclude that a session is created when a successful NCA is sent. If this is the case, then look at Figure 5 on page 13, where there is a "Create session state" in between an NCR and an NCA message. If this NCA had failed, you would still had a session created. So, somehow you need to make Figure 5 congruent with the state machine of the DNCA Agent. You can modify figure 5 (and the surrounding text) to indicate that the session is created after a successful NCA (and not before), or you can modify the state machine line I mentioned, to indicate that not only you should send a failed NCA, but also transition to a Disconn state in order to send an ASR. - On Sections 8.2.2 and 8.2.3, the RESOURCE_FAILURE and the UNKNOWN_BINDING_RULE_NAME have both exactly the same description. The only difference is that the former is a transient failure whereas the latter is a permanent failure. Honestly, I don't understand the difference between both, and when I should generate one or the other. I would suggest that the latter adds a paragraph starting with "The difference between this code and the RESOURCE_FAILURE is ..." Similarly, I don't see the difference between UNKNOWN_BINDING_RULE_NAME and BINDING_FAILURE. I also suggest to clearly explain it or delete one of them. - Section 12, IANA considerations section. You need to mention the registry and subregistry where you want IANA to operate. For example: This document instructs IANA to assign values to the [choose: Commands/AVP Codes/etc] subregistry of the Authentication, Authorization, and Accounting (AAA) Parameters registry. Also, in Tables 1 to 4, under the "Reference" column, you don't need to refer to a section in this draft, but just right "RFC XXXX", which later will be replaced by the RFC number of this draft. Additionally, the headings of each table needs to be congruent with the registry, so, Table 2 should read "AVP Code Attribute Name ", and so on. Take a look at the registry in http://www.iana.org/assignments/aaa-parameters/aaa-parameters.xml and you make some similar table. - I have an unparseable sentence in Section 12. Please split it and make it understandable. To secure the information exchange between the authorizing entity (DNCA Manager) and the NAT device (DNCA Agent) requires bilateral authentication of the involved parties, authorization of the involved parties to perform the required procedures and functions, and procedures to ensure integrity and confidentiality of the information exchange MAY be performed. Nits: - Section 3.3, at the beginning of the section, add "server" as follows: s/The role of the DNCA can be/The role of the DNCA server can be ^^^^^^ - On Section 6.2, you have a duplicated "*[AVP]" towards the end of the command. - On the title of Sections 8.4, 8.5, and 8.6 add "AVPs" as follows: s/Reused from/Reused AVPs from" - On Figure 14 (page 29), towards the bottom, there is a reference to the "V" bit. However, the "V" is never used in this figure, so, I suggest to delete the sentence that describes the "V". - Section 8.7.1 under INITIAL_REQUEST, missing "a" s/is used to install binding at/is used to install a binding at - First paragraph in Section 8.7 various missing articles and other typos: s/that references predefined policy/that reference a predefined policy s/policy template on DNCA Agent/policy template on the DNCA Agent s/contain static bindings, maximum number/contain static binding, a maximum number s/to be allowed, address pool/to be allowed, or an address pool s/external binding address/external binding addresses