Subject: [OPSAWG] Shepherd review of draft-ietf-opsawg-l2nm

From: "Adrian Farrel" <[email-masked]>
Date: Thu, 18 Nov 2021 18:25:11 -0000

I have done my review of draft-ietf-opsawg-l2nm based on revision -10 of
the draft. Sorry it took so long, but there are a few pages of text that
needed to be read.

I have to congratulate the authors on this substantial piece of work. A
lot of time and effort must have gone into it.

It seems to be in pretty reasonable shape to me. Of course, it would be
a surprise if such a long document didn't have a few issues, but most of
what I have below is just nits.

Please feel free to ask for clarification or to refute anything I have
said.

Thanks,

Adrian

====

I see "layer 2" and "Layer 2" etc. I think in the context of L2VPN,
L2NM, etc., "Layer 2" is fine. But others "like Layer 2 connection"
are used inconsistently.

---

Abstract doesn't need "(VPN)"

---

Abstract

s/providers/provider/
s/Also, the document/Also, this document/
s/that defines/that define/

---

1.

OLD
   [RFC8466] defines an L2VPN Service Model (L2SM) YANG data model that
   can be used for Layer 2 Virtual Private Network (L2VPN) service
   ordering matters between customers and service providers.
NEW
   [RFC8466] defines an L2VPN Service Model (L2SM) YANG data model that
   can be used between customers and service providers for ordering
   Layer 2 Virtual Private Network (L2VPN) services.
END

---

1.

s/providers network/provider's network/
s/Also, the document/Also, this document/
s/rather than be limited/rather than being limited/

---

2. Service orchestrator

s/responsible of the CE-PE/responsible for the CE-PE/

---

2.

You might put "VPN node" before "VPN network access" to resolve the
reference.

---

2. Layer 2 VPN Service Network Model (L2NM)

s/providers network/provider's network/

---

Figure 1

It is unclear what +++++++
                   + AAA +
                   +++++++  means, and I don't see any text about it.

Is "Bearer Connection" identical to "Ethernet Segment"? See the
definition of "bearer" in "VPN network access" in section 2. If they are
the same, why use two terms? If they are different, please define bearer
connection.

---

Section 6 and the Ethernet Segment YANG Module comes as a bit of a
surprise.  There is just one mention of that module (in Section 5).

I think the Abstract and Introduction should at least mention the ES
module.

---

Figure 3 appears to have some rather random whitespace. Needs tidying?
Perhaps tabs were used in the original.

Figures 7, 8, 9, 11, 12, 13, 15 are pretty bad, too.

Figures 6, 14, and 22 have this a little.

Figure 17 is pretty much OK, but I don't know that "enumeration" needs
to be on a separate line each time.

---

Section 5

s/is meant to manage L2VPN services/is used to manage L2VPN services/

---

Section 6

   In reference to the structure shown in Figure 3, the following data
   nodes can be included:

Too much passive voice. Can be included:
- where?
- by whom?
- under what circumstances are they included or left out?

---

Section 6

'name'   s/with a service/within a service/

'esi-redundancy-mode'    Not sure, should you...
      s/Single-Active/'single-active'/
      s/All-Active/'all-active/

There are a number of data nodes shown in Figure 3 that don't have
explanations in the text. Is that intentional? It would seem to me that
they should be described in the same way as the other nodes. If you mean
to leave them out, perhaps say so and point at the YANG for definitive
descriptions of the other nodes.

---

Section 7

s/is meant to manage L2VPNs/is used to manage L2VPNs/

---

Section 7.2

 'forwarding-profile-identifier':
      Such policies may consist, for example, at applying Access
      Control Lists (ACLs).

Can't parse this. Maybe "...may consist, for example, of applying..."

---

Section 7.3

s/Such 'vpn-id' is/Such a 'vpn-id' is/

'signaling-type':  Indicates the signaling that is used for setting
   s/setting/setting up/

s/l2tp-signaling'/'l2tp-signaling'/

---

Section 7.3
   'underlay-transport'
      (e.g., an identifier of a VPN+ instance, a virtual network
      identifier, or a network slice name)
I don't know why you have "identifier" for the first two, but "name" for
a network slice. Wouldn't "identifier" serve better for all three cases
as...
      (e.g., an identifier of a VPN+ instance, virtual network, or
      network slice)

   'status'
   s/provision/provisioning/

   'vpn-node'
   s/various 'description' data node/various 'description' data nodes,/

---

Section 7.3

                  Table 1: Valid Signaling Options per VPN
                                Service Type

Maybe delete "Valid"

---

Section 7.4

I think PCP needs to expanded here. (Especially since you don't mean
Port Control Protocol.)

---

Section 7.5.1

s/'bgp-auto-discovery' container/The 'bgp-auto-discovery' container/

---

Section 7.5.2.1

s/this is VLAN-based/a this is VLAN-based/

---

Section 7.5.2.1

   The value of the 'pw-
   encapsulation-type' are taken from the IANA-maintained "iana-bgp-
   l2-encaps" module.

A reference to Section 8.1 would be good.

---

Section 7.5.2.2

s/include a an/include an/

---

Section 7.5.2.3

   The PW type ('pseudowire-type') value is taken from the
   IANA-maintained "iana-pseudowire-types" module.

A reference to Section 8.2 would be good.

---

7.6

   As such, every 'vpn-network-access' MUST belong to
   one and only one 'vpn-node'.

I'm not sure how to interpret this. The vpn-network-access part of the
tree is anchored under vpn-node. So it cannot be in two places at once.

Maybe this is saying that the value of the 'id' node must not appear
twice across the whole instantiation of the data model?

---

Section 7.6

s/' global-parameters-profile'/'global-parameters-profile'/

---

Section 7.6.1

s/OAM 802.3 ah/OAM 802.3ah/

---

Section 7.6.4

'svc-pe-to-ce-bandwidth' and 'svc-ce-to-pe-bandwidth' are missing colons

---

Section 8.1

   The "iana-bgp-l2-encaps" YANG module (Section 8.1) is designed to
   echo the registry available at [IANA-BGP-L2].

This *is* Section 8.1
Instead of "is designed to echo" say "echoes"

---

Section 8.1

   Appendix B lists the
   initial values included in the "iana-bgp-l2-encaps" YANG module.

Initial values of what? I think you are saying that the YANG model is
populated according to the state of the registry at the time of
publication of this document (and that new entries to the registry
should be matched with additional nodes in this model).

So, I would be inclined to leave this out.

And actually, I'd omit Appendix B which doesn't add anything.

---

8.1 and other subsections

It is useful to help resolve references and import clauses by placing
some text before the <CODE BEGINS> to say something like...

   This module makes imports from [RFCaaaa], and makes references to
   [RFCbbbb] and [RFCcccc].

---

Section 8.1

I believe the correct postal details are:

   Internet Assigned Numbers Authority (IANA)
   12025 Waterfront Drive, Suite 300
   Los Angeles CA 90094
   USA

   +1-424-254-5300 (phone)

---

8.1 and 8.2 
     identity layer-2-transport {
       base foo;
       description
         "IP Layer 2 Transport.";
       reference
         "RFC 3032: MPLS Label Stack Encoding";
     }

I don't quite get this. Is the reference wrong? Is the name of the
identity and the description wrong? 

---

Section 8.2

The comments about the introductory paragraphs of 8.1 apply to 8.2.
Also the comment about the contact address.

---

Section 8.2

Why don't atm-aal5 and atm-vp-virtual-trunk have reference clauses?

---
Section 8.4 

I don't find any explanation or reference for the color-type identities
or the color-type leaf. Maybe a description for Figure 20, or a 
description clause for the leaf or the base identity.

---

Section 8.4

I'm puzzled as to which leaf nodes qualify for a default clause and
which don't.

For example, ccm-interval and ccm-holdtime have default values. That
seems reasonable. But message-period and measurement-interval do not.

Similarly, mac-loop-prevention and frequency have defaults, retry-timer
does not.

Can you say how you decided which leaf nodes don't merit a default
value? Can you check that you have assigned a default everywhere one is
needed?

---

Section 8.4

       leaf ce-vlan-preservation {
         type boolean;
         description
           "Preserve the CE-VLAN ID from ingress to egress,i.e.,
            CE-VLAN tag of the egress frame are identical to
            those of the ingress frame that yielded this egress
            service frame. If All-to-One bundling within a site

s/,i.e.,/, i.e.,/
s/are identical/is identical/
s/those/that/

---

Section 8.4

               leaf ne-id {
                 type string;
                 description
                   "Indicates the node's IP address.";
               }

But 7.5 had
   'ne-id':  Includes a unique identifier of the network element where
      the VPN node is deployed.
...and 8.3 had
           leaf ne-id {
             type string;
             description
               "Unique identifier of the network element where the ES
                is configured with a service provider network.";

Now, I know that a node's IP address is usually used as the unique 
identifier of the node, but 7.5 and 8.3 make it look like you could use
something else so long as it is unique.

Additionally, "unique" is probably stronger than you mean, since you are
probably happy with a certain domain of uniqueness.

Compare and contrast with

               leaf router-id {
                 type rt-types:router-id;
                 description
                   "A 32-bit number in the dotted-quad format that is
                    used to uniquely identify a node within an
                    autonomous system (AS). ";
               }

---

Section 8.4
                         leaf pw-description {
                           type string;
                           description
                             "Includes an interface description used to
                              send a human-readable administrative
                              string describing the interface to the
                              remote.";

To the remote what? 
Actually, to be completely clear, are you saying that the interface
description is used to send a string to the remote? Or is this garbled?

Could be...
Includes a human-readable description of the interface. This may be used
when communicating with remote administrative processes to identify the
interface.

---

Section 8.4

There is a set of three members of the container 'connection' that are 
of type string : l2-termination-point, local-bridge-reference, and
bearer-reference. The description classes are clear up to a point, but
"a reference to" doesn't really tell me what I should put in these
objects. 

---

Section 8.4
                             leaf port-speed {
                               type uint32;
                               description
                                 "Port speed.";
                             }

This is missing a units clause. And I would be slightly worried that
uint32 might not be future-proof with some possible units.

---

Section 8.4

                         leaf ce-id {
                           type uint16;
                           description
                             "The PE must know the set of virtual
                              circuits connecting it to the CE and a
                              CE ID identifying the CE within the VPN.";

The first part of the description ("set of VCs connecting to the CE") is
true, but is it relevant to this leaf?

---

Section 9

   The YANG modules specified in this document defines schema for data
   that is designed to be accessed via network management protocols such
   as NETCONF [RFC6241] or RESTCONF [RFC8040] . The lowest NETCONF layer

s/defines schema/define schemas/
s/that is/that are/
s/] ./]./

---

I think 10.2 and 10.3 should make it clearer that IANA is requested to
maintain the two YANG models.

---

Thanks for all the examples. They are helpful.

---

A.1

s/used to configured/used to configure/

---

A.4

s/depictes/depicts/

---

A.4

In Figure 29 is it intentional that the left-hand end of the Emulated
Service is at the edge of the CE, but the right-hand end is in the 
middle of the CE.

---

A.6

s/In such as configuration/In such a configuration/

---

I'm unclear of the purpose of Appendixes B and C. They seem to repeat
information that is found in the relevant IANA registries.

---

From: <[email-masked]>
Date: Fri, 19 Nov 2021 09:43:04 +0000

Hi Adrian, 

Many thanks for the careful review. 

You can track the changes to address your comments at: https://tinyurl.com/l2nm-latest. 

Please see inline for more context. 

Cheers,
Med

> -----Message d'origine-----
> De : OPSAWG <[email-masked]> De la part de Adrian Farrel
> Envoyé : jeudi 18 novembre 2021 19:25
> À : [email-masked]
> Cc : [email-masked]
> Objet : [OPSAWG] Shepherd review of draft-ietf-opsawg-l2nm
> 
> It seems to be in pretty reasonable shape to me. Of course, it would be a
> surprise if such a long document didn't have a few issues, but most of
> I see "layer 2" and "Layer 2" etc. I think in the context of L2VPN, L2NM,
> etc., "Layer 2" is fine. But others "like Layer 2 connection"
> are used inconsistently.
> 

[Med] Thanks. I prefer to leave the current wording. 

> Abstract doesn't need "(VPN)"

[Med] OK.

> ---
> 
> Abstract
> 
> s/providers/provider/
> s/Also, the document/Also, this document/ s/that defines/that define/
> 

[Med] Fixed. 

>    Layer 2 Virtual Private Network (L2VPN) services.

[Med] This is better. thanks. 

> s/Also, the document/Also, this document/ s/rather than be limited/rather
> than being limited/
> 

[Med] Fixed. Thanks. 

> s/responsible of the CE-PE/responsible for the CE-PE/

[Med] ACK.

> You might put "VPN node" before "VPN network access" to resolve the

[Med] Done. 

> s/providers network/provider's network/

[Med] OK

>                    +++++++  means, and I don't see any text about it.

[Med] You are right. AAA was brought from Figure 3 of RFC8466. I prefer to delete it rather than adding text. 

> Is "Bearer Connection" identical to "Ethernet Segment"? See the definition
> of "bearer" in "VPN network access" in section 2. If they are the same,
> why use two terms? If they are different, please define bearer connection.
> 

[Med] We do already have "The attachment circuits (bearers) between Customer Edges (CEs) and Provider Edges (PEs) are terminated in the VPN network access. "

Added "Similar to Section 3 of [RFC8466] CE to PE attachment is achieved through a bearer with a Layer 2 connection on top. The bearer refers to properties of the attachment that are below Layer 2, while the connection refers to Layer 2 protocol-oriented properties."

> I think the Abstract and Introduction should at least mention the ES

[Med] The introduction has already the following:

   Section 8.3 defines a YANG
   module to manage Ethernet Segments (ESes) that are required for
   instantiating EVPNs.

Updated the abstract. Thanks. 

> Perhaps tabs were used in the original.

[Med] For this one, this is exactly the output we get from pyang.  

> Figure 17 is pretty much OK, but I don't know that "enumeration" needs to
> be on a separate line each time.
> 

[Med] :-) It does not have ... but idnits won't be happy. 

This is how trees are adjusted when run with "--tree-line-length 69"

> s/is meant to manage L2VPN services/is used to manage L2VPN services/

[Med] OK

> - under what circumstances are they included or left out?

[Med] Changed to "The description of the data nodes depicted in Figure 3 is as follows:"

> 'name'   s/with a service/within a service/

[Med] Fixed.

>       s/All-Active/'all-active/

[Med] The OLD one is OK because we are trying to be consistent with the use in rfc7432. Thanks.

> descriptions of the other nodes.

[Med] Updated the description to cite 'auto-ethernet-segment-identifier', 'esi-pool-name', and 'ethernet-segment-identifier'. 

> s/is meant to manage L2VPNs/is used to manage L2VPNs/

[Med] ACK. 

> Can't parse this.

[Med] You can't. Only those like me having English being their forth language can parse this!

 Maybe "...may consist, for example, of applying..."

[Med] Fixed. Thank you/

> s/l2tp-signaling'/'l2tp-signaling'/

[Med] Fixed. 

>       identifier, or a network slice name) I don't know why you have
> "identifier" for the first two, but "name" for a network slice. Wouldn't
> "identifier" serve better for all three cases as...

[Med] ACK

>                   Table 1: Valid Signaling Options per VPN

[Med] OK

> ---
> 
> Section 7.4
> 
> I think PCP needs to expanded here. (Especially since you don't mean Port
> Control Protocol.)

[Med] :-)

Updated to "Priority Code Point (PCP)"

> s/'bgp-auto-discovery' container/The 'bgp-auto-discovery' container/

[Med] Fixed. 

> s/this is VLAN-based/a this is VLAN-based/

[Med] Changed to "this is a VLAN-based"

> A reference to Section 8.1 would be good.

[Med] Added. Thanks.

> s/include a an/include an/

[Med] Fixed.  

> A reference to Section 8.2 would be good.

[Med] Agree. Fixed. 

> Maybe this is saying that the value of the 'id' node must not appear twice
> across the whole instantiation of the data model?

[Med] I fully agree. No need to have that sentence. This is the kind of text you borrow since early days of a draft. 

> s/' global-parameters-profile'/'global-parameters-profile'/

[Med] ACK. 

> s/OAM 802.3 ah/OAM 802.3ah/

[Med] Fixed. 

> 'svc-pe-to-ce-bandwidth' and 'svc-ce-to-pe-bandwidth' are missing colons

[Med] Please note that we have "'svc-pe-to-ce-bandwidth' and 'svc-ce-to-pe-bandwidth':" to factorize the description for both directions. No colons are used for 'svc-pe-to-ce-bandwidth' and 'svc-ce-to-pe-bandwidth' because these are not in the same level as the previous bullets. Rearranged the text to avoid this confusion. Sorry.

>    echo the registry available at [IANA-BGP-L2].

[Med] :-)

> Instead of "is designed to echo" say "echoes"

[Med] Fixed. Thanks.

> Initial values of what?

[Med] ... of the registry. This is a  living module if you will. It will be automatically updated by IANA when new values are assigned. 

 I think you are saying that the YANG model is
> publication of this document (and that new entries to the registry should
> be matched with additional nodes in this model).
> 

[Med] A pointer to the IANA registry would be sufficient, but idnits won't be happy as all reference statements have to be called outside the YANG modules. These tables solves this issue. 

> It is useful to help resolve references and import clauses by placing some
> text before the <CODE BEGINS> to say something like...
> 

[Med] Agree. Unlike 8.3 and 8.4, we don't have such statements in 8.1/8.2 because we don't have any imports. 

>    12025 Waterfront Drive, Suite 300

[Med] You are right, but the one in the draft is what is used by IANA for IANA-maintained YANG modules. 

> identity and the description wrong?

[Med] That's what is currently in the registry. We are not modifying it or fixing any issue we may find. 

> Why don't atm-aal5 and atm-vp-virtual-trunk have reference clauses?

[Med] ... because we failed to find what is "[[ATM]]" in the IANA registry (please see Appendix C). Please note that we have a reference statement in 8.1 for atm-aal5. 

Fixed atm-vp-virtual-trunk by adding a pointer to MFA. 

> ---
> Section 8.4
> 
> I don't find any explanation or reference for the color-type identities or
> the color-type leaf. Maybe a description for Figure 20, or a description
> clause for the leaf or the base identity.
> 

[Med] Joe raised a similar comment in the past. We added this general pointer rather that redefining all the inherited item in the L2NM: 

   See also Section 5.10.2.1 of [RFC8466].

That section says the following:

==
   A
   "color-id" will be assigned to a service frame to identify its QoS
   profile conformance.  A service frame is "green" if it is conformant
   with the "committed" rate of the bandwidth profile.  A service frame
   is "yellow" if it exceeds the "committed" rate but is conformant with
   the "excess" rate of the bandwidth profile.  Finally, a service frame
   is "red" if it is conformant with neither the "committed" rate nor
   the "excess" rate of the bandwidth profile.
==

As I expect this to pop-up in other reviews, I updated the yang description with more text. 

> ---
> 
> Section 8.4
> 
> I'm puzzled as to which leaf nodes qualify for a default clause and which
> don't.
> 
> For example, ccm-interval and ccm-holdtime have default values. That seems
> reasonable. But message-period

[Med] Added one. 

 and measurement-interval do not.
[Med] Will check if there is a default to mention.

> Similarly, mac-loop-prevention and frequency have defaults, retry-timer

[Med] Will check. 

> 
> Can you say how you decided which leaf nodes don't merit a default value?
> Can you check that you have assigned a default everywhere one is needed?
> 

[Med] Added some more, but will further check. 

> s/are identical/is identical/

[Med] Fixed.

>                    "Indicates the node's IP address.";

[Med] this is not correct. Updated as follows: 

                "An identifier of the network element where
                 the VPN node is deployed. This identifier
                 uniquely identifies the network element within
                 an administrative domain."
> probably happy with a certain domain of uniqueness.

[Med] Indeed. 

>                               string describing the interface to the

[Med] to the remote peer. Changed to:

               "Includes a human-readable description of 
                the interface. This may be used when 
                communicating with a remote peer." 

FWIW, this echoes this part from RFC4447:

      This arbitrary, and OPTIONAL, interface description string is used
      to send a human-readable administrative string describing the
      interface to the remote.

> There is a set of three members of the container 'connection' that are of
> type string : l2-termination-point, local-bridge-reference, and bearer-
> reference. The description classes are clear up to a point, but "a
> reference to" doesn't really tell me what I should put in these objects.
> 

[Med] The text says ".. such as a layer 2 sub-interface" (for l2-termination-point) and ".. A reference may be a local bridge domain" (for local-bridge-reference). 

For bearer-reference, I made this change: 

OLD: 
   VPN network access:  Is an abstraction that represents the network
      interfaces that are associated to a given VPN node.  Traffic
      coming from the VPN network access belongs to the VPN.  The
      attachment circuits (bearers) between Customer Edges (CEs) and
      Provider Edges (PEs) are terminated in the VPN network access.

NEW:
   VPN network access:  Is an abstraction that represents the network
      interfaces that are associated to a given VPN node.  Traffic
      coming from the VPN network access belongs to the VPN.  The
      attachment circuits (bearers) between Customer Edges (CEs) and
      Provider Edges (PEs) are terminated in the VPN network access.
      A reference to the bearer is
      maintained to allow keeping the link between L3SM and L3NM when
      both models are used in a given deployment.  

> ---
> 
> Section 8.4
>                              leaf port-speed {
>                                type uint32;
>                                description
>                                  "Port speed.";
>                              }
> 
> This is missing a units clause.

[Med] Good catch. Added "units "mbps"

 And I would be slightly worried that
> uint32 might not be future-proof with some possible units.

[Med] I think we are safe with the proposed unit. 
                         
> true, but is it relevant to this leaf?

[Med] Fair. Changed to "Identifies the CE within the VPN."

> s/defines schema/define schemas/

[Med] Fixed. 

> maintain the two YANG models.

[Med] This is already mentioned in 10.1

==
         name: iana-bgp-l2-encaps
         namespace: urn:ietf:params:xml:ns:yang:iana-bgp-l2-encaps
         maintained by IANA: Y <=========
         prefix: iana-bgp-l2-encaps
         reference: RFC XXXX

         name: iana-pseudowire-types
         namespace: urn:ietf:params:xml:ns:yang:iana-pseudowire-types
         maintained by IANA: Y <=========
         prefix: iana-pw-types
         reference: RFC XXXX
==

and repeated in 10.2/3: 
  "This document defines the initial version of the IANA-maintained.."

> Thanks for all the examples. They are helpful.

[Med] Thanks. I hope we can have more integrated tools to check the json with the module structure. 

> s/used to configured/used to configure/

[Med] Fixed. 

> s/depictes/depicts/

[Med] Fixed.

> Service is at the edge of the CE, but the right-hand end is in the middle
> of the CE.
> 

[Med] Thanks for catching this. Fixed. 

This should be fixed in Figure 3 of RFC8214 as well. 

> s/In such as configuration/In such a configuration/

[Med] Fixed. 

> information that is found in the relevant IANA registries.

[Med] This is used as a simple way to cite all the references outside the module. If not, idnits won't he happy.

_________________________________________________________________________________________________________________________

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.

---

From: "Joe Clarke (jclarke)" <[email-masked]>
Date: Fri, 19 Nov 2021 09:49:16 +0000

Thank you for taking on these drafts, Adrian and for a thorough review
of L2NM.

Joe

On 11/18/21 13:25, Adrian Farrel wrote:
> _______________________________________________
> OPSAWG mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/opsawg
>

---

From: "Adrian Farrel" <[email-masked]>
Date: Fri, 19 Nov 2021 10:56:30 -0000

Thanks for the speedy response, Med.

Looks like you are completely on top of this.

A few closures below.

Cheers,
Adrian

[significant snipping]

> [Med] Thanks. I prefer to leave the current wording. 

OK. The RFC editor *will* raise the inconsistency.

For example...

Section 1
   Also, the document defines the initial versions of two IANA-
   maintained modules that define a set of identities of BGP Layer 2
   encapsulation types (Section 8.1)
Section 5
   Also, the L2NM uses the IANA-maintained modules "iana-bgp-l2-encaps"
   (Section 8.1) and "iana-pseudowire-types" (Section 8.2) to identify a
   layer 2 encapsulation type.

> [Med] The OLD one is OK because we are trying to be consistent with the
use in rfc7432. Thanks.

I'm OK with this, but pointing out that evpn-redundancy-mode defines the
values for esi-redundancy-mode, and that uses lower case.

A merging of two issues, below...

> [Med] ... of the registry. This is a  living module if you will. It will
be automatically updated by IANA when new values are assigned. 
> [Med] A pointer to the IANA registry would be sufficient, but idnits won't
be happy as all reference statements have to be called outside the YANG
modules. These tables solves this issue. 
>> It is useful to help resolve references and import clauses by placing
some
> [Med] Agree. Unlike 8.3 and 8.4, we don't have such statements in 8.1/8.2
because we don't have any imports. 

My first point was that if you say "This module references [RFCaaaa]" then
you don't need to use the Appendix to resolve the references.

I am not terribly worried by this issue: just that the two Appendixes seemed
to be a bit odd. In general, I don't like reproducing information in
multiple places because it is hard to see which is normative and how to
resolve discrepancies.

> [Med] ... because we failed to find what is "[[ATM]]" in the IANA registry
(please see Appendix C). Please note that we have a reference statement in
8.1 for atm-aal5. 

I believe it is per RFC 4446 (which is what populated the registry)
   [ATM]     Martini, L., Ed., El-Aawar, N., and M. Bocci, Ed.,
             "Encapsulation Methods for Transport of ATM Over MPLS
             Networks", Work in Progress.
I believe this became RFC 4717. (See Section 6.3.)
I have sent mail to the PALS chairs to ask them whether the registry should
be updated.

>> I don't find any explanation or reference for the color-type identities
or
> [Med] Joe raised a similar comment in the past. We added this general
>   See also Section 5.10.2.1 of [RFC8466].

Ah, good.

Perhaps (but not absolutely necessary)...

"See also Section 5.10.2.1 of [RFC8466] for more discussion of QoS
classification including the use of color types."

That would close the loop for people searching through the document.

> As I expect this to pop-up in other reviews, I updated the yang
description with more text. 

So you may have closed this already

[Med] Fixed. 

> [Med] This is already mentioned in 10.1

Notwithstanding the text you quoted, you are not actually asking IANA to do
the thing (merely stated that it is done).

The point here is that in satisfying 10.1, IANA just copies the text into
the registry. Nothing in the whole of Section 10, asks IANA to create and
maintain the modules. 
Yes, they will probably work it out, but nothing beats being very, very
clear in an IANA considerations section.

> This should be fixed in Figure 3 of RFC8214 as well. 

I'll raise an errata report.

---

From: <[email-masked]>
Date: Fri, 19 Nov 2021 12:27:58 +0000

Re-,

Thanks for the follow-up. 

Please see inline. 

Cheers,
Med

> De : Adrian Farrel <[email-masked]>
> Envoyé : vendredi 19 novembre 2021 11:57
> À : BOUCADAIR Mohamed INNOV/NET <[email-masked]>; draft-
> [email-masked]
> Cc : [email-masked]
> Objet : RE: [OPSAWG] Shepherd review of draft-ietf-opsawg-l2nm
> 
> > [Med] The OLD one is OK because we are trying to be consistent with
> > the
> use in rfc7432. Thanks.
> 

[Med] the identity labels use lower case, not the description. 

> >> I think you are saying that the YANG model is populated according to
> >> the state of the registry at the time of publication of this document
> >> (and that new entries to the registry should be matched with
> >> additional nodes in this model).
> >>
> > [Med] Agree. Unlike 8.3 and 8.4, we don't have such statements in
> > 8.1/8.2

[Med] OK. Went for:

==
This module references [RFC3032], [RFC4446],
   [RFC4448], [RFC4553], [RFC4618], [RFC4619], [RFC4717], [RFC4761],
   [RFC4816], [RFC4842], and [RFC5086].
==

and

==
This module references
   [MFA], [RFC2507], [RFC2508], [RFC3032], [RFC3545], [RFC4448],
   [RFC4553], [RFC4618], [RFC4619], [RFC4717], [RFC4842], [RFC4863],
   [RFC4901], [RFC5086], [RFC5087], [RFC5143], [RFC5795], and [RFC6307].
==

> 
> I am not terribly worried by this issue: just that the two Appendixes
> seemed to be a bit odd. In general, I don't like reproducing information
> in multiple places because it is hard to see which is normative and how to
> resolve discrepancies.
> 
> >> Section 8.2
> >>
> I believe this became RFC 4717. (See Section 6.3.) I have sent mail to the
> PALS chairs to ask them whether the registry should be updated.

[Med] Thanks. Updated the module. it would be good if the registry is formally updated.  

> >> the color-type leaf. Maybe a description for Figure 20, or a

Med] Deal!

> >> I think 10.2 and 10.3 should make it clearer that IANA is requested
> >> to maintain the two YANG models.
> >
> Notwithstanding the text you quoted, you are not actually asking IANA to
> do the thing (merely stated that it is done).

[Med] We ask them to maintain the modules: 

   This document requests IANA to register the following YANG modules in
   the "YANG Module Names" subregistry [RFC6020] within the "YANG
   Parameters" registry:

Please trust me, this is sufficient and the presence of "maintained by IANA:" will save us one usual question from IANA. 

> I'll raise an errata report.

[Med] Thank you.

---

From: "Adrian Farrel" <[email-masked]>
Date: Fri, 19 Nov 2021 12:44:04 -0000

Just to formally close the loop.

You have captured and addressed all the point from my review.

Thanks.

Adrian

-----Original Message-----
From: [email-masked] <[email-masked]> 
Sent: 19 November 2021 12:28
To: [email-masked]; [email-masked]
Cc: [email-masked]
Subject: RE: [OPSAWG] Shepherd review of draft-ietf-opsawg-l2nm

Re-,

Please see inline. 

Cheers,
Med

[Med] OK. Went for:

==

and

==
This module references

> PALS chairs to ask them whether the registry should be updated.

[Med] Thanks. Updated the module. it would be good if the registry is
formally updated.  

Med] Deal!

Please trust me, this is sufficient and the presence of "maintained by
IANA:" will save us one usual question from IANA. 

[Med] Thank you.

____________________________________________________________________________
_____________________________________________

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.

---

From: <[email-masked]>
Date: Mon, 22 Nov 2021 10:14:00 +0000

Hi Adrian, all, 

Thanks. All the agreed changes are addressed in https://datatracker.ietf.org/doc/draft-ietf-opsawg-l2nm/12/. 

It is convenient to use this diff rather than the side-by-side to track the changes: https://www.ietf.org/rfcdiff?url1=draft-ietf-opsawg-l2nm-11&url2=draft-ietf-opsawg-l2nm-12&difftype=--hwdiff

Cheers,
Med

> Envoyé : vendredi 19 novembre 2021 13:44
> > De : Adrian Farrel <[email-masked]> Envoyé : vendredi 19 novembre
> > 2021 11:57 À : BOUCADAIR Mohamed INNOV/NET
> > <[email-masked]>; draft- [email-masked] Cc :
> > [email-masked] Objet : RE: [OPSAWG] Shepherd review of
> > draft-ietf-opsawg-l2nm
> >
> > I'm OK with this, but pointing out that evpn-redundancy-mode defines
> > the values for esi-redundancy-mode, and that uses lower case.
> >
> 
> > >> I think you are saying that the YANG model is populated according
> > >> to the state of the registry at the time of publication of this
> > >> document (and that new entries to the registry should be matched
> > >> with additional nodes in this model).
> > >>
> > be happy as all reference statements have to be called outside the
> > YANG modules. These tables solves this issue.
> > >
> > >>
> > My first point was that if you say "This module references [RFCaaaa]"
> > then you don't need to use the Appendix to resolve the references.
> 
> [Med] OK. Went for:
> 
> ==
> > seemed to be a bit odd. In general, I don't like reproducing
> > information in multiple places because it is hard to see which is
> > normative and how to resolve discrepancies.
> >
> > >> Section 8.2
> > >>
> > I believe this became RFC 4717. (See Section 6.3.) I have sent mail to
> > the PALS chairs to ask them whether the registry should be updated.
> 
> > Notwithstanding the text you quoted, you are not actually asking IANA
> > to do the thing (merely stated that it is done).
> 
> > The point here is that in satisfying 10.1, IANA just copies the text
> > into the registry. Nothing in the whole of Section 10, asks IANA to
> > create and maintain the modules.
> > Yes, they will probably work it out, but nothing beats being very,
> > very clear in an IANA considerations section.
> >
> >
> > >> A.4
> > >>
> > >> In Figure 29 is it intentional that the left-hand end of the
> > >> Emulated Service is at the edge of the CE, but the right-hand end
> > >> is in the middle of the CE.
> > >

---

From: Moti Morgenstern <[email-masked]>
Date: Tue, 23 Nov 2021 04:44:15 +0000

Hi Med,

For esi-redundancy-mode leaf/parameter, the Description refers to "EVPN redundancy mode" instead of "ES redundancy mode".

Regards,
Moti

-----Original Message-----
From: OPSAWG <[email-masked]> On Behalf Of [email-masked]
Sent: Monday, November 22, 2021 12:14 PM
To: [email-masked]; [email-masked]
Cc: [email-masked]
Subject: [EXTERNAL] Re: [OPSAWG] Shepherd review of draft-ietf-opsawg-l2nm

Hi Adrian, all, 

Thanks. All the agreed changes are addressed in https://clicktime.symantec.com/3FXQgvQVPSVWohRYsEsFwzK6H2?u=https%3A%2F%2Fdatatracker.ietf.org%2Fdoc%2Fdraft-ietf-opsawg-l2nm%2F12%2F. 

It is convenient to use this diff rather than the side-by-side to track the changes: https://clicktime.symantec.com/3CNEi8fM9yX8oEcpcvkd2Ao6H2?u=https%3A%2F%2Fwww.ietf.org%2Frfcdiff%3Furl1%3Ddraft-ietf-opsawg-l2nm-11%26url2%3Ddraft-ietf-opsawg-l2nm-12%26difftype%3D--hwdiff

Cheers,
Med

> 2021 13:44 À : BOUCADAIR Mohamed INNOV/NET 
> > >> I see "layer 2" and "Layer 2" etc. I think in the context of 
> > >> L2VPN, L2NM, etc., "Layer 2" is fine. But others "like Layer 2 connection"
> > >> are used inconsistently.
> > >
> >    encapsulation types (Section 8.1) Section 5
> > > [Med] The OLD one is OK because we are trying to be consistent 
> > > with the
> > use in rfc7432. Thanks.
> >
> > I am not terribly worried by this issue: just that the two 
> > Appendixes seemed to be a bit odd. In general, I don't like 
> > reproducing information in multiple places because it is hard to see 
> > which is normative and how to resolve discrepancies.
> >
> > >> Section 8.2
> > >>
> > I believe this became RFC 4717. (See Section 6.3.) I have sent mail 
> > to the PALS chairs to ask them whether the registry should be updated.
> 
> > >    that is designed to be accessed via network management 
> > > protocols
> such
> > >> I think 10.2 and 10.3 should make it clearer that IANA is 
> > >> requested to maintain the two YANG models.
> > >
> > Notwithstanding the text you quoted, you are not actually asking 
> > IANA to do the thing (merely stated that it is done).
> 

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
[email-masked]
https://clicktime.symantec.com/3Brj4YRPPFzU76HVHwRwYRE6H2?u=https%3A%2F%2Fwww.ietf.org%2Fmailman%2Flistinfo%2Fopsawg

Notice: This e-mail together with any attachments may contain information of Ribbon Communications Inc. and its Affiliates that is confidential and/or proprietary for the sole use of the intended recipient. Any review, disclosure, reliance or distribution by others or forwarding without express permission is strictly prohibited. If you are not the intended recipient, please notify the sender immediately and then delete all copies, including any attachments.

Notice: This e-mail together with any attachments may contain information of Ribbon Communications Inc. and its Affiliates that is confidential and/or proprietary for the sole use of the intended recipient. Any review, disclosure, reliance or distribution by others or forwarding without express permission is strictly prohibited. If you are not the intended recipient, please notify the sender immediately and then delete all copies, including any attachments.