[Dime] FW: Gen-ART review of draft-ietf-dime-nat-control-07.txt

["Romascanu, Dan (Dan)" <[e-mail]>]

 

-----Original Message-----
From: Miguel A. Garcia [mailto:[e-mail]] 
Sent: Saturday, March 05, 2011 12:06 AM
To: [e-mail]; [e-mail]; [e-mail];
[e-mail]; Jouni Korhonen; Romascanu, Dan (Dan)
Cc: General Area Review Team
Subject: Gen-ART review of draft-ietf-dime-nat-control-07.txt

I have been selected as the General Area Review Team (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-dime-nat-control-07.txt
Reviewer: Miguel Garcia <[e-mail]> 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





Regards,

       Miguel G.
-- 
Miguel A. Garcia
[phone]
Ericsson Spain