[yang-doctors] Yangdoctors last call review of draft-ietf-opsawg-teas-attachment-circuit-14

[Ebben Aries via Datatracker <[e-mail]>]

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.6.0, yanglint 3.1.0)
- No compiler errors or warnings for tree outputs/instance-data validation

Summary:

This is a last-call review to a previous early review of these related modules
from 2024-01-10.  For the most part the modules are in good shape but there
have been some changes needing clarification (below) between the -03 and -14
versions of the draft.

These modules were reviewed and validated (stub instance-data) in conjunction
with draft-ietf-opsawg-teas-common-ac-12

General comments on the modules:
- ietf-bearer-svc: There is a top-level container added named `locations`.
  Should this not be a keyed list rather?  In addition, no need to repeat the
  prefix `location-` in `location-name` as we already know this is a list of
  locations.  `name` is sufficient here.
- ietf-bearer-svc: Is the `type` for the leaf-list `bearer-lag-member` correct
  in referencing another bearer is all? (Same as bearer-parent-ref)
- Nit: Feel free to update the revision dates with each publish
- Nit: ietf-ac-svc: Why not remain consistent and name
  `s/child-ac-ref/ac-child-ref/` to align with the other `ac-hierarchy`
  leaf-list of `ac-parent-ref`?
- Nit: ietf-ac-svc: container `bgp-max-prefix` is already nested under BGP.
  No reason to prefix as such so if you want to encapsulate in a container for
  future growth, I would suggest something like `./prefix-limit/max-prefixes`.
  In addition, is this fine to be this generic and not per AFI/SAFI?
- Nit: ietf-bearer-svc: For the addition of the 2 new identities
  (syncPHY-type, syncE), is there any reason to diverge casing?
- Nit: ietf-bearer-svc L#154 `s/Reusabel/reusable/`

Example Validated Instance Data:

Features enabled:
*
ietf-vpn-common:ipv4,ipv6,rtg-bgp,rtg-vrrp,rtg-rip,rtg-isis,rtg-ospf,bfd,encryption,inbound-bw,outbound-bw,qos,placement-diversity
* ietf-ac-common:layer2-ac,layer3-ac,server-assigned-reference

<key-chains xmlns="urn:ietf:params:xml:ns:yang:ietf-key-chain">
  <key-chain>
    <name>KC1</name>
    <description>KC1 Description</description>
    <key>
      <key-id>131001</key-id>
      <lifetime>
        <send-accept-lifetime>
          <always/>
        </send-accept-lifetime>
      </lifetime>
      <crypto-algorithm>hmac-sha-512</crypto-algorithm>
    </key>
  </key-chain>
</key-chains>
<attachment-circuits xmlns="urn:ietf:params:xml:ns:yang:ietf-ac-svc">
  <ac-group-profile>
    <name>AGP1</name>
    <service-profile>SPP1</service-profile>
    <service-profile>SPP2</service-profile>
    <l2-connection>
      <encapsulation>
        <type
        xmlns:vpn-common="urn:ietf:params:xml:ns:yang:ietf-vpn-common">vpn-common:ethernet-type</type>
      </encapsulation>
    </l2-connection>
  </ac-group-profile>
  <ac-group-profile>
    <name>AGP2</name>
    <service-profile>SPP1</service-profile>
    <ip-connection>
      <ipv4>
        <local-address>1.1.1.1</local-address>
        <virtual-address>2.2.2.2</virtual-address>
        <prefix-length>31</prefix-length>
        <address-allocation-type
        xmlns:ac-common="urn:ietf:params:xml:ns:yang:ietf-ac-common">ac-common:static-address</address-allocation-type>
        <address>
          <address-id>ID1</address-id>
          <customer-address>10.1.1.1</customer-address>
        </address>
      </ipv4>
      <ipv6>
        <local-address>2001:db8:1000::1</local-address>
        <virtual-address>2001:db8:ffff::ffff</virtual-address>
        <prefix-length>127</prefix-length>
        <address-allocation-type
        xmlns:ac-common="urn:ietf:params:xml:ns:yang:ietf-ac-common">ac-common:static-address</address-allocation-type>
        <address>
          <address-id>ID1</address-id>
          <customer-address>2001:db8:dead::beef</customer-address>
        </address>
      </ipv6>
    </ip-connection>
    <routing-protocols>
      <routing-protocol>
        <id>RP1</id>
        <type
        xmlns:vpn-common="urn:ietf:params:xml:ns:yang:ietf-vpn-common">vpn-common:bgp-routing</type>
        <routing-profiles>
          <id>EPI5</id>
          <type
          xmlns:vpn-common="urn:ietf:params:xml:ns:yang:ietf-vpn-common">vpn-common:import-export</type>
        </routing-profiles>
        <bgp>
          <peer-groups>
            <peer-group>
              <name>PG1</name>
              <local-as>65000</local-as>
              <peer-as>65001</peer-as>
              <address-family
              xmlns:vpn-common="urn:ietf:params:xml:ns:yang:ietf-vpn-common">vpn-common:ipv4</address-family>
              <local-address>10.1.1.1</local-address> <authentication>
                <enabled>true</enabled>
                <keying-material>
                  <enable-ao>true</enable-ao>
                  <ao-keychain>KC1</ao-keychain>
                </keying-material>
              </authentication>
            </peer-group>
          </peer-groups>
          <neighbor>
            <id>N1</id>
            <server-reference>SR1</server-reference>
            <remote-address>10.2.2.2</remote-address>
            <local-address>10.1.1.1</local-address>
            <peer-group>PG1</peer-group>
            <status>
              <admin-status>
                <status
                xmlns:vpn-common="urn:ietf:params:xml:ns:yang:ietf-vpn-common">vpn-common:admin-up</status>
                <last-change>2023-12-30T15:02:11.353Z</last-change>
              </admin-status>
              <oper-status>
                <status
                xmlns:vpn-common="urn:ietf:params:xml:ns:yang:ietf-vpn-common">vpn-common:op-up</status>
                <last-change>2023-12-30T15:02:11.353Z</last-change>
              </oper-status>
            </status>
          </neighbor>
        </bgp>
      </routing-protocol>
      <routing-protocol>
        <id>RP2</id>
        <type
        xmlns:vpn-common="urn:ietf:params:xml:ns:yang:ietf-vpn-common">vpn-common:vrrp-routing</type>
        <vrrp>
          <address-family
          xmlns:vpn-common="urn:ietf:params:xml:ns:yang:ietf-vpn-common">vpn-common:ipv4</address-family>
        </vrrp>
      </routing-protocol>
      <routing-protocol>
        <id>RP3</id>
        <type
        xmlns:vpn-common="urn:ietf:params:xml:ns:yang:ietf-vpn-common">vpn-common:rip-routing</type>
        <rip>
          <address-family
          xmlns:vpn-common="urn:ietf:params:xml:ns:yang:ietf-vpn-common">vpn-common:ipv4</address-family>
        </rip>
      </routing-protocol>
      <routing-protocol>
        <id>RP4</id>
        <type
        xmlns:vpn-common="urn:ietf:params:xml:ns:yang:ietf-vpn-common">vpn-common:isis-routing</type>
        <isis>
          <address-family
          xmlns:vpn-common="urn:ietf:params:xml:ns:yang:ietf-vpn-common">vpn-common:ipv4</address-family>
          <area-address>49.0000.0001.0000</area-address>
        </isis>
      </routing-protocol>
      <routing-protocol>
        <id>RP5</id>
        <type
        xmlns:vpn-common="urn:ietf:params:xml:ns:yang:ietf-vpn-common">vpn-common:ospf-routing</type>
        <ospf>
          <address-family
          xmlns:vpn-common="urn:ietf:params:xml:ns:yang:ietf-vpn-common">vpn-common:ipv4</address-family>
          <area-id>0.0.0.0</area-id> <metric>100</metric>
        </ospf>
      </routing-protocol>
      <routing-protocol>
        <id>RP6</id>
        <type
        xmlns:vpn-common="urn:ietf:params:xml:ns:yang:ietf-vpn-common">vpn-common:static-routing</type>
        <static>
          <cascaded-lan-prefixes>
            <ipv6-lan-prefix>
              <lan>2001:db8:1234:ffff::/64</lan>
              <lan-tag>LT1</lan-tag>
              <next-hop
              xmlns:ac-common="urn:ietf:params:xml:ns:yang:ietf-ac-common">ac-common:local-link</next-hop>
            </ipv6-lan-prefix>
          </cascaded-lan-prefixes>
        </static>
      </routing-protocol>
    </routing-protocols>
    <oam>
      <bfd>
        <session>
          <id>SID1</id>
          <local-address>10.1.1.1</local-address>
          <remote-address>10.2.1.1</remote-address>
          <profile>EPI3</profile>
          <holdtime>180</holdtime>
          <status>
            <admin-status>
              <status
              xmlns:vpn-common="urn:ietf:params:xml:ns:yang:ietf-vpn-common">vpn-common:admin-up</status>
              <last-change>2023-12-30T15:02:11.353Z</last-change>
            </admin-status>
            <oper-status>
              <status
              xmlns:vpn-common="urn:ietf:params:xml:ns:yang:ietf-vpn-common">vpn-common:op-up</status>
              <last-change>2023-12-30T15:02:11.353Z</last-change>
            </oper-status>
          </status>
        </session>
      </bfd>
    </oam>
    <security>
      <encryption>
        <enabled>true</enabled>
        <layer>layer3</layer>
      </encryption>
      <encryption-profile>
        <customer-key-chain>KC1</customer-key-chain>
      </encryption-profile>
    </security>
    <service>
      <mtu>9000</mtu>
      <svc-pe-to-ce-bandwidth>
        <bandwidth>
          <bw-type
          xmlns:vpn-common="urn:ietf:params:xml:ns:yang:ietf-vpn-common">vpn-common:bw-per-service</bw-type>
          <cir>10000</cir> <cbs>10000</cbs> <eir>10000</eir> <ebs>10000</ebs>
          <pir>10000</pir> <pbs>10000</pbs>
        </bandwidth>
      </svc-pe-to-ce-bandwidth>
    </service>
  </ac-group-profile>
  <placement-constraints>
    <constraint>
      <constraint-type
      xmlns:vpn-common="urn:ietf:params:xml:ns:yang:ietf-vpn-common">vpn-common:pop-diverse</constraint-type>
      <target>
        <group>
          <group-id>GID1</group-id>
        </group>
      </target>
    </constraint>
  </placement-constraints>
  <ac>
    <name>AC1</name>
    <customer-name>CUSTOMER1</customer-name>
    <description>Attachment Circuit #1</description>
    <requested-start>2023-12-30T14:52:51.353Z</requested-start>
    <requested-stop>2025-12-30T00:00:00.000Z</requested-stop>
    <actual-start>2023-12-30T15:02:10.003Z</actual-start>
    <role
    xmlns:ac-common="urn:ietf:params:xml:ns:yang:ietf-ac-common">ac-common:nni</role>
    <peer-sap-id>PSID1</peer-sap-id>
    <ac-group-profile-ref>AGP2</ac-group-profile-ref>
    <ac-parent-ref>AC2</ac-parent-ref> <child-ac-ref>AC3</child-ac-ref> <group>
      <group-id>GID1</group-id>
      <precedence
      xmlns:ac-common="urn:ietf:params:xml:ns:yang:ietf-ac-common">ac-common:primary</precedence>
    </group>
    <service-ref>
      <service-type
      xmlns:vpn-common="urn:ietf:params:xml:ns:yang:ietf-vpn-common">vpn-common:l3vpn</service-type>
      <service-id>SID1</service-id>
    </service-ref>
    <server-reference>SAR1</server-reference>
    <service-profile>SPP1</service-profile>
    <ip-connection>
      <ipv6>
        <local-address>2001:db8::1</local-address>
        <virtual-address>2001:db8::2</virtual-address>
        <prefix-length>128</prefix-length>
        <address-allocation-type
        xmlns:ac-common="urn:ietf:params:xml:ns:yang:ietf-ac-common">ac-common:static-address</address-allocation-type>
      </ipv6>
    </ip-connection>
    <service>
      <qos>
        <qos-profiles>
          <qos-profile>
            <profile>EPI2</profile>
            <direction
            xmlns:vpn-common="urn:ietf:params:xml:ns:yang:ietf-vpn-common">vpn-common:both</direction>
          </qos-profile>
        </qos-profiles>
      </qos>
      <access-control-list>
        <acl-profiles>
          <acl-profile>
            <profile>EPI4</profile>
          </acl-profile>
        </acl-profiles>
      </access-control-list>
    </service>
  </ac>
  <ac>
    <name>AC2</name>
  </ac>
  <ac>
    <name>AC3</name>
  </ac>
</attachment-circuits>
<specific-provisioning-profiles xmlns="urn:ietf:params:xml:ns:yang:ietf-ac-svc">
  <valid-provider-identifiers>
    <encryption-profile-identifier>
      <id>EPI1</id>
    </encryption-profile-identifier>
    <qos-profile-identifier>
      <id>EPI2</id>
    </qos-profile-identifier>
    <failure-detection-profile-identifier>
      <id>EPI3</id>
    </failure-detection-profile-identifier>
    <forwarding-profile-identifier>
      <id>EPI4</id>
    </forwarding-profile-identifier>
    <routing-profile-identifier>
      <id>EPI5</id>
    </routing-profile-identifier>
  </valid-provider-identifiers>
</specific-provisioning-profiles>
<service-provisioning-profiles xmlns="urn:ietf:params:xml:ns:yang:ietf-ac-svc">
  <service-profile-identifier>
    <id>SPP1</id>
  </service-profile-identifier>
  <service-profile-identifier>
    <id>SPP2</id>
  </service-profile-identifier>
</service-provisioning-profiles>
<locations xmlns="urn:ietf:params:xml:ns:yang:ietf-bearer-svc">
  <customer-name>CN1</customer-name>
  <role
  xmlns:ac-common="urn:ietf:params:xml:ns:yang:ietf-ac-common">ac-common:nni</role>
  <local-as>65000</local-as> <peer-as>1000</peer-as> <location>
    <location-name>LOC1</location-name>
  </location>
</locations>
<bearers xmlns="urn:ietf:params:xml:ns:yang:ietf-bearer-svc">
  <placement-constraints>
    <constraint>
      <constraint-type>network-termination-hint</constraint-type>
      <target>
        <group>
          <group-id>G1</group-id>
        </group>
      </target>
    </constraint>
    <constraint>
      <constraint-type
      xmlns:vpn-common="urn:ietf:params:xml:ns:yang:ietf-vpn-common">vpn-common:pop-diverse</constraint-type>
      <target>
        <all-other-bearers/>
      </target>
    </constraint>
    <constraint>
      <constraint-type
      xmlns:vpn-common="urn:ietf:params:xml:ns:yang:ietf-vpn-common">vpn-common:pe-diverse</constraint-type>
      <target>
        <all-other-groups/>
      </target>
    </constraint>
  </placement-constraints>
  <bearer>
    <name>B1</name>
    <description>Description for B1</description>
    <groups>
      <group>
        <group-id>G1</group-id>
      </group>
    </groups>
    <op-comment>Op comment</op-comment>
    <bearer-parent-ref>B2</bearer-parent-ref>
    <bearer-lag-member>B2</bearer-lag-member>
    <sync-phy-capable>true</sync-phy-capable>
    <sync-phy-enabled>true</sync-phy-enabled>
    <sync-phy-type>syncE</sync-phy-type>
    <provider-location-reference>LR1</provider-location-reference>
    <customer-point>
      <identified-by>site-and-device-id</identified-by>
      <device>
        <device-id>devid1</device-id>
        <location>
          <location-name>SJC01</location-name>
          <address>555 Anystreet</address>
          <postal-code>95123</postal-code>
          <state>CA</state>
          <city>San Jose</city>
          <country-code>US</country-code>
        </location>
      </device>
    </customer-point>
    <type>ethernet</type>
    <ac-svc-ref>AC1</ac-svc-ref>
    <requested-start>2023-12-30T14:52:51.353Z</requested-start>
    <requested-stop>2025-12-30T00:00:00.000Z</requested-stop>
    <actual-start>2023-12-30T15:02:10.003Z</actual-start>
    <status>
      <admin-status>
        <status
        xmlns:vpn-common="urn:ietf:params:xml:ns:yang:ietf-vpn-common">vpn-common:admin-up</status>
        <last-change>2023-12-30T15:02:11.353Z</last-change>
      </admin-status>
      <oper-status>
        <status
        xmlns:vpn-common="urn:ietf:params:xml:ns:yang:ietf-vpn-common">vpn-common:op-up</status>
        <last-change>2023-12-30T15:02:11.353Z</last-change>
      </oper-status>
    </status>
  </bearer>
  <bearer>
    <name>B2</name>
  </bearer>
</bearers>