Re: [core] js review of draft-ietf-core-sid-12

[Ivaylo Petrov <[e-mail]>]

Hi Juergen,

Thank you very much for your new comments! Please find my answers below.

On Tue, Jun 23, 2020 at 6:59 PM Juergen Schoenwaelder <
[e-mail]> wrote:

> On Wed, Jun 10, 2020 at 11:16:47AM +0200, Ivaylo Petrov wrote:
>
> > > - Is it CoRECONF or CORECONF? And I find the term CORECONF confusing.
> > > > >   We have two protocols called NETCONF and RESTCONF and now we add
> > > > >   another protocol called CoMI and we call CoMI together with YANG
> > > > >   CBOR and SIDs CORECONF?
> > > > >
> > > > >   1) NETCONF  + YANG + XML      serialization + path naming -> ?
> > > > >   2) RESTCONF + YANG + XML|JSON serialization + path naming -> ?
> > > > >   3) CoMI     + YANG + CBOR     serialization + SID naming  ->
> CORECONF
> > > > >
> > > > >   We do not have a term for 1) and 2) and then we have a term for
> 3)
> > > > >   which, however, looks more like the protocol names used in 1) and
> > > > >   2). This comment is not specific to this ID, but the asymmetry
> > > > >   showed up while reading the SID document, I had to look at other
> IDs
> > > > >   to understand how things are named. And the SID document says
> > > > >
> > > > >    YANG is a language designed to model data accessed using one of
> the
> > > > >    compatible protocols (e.g.  NETCONF [RFC6241], RESCONF
> [RFC8040] and
> > > > >    CoRECONF [I-D.ietf-core-comi]).
> > > > >
> > > > >   Then I read the CoMI abstract. It first says CoMI is "a CoAP
> > > > >   Management Interface", it then says "The complete solution
> composed
> > > > >   of CoMI, [I-D.ietf-core-yang-cbor] and [I-D.ietf-core-sid] is
> called
> > > > >   CORECONF." and finally it states that "CORECONF extends the set
> of
> > > > >   YANG based protocols, NETCONF and RESTCONF, with the capability
> to
> > > > >   manage constrained devices and networks.". So I am confused, is
> > > > >   CORECONF a protocol as stated in this document? Or is CoMI a
> > > > >   protocol? (What is then the difference between a "Management
> > > > >   Interface" and a management protocol?) I am not sure whether I
> get
> > > > >   to review comi, hence I mention my confusion here as I hit it
> while
> > > > >   reviewing the sid document.
> > > > >
> > > >
> > > > [IP]: Currently this is indeed somewhat confusing. The proposed
> change
> > > from
> > > > Michael Richardson was to at least have CORECONF in the title of the
> CoMI
> > > > document. I am wondering if that might still leave some of the
> confusion.
> > > > For me the simple solution is in this document to refer to CoMI, not
> > > > CORECONF and let CoMI draft define what CORECONF actually is. Unless
> you
> > > > think this will still not resolve the issue, this is going to be my
> way
> > > > forward.
> > >
> > > Avoiding CORECONF in this document helps to limit the problem. If CoMI
> > > is the name of the protocol, I would hope we do not need CORECONF at
> > > all. But then CORECONF is all over the place in
> > > draft-ietf-core-comi-09.txt, it actually looks like the protocol is
> > > called CORECONF and not CoMI. I really believe this terminology
> > > confusion needs to be resolved in the WG so the WG actually knows and
> > > agrees on the name of the technology they standardize.
>
> I am not sure whether this got resolved...
>

[IP]: You are right - I realized that omission and I have started a
discussion (so far not many opinions on it though) here [1] focusing mostly
on this point. I have proposed 3 options for a way forward - use only
CORECONF, use only CoMI or find good enough reason to have the clearly CoAP
part have a name (CoMI) and CoMI + Content format to be called CORECONF. I
currently don't see a reason for having both CoMI and CORECONF as names, so
I would rather go with one of the other two options. I will try to see if
in the past people were more in favour of one or the other. I believe
Carsten would prefer CORECONF and at the top of my head I don't remember
anyone voicing any preferences, but I will check that and propose it as the
default way forward if there are no replies in the next 6-7 days.

[1]: https://mailarchive.ietf.org/arch/msg/core/x9RJkfnQgW0Rp3LmHd2CkbOA4DY/

> > > - This description makes little sense to me:
> > > > >
> > > > >   typedef sid-file-version-identifier {
> > > > >     type uint64;
> > > > >     description
> > > > >       "Optional attribute that gives information about the .sid
> file
> > > > >        version.";
> > > > >   }
> > > > >
> > > > >   This is a type definition. Why does the description talk about an
> > > > >   optional attribute? The type should not state whether something
> > > > >   using the type is optional or not. (And I would prefer to avoid
> > > > >   'attribute', better use YANG defined terms or just describe that
> > > > >   this type represents a version number for a SID file.)
> > > > >
> > > >
> > > > [IP]: I believe now it should be more clear.
> > >
> > > Yes. I wonder though, is this a simple linear counter? Or can it be
> > > anything as long as newer > older is satisfied? Or is this just a tag
> > > that needs to match and it does not imply any order semantics?
> > >
> >
> > [IP]: The intention was to be newer > older without any implied
> semantics. I
> > rephrased the text to capture this.
> > Old:
> >            "Optional leaf that specifies the version number of the .sid
> > file.
> >           .sid files and the version
> >           sequence are specific to a given YANG module revision.
> >           This number starts at zero when there is a YANG module update.
> >           This number can distinguish updates to the SID file which are
> the
> > result of
> >           new processing, or the result of reported errata.";
> > New:
> >            "Optional leaf that specifies the version number of the .sid
> > file.
> >           .sid files and the version sequence are specific to a given
> YANG
> >           module revision. This number starts at zero when there is a new
> > YANG
> >           module revision and increases monotonically.  This number can
> >           distinguish updates to the .sid file which are the result of
> new
> >           processing, or the result of reported errata.";
>
> The YANG versioning aims at supporting version histories that are more
> complex than just a simple linear history. Hence, a simple linear
> order of the sid version number may have limitations.
>

[IP]: The .sid file history is linked to a YANG file version and is
supposed to mostly/only fix errors in the generation. Any change in the
YANG file version (be it linear or nonlinear) is expected to reset the .sid
file version to 0. Please let me know if you believe that this is still not
going to work well in some cases or if you think that the text is not clear
enough.

> >   s/Identifies a schema-node path string/A schema-node path"
> > > > >
> > > > >   It is a bit confusing to define a schema-node path by way of
> > > > >   reference to an instance identifier. I understand that you borrow
> > > > >   the namespace encoding from the way JSON encode instance
> identifiers
> > > > >   but this type really represents what RFC 7950 calls an absolute
> > > > >   schema node identifier, no? Is the term schema-node path actually
> > > > >   needed or is it the same as absolute schema node identifier? Or
> is
> > > > >   the difference between the two how namespaces are represented?
> > > > >
> > > >
> > > > [IP]: I might have misunderstood something, but my understanding is
> that
> > > > the prefix related to a module could be changed during an import,
> whereas
> > > > here we really want to use the module name as a more stable
> identifier.
> > > The
> > > > difference between absolute schema node identifier and schema-node
> path
> > > is
> > > > that we mandate the use of module name and not prefix as defined in
> RFC
> > > > 7950.
> > >
> > > Well, what you model here is an absolute schema node path, except that
> > > prefixes are replaced by module names. Note that refering to
> > > instance-identifier as defined in RFC 7951 has the problem, the RFC
> > > 7951 definition of an instance-identifier also includes prefixes
> > > instead of module names.
> > >
> >
> > [IP]: I might be misunderstanding your statement or the text in RFC 7951,
> > but if I read sec 6.11. from RFC 7951 correctly,
> >
> > The leftmost (top-level) data node name is always in the
> > namespace-qualified form.
> >
> >
> > In sec 4 of RFC 7951 the namespace-qualified form seems to only use the
> > module name and not the prefix. My impression seems to be supported also
> by
> > the example in this section. Due to this I believe the current text is
> > actually correct.
>
> In an ideal world, the definitions in this document would depend on
> the YANG specification and not on some other encoding rules. And I
> think what you are dealing with are absolute-schema-nodeid with the
> additional rule that prefix values in the node-identifier production
> are module names.
>
> The way the instance-identifier type has been defined is a bit
> problematic since it is rather XML encoding specific. Hence to get
> what you want, you have to import the JSON specific solution. If we
> ever do YANG 2.0 and factor out the XML specific things from the core
> language, this will likely be addressed. Note that the instance
> identifier includes predicates in square brackets, which I think your
> schema-node-path does not need.
>

[IP]: I believe I see your point. My understanding is that you would prefer
this document to not depend on the JSON encoding, but rather define that
encoding in this document using as a basis the YANG specification. That
would be fine for me, simply let me know if I understood you correctly.

/js
>
> --
> Juergen Schoenwaelder           Jacobs University Bremen gGmbH
> Phone: [phone]         Campus Ring 1 | 28759 Bremen | Germany
> Fax:   [phone]         <https://www.jacobs-university.de/>
>

Thanks again!

Best regards,
Ivaylo Petrov