Re: [Last-Call] Rtgdir telechat review of draft-ietf-opsawg-l2nm-16

[Yingzhen Qu <[e-mail]>]

Hi Med,

Thanks for working on my comments. Just one question inline below.

Thanks,
Yingzhen

> On May 23, 2022, at 11:46 PM, <[e-mail]> <[e-mail]> wrote:
> 
> Hi Yingzhen,
> 
> Many thanks for the review. 
> 
> Please see inline. 
> 
> Cheers,
> Med
> 
>> -----Message d'origine-----
>> De : Yingzhen Qu via Datatracker <[e-mail] <mailto:[e-mail]>>
>> Envoyé : mardi 24 mai 2022 01:28
>> À : [e-mail] <mailto:[e-mail]>
>> Cc : [e-mail] <mailto:[e-mail]>; [e-mail] <mailto:[e-mail]>;
>> [e-mail] <mailto:[e-mail]>
>> Objet : Rtgdir telechat review of draft-ietf-opsawg-l2nm-16
>> 
>> Reviewer: Yingzhen Qu
>> Review result: Has Nits
>> 
>> I have been selected as the Routing Directorate reviewer for this
>> draft. The Routing Directorate seeks to review all routing or
>> routing-related drafts as they pass through IETF last call and
>> IESG review, and sometimes on special request. The purpose of the
>> review is to provide assistance to the Routing ADs.
>> For more information about the Routing Directorate, please see 
>> http://trac.tools.ietf.org/area/rtg/trac/wiki/RtgDir
>> 
>> Although these comments are primarily for the use of the Routing
>> ADs, it would be helpful if you could consider them along with any
>> other IETF Last Call comments that you receive, and strive to
>> resolve them through discussion or by updating the draft.
>> 
>> Document: draft-ietf-opsawg-l2nm-16
>> Reviewer: Yingzhen Qu
>> Review Date: 05/23/2022
>> IETF LC End Date: unknown
>> Intended Status: Standards Track
>> 
>> Summary:
>> Thanks to the authors for working on this document. I think it's
>> almost ready for publication. There is one error in the appendix
>> A.2 example that should be fixed before publication, and there are
>> a few minor issues/nits that may be considered before publication.
>> 
>> Major Issues:
>> 
>> Appendix A.2:
>> libyang[0]: Node "ldp-pw-type" not found as a child of "ldp-or-
>> l2tp" node.
>> (path: Schema location
>> /ietf-l2vpn-ntw:l2vpn-ntw/vpn-services/vpn-service/vpn-nodes/vpn-
>> node/signaling-option/signaling-option/ldp-or-l2tp/ldp-or-l2tp,
>> data location /ietf-l2vpn-ntw:ldp-or-l2tp, line number 61.) This
>> should "t-ldp-pw-type".
>> 
> 
> [Med] Good catch. Fixed. Thanks.
> 
>> General Comment:
>> A lot of descriptions about nodes exported from RFC9181 in section
>> 7 are redundant from RFC9181.
> 
> [Med] We tried to find a balance between pointing to RFC9181 and ease the readability of the document. 
> 
>> 
>> Minor Issues and Nits (line numbers are from idnits):
>> 
>> 397        Also, the L2NM uses the IANA-maintained modules "iana-
>> bgp-l2-encaps"
>> 398        (Section 8.1) and "iana-pseudowire-types" (Section 8.2)
>> to identify a
>> 399        Layer 2 encapsulation type.
>> 
>> [nits]: encapsulation type and pseudowire type.
>> 
> 
> [Med] ACK. Changed to "encapsulation and pseudowire types"
> 
>> 409        view of the L2VPN service.  Such a view is only visible
>> within the
>> 410        service provider and is not exposed outside (to
>> customers, for
>> 411        example).
>> 
>> [nits]: Visible within/visible to
> 
> [Med] Fixed, thanks.
> 
>> 
>> 500        'name':  Sets a name to uniquely identify an ES within
>> a service
>> 501           provider network.  This name is referenced in the
>> VPN network
>> 502           access level of the L2NM (Section 7.6).
>> 
>> [major]: I don't see where the "name" is referenced.
> 
> [Med] This is called here:
> 
>                       +--rw group* [group-id]
>                       |  +--rw group-id                       string
>                       |  +--rw precedence?
>                       |  |       identityref
>                =====> |  +--rw ethernet-segment-identifier?   string
> 
> 
> A.4 provides an example how the segment is created and then how the name is called in the L2NM.
> 
> Updated the text with a pointer to that appendix. 

[Yingzhen]: “ethernet-segment-identifier” is defined as string here in stead of a leafref, why?
> 
>> 
>> 577        The 'vpn-profiles' container is used by the provider to
>> maintain a
>> 578        set of common VPN profiles that apply to VPN services
>> (Section 7.2).
>> 
>> [nits]: to be consistent with section 7.2: to maintain/to define
>> and maintain
>> 
> 
> [Med] OK. 
> 
>> 629        This document does not make any assumption about the
>> exact definition
>> 630        of these profiles.  The exact definition of the
>> profiles is local to
>> 631        each VPN service provider.
>> 
>> [nits]: These two sentences should be rewritten to be concise.
> 
> [Med] OK, shortened to: 
> 
> "The exact definition of these profiles is local to each VPN service provider."
> 
>> 
>> 2785             leaf name {
>> 2786               type string;
>> 2787               description
>> 2788                 "Includes the name of the Ethernet Segment
>> (ES).";
>> 2789             }
>> 
>> [nits]: please update the description
> 
> [Med] Updated to:
> 
>          "Includes the name of the Ethernet Segment (ES) that 
>           is used to unambiguously identify an ES.";
> 
>> 
>> 3824                   container active-global-parameters-profiles
>> {
>> 3825                     description
>> 3826                       "Container for a list of global
>> parameters
>> 3827                        profiles.";
>> 3828                     list global-parameters-profile {
>> 3829                       key "profile-id";
>> 3830                       description
>> 3831                         "List of active global parameters
>> profiles.";
>> 3832                       leaf profile-id {
>> 3833                         type leafref {
>> 3834                           path "../../../../../global-
>> parameters-profiles"
>> 3835                              + "/global-parameters-
>> profile/profile-id";
>> 3836                         }
>> 3837                         description
>> 3838                           "Points to a global profile defined
>> at the
>> 3839                            service level.";
>> 3840                       }
>> 3841                       uses parameters-profile;
>> 3842                     }
>> 3843                   }
>> 
>> [question]: profile-id is a leafref to the "global-parameters-
>> profile", where grouping "parameters-profile" is already
>> included/used, why is it used here again? is it for configuration
>> overwritten?
>> 
> 
> [Med] We need to call out a specific profile (and use the same id) to be activated at the node level. Some of the parameters will be overridden. 
> 
> 
> 
> _________________________________________________________________________________________________________________________
> 
> 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.