Re: [yang-doctors] Yangdoctors early review of draft-ietf-opsawg-ntw-attachment-circuit-04

[[e-mail]]

Hi Martin,

Thanks for the review. 

Please see inline.

Cheers,
Med

> -----Message d'origine-----
> De : Martin Björklund via Datatracker <[e-mail]>
> Envoyé : mercredi 24 janvier 2024 16:20
> À : [e-mail]
> Cc : [e-mail];
> [e-mail]
> Objet : Yangdoctors early review of draft-ietf-opsawg-ntw-attachment-
> circuit-04
> 
> Reviewer: Martin Björklund
> Review result: Ready with Issues
> 
> Here is my YANG doctor's review of draft-ietf-opsawg-ntw-attachment-
> circuit-04.
> 
> o  There are several typedefs defined on the form:
> 
>   typedef attachment-circuit-reference {
>     type leafref {
>       path "/nw:networks/nw:network/nw:node/ac-ntw:ac/ac-ntw:name";
>     }
> 
>   Note that this path spans three lists (network, node, ac).  Unless
>   it is guaranteed that the `ac-ntw:ame` value is unique within all
>   networks and all nodes, this typedef won't work (or rather, it may
>   refer to more than one ac).
> 

[Med] Good catch. We don't have that constraint on the "ac-ntw:name". Please see the candidate changes to fix this at: https://author-tools.ietf.org/api/iddiff?url_1=https://boucadair.github.io/attachment-circuit-model/draft-ietf-opsawg-ntw-attachment-circuit.txt&url_2=https://boucadair.github.io/attachment-circuit-model/boucadair-patch-5/draft-ietf-opsawg-ntw-attachment-circuit.txt. The use of groupings for references is consistent with the approach in RFC 8345.

> 
> o typedef ac-profile-reference {
>     type leafref {
>       path "/nw:networks/nw:network/ac-profile/name";
>     }
> 
>   The nodes should have prefixes:
> 
>       path "/nw:networks/nw:network/ac-ntw:ac-profile/ac-ntw:name";
> 

[Med] Fixed.

> 
> o   leaf site-of-origin {
>       when "../address-family = 'vpn-common:ipv4' "
>          + "or 'vpn-common:dual-stack'" {
> 
>     leaf ipv6-site-of-origin {
>       when "../address-family = 'vpn-common:ipv6' "
>          + "or 'vpn-common:dual-stack'" {
> 
> 
>    Use 'derived-from-or-self' instead of comparison.
> 

[Med] Agree. Fixed.

> 
> o  Some lists have plural-names: routing-profiles, ipv4-lan-prefixes,
>    ipv6-lan-prefixes.  Usually lists should have singular names.
> 
 
[Med] Fixed.

> o typedef encryption-profile-reference {
>     ...
>     description
>       "Defines a type to an encryption profile for referencing
>        purposes.";
>   }
> 
>   Perhaps "Defines a reference to an encryption profile"?
> 
>   (Same for 4 more typedefs)
> 
> 

[Med] Works for me. Thanks.

> 
> /martin
> 

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