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

[[e-mail]]

Hi Ebben, 

Thank you for the review. 

A new version that takes into account the review can be seen at: https://author-tools.ietf.org/iddiff?url2=draft-ietf-opsawg-teas-attachment-circuit-04

Please see inline for more context. 

Cheers,
Med

> -----Message d'origine-----
> De : Ebben Aries via Datatracker <[e-mail]>
> Envoyé : mercredi 10 janvier 2024 23:58
> À : [e-mail]
> Cc : [e-mail];
> [e-mail]
> Objet : Yangdoctors early review of draft-ietf-opsawg-teas-
> attachment-circuit-03
> 
> 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 2.1.128,
> yangson 1.4.19)
> - No compiler errors or warnings for tree outputs
> 
> NOTE: These modules were reviewed and validated (stub instance-
> data) in conjunction with draft-ietf-opsawg-teas-common-ac-02 and
> I did my best to separate comments out to each even though
> validation crosses the 2 reviews
> 
> General comments on the draft:
> - Section 5.1/5.2: Move the "file" declaration in <CODE BEGINS>
> up to align
>   and quote the filename otherwise published IETF tooling will
> fail to parse
>   correctly

[Med] Fixed.

> 
> General comments on the modules:
> - Similar comment to that in the `ietf-ac-common` review in that
> if there is
>   intention for other modules to import and use then ensure any
> must/when
>   statements are fully qualified.  L#272-273 in `ietf-bearer-svc`
> are one such
>   example.

[Med] ACK.

> - For `status/admin-status/last-change`, this leaf is `r/w` and
> while I
>   realize this is reuse from `ietf-vpn-common`, it seems that
> this is
>   incorrect and should be reflected as pure `r/o` state.

[Med] Actually, no. Unlike the operational status, the client can control that for administrative status as well. Think about scheduled operations for example. 

  A
> client is not
>   going to "write" this value to a server however this is an
> inheritance/reuse
>   issue if you agree
> 
> Example Validated Instance Data (post qualification fixes):

[Med] Thank you. I put this example at https://github.com/boucadair/attachment-circuit-model/blob/main/xml-examples/svc-full-instance.xml and cited it in the draft with an acknowledgement :-) 

> 
> <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>
>                 <enable>true</enable>
>                 <keying-material>
>                   <enable-ao>true</enable-ao>
>                   <ao-keychain>KC1</ao-keychain>
>                 </keying-material>
>               </authentication>
>             </peer-group>
>           </peer-groups>
>           <neighbor>
>             <id>N1</id>
>             <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-protocols>
>     <oam>
>       <bfd>
>         <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>
>       </bfd>
>     </oam>
>     <security>
>       <encryption>
>         <enabled>true</enabled>
>         <layer>layer3</layer>
>       </encryption>
>       <encryption-profile>
>         <customer-key-chain>KC1</customer-key-chain>
>       </encryption-profile>
>     </security>
>     <service>
>       <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>
>     <peer-sap-id>PSID1</peer-sap-id>
>     <ac-group-profile>AGP2</ac-group-profile>
>     <ac-parent-ref>AC2</ac-parent-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>
>     <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>
> </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>
>     <bfd-profile-identifier>
>       <id>EPI3</id>
>     </bfd-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>
> <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>
>     <id>B1</id>
>     <description>Description for B1</description>
>     <groups>
>       <group>
>         <group-id>G1</group-id>
>       </group>
>     </groups>
>     <op-comment>Op comment</op-comment>
>     <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>
>     <requested-type>ethernet</requested-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>
> </bearers>
> 
> 

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