Subject: [Dime] WGLC Comments on draft-ietf-dime-rfc3588bis-17.txt, pp. 14-20

From: "Glen Zorn" <[email-masked]>
Date: Mon, 8 Jun 2009 16:59:58 +0700

EDITORIAL

Section 1.2
-----------
In the definition of "Diameter Agent", s/Diameter node/Diameter Node/

In the definition of "Diameter Node", s/Diameter node/Diameter Node/

The definition of a "Diameter Peer" is relational in nature; I think that
the definition might be usefully revised to better express this quality.
Maybe something like "If a Diameter Node shares a direct transport
connection with another Diameter Node, it is a Diameter Peer to that
Diameter Node."

In the definition of "Diameter Server", s/one/Diameter Node/

In the definition of "Downstream", s/home server/Home Server/

In the definition of "Interim accounting", s/in the case of a device reboot
or other network problem prevents the reception/in case a device reboot or
other network problem prevent the delivery/

In the definition of "Real-time Accounting" s/The Diameter Credit Control
Application [RFC4006] is the application that/The Diameter Credit Control
Application [RFC4006] is an example of an application that/

In the definition of "Upstream", s/home server/Home Server/

In the definition of "User", I find the use of "client device" confusing: I
think that the term "client" in this document should only be used to refer
to Diameter Clients, but this is not the case here.  Suggest changing
"client device" to "device".

Section 1.3
-----------
s/From the point of extensibility/From the point of view of extensibility/

s/Protocol designer/Protocol designers/

Section 1.3.1
-------------
s/of the documents that defines/of the document that defines/

s/where as values/whereas values/

Section 1.3.2
-------------
s/instead of the base/instead of a base/

Section 1.3.3
-------------
I find this section a little bit confusing; I suggest rewriting it as
follows:

OLD:
   A new Command Code MUST be allocated when new required AVPs (those
   indicated as {AVP}) are added, deleted or are redefined (for example
   by changing a required AVP into an optional one).

   Furthermore, when a command is modified with respect to the number of
   round trips then a new Command Code has to be registered.

   A change to the ABNF of a command, such as described above, MUST
   result in the definition of a new Command Code.  This subsequently
   leads to the need to define a new Diameter Application for any
   application that will use that new Command.

   The IANA considerations for commands are discussed in Section 11.2.1.

NEW:
   A new Command Code MUST be allocated when required AVPs (those
   indicated as {AVP} in the ABNF definition) are added to, deleted from or
redefined in (for example,
   by changing a required AVP into an optional one) an existing command.

   Furthermore, if the transport characteristics of a command are changed
(for example, with respect to the number of
   round trips required) a new Command Code MUST be registered.

   A change to the ABNF of a command, such as described above, MUST
   result in the definition of a new Command Code.  This subsequently
   leads to the need to define a new Diameter Application for any
   application that will use that new Command.

   The IANA considerations for commands are discussed in Section 11.2.1.

Section 1.3.4
-------------
I'm pretty sure that "mandatorily" is not an actual English word ;-).  I
suggest changing "The M-bit allows the sender to indicate to the receiver
whether the semantics of an AVP and it's content has to be understood
mandatorily or not." to "The M-bit allows the sender to indicate to the
receiver whether or not understanding the semantics of an AVP and its
content is mandatory."

TECHNICAL

Section 1.2
I think that the definition of "Diameter Client" is a bit too restrictive:
it's not hard to imagine Diameter clients that are not strictly speaking
devices or don't sit at the edge of a network or do not (directly) perform
access control functions.  I suggest rewriting the definition in terms of
Diameter Node, maybe something like:
"A Diameter Client is a Diameter Node that supports Diameter client
applications as well as the base protocol.  Diameter Clients are often
implemented in devices situated at the edge of a network and provide access
control services for that network.  Typical examples of Diameter Clients
include the Network Access Server (NAS) and the Mobile IP Foreign Agent
(FA)."  

In the definition of "Downstream", a more general statement might be
obtained by replacing "access device" with "Diameter Client"

The definition of "Session state" doesn't really make sense to me: it
doesn't actually define session state, but does define a stateful agent.

In the definition of "Upstream", a more general statement might be obtained
by replacing "access device" with "Diameter Client"

---

From: "Glen Zorn" <[email-masked]>
Date: Mon, 15 Jun 2009 18:48:13 +0700

EDITORIAL

Section 1.3.4
-------------
Under "M-bit Setting", paragraph 1, CHANGE "Adding an AVP with the M-bit in
the MUST column of the AVP flag table to an existing Command/Application" TO
"An AVP with the M-bit in the MUST column of the AVP flag table is added to
an existing Command/Application"

Under "M-bit Setting", paragraph 2, CHANGE "Adding an AVP with the M-bit in
the MAY column of the AVP flag table to an existing Command/Application" TO
"An AVP with the M-bit in the MAY column of the AVP flag table is added to
an existing Command/Application"

Clarification: Suggest changing 
   An implementation MAY add arbitrary optional AVPs with the M-bit
   cleared to a command defined in an application, including vendor-
   specific AVPs without needing to define a new application.  This can
   be done if the commands ABNF allows for it.  Please refer to Section
   11.1.1 for details.
to
   If the ABNF definition of a command allows it, an implementation may 
   add arbitrary optional AVPs with the M-bit cleared (including 
   vendor-specific AVPs) to that command without needing to define a 
   new application.  Please refer to Section 11.1.1 for details.

Section 2.1
-----------
Paragraph 3, s/backwards compatibility purpose/for purposes of backward
compatibility/

Paragraph 5, it would be really nice to have a reference to section 12 after
"the Tc timer"; in fact, to make such reference easier it might be a good
idea to add subsections of section 12 for each of the configurable
parameters discussed there

In the last paragraph, it took a couple of readings to make sense of the
last 3 sentence; for clarification, suggest changing "If Diameter receives
data up from TCP that cannot be parsed or identified as a Diameter error
made by the peer, the stream is compromised and cannot be recovered." to "If
Diameter receives data from the lower layer that cannot be parsed or
identified as a Diameter error made by the peer, the stream is compromised
and cannot be recovered."

Section 2.3
-----------
The second sentence of paragraph one seems rather poorly written to me:
suggest changing "For a given application, advertising support of" to
"Advertising support of"

Same comment on paragraph 2; suggest changing
   An implementation MAY add arbitrary optional AVPs with the M-bit
   cleared to a command defined in an application, including vendor-
   specific AVPs only if the commands ABNF allows for it.  Please refer
   to Section 11.1.1 for details.
to
   Implementations MAY add arbitrary optional AVPs with the M-bit
   cleared (including vendor-specific AVPs) to a command defined in an    
   application, but only if the command's ABNF syntax specification 
   allows for it.  Please refer to Section 11.1.1 for details.

Section 2.5
-----------
I had no idea that a session would (or even could) spawn!  Suggest changing 
   A session is a logical concept at the application layer, it spawns from
the Diameter
   client to the Diameter server and is identified via the Session-Id AVP.
to
   A session is a logical concept at the application layer existing between
the Diameter
   client and the Diameter server; it is identified via the Session-Id AVP.
in paragraph 2.

In paragraph 4, s/application specific/application-specific/

Section 2.6
-----------
Under "Static or Dynamic", s/configured, or/configured or/

Section 2.7
-----------
Under "Local Action", bullet 4 s/Section 6.1.9 for redirect
guidelines./Section 6.1.8 for redirect guidelines.

Under "Server Identifier", s/One or more servers the message is to be routed
to./One or more servers to which the message is to be routed./

Under "Static or Dynamic", s/configured, or/configured or/

Under "Expiration Time", s/Specifies the time which/Specifies the time at
which/

Section 2.8
-----------
In paragraph 1, s/In addition to client and servers/In addition to clients
and servers/

In paragraph 4, s/information; by/information by/

Same paragraph, s/considered active either until it is notified otherwise,
or by expiration.  Each authorized session has an expiration/considered
active either until the agent is notified otherwise, or the session expires.
Each authorized session has an expiration time/

TECHNICAL

Section 2
---------
I don't really understand this section, nor even why it is included  For
example:

   Diameter Clients MUST support the base protocol, which includes
   accounting.  In addition, they MUST fully support each Diameter
   application that is needed to implement the client's service, e.g.,
   NASREQ and/or Mobile IPv4.  A Diameter Client that does not support
   both NASREQ and Mobile IPv4, MUST be referred to as "Diameter X
   Client" where X is the application which it supports, and not a
   "Diameter Client".

There are a ton of Diameter applications; what's so special about NASREQ &
MIPv4?  Even if there is a rationale for keeping this part of the original
text, section 2 in bis doesn't seem like any improvement of section 2 of RFC
3588.