Subject: [Dime] Review of rfc3588-bis-11 - Part 1

From: Mark Jones <[email-masked]>
Date: Tue, 26 Aug 2008 17:22:33 -0400

Here is my review of sections 1 to 5 of rfc3588bis. I'll review the remainder of the draft before the end of the week. The majority of my comments are editorial.

---

1.1.  Diameter Protocol

   AVPs may be added arbitrarily to Diameter messages,
   so long as the requirements of a message's ABNF are met and the ABNF
   allows for it.

The clause "and the ABNF allows for it" appears to be redundant.

---

1.1.3.  Changes from RFC3588

s/obselete/obsolete
s/fixes state machine/fixes to the state machine/
s/is now shown here/is not shown here/

---

1.2.  Approach to Extensibility

s/semantic/semantics/

---

1.2.3.   Creating New Commands

   A new Command Code has to 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).

Suggest rewording to:

A new Command Code MUST be allocated when (a) new required AVPs (those indicated as {AVP}) are added or deleted, (b) new mandatory-to-understand AVPs (those with the M-bit set) are added or deleted or (c) existing AVPs are redefined (for example by changing a required AVP into an optional one).

---

I suggest clarifying somewhere in this section that the IANA managed Command Code name space is flat, i.e. it is not a per-application name space. The reuse of existing commands in a new application does not free the application designer from the ABNF constraints of the original command.

---

1.2.4.   Creating New Diameter Applications

        If the M-bit is set by the sender and the
      receiver does not understand the AVP or the values carried within
      that AVP then a failure is generated (see Section 7).

Section 7 describes how receiver deals with a request containing a mandatory-to-understand AVP that it does not understand. Do we specify how a receiver deals with an answer? Terminating the session maybe not always be the desired behaviour. Should we leave it to application to specify?

---

      Adding an AVP with the M-bit in the MUST column of the AVP flag
      table to an existing Command/Application requires a new Diameter
      Application Id to be assigned to that Application.

I understand that adding an M-bit AVP requires a new Command but that is not mentioned in this paragraph. It is then this new command that triggers the requirement for a new application.

---

Remove the reference to the MAY column (assuming agreement on removal from section 4.5).

---

Suggest rewording the note on M-bit setting to:

Note: The M-bit setting for a given AVP is specific to an Application and each command within that Application which includes the AVP. That is, if an AVP appears in two commands for application Foo and the M-bit settings are different in each command, then there should be two AVP flag tables describing when to set the M-bit.

---

The last paragraph in section 1.2.4 doesn't mention the M-bit which appears to contradict the earlier statement on setting it requiring a new application id. Suggest rewording to remove the contradiction:

   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.

---

1.3.  Terminology

s/Session-Identifier/Session-Id/

---

2.  Protocol Overview

s/support also both/support both/

---

2.3.  Diameter Application Compliance

The last paragraph already appears in section 1.2.4. If it is still required here, I suggest rewording "non-mandatory AVPs" to "optional AVPs with the M-bit cleared" for consistency.

---

2.7.  Routing Table

Inconsistency wrt normative statements in the descriptions of Realm Name and Application Identifier fields: Realm Name is "typically used as primary key" whereas Application Identifier "MUST be used as a secondary key". Why wouldn't the Realm Name be the mandatory primary key?

---

3.  Diameter Header

Suggest rewording second paragraph describing Application-ID field:

      The value of the application-id field in the header MUST be the
      same as any relevant application-id AVPs contained in the message.

---

4.1.  AVP Header

s/ingnored/ignored/

---

Suggest rewording paragraph on 'M' bit setting to:

      The 'M' bit MUST be set according to the rules defined in the
      application specification which introduces or re-uses this AVP.
      Within a given application, the M-bit setting for an AVP is either
      defined for all command types or for each command type.

---

4.4.   Grouped AVP Values

   However, the encapsulated AVPs with 'M' (mandatory) bit set MUST
   belong to the Diameter application the Grouped APV is used in.

I don't understand what "belong" means in this sentence. Does it mean new AVPs that are defined in this application?

s/APV/AVP/

---

4.5.  Diameter Base Protocol AVPs

Strongly suggest removing the MAY column. For the confusion it has caused, it MUST die NOW.

---

5.2.  Diameter Peer Discovery

s/requestor/requester/

---

5.3.  Capabilities Exchange

s/indentifier/identifier/

---

5.6.5.  Capabilities Update

   A Diameter node MUST initiate peer capabilities update by sending a
   Capabilities-Exchange-Req (CER) to all its peers which supports peer
   capabilities update and is in OPEN state.

How does the Diameter node know which peers support peer capabilities update? I understood the goal of the bis was to clarify/fix existing functionality so I am surprised we are adding mandatory new functionality. I suggest removing this and instead encourage interested parties to craft a "Peer Capabilities Update" application that can be advertised by CER/CEA.

---

Regards
Mark
_______________________________________________
DiME mailing list
[email-masked]
https://www.ietf.org/mailman/listinfo/dime

---

From: Victor Fajardo <[email-masked]>
Date: Wed, 27 Aug 2008 10:42:41 -0400

Hi Mark,

Thanks for the review. Most of the editorial stuff looks ok to me. 

> A new Command Code MUST be allocated when (a) new required AVPs (those indicated as {AVP}) are added or deleted, (b) new mandatory-to-understand AVPs (those with the M-bit set) are added or deleted…

Just for clarification. Was requirement (b) above something we agreed on 
in the extensibility design (I'm having problems trying to find a 
reference for this in the extensibility discussion) ? My understanding 
was that only ABNF changes and message round-trips triggers a new 
command code; maybe I'm wrong. Also, (b) seems to imply there is a loose 
association between M-bit AVPs and ABNF ... something we are trying to 
avoid.

> I suggest clarifying somewhere in this section that the IANA managed Command Code name space is flat, i.e. it is not a per-application name space. The reuse of existing commands in a new applicatio…

Sure.

> Section 7 describes how receiver deals with a request containing a mandatory-to-understand AVP that it does not understand. Do we specify how a receiver deals with an answer? Terminating the sessio…

It is probably better to leave it up to the application to decide. In 
this way, we don't hinder robust error recovery schemes the application 
may introduce.

> I understand that adding an M-bit AVP requires a new Command but that is not mentioned in this paragraph. It is then this new command that triggers the requirement for a new application.

See above.

> Remove the reference to the MAY column (assuming agreement on removal from section 4.5).

I'm still waiting for any objections on the MAY column removal. Hannes ?

> Note: The M-bit setting for a given AVP is specific to an Application and each command within that Application which includes the AVP. That is, if an AVP appears in two commands for application Foo…

Sounds good.

>    commands ABNF allows for it.  Please refer to Section 11.1.1 for

Ok.

> The last paragraph already appears in section 1.2.4. If it is still required here, I suggest rewording "non-mandatory AVPs" to "optional AVPs with the M-bit cleared" for consistency.

Ok.

> Inconsistency wrt normative statements in the descriptions of Realm Name and Application Identifier fields: Realm Name is "typically used as primary key" whereas Application Identifier "MUST be use…

Good catch.

>       same as any relevant application-id AVPs contained in the message.

Ok.

>       defined for all command types or for each command type.

Ok.

> I don't understand what "belong" means in this sentence. Does it mean new AVPs that are defined in this application?

I don't understand that sentence either. I suggest to remove it since if 
the grouped AVP with M bit is received and understood by the app then it 
applies that the app already understands the sematics/usage/presence of 
the encapsulated AVPs.

> Strongly suggest removing the MAY column. For the confusion it has caused, it MUST die NOW.

See above :):):)

> How does the Diameter node know which peers support peer capabilities update? I understood the goal of the bis was to clarify/fix existing functionality so I am surprised we are adding mandatory ne…

The problem is actually the opposite of what you describe. The 
capabilities update has always been in the statemachine in RFC3588 and 
everybody was trying to implement it but nobody knows exactly what it 
was suppose to do (see notes from the very first interop). The text in 
11 was proposed by implementors as an "easy" way to negotiate through 
issue  ...

if folks REALLY want to create a new capabilities update then I would 
agree with your suggestion with some exceptions: a) keep this current 
text for backward compatibility with existing implementation b) write a 
new extension application c) use a different command code instead of 
CER/CEA

-- victor

---

From: Mark Jones <[email-masked]>
Date: Wed, 27 Aug 2008 14:40:48 -0400

(Message subject changed to focus discussion...)

Hi Victor,

Comments inline...

> > How does the Diameter node know which peers support peer
> capabilities update? I understood the goal of the bis was to
> clarify/fix existing functionality so I am surprised we are
> adding mandatory new functionality. I suggest removing this
> and instead encourage interested parties to craft a "Peer
> Capabilities Update" application that can be advertised by CER/CEA.
> >
>

I undestood that the capabilities update was allowed due to an oversight in the original RFC3588 state machine. Since nobody knows exactly what it was supposed to do, my guess is that the implementations out there either don't support capabilities update at all or don't support it in an interopable fashion. Given this starting point, maybe an easier way to negotiate the issue would be to remove it from the state machine in 3588bis, i.e. no CER/CEA in open state.

> new extension application c) use a different command code instead of

I agree with (b) and (c).

Regards
Mark

---

From: Victor Fajardo <[email-masked]>
Date: Wed, 27 Aug 2008 15:41:54 -0400

Hi Mark,

> I undestood that the capabilities update was allowed due to an oversight in the original RFC3588 state machine. Since nobody knows exactly what it was supposed to do, my guess is that the implement…

I hoping bis has enough changes to justify discarding earlier agreements 
with early diameter adaptors/implementors; i.e. updating to bis-12 will 
mandate changes to the base diameter stack implementation regardless. In 
which case, I'm ok with removing capabilities update from this document.

Any opinions ?

-- victor

---

From: Sarkar Biplab <[email-masked]>
Date: Thu, 28 Aug 2008 10:31:02 +0530

Hi All,

I think in case we change the base diameter stack implementation we
might also have to change the diameter message version number.

Thanks
Biplab

---

From: "Glen Zorn" <[email-masked]>
Date: Thu, 28 Aug 2008 16:56:10 +0700

Victor Fajardo <mailto://[email-masked]> writes:

> Hi Mark,
> 
> > I undestood that the capabilities update was allowed due to an
> oversight in the original RFC3588 state machine. Since nobody knows
> exactly what it was supposed to do, my guess is that the
> implementations out there either don't support capabilities update at
> all or don't support it in an interopable fashion. Given this starting
> point, maybe an easier way to negotiate the issue would be to remove it
> from the state machine in 3588bis, i.e. no CER/CEA in open state.
> >
> 
> mandate changes to the base diameter stack implementation regardless.

Then don't we need to change the version number in the header?

> In

If capabilities update is removed, would we still have to change the
version?

---

From: Victor Fajardo <[email-masked]>
Date: Thu, 28 Aug 2008 09:48:36 -0400

Hi Glen,

> Then don't we need to change the version number in the header?

Hmm.. I think I was mistaken in that statement. So far, implementations 
has been able to accommodate the bis without the need to rev up the base 
numbers. So pls dis-regard my comments.

> If capabilities update is removed, would we still have to change the

Good question. I'm hoping not to change the rev. In which case we may 
need to keep the capabilities update in the bis. Any opinions ?

-- victor

---

From: Victor Fajardo <[email-masked]>
Date: Thu, 28 Aug 2008 10:23:09 -0400

Hi,

Going through some offline discussion, the consensus seems to be:

a) Remove capabilities update completely from bis
b) write a new extension application for capabilities update
c) use new command code for (b) to negotiate the updates

Any opinions/comments ?

-- victor

> ______________________________________________________________________
> This email has been scanned by the MessageLabs Email Security System.
> For more information please visit http://www.messagelabs.com/email 
> ______________________________________________________________________
>
>
>

---

From: <[email-masked]>
Date: Thu, 28 Aug 2008 09:50:38 -0500

I agree with this.  

For what it is worth, this was something we discussed about 4 years ago
as well, that Negotiation could be added on as an extension later.

John 

>-----Original Message-----
>From: [email-masked] [mailto:[email-masked]] On 
>Behalf Of ext Victor Fajardo
>Sent: 28 August, 2008 07:23
>To: Victor Fajardo
>Cc: [email-masked]
>Subject: Re: [Dime] Peer Capabilities Update
>
>Hi,
>
>>>> implementations out there either don't support capabilities update 
>>>> at all or don't support it in an interopable fashion. Given this 
>>>> starting point, maybe an easier way to negotiate the issue 
>would be 
>>>> to remove it from the state machine in 3588bis, i.e. no 
>CER/CEA in open state.
>>>>     
>>>> I hoping bis has enough changes to justify discarding earlier 
>>>> agreements with early diameter adaptors/implementors; i.e. 
>updating 
>>>> to bis-12 will mandate changes to the base diameter stack 
>>>> implementation regardless.
>>>>     
>>>>       
>> Hmm.. I think I was mistaken in that statement. So far, 
>> implementations has been able to accommodate the bis without 
>the need 
>> to rev up the base numbers. So pls dis-regard my comments.
>>
>>   
>>>   
>>>     
>>>> In