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

[[e-mail]]

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.

>   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.

 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. 

> 
> - (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."

> 
> Module ietf-dots-signal-channel:
> ----------------------------------------
> - IMO, the module prefix 'signal' is too generic for what is
> specific to
>   message payload over a protocol that uses CoAP.  Looking at all
> the
>   published RFCs for dots, we have the same use of generic prefixes.
> Since
>   the prefix is of local scope, it is not an entirely large issue
> but in
>   hindsight should have had some better consistency especially when
> the
>   recommendation to module importers is to utilize the defined
> prefix of said
>   module.

[Med] Fair enough. Changed to "ietf-dots-signal".

> 
> - This module imports and references 'ietf-dots-data-channel' as
> prefix
>   'ietf-data'.  Not only is this too generic as I mentioned above,
> it is not
>   using the suggested prefix of 'data-channel' (which is also too
> generic) as
>   defined within that module (RFC7950 Section 7.1.4)
> 
>    "When used inside the "module" statement, the "prefix" statement
>     defines the prefix suggested to be used when this module is
> imported."

[Med] Fixed. Updated to use 'data-channel'.

> 
> - Minor nit: It appears all author email addresses have been updated
> since the
>   RFC8782 variant replacing '@' with '&'

[Med] Fixed. 

> 
> Module iana-dots-signal-channel:
> ----------------------------------------
> - Similar to what I mention above regarding the module prefix, we
> use a
>   generic 'iana-signal' prefix here which is inconsistent and not
> descriptive
>   of the overall intent of the module.

[Med] Updated to "iana-signal-channel".

> 
> Overall, I'm not sure if the other referenced and published DOTS
> YANG modules went through a YD review but the comments above really
> should be addressed in a new revision of those modules.

[Med] Sure. We will double check those. 

  Once the
> above is addressed and possibly some discussion around when type
> restrictions are not conveyed within the model, it is probably on
> the right track. 

[Med] Great, thanks. We can add some clarification as needed, but as indicated above, if you can share the attributes you are referring to, this will be helpful. Thanks.  

 Since this is not the usual use of YANG in the
> context of YD reviews, it is a bit of an on the fly analysis which
> may require some additional discussion among other YANG doctors.
> 

[Med] Sure.


_________________________________________________________________________________________________________________________

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.