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

[[e-mail]]

Hi Ebben, 

Thank you for the review.

The changes to take into account your comments can be tracked here: https://author-tools.ietf.org/api/iddiff?url_1=https://boucadair.github.io/attachment-circuit-model/draft-ietf-opsawg-teas-attachment-circuit.txt&url_2=https://boucadair.github.io/attachment-circuit-model/yangdoctor-lc-review/draft-ietf-opsawg-teas-attachment-circuit.txt.

Please see inline. 

Cheers,
Med

> -----Message d'origine-----
> De : Ebben Aries via Datatracker <[e-mail]>
> Envoyé : mardi 6 août 2024 01:53
> À : [e-mail]
> Cc : [e-mail];
> [e-mail]; [e-mail]
> Objet : Yangdoctors last call review of draft-ietf-opsawg-teas-
> attachment-circuit-14
> 
> 
> 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
> 

[Med] Thanks, Ebben.


> 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?

[Med] No, that's not needed for the intended usage:

   In some deployments, a customer may first retrieve a list of
   available presence locations before actually placing an order for a
   bearer creation.  The request may be filtered based upon a customer
   name, role of the bearer, etc.  The retrieved location name may be
   then referenced in the bearer creation request ("provider-location-
   reference").

Please refer also to the example in https://datatracker.ietf.org/doc/html/draft-ietf-opsawg-teas-attachment-circuit-14#name-retrieve-interconnection-lo.

Added a pointer to this example.

  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.

[Med] I lost the context why we prefixed it, but I think the proposed change is OK.

> - 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)

[Med] I think so.

> - Nit: Feel free to update the revision dates with each publish

[Med] Done.

> - 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`?

[Med] Works for me. Done.

> - 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?

[Med] The name is kept as such on purpose to ease mapping with existing models such as L3NM. We do have a note in the spec to highlight this:

   In order to ease the mapping between the service model and underlying
   network models (e.g., the L3VPN Network Model (L3NM), SAP), the name
   conventions used in existing network data models are reused as much
   as possible.  For example, "local-address" is used rather than
   "provider-address" (or similar) to refer to an IP address used in the
   provider network.  This approach is consistent with the automation
   framework defined in [RFC8969].

The structure is also genetic and not per AFI. If further control is needed at the service using the AC. See the provision in the L3NM, for example.

> - 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/`
> 

[Med] Fixed both

> Example Validated Instance Data:

[Med] Thanks for sharing this. As you may notice, we have acked this effort in the draft.

   Thanks to Ebben Aries for the YANG Doctors review and for providing
   [Instance-Data].
____________________________________________________________________________________________________________
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.