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

[Ebben Aries <[e-mail]>]

Thx Med - one comment inline...

On 2024-01-15 06:59:58, [e-mail] wrote:
> [External Email. Be cautious of content]
> 
> 
> Hi Ebben,
> 
> Thank you for the review.
> 
> A new version that takes into account the review can be seen at: https://urldefense.com/v3/__https://author-tools.ietf.org/iddiff?url2=draft-ietf-opsawg-teas-attachment-circuit-04__;!!NEt6yMaO-gk!HXVDToOssxIY9VMDmKqkKUPCGGv49M5Ey707NIlh9DgCqdza_Lc--Ls96qov_kNmayL2_P2Jx6ScROMTM-o0w6Hh$
> 
> 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.

I'm conflicted why this would reside in modeling for the use-case you
describe as this would entail a pattern that would need to be considered
across _all_ modeling.  I'm not sure I've seen this pattern arise
before.

RFC7758 is one such example of how scheduled operations would be handled
at the protocol messaging layer and not scattered among the
data-content.

`last-change` also seems like a misnomer here for the administrative
client induced use-case which I don't see mention of intent within
RFC9181.

>   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://urldefense.com/v3/__https://github.com/boucadair/attachment-circuit-model/blob/main/xml-examples/svc-full-instance.xml__;!!NEt6yMaO-gk!HXVDToOssxIY9VMDmKqkKUPCGGv49M5Ey707NIlh9DgCqdza_Lc--Ls96qov_kNmayL2_P2Jx6ScROMTM7M963ko$  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.