Hi Martin,

Thanks for the review. 

Please see inline.

Cheers,
Med

> -----Message d'origine-----
> De : Martin Björklund via Datatracker <nore...@ietf.org>
> Envoyé : mercredi 24 janvier 2024 16:20
> À : yang-doct...@ietf.org
> Cc : draft-ietf-opsawg-ntw-attachment-circuit....@ietf.org;
> opsawg@ietf.org
> 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.
_______________________________________________
OPSAWG mailing list
OPSAWG@ietf.org
https://www.ietf.org/mailman/listinfo/opsawg

Reply via email to