Subject: Re: [netmod] review of draft-schoenw-netmod-smi-yang-02.txt

From: Juergen Schoenwaelder <[email-masked]>
Date: Tue, 5 Apr 2011 10:57:59 +0200

On Tue, Mar 29, 2011 at 10:01:27AM +0200, Martin Bjorklund wrote:
> Hi,
> 
> I have reviewed this document, and here are my comments:
> 
> 
> o  In general, I think the mapping would be more clear if you used 2119
>    language.  For example, the first three paragraphs in section 4 are
>    not clear currently:
> 
>      SMIv2 modules are mapped to corresponding YANG modules.  The YANG
>      module name is the same as the SMIv2 module name.
> 
>    this is a MUST
> 
>      The YANG namespace is constructed out of a constant prefix
>      followed by the SMIv2 module name.  Since SMIv2 module names are
>      unique, the resulting YANG namespace is unique.  The registered
>      prefix is urn:ietf:params:xml:ns:yang:smiv2:, see the IANA
>      considerations section.
> 
>    this is also a MUST
> 
>      The YANG prefix is derived from the SMIv2 module name using the
>      module prefix generation algorithm described in Section 3. 
> 
>    this is a MAY.
> 
>      The
>      YANG prefix is supposed to be short and it must be unique within
>      the set of all prefixes used by a YANG module.  The algorithm
>      described in Section 3 generates such prefixes.

I have thrown in a few MUST and MAYs.
 
> o  Table 3 can be pruned by removing the explict SNMPv2-CONF symbols.
>    SNMPv2-CONF is also listed as 'remove all symbols'.

OK

> o  In general, you should list in your translation rules sections where
>    there's an ietf-yang-smiv2 extension statement available, so
>    e.g. where it says:
> 
>      o  The SMIv2 MAX-ACCESS clause is ignored.
> 
>    it could say that a smiv2:max-access statement MAY be generated.

I am adding explicit text that smiv2 extension statements MAY be
generated to capture these things.

> o  I think section 5 should mention that a container is generated from
>    the MODULE-IDENTITY (this is currently mentioned in section 7.1).

I will detail the generation of the containter to avoid the forward
reference and to keep the structure consistent (even though my
implementation does it differently).
 
> o  Section 7 says:
> 
>      The mapping suppresses many structural OBJECT IDENTIFIER
>      assignments that are typically used to organize the OBJECT
>      IDENTIFIER tree.
> 
>   I don't think this is true anymore, is it?

Yep, I have removed this sentence, although we may have to reconsider
this...
 
> o  Section 7.1 says:
> 
>      o  Object identifier assignments through ASN.1 value assignments or
>         through the invocation of a MODULE-IDENTITY clause are translated
>         to YANG container statements.
> 
>   I think more text is needed.  It is not clear from this description
>   how containment is done.  For example, given this:
> 
>     bar OBJECT IDENTIFIER ::= { foo 1 }
>     baz OBJECT IDENTIFIER ::= { bar 1 }
> 
>   it is not clear if the result is:
> 
>     container bar { ... }
>     container baz { ... }
> 
>   or
> 
>     container bar {
>       container baz { ... }
>       ...
>     }

OK
 
> o  Section 7.1, last bullet:
> 
>    o  Implementations MAY suppress the generation of YANG containers for
>       object identifiers that only contain SMIv2 conformance
>       definitions.
> 
>    Same if the oid only contains notifications, I assume.

Yep. 
 
> o  Section 9.2, the TODO.
> 
>   I suggest that only an identity is generated in the normal case, but
>   if there is also some other oid defined under the object-identity
>   oid, then a container is generated.

This makes sense. Although we may have to revisit the question whether
we reproduce the container structure in the first place. I will post a
separate open issue for this.
 
> o  Section 10.1
> 
>       The name of this container
>       is the name of the notification and the name of the current
>       concatenated by a hyphen.
> 
>    I must miss something obvious here, but why is the name of the
>    notification prepended to the container name?

Historical reasons. ;-) I will create a separate thread to resolve
this issue.
 
> o  Section 10.1
> 
>    
>       is the name of the notification and the name of the current
>       concatenated by a hyphen.
> 
>    this should be ... current object concatenated ...

fixed 

> o  Section 10.1
> 
>       If the current object belongs a conceptual table, then a
>       sequence of leaf statements is generated for each INDEX of the
>       SMIv2 conceptual table. Next, a leaf statement is generated for
>       the current object.
> 
>    This needs to specify that the type of the leaf is a leafref.
>    It also needs to specify what the name of the leaf is.

fixed
 
> o  Section 10.1
> 
>       All container leafs are marked as config false.
> 
>    Not necessary.  Remove.  (Section 7.14 of RFC 6020 says:
> 
>      If a "config" statement is present for any node in the
>      notification tree, the "config" statement is ignored.)

fixed

Thanks for this helpful review.

/js

-- 
Juergen Schoenwaelder           Jacobs University Bremen gGmbH
Phone: +49 421 200 3587         Campus Ring 1, 28759 Bremen, Germany
Fax:   +49 421 200 3103         <http://www.jacobs-university.de/>