[Dime] AD review draft-ietf-dime-app-design-guide

[Benoit Claise <[e-mail]>]

Dear authors,

Sorry for dropping the ball on this one.
If any of the points was already discussed/addressed by the WG, feel 
free to let me know.


- When I read the document, it looked to me as a BCP.
BCP definition from RFC 2026:

    5.  BEST CURRENT PRACTICE (BCP) RFCs

        The BCP subseries of the RFC series is designed to be a way to
        standardize practices and the results of community deliberations.

Interestingly, the charter mentions:
May 2012, Submit 'Diameter Application Design Guidelines' to the IESG 
for consideration as a BCP document

If you go to BCP, don't forget to update the abstract, and the writeup.
Also, BCPs usually use the RFC2119 keywords (ex: RFC 7013)
Example:
OLD:

    Diameter
    protocol designers are then strongly advised to carefully consider

NEW:

    Diameter
    protocol designers SHOULD NOT consider

OLD:

    Instead of using
    an Enumerated AVP for a Boolean flag, protocol designers are again
    encouraged to use AVPs of type Unsigned32 or Unsigned64 AVP in which
    the data field would be defined as


NEW:

    Instead of using
    an Enumerated AVP for a Boolean flag, protocol designers SHOULD
    use AVPs of type Unsigned32 or Unsigned64 AVP in which
    the data field would be defined as

Finally, with a BCP, RFC 6733 could be a normative reference, which I 
believe it should.


- Editorial
Please don't use "we" in RFCs

-
Section 3

   Major Extension:  Enhancing an application that requires the
       definition of a new Diameter application.

       Typical examples would be the creation of a new command for
       providing functionality not supported by existing applications or
       the definition of a new AVP with the M-bit set to be carried in an
       existing command.  For such extension, a significant specification
       effort is required and a careful approach is recommended.

Do you want to mention that Major Extension have backward compatibility 
issue, as opposed to the minor one?

- Editorial

       Typical examples would be the creation of a new command for
       providing functionality not supported by existing applications or
       the definition of a new AVP with the M-bit set to be carried in an
       existing command.  For such extension, a significant specification
       effort is required and a careful approach is recommended.

NEW:

       Typical examples would be the creation of a new command for
       providing functionality not supported by existing applications or
       the definition of a new mandatory AVP set to be carried in an
       existing command.  For such extension, a significant specification
       effort is required and a careful approach is recommended.

Justification:

	* Minor extension speaks about "optional"
	* The M-bit is explained in 4.3.1


- Section 3
I see Minor Extension versus Major Extension, and I tried to match the 
following classifications to Minor or Major

    1.  Addition of new functionality to an existing Diameter application
        without defining a new application.

    2.  Addition of new functionality to an existing Diameter application
        that requires the definition of a new application.

    3.  The definition of an entirely new Diameter application to offer
        functionality not supported by existing applications.

    4.  The definition of a new generic functionality that can be reused
        across different applications.

2 and 3 are Major
For 1, I thought about Minor, but the following sentence "or the 
definition of a new AVP with the M-bit set to be carried in an existing 
command." in the Major paragraph confuses me.
I guess that 4 is Major, but it's not mentioned.
Can you please better explain the mapping between the 4 categories and 
Minor/Major extensions
Alternatively, or maybe on top of that, explain which of 4.X and 5.X are 
Minor/Major extensions would be beneficial.

- Section 3
I don't understand what your message is with:

        We would also like to remind that the definition of a new Diameter
        application and the definition of a new command should be something
        to avoid as much as possible.  In the past, there has been some
        reluctance to define new commands and new applications.  With the
        modified extensibility rules provided by [RFC6733  <http://tools.ietf.org/html/rfc6733>], registering new
        commands and new applications does not lead to additional overhead
        for the specification author in terms of standardization process.
        Registering new functionality (new commands, new AVPs, new
        applications, etc.) with IANA remains important to avoid namespace
        collisions, which will likely lead to deployment problems.

"should be something to avoid as much as possible", is this valid today?
Because the next sentence speaks about the past, then the next sentence 
seems like it's fixed now with 6733.

- Editorial:

    With the
    modified extensibility rules provided by [RFC6733  <http://tools.ietf.org/html/rfc6733>], registering new
    commands and new applications does not lead to additional overhead
    for the specification author in terms of standardization process.
    Registering new functionality (new commands, new AVPs, new
    applications, etc.)

"etc.": What does it refer to? new AVP value is the only one I can think of.
Worth removing "etc." and specifying the full list?


- Application and command

    Major Extension:  Enhancing an application that requires the
       definition of anew Diameter application.

       Typical examples would be the creation of anew command  for
       providing functionality not supported by existing applications or
       the definition of a new AVP with the M-bit set to be carried in an
       existing command.  For such extension, a significant specification
       effort is required and a careful approach is recommended.


      4.1
      <http://tools.ietf.org/html/draft-ietf-dime-app-design-guide-21#section-4.1>.
      Adding a New Command

    Adding a new command is considered as a major extension and requires
    a new Diameter application to be defined.

I'm not clear on the application/command boundary.
Why do we need a new application for a new command?
Why can't I add a command to an existing application?
There are commands that are for all applications/independent of the 
application, no?
     CER/CEA (Capabilities-Exchange-Request)
This contradicts:

    Before adding or_importing_a command, application designers
    should consider the following ...

This also appears to contradict, in section 6
    Generic Diameter extensions are AVPs, commands or applications that
    are designed_to support other Diameter applications_.

My issue comes from the fact that there are no "application" and 
"command" definitions.
Draft says:


    2
    <http://tools.ietf.org/html/draft-ietf-dime-app-design-guide-21#section-2>.
    Terminology

    This document reuses the terminology defined in [RFC6733]  <http://tools.ietf.org/html/rfc6733>

However, RFC 6733 terminology doesn't contain the application and 
command definitions.
Talking to Jouni, I now understand that a command is always specified 
within the context of an application.
This should be clarified.

Also, from section 5.2:

        As a general recommendation, commands should not be defined from
        scratch.  It is instead recommend to re-use an existing command
        offering similar functionality and use it as a starting point.

Based on my latest understanding (a command is always specified within 
the context of an application), the only justification for the above 
text is code reuse, right. Please mention it.

- Editorial

    Adding a new command is considered as a major extension and requires
    a new Diameter application to be defined.  Adding a new command to an
    application means ...

A new command addition is always to an application, right?
If yes, why do you make the distinction between the sentences above

- Application version?

    An exception might be if the
        intent of the deletion is to create a newer version of the same
        application that is somehow simpler than the previous version.

         ...

    o  Would the new AVP be used to differentiate between old and new
       versions of the same application whereby the two versions are not
       backward compatible?

Readers might be understanding that diameter applications having a 
version field.
This is not the case. Please rephrase.
Also, mention that a new version of an application is a new application.
I guess it needs to be mentioned in 7.d


- Section 4.3.2
For the reader convenience, I would mention the convention behind { AVP 
} and [ AVP ], or at least give a reference.

- Section 5.3
OLD:

    Some existing
    specifications do not adhere to this rule for historical reasons.
    However, this guidance should be followed to avoid routing problems.

NEW:
    Some existing
    specifications do not adhere to this rule for historical reasons.
    However, this guidance should be followed for new applications to avoid routing problems.


In the same section, why "In general" in the next sentence? It 
contradicts with "must use"

    In general, when a new application has been allocated with a new
        Application Id and it also reuses existing commands with or without
        modifications, it must use the newly allocated Application Id in the
        header and in all relevant Application Id AVPs (Auth-Application-Id
        or Acct-Application-Id) present in the commands message body.


- Editorial, section 5.5

OLD:
    Based on these considerations, protocol designers should carefully
    appraise whether the application currently defined relies on it's own
    session management concept or whether

NEW:
    Based on these considerations, protocol designers should carefully
    appraise whether the application currently defined relies on its own
    session management concept or whether


- Editorial in section 5.7
OLD:

Destination- Realm

NEW:
Destination-Realm



- The section 5.8 should mention RFC4005bis

- Section 5.9

  Applications that do not understand these AVPs can discard
    them upon receipt.

Generic comment: Each time there is a sentence like this one above, we 
should mention RFC 6733 as the reference.
This document is not an extension/deviation to RFC 6733.

- Do you have a good reason to add a reference to a work-in-progress 
that didn't progress since 2001?

    [I-D.calhoun-diameter-res-mgmt]
               Calhoun, P., "Diameter Resource Management Extensions",
               draft-calhoun-diameter-res-mgmt-08.txt  <http://tools.ietf.org/html/draft-calhoun-diameter-res-mgmt-08.txt>  (work in progress),
               March 2001.


- Editorial (section 6)
I can't parse the following sentence:

      Backward Compatibility:

           With the design of generic extensions an protocol designer has to
           consider with potential concerns about how existing applications
           deal with the new extension they do not understand.


- Contributors:
If Victor and Hannes are authors, then they shouldn't be in the 
contributors list.
Btw, I don't see an affiliation for Victor in the document header. I 
believe the common practice is to write down "consultant"

Did the WG ever considered the following questions:
- Any naming convention for new applications?
- When it is not a good idea to define diameter applications? Let me 
make something up: to push syslog message

Regards, Benoit