Re: [OPSAWG] Yangdoctors early review of draft-ietf-opsawg-teas-attachment-circuit-03
Thx Med - one comment inline... On 2024-01-15 06:59:58, mohamed.boucad...@orange.com 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 > > Envoyé : mercredi 10 janvier 2024 23:58 > > À : yang-doct...@ietf.org > > Cc : draft-ietf-opsawg-teas-attachment-circuit@ietf.org; > > opsawg@ietf.org > > 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: > > - ietf-ac-...@2023-11-13.yang > > - ietf-bearer-...@2023-11-13.yang > > > > 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 > > 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 :-) > > > > > > > > > KC1 > > KC1 Description > > > > 131001 > > > > > > > > > > > > hmac-sha-512 > > > > > > > > > > > > AGP1 > > SPP1 > > SPP2 > > > > > > > xmlns:vpn-common="urn:ietf:params:xml:ns:yang:ietf-vpn- > > common">vpn-common:ethernet-type > > > > > > > > > > AGP2 > > SPP1 > > > > > > 1.1.1.1 > > 2.2.2.2 > > 31 > > > xmlns:ac-common="urn:ietf:params:xml:ns:yang:ietf-ac- > > common">ac-common:static-address > > > > ID1 > > 10.1.1.1 > > > > > > > > 2001:db8:1000::1 > > 2001:db8::: > > 127 > > > xmlns:
Re: [OPSAWG] Yangdoctors early review of draft-ietf-opsawg-teas-common-ac-02
Thx Med for addressing my comments. The updates look good to me.
/ebben
On 2024-01-11 14:30:09, mohamed.boucad...@orange.com wrote:
> [External Email. Be cautious of content]
>
>
> Hi Ebben,
>
> Thank you for the review.
>
> Updated the spec to take into account your comments as you can see at:
> https://urldefense.com/v3/__https://author-tools.ietf.org/api/iddiff?doc_1=draft-ietf-opsawg-teas-common-ac_2=https:**Aboucadair.github.io*attachment-circuit-model*draft-ietf-opsawg-teas-common-ac.txt__;Ly8vLw!!NEt6yMaO-gk!DFSr2_ATQcCtt5PSDds7C4uS-HfuciT-F6O_lx3_dBJOFrX_WcRuzQ9NokZvhNBpfw0WB1Xwbh2xNcK3YEwuwywW$
>
> Please see inline for more context.
>
> Cheers,
> Med
>
> > -Message d'origine-
> > De : Ebben Aries via Datatracker
> > Envoyé : mercredi 10 janvier 2024 23:57
> > À : yang-doct...@ietf.org
> > Cc : draft-ietf-opsawg-teas-common-ac@ietf.org;
> > opsawg@ietf.org
> > Objet : Yangdoctors early review of draft-ietf-opsawg-teas-
> > common-ac-02
> >
> > Reviewer: Ebben Aries
> > Review result: On the Right Track
> >
> > 1 module in this draft:
> > - ietf-ac-com...@2023-11-13.yang
> >
> > YANG compiler errors or warnings (pyang 2.6.0, yanglint 2.1.128,
> > yangson 1.4.19)
> > - No compiler errors or warnings
> >
> > NOTE: This module was reviewed and validated (stub instance-data)
> > in conjunction with draft-ietf-opsawg-teas-attachment-circuit-03
> > and I did my best to separate comments out to each even though
> > validation crosses the 2 reviews
> >
>
> [Med] Thank you.
>
> > General comments on the draft:
> > - Section 3: There is reference to the tree and the tree being
> > too long but
> > this module is solely groupings, identities and typedefs thus
> > there is no
> > implementation of a standalone data tree in this common module.
> > Is this
> > moreso in reference to a tree output that might include
> > `--tree-print-groupings` as seen in a subsequent section?
>
> [Med] Yes because we were assuming that the reader is familiar with
> rfc8340#section-3.2. Added a note to make this explicit.
>
> > - Section 4: Move the "file" declaration in up to
> > align and
> > quote "ietf-ac-com...@2023-11-13.yang" otherwise published IETF
> > tooling will
> > fail to parse correctly
>
> [Med] Fixed.
>
> >
> > General comments on the module:
> > - Suggestion: Insert appropriate line breaks throughout module
> > statements for
> > readability
>
> [Med] I think that we are already following the practices for published
> modules in that matter: no line breaks inside the groupings/containers.
>
> > - L#509: must statement needs to qualify identities as 'ac-
> > common:slaac' and
> > 'ac-common:provider-dhcp-slaac' otherwise the imports/uses will
> > assume
> > localization (Suggest auditing all 'when' and 'must' statements
> > that
> > reference identities to ensure full qualification for any
> > future imports)
>
> [Med] Fixed.
>
> > - L#1329: Address/remove comment
>
> [Med] Fixed.
>
> > - L#1404: s/type\{/type \{/
>
> [Med] Fixed.
>
> > - Minor nit: s/Indciates/Indicates/ -> "Indciates the actual date
> > and time
> > when the service"
>
> [Med] Fixed.
>
> > - For "vlan-id" related leaves, should these be scoped in the
> > uint16 space?
>
> [Med] Yes, they should. Thanks for catching this. Fixed.
>
> > - For the `pseudowire` and `vpls` groupings, there is a
> > duplication of nodes.
> > Does it make sense to consolidate as much as possible and only
> > diverge where
> > necessary?
> >
>
> [Med] I don't think there is value to do that here as we only have one common
> leaf. Thanks.
>
>
> 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
[OPSAWG] 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: - ietf-ac-...@2023-11-13.yang - ietf-bearer-...@2023-11-13.yang 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 up to align and quote the filename otherwise published IETF tooling will fail to parse correctly 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. - 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. 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): KC1 KC1 Description 131001 hmac-sha-512 AGP1 SPP1 SPP2 vpn-common:ethernet-type AGP2 SPP1 1.1.1.1 2.2.2.2 31 ac-common:static-address ID1 10.1.1.1 2001:db8:1000::1 2001:db8::: 127 ac-common:static-address ID1 2001:db8:dead::beef RP1 vpn-common:bgp-routing EPI5 vpn-common:import-export PG1 65000 65001 vpn-common:ipv4 10.1.1.1 true true KC1 N1 10.2.2.2 10.1.1.1 PG1 vpn-common:admin-up 2023-12-30T15:02:11.353Z vpn-common:op-up 2023-12-30T15:02:11.353Z EPI3 180 vpn-common:admin-up 2023-12-30T15:02:11.353Z vpn-common:op-up 2023-12-30T15:02:11.353Z true layer3 KC1 vpn-common:bw-per-service 1 1 1 1 1 1 vpn-common:pop-diverse GID1 AC1 CUSTOMER1 Attachment Circuit #1 2023-12-30T14:52:51.353Z 2025-12-30T00:00:00.000Z 2023-12-30T15:02:10.003Z PSID1 AGP2 AC2 GID1 ac-common:primary vpn-common:l3vpn SID1 SPP1 2001:db8::1 2001:db8::2 128 ac-common:static-address EPI2 vpn-common:both EPI4 AC2 EPI1 EPI2 EPI3 EPI4 EPI5 SPP1 SPP2 network-termination-hint G1 vpn-common:pop-diverse vpn-common:pe-diverse B1 Description for B1 G1 Op comment site-and-device-id devid1 SJC01 555 Anystreet 95123 CA San Jose US ethernet AC1 2023-12-30T14:52:51.353Z 2025-12-30T00:00:00.000Z 2023-12-30T15:02:10.003Z vpn-common:admin-up 2023-12-30T15:02:11.353Z vpn-common:op-up 2023-12-30T15:02:11.353Z ___ OPSAWG mailing list OPSAWG@ietf.org https://www.ietf.org/mailman/listinfo/opsawg
[OPSAWG] Yangdoctors early review of draft-ietf-opsawg-teas-common-ac-02
Reviewer: Ebben Aries
Review result: On the Right Track
1 module in this draft:
- ietf-ac-com...@2023-11-13.yang
YANG compiler errors or warnings (pyang 2.6.0, yanglint 2.1.128, yangson 1.4.19)
- No compiler errors or warnings
NOTE: This module was reviewed and validated (stub instance-data) in
conjunction with draft-ietf-opsawg-teas-attachment-circuit-03 and I did my
best to separate comments out to each even though validation crosses the 2
reviews
General comments on the draft:
- Section 3: There is reference to the tree and the tree being too long but
this module is solely groupings, identities and typedefs thus there is no
implementation of a standalone data tree in this common module. Is this
moreso in reference to a tree output that might include
`--tree-print-groupings` as seen in a subsequent section?
- Section 4: Move the "file" declaration in up to align and
quote "ietf-ac-com...@2023-11-13.yang" otherwise published IETF tooling will
fail to parse correctly
General comments on the module:
- Suggestion: Insert appropriate line breaks throughout module statements for
readability
- L#509: must statement needs to qualify identities as 'ac-common:slaac' and
'ac-common:provider-dhcp-slaac' otherwise the imports/uses will assume
localization (Suggest auditing all 'when' and 'must' statements that
reference identities to ensure full qualification for any future imports)
- L#1329: Address/remove comment
- L#1404: s/type\{/type \{/
- Minor nit: s/Indciates/Indicates/ -> "Indciates the actual date and time
when the service"
- For "vlan-id" related leaves, should these be scoped in the uint16 space?
- For the `pseudowire` and `vpls` groupings, there is a duplication of nodes.
Does it make sense to consolidate as much as possible and only diverge where
necessary?
___
OPSAWG mailing list
OPSAWG@ietf.org
https://www.ietf.org/mailman/listinfo/opsawg
[OPSAWG] Yangdoctors early review of draft-ietf-opsawg-sbom-access-02
Reviewer: Ebben Aries Review result: Almost Ready Apologies for not turning this around sooner. Structure wise, the model is fairly sound. Most of the comments below are either nits/wording, slight adjustments and questions/clarifications. 1 module in this draft: - ietf-mud-transpare...@2021-07-06.yang No YANG compiler errors or warnings (pyang 2.5.0, yanglint 2.0.88, confdc 7.2.3.4) - L#364: CODE BEGINS : filename must be defined on same line for tools such as rfcstrip to correctly parse the module contents Module ietf-mud-transpare...@2021-07-06.yang: - import `ietf-inet-types` should reference RFC 6991 per https://tools.ietf.org/html/draft-ietf-netmod-rfc6087bis-15#section-4.7 - import `ietf-mud` should reference RFC 8520 per https://tools.ietf.org/html/draft-ietf-netmod-rfc6087bis-15#section-4.7 - L#016 - Minor nit: looks like L#17 should be moved up here - L#018-020 - Minor nit: adjust email address formatting per https://tools.ietf.org/html/draft-ietf-netmod-rfc6087bis-15#appendix-C - The type and enum members are identically defined for `sbom-local-well-known` and `vuln-local-well-known`. Is this something you can leverage by using a typedef or a grouping or is there intention to keep these separately defined? - When retrieving say an 'sbom' from the device, is it assumed that it be via `sbom-local-well-known`? What if it is necessary to host this on an alternate port for one of the communication protocols chosen? Would this scenario then best use `sbom-url` to define a static URI? (Same question applies to vuln as well) - Independent of the answer to the above question, is `cloud` the best choice or wording for the other case statement under the retrieval method choices? It seems to be that we have 3 cases for sbom/vuln retrieval methods which correspond to the draft wording at L#176-180 which seems to not pair identically. * on devices themselves: Could be /.well-known/ or a static URI could it not? * on a website: Static URI only * out-of-band: Static URI only - should this leaf be named something closer to that vs. 'contact'? General comments on the draft/modules: - L#0021: s/provide access this/provide access to this/ - L#0117: s/bills of material/bill of materials/ - L#0627: JSON example needs correct prefix for the augment `ietf-mud-transparency:transparency` - L#0941: s/not be/not been/ - L#0961: s/authoration/authorization/ - Since `ietf-mud-transparency` imports `ietf-inet-types`, a normative reference must be added per https://tools.ietf.org/html/draft-ietf-netmod-rfc6087bis-15#section-3.9 ___ OPSAWG mailing list OPSAWG@ietf.org https://www.ietf.org/mailman/listinfo/opsawg
Re: [OPSAWG] New Version Notification for draft-zheng-opsawg-tacacs-yang-02.txt
A few quick observations on the model - The model defines the client configuration and state parameters only but to be functional for operator use w/ AAA needs a few other things, otherwise this by itself is incomplete - There should likely be an identity of 'tacacsplus' that is base off ietf-system:authentication-method - The 'user-authentication-order' must restrictions in ietf-system would need to be accounted for as is done for radius - Is there intention to add an equivalent 'tacacsplus-authentication' feature much like there is for radius? Thx /ebben On Jun 20 13:04 PM, Wubo (lana) wrote: > Dear WG, > > We update the 02 version of draft-zheng-opsawg-tacacs-yang-02 to address the > comments from 104 meeting. > https://tools.ietf.org/html/draft-zheng-opsawg-tacacs-yang-02 > > Here are some major changes in this version: > - This draft is focused on TACACS+ Client only YANG. > - Change the module name to ietf-system-tacacsplus. > - Group the all the rw objects together by changing timeout to server > specific. > - Change "network-instance" to "vrf-instance" to make it specific and add > text to describe it. > - Add "source-interface" as a choice to accommodate one more implementation. > > Please help to review the document, comments and suggestions are welcome! > > Thanks, > Bo > > > -邮件原件- > 发件人: internet-dra...@ietf.org [mailto:internet-dra...@ietf.org] > 发送时间: 2019年6月20日 20:38 > 收件人: wangzitao ; Wubo (lana) ; > Zhengguangying (Walker) ; Wubo (lana) > ; wangzitao > 主题: New Version Notification for draft-zheng-opsawg-tacacs-yang-02.txt > > > A new version of I-D, draft-zheng-opsawg-tacacs-yang-02.txt > has been successfully submitted by Bo Wu and posted to the IETF repository. > > Name: draft-zheng-opsawg-tacacs-yang > Revision: 02 > Title:Yang data model for TACACS+ > Document date:2019-06-20 > Group:Individual Submission > Pages:14 > URL: > https://www.ietf.org/internet-drafts/draft-zheng-opsawg-tacacs-yang-02.txt > Status: > https://datatracker.ietf.org/doc/draft-zheng-opsawg-tacacs-yang/ > Htmlized: https://tools.ietf.org/html/draft-zheng-opsawg-tacacs-yang-02 > Htmlized: > https://datatracker.ietf.org/doc/html/draft-zheng-opsawg-tacacs-yang > Diff: > https://www.ietf.org/rfcdiff?url2=draft-zheng-opsawg-tacacs-yang-02 > > Abstract: >This document defines a YANG modules that augment the System data >model defined in the RFC 7317 with TACACS+ client model. The data >model of Terminal Access Controller Access Control System Plus >(TACACS+) client allows the configuration of TACACS+ servers for >centralized Authentication, Authorization and Accounting. > >The YANG modules in this document conforms to the Network Management >Datastore Architecture (NMDA) defined in RFC 8342. > > > > > > Please note that it may take a couple of minutes from the time of submission > until the htmlized version and diff are available at tools.ietf.org. > > The IETF Secretariat > > ___ > OPSAWG mailing list > OPSAWG@ietf.org > https://www.ietf.org/mailman/listinfo/opsawg ___ OPSAWG mailing list OPSAWG@ietf.org https://www.ietf.org/mailman/listinfo/opsawg
