Re: [yang-doctors] Yangdoctors early review of draft-ietf-dots-rfc8782-bis-00

[[e-mail]]

Hi Ebben, 

Thank you. An udpated verison that takes into account your review is available at: 

https://tools.ietf.org/html/draft-ietf-dots-rfc8782-bis-01 
https://datatracker.ietf.org/doc/html/draft-ietf-dots-rfc8782-bis-01 

A diff from the previous version is available at:
https://github.com/boucadair/rfc8782-yang-update/blob/master/Ebben-Review.pdf 

Cheers,
Med

> -----Message d'origine-----
> De : Ebben Aries [mailto:[e-mail]]
> Envoyé : vendredi 25 septembre 2020 02:53
> À : BOUCADAIR Mohamed TGI/OLN <[e-mail]>
> Cc : [e-mail]; [e-mail]; draft-ietf-dots-rfc8782-
> [e-mail]
> Objet : Re: Yangdoctors early review of draft-ietf-dots-rfc8782-bis-
> 00
> 
> On Sep 24 08:36 AM, [e-mail] wrote:
> > Hi Ebben, all,
> >
> > Thank you for the review.
> >
> > Please see inline.
> >
> > Cheers,
> > Med
> >
> > > -----Message d'origine-----
> > > De : Ebben Aries via Datatracker [mailto:[e-mail]]
> Envoyé :
> > > jeudi 24 septembre 2020 03:38 À : [e-mail] Cc :
> > > [e-mail]; [e-mail]
> > > Objet : Yangdoctors early review of draft-ietf-dots-rfc8782-bis-
> 00
> > >
> > > Reviewer: Ebben Aries
> > > Review result: On the Right Track
> > >
> > > 2 modules in this draft:
> > > - [e-mail]
> > > - [e-mail]
> > >
> > > YANG compiler errors or warnings (pyang 2.3.2, yanglint 1.9.2)
> > > - No compiler errors or warnings however pyang 2.3.2 is
> currently
> > > the only
> > >   compiler verified to emit YANG sx:structure data in tree
> format.
> > > Instance
> >
> > [Med] I confirm this is what we are using.
> >
> > >   data could not yet easily be validated given the current
> > > linters/compilers.
> > >
> > > General comments on the draft/modules:
> > > ----------------------------------------
> > > The modules defined in this draft are an update replacement to
> those
> > > defined in RFC8782 with only 1 being updated from the prior
> > > publication.
> > >   - [e-mail] (updated)
> > >   - [e-mail]
> > >
> > > - First, I share some of the same comments as the previous
> review in
> > > that it
> > >   takes some additional thought on modeling structures for
> alternate
> > > protocol
> > >   communications with the YANG language.  Overall, I think this
> is
> > > likely fine
> > >   but also question where the draft/RFC specification outlines
> rules
> > > that are
> > >   not conveyed within the data-model.  Some examples include
> > > specifying
> > >   mandatory attributes but not making use of the YANG
> 'mandatory'
> > > statement.
> >
> > [Med] We are using "mandatory" statements when this makes sense
> (e.g.,
> > alt-server, hb-status). It is not used for some attributes that
> may
> > appear in both directions, but are not mandatory in one direction.
> For
> > example, "lifetime" is mandatory in a mitigation request, but it
> is
> > when the server replies with a conflict message.
> >
> > FWIW, a constraint we had is to reuse as much as possible the same
> > CBOR key/attribute names. This is not possible if we overload the
> > module with "choice"s:
> >
> > ==
> >    Each "case" may
> >    contain multiple nodes, but each node may appear in only one
> "case"
> >    under a "choice".
> > ==
> >
> > This is why what is normative for an implementation is the core of
> the
> > specification not the YANG module itself.
> >
> 
> Ack - however just like other verbiage put within the YANG
> description statements to indicate valid/invalid values (some of
> which is a direct copy from the document detail), I might expect at
> a bare minimum the description for the node carry when a node is
> mandatory within the payload or not if it cannot be expressed via a
> language statement as you indicate - e.g. This node is mandatory for
> request and info retrieval operations.
> 
> Also keep in mind, YD reviews likely have not dealt with many such
> cases yet so a best practice for these types of use-cases does not
> necessarily exist yet.
> 
> > >   Other cases are stating the intent of reserved or invalid
> integers
> > > but not
> > >   putting the same restrictions on the types under a given data
> > > node.
> >
> > [Med] Can you please indicate which data node are you referring
> to? Thanks.
> >
> 
> A common example to point out...
> 
> An unbounded int32 with the description yielding the rules around
> valid values.  From a pure type perspective, all values that fall
> within the
> int32 range are valid and data validation is only then left to the
> running implementation and conveying invalid boundaries by way of
> protocol error messaging.
> 
> 105       leaf lifetime {
> 106         type int32;
> 107         units "seconds";
> 108         default "3600";
> 109         description
> 110           "Indicates the lifetime of the mitigation request.
> 111
> 112            A lifetime of '0' in a mitigation request is an
> 113            invalid value.
> 114
> 115            A lifetime of negative one (-1) indicates indefinite
> 116            lifetime for the mitigation request.";
> 117       }
> 
> Now to be fair, this also exists in datastore related modules where
> the instance data can be valid from a data-model conformance
> standpoint but still not meet the requirements of the implementation
> - all such cases make it harder for the model consumer to fully
> validate instance data outside of the complete implementation.
> 
> The tradeoff of leaving this outside the schema is likely fine here.
> 
> >  Overall
> > >   this means that you cannot validate the instance/payload data
> > > according to
> > >   the data-model alone.
> >
> > [Med] Agree. The data model alone is not sufficient for an
> > implementer, it should rely on the detailed spec.
> >
> > >
> > > - Previous review suggested the main edits required switching
> the
> > > top-level
> > >   container 'dots-signal'.  This has now been fulfilled by that
> of
> > > an
> > >   sx:structure in this revision.
> > >
> >
> > [Med] Thanks.
> >
> > > - A recent discussion among YANG doctors suggests the use of
> > > normative
> > >   language (RFC2119) in YANG description statements. For example
> > > 'should' =>
> > >   'SHOULD', 'must' => 'MUST', etc... A benefit to this approach
> is
> > > RFC
> > >   comprehension even when the module is stripped from the source
> > > document it
> > >   is contained under.
> >
> > [Med] Thank you for sharing this. I understand the benefit for the
> > "classic" use of YANG, but in our case I'm afraid thus this will
> be
> > redundant with the core specification itself.
> >
> 
> YANG modules can become detached from the IETF document containing
> them (and is the intention for the normal uses as you mention) and
> for this purpose, the module should be as descriptive as possible to
> live on its own.
> 
> Now for this use, you are defining solely message structures which
> can likely still be used for library code generation on its own
> without further context.  The implementation however will need to
> take into account the specification to build upon and this is fine.
> 
> And yes, for all other cases I mention the intention is to be
> redundant carrying the verbiage from the specification detail into
> the YANG description statements for consistency - especially here
> where a protocol specification will already carry this in the
> detail.
> 
> > >
> > > - (draft) L887 nit: "As a reminder, the prefix length must be
> less
> > > than or
> > >   equal to 32 (for IPv4) for 128 (for IPv6)"
> >
> > [Med] Fixed. Changed to:
> >
> > "As a reminder, the prefix length must be less than or equal to 32
> for IPv4 and 128 for IPv6."


_________________________________________________________________________________________________________________________

Ce message et ses pieces jointes peuvent contenir des informations confidentielles ou privilegiees et ne doivent donc
pas etre diffuses, exploites ou copies sans autorisation. Si vous avez recu ce message par erreur, veuillez le signaler
a l'expediteur et le detruire ainsi que les pieces jointes. Les messages electroniques etant susceptibles d'alteration,
Orange decline toute responsabilite si ce message a ete altere, deforme ou falsifie. Merci.

This message and its attachments may contain confidential or privileged information that may be protected by law;
they should not be distributed, used or copied without authorisation.
If you have received this email in error, please notify the sender and delete this message and its attachments.
As emails may be altered, Orange is not liable for messages that have been modified, changed or falsified.
Thank you.