Re: comments on draft-frascone-aaa-xml-dictionary-00.txt
From: David Frascone (davefrascone.com)
Date: 1 May 2002 21:34:06 -0000
I'm glad someone else is using the dictionary!  I agree completely with
your addition of the proxyable and error bit.  They were introduced after
this document was created.  I'll implement those changes right away.

I'm copying the other authors on this response.

Comments inline.

On Wednesday, 01 May 2002, Yoshihiro Ohba wrote:
> o The "requestrules" and "answerrules" attributes in "command" element
>   should have a set of "avprule" attributes to
>   define which AVPs are expected to appear in each command.

I'm not sure I know what you mean by "expected"  The diameter protocol has
required AVPs.  Any number of optional avps can be "expected" in any diameter
message.  That's why we didn't try to recreate the ABNF, but instead, gave
a simple set of rules that would be easy to implement at the protocol
layer, before an application (i.e. nasreq) started doing validation.


>   In addition, such a set of "avprule" attributes should be classified 
>   into "fixed", "required", "optional" portions, to be equivalent 
>   with the ABNF-based command definition described in the Diameter 
>   base specification.

As I stated before, "optional" makes no sense.  The other two, "fixed" and
"required" are already implemented (albeit, without using the words "fixed"
and "required") by the position, maximum, and minimum attributes.

> o The same thing should apply to the "grouped" element".

I don't understand this statement at all.  Unless you're stating that
we should defined grouped AVPs by allowing them to define optional avps, 
position, etc.

If this is the case, then I think we have different definitions of what this
dictionary is going to be used for.  This dictionary is *NOT* meant to be
a replacement, or even an exact representation of the ABNF/rules of the 
Diameter Protocol.  The dictionary has few functions:

It provides a maping for AVP Codes, Values, and Commands into Character
String Mnemonics.

It defines *some* rules for AVPs so that protocol errors can be detected very
early in message processing.

> o The "command" attributes in the "base" element should come
>   after "typedefn" and "avp" attributes since 
>   the "requestrules" and "answerrules" are going to 
>   contain "avprule" attributes which has IDREF to the 
>   "name" attributes of "avp".

Are forward references not allowed?  I had no problem getting libxml to
parse the data.  The reason it is layed out like that is so applications may
be included as external entities.

> o Once "fixed", "required" and "optional" attributes are defined 
>   for "command" and "grouped" elements, the "position" infomation 
>   might not be necessary for "avprule" element.

This I agree with, but, I like the way it is now.  Let's see what the other
authors have to say about it.
> 
> ---- suggested schema file ----
>      <!-- <!ELEMENT base (command*, typedefn+, avp+)> Modified by Ohba -->
>      <!ELEMENT base (typedefn+, avp+, command*)>

I'm not sure this is necessary, but if some xml parsers are picky, we might
have other problems in our entities.  What parser are you using?  A home
grown one?

>      <!-- Modified by Ohba
>      <!ELEMENT grouped (gavp+)>
>      -->
>      <!ELEMENT grouped (fixed* required* optional* fixed*)>
>      <!-- Deleted by Ohba -->
>      <!ELEMENT gavp EMPTY>
>      <!ATTLIST gavp
>           name IDREF #REQUIRED
>           vendor-id CDATA #IMPLIED
>      >
>      -->

Ok, your grouped element does not contain AVPs.  I'm not sure what you're
doing here (or why).  

>      <!-- <!ELEMENT requestrules (avprule+)> Modified by Ohba -->
>      <!ELEMENT requestrules (fixed* required* optional* fixed*)>

I *think* I see what you're trying to do here.  But, since "fixed" is
more of an attribute (logically, not syntatically), I'm not sure what
the meaning of having zero or more "fixed's" around is.  Using your
terminology, I think something like this is more appropriate (although,
again, I do not agree with this change.  I like it the way it was)
And, I don't know why you have two fixed's.

   <!ELEMENT requestrules (avprule+)>
   <!ELEMENT avprule EMPTY>
      <!ATTLIST avprule
         fixed (yes|no) "no"
         required (yes|no) "no"
         optional (yes|no) "yes">

Of course, now there is a bit more checking to make sure required and
optional are not both yes.

>     <!-- Diameter Base Protocol Command Codes -->
>     <command name="PROXIABLE-ERROR" code="0" pbit="1" ebit="1">
>       <answerrules>
>         <fixed>
>           <avprule name="Session-Id" maximum="1"/>
>         </fixed>
>         <required>
>           <avprule name="Origin-Host" maximum="1" minimum="1"/>
>           <avprule name="Origin-Realm" maximum="1" minimum="1"/>
>           <avprule name="Result-Code" maximum="1" minimum="1"/>
>         </required>
>         <optional>
>           <avprule name="Origin-State-Id" maximum="1" minimum="1"/>
>           <avprule name="Error-Reporting-Host" maximum="1" minimum="1"/>
>           <avprule name="Proxy-Info" maximum="1" minimum="1"/>
>           <avprule name="AVP"/>
>       </optional>
>       </answerrules>
>     </command>

Ok, I see where you were going, but this is very confusing to me.  And, more
importantly, I think it is going to be useless information to a parser. 
(Well, to be honest, not useless, but VERY difficult to have a parser 
implement all these rules to do runtime symantic checking of Diameter
messages)

>   <!-- Include extensions here (from external entities) -->
>   &nasreq;
>   &mobileipv4;
>   &sunping;

I like these at the beginning, not at the end.

-- 
David Frascone

          Waiter, there's no fly in my soup! - Kermit

Results generated by Tiger Technologies using MHonArc.