Hi Éric, 

Thank you for the review. 

Please see inline.

Cheers,
Med

> -----Message d'origine-----
> De : Éric Vyncke via Datatracker <nore...@ietf.org>
> Envoyé : lundi 13 janvier 2025 17:43
> À : The IESG <i...@ietf.org>
> Cc : draft-ietf-opsawg-teas-common...@ietf.org; opsawg-
> cha...@ietf.org; opsawg@ietf.org; rro...@ciena.com;
> rro...@ciena.com
> Objet : Éric Vyncke's No Objection on draft-ietf-opsawg-teas-
> common-ac-14: (with COMMENT)
> 
> 
> Éric Vyncke has entered the following ballot position for
> draft-ietf-opsawg-teas-common-ac-14: No Objection
> 
> When responding, please keep the subject line intact and reply to
> all email addresses included in the To and CC lines. (Feel free
> to cut this introductory paragraph, however.)
> 
> 
> -----------------------------------------------------------------
> COMMENT:
> -----------------------------------------------------------------
> 
> 
> # Éric Vyncke, INT AD, comments for draft-ietf-opsawg-teas-
> common-ac-14
> CC @evyncke
> 
> Thank you for the work put into this document.
> 
> Please find below some non-blocking COMMENT points (but replies
> would be
> appreciated even if only for my own education), and some nits.
> 
> Special thanks to Reza Rokui for the shepherd's detailed write-up
> including the
> WG consensus *but* the justification of the intended status is
> rather light:
> being "valuable" is not enough to be a "standard".
> 
> I hope that this review helps to improve the document,
> 
> Regards,
> 
> -éric
> 
> ## COMMENTS (non-blocking)
> 
> Thanks for using SVG graphics, it makes reading the HTML
> rendering much easier
> and nicer.
> 
> ### Abstract
> 
> Should 'model' be used rather than 'module' in `The document
> specifies a common
> attachment circuits (ACs) YANG module` in order to be consistent
> with the title
> ? I understand that a model can consist of a single module.
> 

[Med] OK. Updated accordingly.

> ### Section 1
> 
> Should `ancillary node` be better explained as it can mean
> multiple things
> depending on the context?

[Med] Right. Simplified that long sentence to also be consistent with a change 
we made to the ACaaS spec.

> 
> Figure 1, when the text writes `The same CE may terminate
> multiple ACs.` it
> would help if the text mentioned CE3 & CE4 and the associated
> bearer (assuming
> that `b*` is for a bearer).
> 

[Med] Sure. Done.

> ### Section 2
> 
> When defining "bearer", please associate  the previously defined
> terms (CE,
> CPE) with `customer node`.
> 

[Med] s/customer node/CE

> ### Section 3
> 
> Not really about this I-D, but I find the use of `ietf-ac-glue`
> rather than
> 'ietf-ac-bind' a little too trivial especially when the text is
> about `To bind
> Layer 2 VPN` ;-)
> 
> ### Section 4
> 
> Thanks for splitting the whole tree in parts, it helps.
> 
> ### Section 4.1
> 
> It is rather unclear what `server` means in this document, even
> if I can guess
> some controllers in the SP domain.

[Med] Is there any specific occurrence that you think is ambiguous?

> 
> ### Section 4.3
> 
> Should there be a `*l3*-tunnel-type` in addition to `*l2*-tunnel-
> type` ?
> 

[Med] Good catch. The module defines 'l3-tunnel-type' right after 
'l2-tunnel-type' but not listed in the narrative text. Fixed. 

> Should the layer of the next hop be specified in `local-defined-
> next-hop`?
> 

[Med] The layer is determined by where that type is called. See, e.g., 
ipv*-static-rtg-entry. 

> Should there be reference to `UNI`and `NNI`?
> 

[Med] Sure. Added this NEW text: 

"The reader may refer to {{MEF6}}, {{MEF17}}, {{?RFC6004}}, or {{?RFC6215}} for 
examples of discussions regarding the use of UNI and NNI reference points."

With 
* MEF6: https://www.mef.net/Assets/Technical_Specifications/PDF/MEF_6.pdf
* MEF17: https://www.mef.net/wp-content/uploads/2015/04/MEF-17.pdf

> About figure 5 explanations about ipv6-connection, should there
> be a
> possibility for multiple IPv6 addresses per service ?

[Med] We considered it as you see in 
https://github.com/boucadair/attachment-circuit-model/issues/6, but the 
conclusion is that this can be handled at the AC level for specific deployment 
cases (network merging, etc.). Please note that the model supports 
primary/backup addresses for both v4/v6.

 Should
> there be SLAAC for
> dynamic address allocation ?
> 

[Med] It can be if the allocation type is 'provider-dhcp-slaac'. If it is 
'slaac', then no need to have extra parameters under dynamic choice because the 
required info will ne carried local-address/prefix-length per the following in 
the spec:

      Note that if 'address-allocation-type' is set to 'slaac', the
      Prefix Information option of Router Advertisements that will be
      issued for SLAAC purposes will carry the IPv6 prefix that is
      determined by 'local-address' and 'prefix-length'.

> Should there a choice on how to authenticate OSPFv3 either RFC
> 4552 or RFC 7166
> ?
> 

[Med] We used to have ipsec as an option but we removed that part of the code 
it as we wanted to have something **common** for OSPF independent of the 
version. 

FYI, device models implemented out there such as 
https://github.com/openconfig/public/tree/master/release/models/ospf do not 
even cover ospfv3, let alone an internal option. 

> ### Section 5
> 
> In the vxlan grouping, the modules writes `"Specifies the VXLAN
> access mode. By
> default, the peer mode is set to 'static-mode'.";` but I see
> nothing in the
> YANG module to set a default (unsure whether it is possible
> though).
> 

[Med] We are not using the "default" statement on purpose here as this would 
prevent correct use of profiling (ACaaS/ac-nw includes many of such profiles). 
For example, let's assume an AC that reuses the vxlan grouping. Let's also 
assume that we factorize a set of vxlan parameters that we call under a set of 
ACs (and thus avoid repeating the same vxlan info). The default value will 
override the one indicated value in the profile, which is problematic.

We are following this reco from rfc8407 

   *  Do not include a "default" substatement on a leaf or choice unless
      the value applies on all possible contexts.


> ## NITS (non-blocking / cosmetic)
> 
> s/Layer 2 VPN/layer-2 VPN/ ? same for layer 3 of course
> 

[Med] The current wording is consistent with other RFC uses such as in RFC9291. 
Will maintain it.

FWIW, all the changes made so far can be tracked at: 
https://author-tools.ietf.org/api/iddiff?url_1=https://boucadair.github.io/attachment-circuit-model/draft-ietf-opsawg-teas-common-ac.txt&url_2=https://boucadair.github.io/attachment-circuit-model/Eric-IESG-review/draft-ietf-opsawg-teas-common-ac.txt

____________________________________________________________________________________________________________
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
To unsubscribe send an email to opsawg-le...@ietf.org

Reply via email to