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

<lionel.morand@orange.com> Tue, 08 April 2014 17:04 UTC

Return-Path: <lionel.morand@orange.com>
X-Original-To: dime@ietfa.amsl.com
Delivered-To: dime@ietfa.amsl.com
Received: from localhost (ietfa.amsl.com [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id B9FAD1A048A for <dime@ietfa.amsl.com>; Tue, 8 Apr 2014 10:04:20 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: 0.802
X-Spam-Level:
X-Spam-Status: No, score=0.802 tagged_above=-999 required=5 tests=[BAYES_50=0.8, FREEMAIL_FROM=0.001, HTML_MESSAGE=0.001, RCVD_IN_DNSWL_NONE=-0.0001, SPF_PASS=-0.001, UNPARSEABLE_RELAY=0.001] autolearn=ham
Received: from mail.ietf.org ([4.31.198.44]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id HVTMmBP_QbQD for <dime@ietfa.amsl.com>; Tue, 8 Apr 2014 10:04:15 -0700 (PDT)
Received: from relais-inet.francetelecom.com (relais-ias245.francetelecom.com [80.12.204.245]) by ietfa.amsl.com (Postfix) with ESMTP id A740C1A0489 for <dime@ietf.org>; Tue, 8 Apr 2014 10:04:07 -0700 (PDT)
Received: from omfeda05.si.francetelecom.fr (unknown [xx.xx.xx.198]) by omfeda11.si.francetelecom.fr (ESMTP service) with ESMTP id DD72C1B8342; Tue, 8 Apr 2014 19:04:06 +0200 (CEST)
Received: from Exchangemail-eme1.itn.ftgroup (unknown [10.114.1.183]) by omfeda05.si.francetelecom.fr (ESMTP service) with ESMTP id BB64E18006C; Tue, 8 Apr 2014 19:04:06 +0200 (CEST)
Received: from PEXCVZYM13.corporate.adroot.infra.ftgroup ([fe80::cc7e:e40b:42ef:164e]) by PEXCVZYH02.corporate.adroot.infra.ftgroup ([::1]) with mapi id 14.03.0181.006; Tue, 8 Apr 2014 19:04:06 +0200
From: lionel.morand@orange.com
To: Benoit Claise <bclaise@cisco.com>, "draft-ietf-dime-app-design-guide.all@tools.ietf.org" <draft-ietf-dime-app-design-guide@tools.ietf.org>
Thread-Topic: [Dime] AD review draft-ietf-dime-app-design-guide
Thread-Index: AQHPTlKK04bn8aLKgkqsyndd0aygjpsHsEvg
Date: Tue, 08 Apr 2014 17:04:05 +0000
Message-ID: <22885_1396976646_53442C06_22885_3037_1_6B7134B31289DC4FAF731D844122B36E54D5C4@PEXCVZYM13.corporate.adroot.infra.ftgroup>
References: <52D9030B.3010402@cisco.com> <533BD276.7000401@cisco.com>
In-Reply-To: <533BD276.7000401@cisco.com>
Accept-Language: fr-FR, en-US
Content-Language: fr-FR
X-MS-Has-Attach:
X-MS-TNEF-Correlator:
x-originating-ip: [10.197.38.2]
Content-Type: multipart/alternative; boundary="_000_6B7134B31289DC4FAF731D844122B36E54D5C4PEXCVZYM13corpora_"
MIME-Version: 1.0
X-PMX-Version: 6.0.3.2322014, Antispam-Engine: 2.7.2.2107409, Antispam-Data: 2014.4.8.91220
Archived-At: http://mailarchive.ietf.org/arch/msg/dime/PH64unILe4bk-T5PgBS_tKRORqI
Cc: dime mailing list <dime@ietf.org>
Subject: Re: [Dime] AD review draft-ietf-dime-app-design-guide
X-BeenThere: dime@ietf.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: Diameter Maintanence and Extentions Working Group <dime.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/dime>, <mailto:dime-request@ietf.org?subject=unsubscribe>
List-Archive: <http://www.ietf.org/mail-archive/web/dime/>
List-Post: <mailto:dime@ietf.org>
List-Help: <mailto:dime-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/dime>, <mailto:dime-request@ietf.org?subject=subscribe>
X-List-Received-Date: Tue, 08 Apr 2014 17:04:20 -0000

Hi Benoit,

Thank you for the deep review.
Please find my feedback below.

Regards,

Lionel

De : DiME [mailto:dime-bounces@ietf.org] De la part de Benoit Claise
Envoyé : mercredi 2 avril 2014 11:04
À : draft-ietf-dime-app-design-guide.all@tools.ietf.org
Cc : dime mailing list
Objet : [Dime] AD review draft-ietf-dime-app-design-guide

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
[LM] discussed in another email thread.

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
[LM] Should I say "I"? :)


-
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?
[LM] Yes. I'm assuming that you would prefer to say it explicitly.


- 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

[LM] I would say that "mandatory" and "M-bit" set are the same and you need to refer to RFC6733 to understand the meaning anyway. But I would be prefer to keep "M-bit set" to avoid the common ambiguity with "required AVP" that indicates the mandatory presence of an AVP. I can add an explicit reference to section 4.1. of RFC6733.

- 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
[LM] right

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.

[LM] (1) is "Minor" as no new application is required. If the new functionality would have been supported by an AVP with the M-bit set, it would have caused the creation of a new application (cf. section 4.3.1: "A mandatory AVP cannot be added to an existing command without defining a new Diameter application"

I guess that 4 is Major, but it's not mentioned.
[LM] can be both as described in section 6.

Can you please better explain the mapping between the 4 categories and Minor/Major extensions
[LM] the whole document is about clarifying these points. Each point is further described in the section 4, 5 and 6.

Alternatively, or maybe on top of that, explain which of 4.X and 5.X are Minor/Major extensions would be beneficial.
[LM] I will try to clarify when required by highlighting "Minor/Major"


- 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.
[LM] Some inconsistency due to the modification of the IANA allocation modalities. I propose to:

-           remove the first sentence.

-          Clarify that "in the past" refers to RFC3588

-          Remove the last sentence as it is somehow clarify in the section 7.

-          Add a sentence that the general guideline "try to reuse as much as possible" still applies.


- 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?
[LM] removed.


- Application and command

   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.

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?
[LM] using an unknown command in an existing application will cause a protocol error as per the RFC 6733.
   DIAMETER_COMMAND_UNSUPPORTED 3001

      This error code is used when a Diameter entity receives a message
      with a Command Code that it does not support.
Therefore non-supporting nodes will never be able to process the request.

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)
[LM] CER/CEA are part of an application, the application "0" i.e. the base protocol. The requirement is that all diameter peers support AT LEAST the base protocol application.

This contradicts:

   Before adding or importing a command, application designers

   should consider the following ...

[LM] here, we are in the section "Adding a new command to an existing application" and it is clearly said that it is a major extension that will cause the creation of a new application.



This also appears to contradict, in section 6

   Generic Diameter extensions are AVPs, commands or applications that

   are designed to support other Diameter applications.

[LM] not so. Any new application can reuse an existing command. For instance, Session Termination commands are reused in every session-related 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.
[LM] ok


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.
[LM] it refers to the text in the section 3.
   It will reduce the time to finalize
   specification writing, and it will lead to a smaller implementation
   effort as well as reduce the need for testing.  In general, it is
   clever to avoid duplicate effort when possible.
Will be clarified.


- 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
[LM] It is about adding a command to an EXISTING application. The first sentence will say "Adding a new command to an existing application is considered" and "to an application" will be removed from the second sentence.


- 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.
[LM] I think we can speak about new version of the specification and variant of the application

Also, mention that a new version of an application is a new application.
I guess it needs to be mentioned in 7.d
[LM] OK for the clarification but not in 7.d as this section is only about interaction with IANA.

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

[LM] I would clarify that "a required AVP (an AVP indicated as {AVP} in the command CCF)" and "an optional AVP (an AVP indicated as [AVP] in the command CCF)"


- 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.

[LM] ok

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.

[LM]  "In general" must be removed and "must use" will be "should use".

- 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

[LM] ok

- Editorial in section 5.7
OLD:

Destination- Realm



NEW:

Destination-Realm

[LM] ok


- The section 5.8 should mention RFC4005bis
[LM] already covered by a previous comment :)


- 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.
[LM] ok


- 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.

[LM] I think that it was as illustration of general-purpose extension based on new commands. Other existing examples are only based on introduction of new AVP. But I agree that it is weird to make reference to expired drafts :)  I would rather refer to Realm-Based Redirection (RFC 7075) and Diameter Priority AVP (RFC6735)

- 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.

[LM] what about:



      When defining generic extensions designed to be supported by existing

      Diameter applications, protocol designers have to consider the

      potential impacts of the introduction of the new extension on the behavior

      of node that would not be yet upgraded to support/understand this new extension.



- Contributors:
If Victor and Hannes are authors, then they shouldn't be in the contributors list.
[LM] this list gives the members of the design team, not the contributors.

Btw, I don't see an affiliation for Victor in the document header. I believe the common practice is to write down "consultant"
[LM] I will try to contact him to see what he would like to see indicated as affiliation.


Did the WG ever considered the following questions:
- Any naming convention for new applications?
[LM] no convention so far.

- When it is not a good idea to define diameter applications? Let me make something up: to push syslog message
[LM] let me two days and I will define a nice application for that :) I cannot see real key criteria to formally say "defining a Diameter application for that purpose is a bad idea". If your example was for "to reinvent the wheel", it seems that this is only a criterion for WG chairs and Ads, but not anymore for the industry.


Regards, Benoit









_________________________________________________________________________________________________________________________

Ce message et ses pieces jointes peuvent contenir des informations confidentielles ou privilegiees et ne doivent donc
pas etre diffuses, exploites ou copies sans autorisation. Si vous avez recu ce message par erreur, veuillez le signaler
a l'expediteur et le detruire ainsi que les pieces jointes. Les messages electroniques etant susceptibles d'alteration,
Orange decline toute responsabilite si ce message a ete altere, deforme ou falsifie. Merci.

This message and its attachments may contain confidential or privileged information that may be protected by law;
they should not be distributed, used or copied without authorisation.
If you have received this email in error, please notify the sender and delete this message and its attachments.
As emails may be altered, Orange is not liable for messages that have been modified, changed or falsified.
Thank you.