Early Review of draft-ietf-opsawg-ntw-attachment-circuit-04
review-ietf-opsawg-ntw-attachment-circuit-04-yangdoctors-early-bjorklund-2024-01-24-00

Request Review of draft-ietf-opsawg-ntw-attachment-circuit
Requested revision No specific revision (document currently at 09)
Type Early Review
Team YANG Doctors (yangdoctors)
Deadline 2024-01-12
Requested 2023-12-14
Requested by Tianran Zhou
Authors Mohamed Boucadair , Richard Roberts , Oscar Gonzalez de Dios , Samier Barguil , Bo Wu
I-D last updated 2024-01-24
Completed reviews Rtgdir Last Call review of -09 by Gyan Mishra
Rtgdir Early review of -05 by Gyan Mishra (diff)
Yangdoctors Early review of -04 by Martin Björklund (diff)
Comments 
There is a group of related documents, including:
draft-ietf-opsawg-teas-common-ac
draft-ietf-opsawg-teas-attachment-circuit
draft-ietf-opsawg-ntw-attachment-circuit
draft-ietf-opsawg-ac-lxsm-lxnm-glue
Assignment Reviewer Martin Björklund
State Completed
Request Early review on draft-ietf-opsawg-ntw-attachment-circuit by YANG Doctors Assigned
Posted at https://mailarchive.ietf.org/arch/msg/yang-doctors/XkZZ7z-hHlDrILX7SN6IRLqhG08
Reviewed revision 04 (document currently at 09)
Result Ready w/issues
Completed 2024-01-24
review-ietf-opsawg-ntw-attachment-circuit-04-yangdoctors-early-bjorklund-2024-01-24-00
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).


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";


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.


o  Some lists have plural-names: routing-profiles, ipv4-lan-prefixes,
   ipv6-lan-prefixes.  Usually lists should have singular names.


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)



/martin