Thanks for the speedy response, Med.

Looks like you are completely on top of this.

A few closures below.

Cheers,
Adrian

[significant snipping]

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

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.

>> Section 6
>> 
>> 'esi-redundancy-mode'    Not sure, should you...
>>       s/Single-Active/'single-active'/
>>       s/All-Active/'all-active/
> 
> [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...

>> Section 8.1
>> 
>>    Appendix B lists the
>>    initial values included in the "iana-bgp-l2-encaps" YANG module.
>> 
>> 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
>> 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.
> 
> [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. 
>
>> 
>> 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].
> 
> [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.

>> Section 8.2
>> 
>> 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. 

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.

>> 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].

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

> 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/] ./]./
> 

[Med] Fixed. 

>> ---
>> 
>> I think 10.2 and 10.3 should make it clearer that IANA is requested to
>> maintain the two YANG models.
> 
> [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.


>> 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.
> 
> [Med] Thanks for catching this. Fixed. 
>
> This should be fixed in Figure 3 of RFC8214 as well. 

I'll raise an errata report.


_______________________________________________
OPSAWG mailing list
OPSAWG@ietf.org
https://www.ietf.org/mailman/listinfo/opsawg

Reply via email to