Subject: Re: [Netconf] RP comments on draft-ietf-netconf-yang-library-03

From: Randy Presuhn <[email-masked]>
Date: Wed, 20 Jan 2016 22:53:01 -0800 (GMT-08:00)

Hi -

>From: Martin Bjorklund <[email-masked]>
>Sent: Jan 20, 2016 7:00 AM
>To: [email-masked]
>Cc: [email-masked]
>Subject: Re: [Netconf] RP comments on draft-ietf-netconf-yang-library-03
>
>Hi Randy,
>
>Thank you for your review!  Comments inline.
>
>Randy Presuhn <[email-masked]> wrote:
>> Hi -
>> 
>> I was asked to take a look at draft-ietf-netconf-yang-library-03,
>> so here are a few of my comments.
>> 
>> Comments on draft-ietf-netconf-yang-library-03.txt
>> 
>> Abstract:
>>   First sentence includes the phrase "YANG modules
>>   used by a device to represent management and protocol
>>   information."  It's unclear what is meant by "protocol
>>   information", particularly since the following sentence
>>   uses "protocols" to mean (AFAICT) seomthing else.
>> 
>>   Second sentence gives a requirement for cache management,
>>   but the document doesn't address it in any meaningful way.
>>   This sentence should either be deleted or amended to declare
>>   that the requirement is recognized but solutions to the
>>   problem are outside the scope of this document.
>
>How about this NEW Abstract:
>
>  This document describes a YANG library, which provides information
>  about all the YANG modules used by a server.  Simple caching
>  mechanisms are provided to allow clients to minimize retrieval of
>  this information.

This addresses the thrust of my comment.

But....
I wonder whether it might be better (in this instance, since
abstracts are read with relatively little context) to replace
"server" with something that would make it clear that NETCONF
(and other management protocol) servers were intended, and not
servers in some more general sense.  "Device" was problematic
because a device might host multiple (NETCONF) servers; I realize
this is intended to work with more than just NETCONF, but the naked
term "server" in this particular case (without benefit of the
definitions section) leaves me uncomfortable.

>> Introduction:
>>   First paragraph states "This information changes very
>>   infrequently, so it is important that clients be able
>>   to cache the YANG library and easily identify if their
>>   cache is out-of-date."  The assertion "changes very
>>   infrequently" may characterize some existing implementations
>>   today, but is this a necessary, *architectural* implication
>>   for all systems using YANG to model information?   I'd like
>>   to see either documention of this alleged archictectual
>>   limitation, revision of this sentence to identify this
>>   limiting assumption of this particular model, or if there
>>   is no *architectural* limitation and this model could
>>   in fact be useful in dynamic environments, re-working
>>   this sentence to justify cacheing without presuming
>>   a nearly-static environment.
>
>I think this text simply means that the module set "normally" changes
>less often than a client access the server, and thus that caching can
>be useful.
>
>Maybe we can simply say:
>
>OLD:
>
>  If a large number of YANG modules are utilized by the server, then
>  the YANG library information needed can be relatively large.  This
>  information changes very infrequently, so it is important that
>  clients be able to cache the YANG library and easily identify if
>  their cache is out-of-date.
>
>NEW:
>
>  If a large number of YANG modules are utilized by the server, then
>  the YANG library information needed can be relatively large.  Thus
>  it is important that clients be able to cache the YANG library and
>  easily identify if their cache is out-of-date.

Works for me.  I'd replace the last "if" with "whether" as a courtesy
to non-native speakers.

Does the second occurrance of "YANG library" refer to the same thing
as the previous occurance of "YANG library information"?  In my mind
the summary represented by this "library" module and the actual
definitions thus summarized are two distinct things, and a client,
depending on what that client needs to do, might need to only cache
one or the other.

>>   Second paragraph states "Typically, a firmware upgrade is
>>   required to change the set of YANG modules used by a server."
>>   While this might be true of some implementations, I see no
>>   architectural justification whatsoever for this limitation/
>>   assumption.  Suggest deleting this sentence.
>
>Ok.
>
>>   Third paragraph states "The following information is needed
>>   by a client application (for each YANG module in the library)
>>   to fully utilize the YANG data modeling language:"  The following
>>   list makes no sense for this introductory sentence, so I suggest
>>   fixing the introductory sentence.  I think the gist of the
>>   problem is (1) that knowing what a YANG module means requires
>>   looking at other YANG modules, and knowing *exactly* what those
>>   other modules are CANNOT in general be determined by looking only
>>   at the module making the reference.  It's not a problem of
>>   "fully utiliz[ing] the YANG data modeling language."  Furthermore,
>>   (2) knowing that a system "implements" a particular module is
>>   also insufficient for knowing how to interact with it.  This, too,
>>   is not a matter of utilizing the language, but rather one of
>>   determining what the model actually implemented by a system might
>>   be by starting with one model and applying changes to it.
>
>I agree that the words "fully utilize the YANG data modeling language"
>are bad.  Maybe we can just remove that last part of the sentence?

Works for me.

>>   What is the rationale for the optionality in this requirement?
>> |   o  deviation list: The name of each YANG module used for deviation
>> |      statements SHOULD be identified.
>
>I think it should be MUST.

That answers my question.  :-)

>> 2.1.1 module-set-id
>> 
>>   There's an ambiguity surrounding the word "unique" here.  If the
>>   set of modules and submodules reverts to some previous set, is the
>>   module-set-id *required* to revert to the corresponding prior value,
>>   or is it sufficient for the value to take on a new value every time
>>   the set of modules and submodules changes?
>
>Since it is only used to help caching on the client, it is ok if the
>value changes every time the set changes.  Do you think this needs to
>be clarified?

Yes.  It's easy to read the existing language as mandating some
kind of implementation-specific guaranteed-collision-free hashing.
It sounds like some much easier to implement and verify would meet
the requirement.

>>   Second paragraph: pronoun antecedant agreement: there is no plural
>>   noun for "them" to refer to.  Replace "them" with "it".
>
>Ok.
>
>> 2.1.2 yang-protocol
>> 
>>   This section left me with a few of questions about what was
>>   really being modeled and how it might possibly be implemented:
>>     - how does the implementation of this model discover that
>>       YANG models are being used by one or more protocols
>>       within a system?
>>     - when we talk about "protocols that are using the YANG
>>       library" do we only mean in the server role?
>>     - is there really a conflict if two protocols employ the same
>>       model but use different datastores?
>>     - what if one protocol is supported by multiple servers
>>       within a system?
>>     - what happens when a protocol uses a YANG model and doesn't
>>       tell the yang-library implementation?
>
>It was already agreed on the ML to remove everything about protocols,
>including this section.  Instead, this paragraph is added (in the next
>revision) to Introduction:
>
>  If the server implements multiple protocols to access the
>  YANG-defined data, each such protocol has it's own conceptual
>  instantiation of the YANG library.

Works for me.

>> In the Model:
>>    Why is the following "MUST NOT" necessary?  I can understand it
>>    as a *consequence* of other (IMO unfortunate) rules, but do not
>>    see why it is an architecturally neccesary requirement on this model.
>> 
>>                   For YANG 1.1 modules, there MUST NOT be more than one
>>                   module entry with conformance 'implement' for a
>>                   particular module name.
>
>You are correct.  In the upcoming revision this text is changed to:
>
>               For YANG version 1.1 modules, there is at most one
>               module entry with conformance 'implement' for a
>               particular module name, since YANG 1.1 requires that at
>               most one revision of a module is implemented.

I think this is ok if it leave open the posibility of this
module continuing to work if some future version of Yang doesn't
suffer from the one revision limitation.  (In most of the systems
I worked on, that limitation would have been a complete non-starter.
But I digress... )

>>    This statement leads me to another question:
>>                   For import statements that do not specify a revision
>>                   date, the most recent revision in the library SHOULD
>>                   be used by the server.";
>
>This sentence was also removed, and corresponding text is added to
>6020bis (thus only for YANG 1.1 modules).

This was in the library module as well...

>>    I think it would be helpful to be a little more specific about
>>    potential interactions with imports and the decision to represent
>>    an unknown version with a zero-length string. When multiple versions
>>    are present, is the one lacking a revision string *necessarily*
>>    considered the oldest, and thus "SHOULD" not the match for an import
>>    without revision if any other versions are present?
>
>I think this makes sense.  I can see how the first version of a module
>is written w/o a revision statement, but not how the first has a
>revision statement, and the next version removes it.

Cool.

Randy

---

From: Martin Bjorklund <[email-masked]>
Date: Thu, 21 Jan 2016 12:15:24 +0100 (CET)

> definitions section) leaves me uncomfortable.

How about:

  This document describes a YANG library, which provides information
  about all the YANG modules used by a network management server
  (e.g., a Network Configuration Protocol (NETCONF) server).

Ok.

> as the previous occurance of "YANG library information"?

Yes.  Should we use "YANG library contents" in both places instead?

> In my mind

[...]

> It sounds like some much easier to implement and verify would meet

Would this be better?

OLD:

  This mandatory leaf contains a unique implementation-specific
  identifier representing the current set of modules and submodules.
  This can for example be a checksum of all modules and submodules.

NEW:

  This mandatory leaf contains a unique implementation-specific
  identifier representing the current set of modules and submodules.

  The value of this leaf MUST change if the set of modules and
  submodules in the YANG library changes.  There is no requirement
  that the same set always results in the same  module-set-id value.

  The value can for example be a timestamp or a checksum of all
  modules and submodules.

/martin