Re: [core] draft-ietf-core-sid-02 Issues

[Andy Bierman <[e-mail]>]

On Mon, Nov 27, 2017 at 1:03 PM, Michel Veillette <
[e-mail]> wrote:

> Hi Andy
>
>
>
> I first want to confirm the list of changes you  are proposing.
>
>
>
>    1. Paths within 'label' represented using a canonical representation
>    (YANG 1.1 ABNF  for a schema node identifier, except module names are used
>    instead of prefixes)
>    2. 'type' implemented using an enumeration
>    3. 'module' and 'submodule' combined within the same 'type' (module)
>    4. 'node', 'notification', 'rpc' and 'action' combined within the same
>    'type' (data)
>
>
>



It's still time to make such changes, but I will like to limit the number
> of iterations to avoid too much instabilities in the registry. If we move
> ahead, let try to get a large consensus.
>
>
>
> My only question about these changes are relative to 'identity' and
> 'feature'.
>
>
>
> 'module' and 'submodule' are part of a global namespace.
>
> 'data' are represented using a canonical representation which make them
> globally unique.
>
> Should we also adopt some kind of canonical representation for 'feature'
> and 'identity'?
>
> Should we make the 'label' field globally unique to simplify the
> aggregation of .sid files?
>
>
>


I think that identity SID entries are currently broken.
Only the identity name is included in the label, not the base.
YANG 1.1 allows multiple bases, so listing 1 base is broken.

Identity and feature names are unique within the module that defines them.
The SIDs for them will be globally unique.
The SID entry for a given module does not need to identify the module
itself.
That is implied.


  typdef foo {
     type identityref {
         base foo;
      }
  }

   identity foo;

   feature foo;

    leaf foo {
        if-feature foo;
        type foo;
    }

All legal YANG -- the SID labels call contain "foo", and each one has a
different type.

I see your issue -- a tool cannot compare an entry from 2 different modules
for feature and identity definitions. However it is not relevant in YANG
to compare these names across modules.    Feature A:foo and B:foo have no
relation
to each other, just a name collision resolved by the module namespace scope.

I do not anticipate any other changes to SID.
Sorry I did not do this review months ago when the draft came out.
I want to implement CoMI w/CBOR/SID soon, and I think the CBOR/SID part
is almost ready.



Andy






> Notes:
>
>    - See https://comi.space/file/ietf/public/[e-mail] for
>    examples using the current  .sid format of data nodes defined using the
>    augment statement.
>    - See https://comi.space/file/ietf/public/[e-mail]
>    for examples using the current  .sid format of identity.
>
>
>
> Regards,
>
> Michel
>
>
>
> *From:* core [mailto:[e-mail]] *On Behalf Of *Andy Bierman
> *Sent:* Monday, November 13, 2017 4:41 PM
> *To:* Core <[e-mail]>
> *Subject:* [core] draft-ietf-core-sid-02 Issues
>
>
>
> Hi,
>
>
>
> Here are some comments on SID draft:
>
>
>
>   - The SID design does not support augment correctly.
>
>     It is not mentioned at all in the draft.
>
>     The "label" leaf design does not allow for YANG identifier
>
>     names from other modules. There is also no need to
>
>     overload the label field with extra semantics about
>
>     the data type.  This actually breaks the SID mapping
>
>     since "label" is part of the key. (It needs a simple
>
>     and canonical representation.)
>
>
>
>     Example of SID broken for augment:
>
>
>
>       module A {
>
>         ...
>
>         container top {
>
>           leaf leaf1 { type string; }
>
>         }
>
>       }
>
>
>
>       module B {
>
>         import A { prefix A; }
>
>         ...
>
>
>
>         augment /A:top {
>
>           leaf leaf1 { type int32; }
>
>         }
>
>         container top;
>
>       }
>
>
>
>
>
>   - the module revision date is always 2015-12-16.
>
>     The revision date is supposed to match the I-D revision date
>
>     so YANG compilers can tell the different versions apart
>
>
>
>   - typedef yang-identifier
>
>     do not clone this type.  Use the real yang-identifier in
>
>     ietf-yang-types (RFC 6991(
>
>
>
>   - The 'mandatory true' statements are redundant for key leafs
>
>     and should be removed (for key leafs only)
>
>
>
>   - leaf 'type' needs to be an enumeration, not a string with patterns
>
>     to do the same thing as an enumeration.  This field needs to be
>
>     a simple identifier, not a union of complex patterns.
>
>
>
>     This type is not defined correctly.  It needs to identify the
>
>     YANG identifier namespace type, as defined in RFC 7950, sec. 6.2.1
>
>     Modules and submodules are in the same namespace.
>
>     They are not separate things as defined in the SID draft.
>
>
>
>     The following "yang-ns-id" typedef definition shows the
>
>     full namespace, and "sid-ns-id" shows the subset relevant to CoMI.
>
>     The leafs "type" and "label" fix the problems with the
>
>     current design and allow the "label" field to support augment-stmt.
>
>     The only allowed form is also the canonical form, allowing the
>
>     { type, label } tuples to be easily and correctly compared.
>
>
>
>
>
>      typedef yang-ns-id {
>
>         type enumeration {
>
>            enum module {
>
>              description
>
>                "All module and submodule names share the same
>
>                 global module identifier namespace.";
>
>            }
>
>            enum extension {
>
>              description
>
>                "All extension names defined in a module and its
>
>                 submodules share the same extension identifier
>
>                 namespace.";
>
>            }
>
>            enum feature {
>
>              description
>
>                "All feature names defined in a module and its
>
>                 submodules share the same feature identifier
>
>                 namespace.";
>
>            }
>
>            enum identity {
>
>              description
>
>                "All identity names defined in a module and its
>
>                submodules share the same identity identifier
>
>                namespace.";
>
>            }
>
>            enum type {
>
>              description
>
>                "The namespace for all derived type names, as
>
>                defined in YANG.";
>
>
>
>            }
>
>            enum grouping {
>
>              description
>
>                "The namespace for all grouping names, as defined
>
>                in YANG.";
>
>            }
>
>            enum data {
>
>              description
>
>                "The namespace for all data nodes, as defined in YANG.";
>
>            }
>
>            enum case {
>
>              description
>
>                "All cases within a choice share the same case
>
>                 identifier namespace.  This namespace is scoped
>
>                 to the parent choice node.";
>
>            }
>
>         }
>
>         description
>
>           "A YANG namespace identifier specifies the identifier
>
>            namespace within a YANG module and its submodules.
>
>            An identifier is only required to be unique within
>
>            a specific namespace.";
>
>          reference
>
>            "RFC 7950, The YANG 1.1 Data Modeling Language;
>
>             Section 6.2.1: Identifiers and Their Namespaces.";
>
>      }
>
>
>
>      typedef sid-ns-id {
>
>         type yang-ns-id {
>
>            enum module {
>
>              description
>
>                "All module and submodule names share the same
>
>                 global module identifier namespace.";
>
>            }
>
>            enum feature {
>
>              description
>
>                "All feature names defined in a module and its
>
>                 submodules share the same feature identifier
>
>                 namespace.";
>
>            }
>
>            enum identity {
>
>              description
>
>                "All identity names defined in a module and its
>
>                submodules share the same identity identifier
>
>                namespace.";
>
>            }
>
>            enum data {
>
>              description
>
>                "The namespace for all data nodes, as defined in YANG.";
>
>            }
>
>         }
>
>         description
>
>           "A SID namespace identifier specifies the identifier
>
>            namespace within a YANG module and its submodules,
>
>            as used within the a SID registry mapping.";
>
>      }
>
>
>
>      typedef sid-path {
>
>        type string;
>
>        description
>
>          "Identifies a schema-node path string for use in the
>
>           SID registry.  This string format follows the rules
>
>           for an instance-identifier, as defined in RFC 7959,
>
>           except that no predicates are allowed.
>
>
>
>           This format is intended to support the YANG 1.1 ABNF
>
>           for a schema node identifier, except module names
>
>           are used instead of prefixes, as specified in RFC 7951.";
>
>        reference
>
>          "RFC 7950, The YANG 1.1 Data Modeling Language;
>
>           Section 6.5: Schema Node Identifier;
>
>           RFC 7951, JSON Encoding of YANG Data;
>
>           Section 6.11: The instance-identifier type";
>
>      }
>
>
>
>      leaf type {
>
>        type sid-ns-id;
>
>        description
>
>          "The SID namespace Identifier type for this entry";
>
>      }
>
>
>
>      leaf label {
>
>        type union {
>
>          type sid-path;
>
>          type yang:yang-identifier;
>
>        }
>
>        description
>
>          "The label identifying this mapping entry.
>
>           If the corresponding 'type' field is 'module',
>
>           'feature', or 'identity', then this field MUST
>
>           contain a valid yang-identifier string.
>
>
>
>           If the corresponding 'type' field is 'data',
>
>           then this field MUST contain a valid sid-path string.";
>
>       }
>
>
>
>
>
>   Example for module A and B:
>
>
>
>     module A:
>
>       module, "A", 1700
>
>       data, "/A:top", 1701
>
>       data, "/A:top/leaf1", 1702
>
>
>
>     module B:
>
>       module, "B", 2700
>
>       data, "/A:top/B:leaf1", 2701
>
>       data, "/B:top", 2702
>
>
>
>
>
>
>
>
>
> Andy
>
>
>