Re: [OPSAWG] 4026 as a downref: Document shepherd review of draft-ietf-opsawg-yang-vpn-service-pm-06

[Adrian Farrel <[e-mail]>]

Thanks for all the work Bo,

We're just waiting on IPR responses from:
  [e-mail]
  [e-mail]
  [e-mail]
  [e-mail]

And I'll need the chairs to set the Intended Status of the document in the
Datatracker.

Cheers,
Adrian

-----Original Message-----
From: Wubo (lana) <[e-mail]> 
Sent: 05 May 2022 14:19
To: [e-mail]; 'tom petch' <[e-mail]>;
[e-mail]
Cc: 'opsawg' <[e-mail]>
Subject: RE: 4026 as a downref: Document shepherd review of
draft-ietf-opsawg-yang-vpn-service-pm-06

Hi Tom, Adrian, all,

Many thanks for your helpful comments. 

Rev-08 has the following changes:
- Replaces the choice definition of "vpn-pm-type" to container to allow both
PM type setting.
- Aligns YANG security guidelines
- Adds STAMP RFC 8762 as one of PM measurement protocol
- Updates P, PE YANG descriptions and move RFC 4026 to informative reference

Please let us know if any further change is needed.
https://www.ietf.org/rfcdiff?url2=draft-ietf-opsawg-yang-vpn-service-pm-08

Thanks,
Bo

-----Original Message-----
From: Adrian Farrel [mailto:[e-mail]] 
Sent: Thursday, May 5, 2022 4:59 PM
To: 'tom petch' <[e-mail]>; Wubo (lana) <[e-mail]>;
[e-mail]
Cc: 'opsawg' <[e-mail]>
Subject: 4026 as a downref: Document shepherd review of
draft-ietf-opsawg-yang-vpn-service-pm-06

Just chiming in on this thread.

Don't be frightened of downrefs.  They are just a small piece of process
easily handled.

Better to set the reference correctly. If it is necessary to read RFC 4026
in order to understand part of this document, then it is a normative
reference. Otherwise, of course, it is an Informative reference.

Adrian

-----Original Message-----
From: tom petch <[e-mail]>
Sent: 27 April 2022 12:35
To: Wubo (lana) <lana.wubo=[e-mail]>;
[e-mail]; [e-mail]
Cc: 'opsawg' <[e-mail]>
Subject: Re: Document shepherd review of
draft-ietf-opsawg-yang-vpn-service-pm-06

From: OPSAWG <[e-mail]> on behalf of Wubo (lana)
<lana.wubo=[e-mail]>
Sent: 25 April 2022 14:13

Hi Adrian,

About the issue on Normative Reference, RFC4026 as specific, the authors
think this will cause downref since RFC4026 is an Informational draft.

<tp>

True but totally irrelevant.  The issue is whether or not the RFC is needed
in order to understand the I-D, the consequences thereof are irrelevant.
IMHO it is needed to make sense of 'p' so it is a Normative Reference.  To
do otherwise is to game the system (which opsawg-l3sm-l3nn does!).

Tom Petch

p.s. I am feeling stroppy today - where has the IETF e-mail service gone?
DoS attack?

We still suggest RFC4026 as an informative reference because the model just
references it as informational.

Thanks,
Bo

-----Original Message-----
From: OPSAWG [mailto:[e-mail]] On Behalf Of Wubo (lana)
Sent: Monday, April 25, 2022 4:44 PM
To: [e-mail]; [e-mail]
Cc: 'opsawg' <[e-mail]>
Subject: Re: [OPSAWG] Document shepherd review of
draft-ietf-opsawg-yang-vpn-service-pm-06

Hi Adrian,

Many thanks for your detailed review. We have released Rev-07 to address
these issues, see if they are fully addressed.
https://www.ietf.org/rfcdiff?url2=draft-ietf-opsawg-yang-vpn-service-pm-07

Please also find some replies inline.

Thanks,
Bo

-----Original Message-----
From: Adrian Farrel [mailto:[e-mail]]
Sent: Saturday, April 23, 2022 12:35 AM
To: [e-mail]
Cc: 'opsawg' <[e-mail]>
Subject: Document shepherd review of
draft-ietf-opsawg-yang-vpn-service-pm-06

Hi,

I'm the document shepherd for this document as it moves beyond the WG for
requested publication as an RFC.

I reviewed the draft at -03 during WG last call, so my comments here are
basically editorial with only a few small questions.

If the authors could produce a new revision, I will start work on the
shepherd write-up.

One other point: can someone say whether this draft has been shared with the
IPPM working group?

Thanks,
Adrian

===

Introduction.

First sentence could use a reference to RFC 6020.
[Bo Wu] Fixed.

---

Introduction

OLD
   It defines that the performance
   measurement telemetry model to be tied with the service, such as
   Layer 3 VPN and Layer 2 VPN, or network models to monitor the overall
   network performance or Service Level Agreement (SLA).
NEW
   It defines that the performance
   measurement telemetry model should be tied to the services (such as
   a Layer 3 VPN or Layer 2 VPN) or to the network models to monitor the
   overall network performance and the Service Level Agreements (SLAs).
END

 [Bo Wu] Fixed.
---

2.1

OLD
   SLA     Service Level Agreements
NEW
   SLA     Service Level Agreement
END

[Bo Wu] Fixed.
---

3.

   For example, the
   controller can use information from [RFC8345], [I-D.ietf-opsawg-sap]
   or VPN instances.

I think this is where there should be a reference to RFC 9182 and
draft-ietf-opsawg-l2nm.

[Bo Wu] Fixed.
---

3.1

s/dynamic-changing/dynamic/
[Bo Wu] Fixed.

---

4.

OLD
   This document defines the YANG module, "ietf-network-vpn-pm", which
   is an augmentation to the "ietf-network" and "ietf-network-topology".
NEW
   This document defines the YANG module, "ietf-network-vpn-pm", which
   is an augmentation to the "ietf-network" and "ietf-network-topology"
   modules.
END

[Bo Wu] Fixed.
---

4.

Would it be more consistent if the box on the right of Figure 2 showed
"ietf-network-vpn-pm"?
[Bo Wu] Fixed.

---

I think that Figure 3 could use a little tidying.
- Some gaps in lines
- A couple of lines slightly out of place
- S2A and S2B are confusinly places
- The cross-over of VN3-N2 and VN1-N1 is unclear
- Wording of the Legend a little unclear

How about...


                     VPN 1                       VPN 2
          +------------------------+   +------------------------+
         /                        /   /                        /
        / S1C_[VN3]:::           /   /                        /
       /         \   :::::      /   / S2A_[VN1]____[VN3]_S2B /
      /           \       :::  /   /      :          :      / Overlay
     /             \         :::::::::::: :          :     /
    / S1B_[VN2]____[VN1]_S1A /   /       :           :    /
   +---------:-------:------+   +-------:-:----------:---+
             :        :     ::::::::::::   :         :
             :         :   :                :        :
   Site-1A   :  +-------:-:------------------:-------:-----+ Site-1C
     [CE1]___:_/_______[N1]___________________[N2]___:____/__[CE3]
             :/       / / \             _____//     :    /
   [CE5]_____:_______/ /    \     _____/     /    ::    /
 Site-2A    /:        /       \  /          /   ::     /
           / :                [N5]         /  ::      / Underlay Network
          /   :     /       __/ \__       / ::       /
         /     :   /    ___/       \__   /::        /
Site-1B /       : / ___/              \ /:         /  Site-2B
[CE2]__/________[N4]__________________[N3]________/____[CE4]
      /                                          /
     +------------------------------------------+

    Legend:
    N:Node   VN:VPN-Node  S:Site  CE:Customer Edge
    __  Link within a network layer
    :   Mapping between network layers

[Bo Wu] Fixed. Thanks for helping to correct the figure.
---

4.1

s/topologies are both built/topologies are built/ [Bo Wu] Fixed.

---

The legend for Figure 4 should include "TP" (if TPs are actually relevant to
the figure and aren't something you should remove - the text doesn't mention
them, and they don't really seem to be important in Section 4.1).

Probably, TP should be added to the list in Section 2.1 with a reference to
where TP is properly explained. 4.4 would then be able to lean on that
definition.

[Bo Wu] Fixed. Thanks for catching this. The reference of TP has been added
in Section 2.1.
---

4.1

s/VPN PM can provides/VPN PM can provide/

[Bo Wu] Fixed.
---

4.2

s/[RFC9181])./[RFC9181]./

[Bo Wu] Fixed.
---

4.2 etc.

Not sure why 'mac-num' has that name when you use 'ipv4' and 'ipv6'
not 'ipv4-num' and 'ipv6-num'.  This is highly unimportant, but might be
something to fix purely for consistency of appearance.

[Bo Wu] Fixed.
---

4.4

   The 'links' are classified into two types: topology link defined in
   [RFC8345] and abstract link of a VPN between PEs.

Would be nice to give a reference for the abstract link as well.
[Bo Wu] Fixed. The abstract one is defined in this module.

---

4.4

   The performance data of a link is a collection of counters that
   report the performance status.

Perhaps "counters and gauges"?
[Bo Wu] Fixed.

---

5. and  10.

I think that all documents referenced from 'reference' clauses should be
Normative References. I found 3 (4026, 4364, 8571) that are not.
There might be a good reason (if so tell me) or this could be an oversight.
[Bo Wu] Fixed. Sorry for the oversight, not fully corrected. Tom also
pointed this out .

---

5.

It's not really your fault, but I hate to see types redefined, especially
with the same name.

     typedef percentage {
       type decimal64 {
         fraction-digits 5;
         range "0..100";
       }
       description
         "Percentage.";
     }

...appears exactly like this in RFC 8532. This makes me think that it should
possibly be in a common types module somewhere. Possibly nothing you can do
about this at this stage.

Do we have a way of flagging desirable common types to Netmod?

Is there value in you using a different name for this type just to set it in
the context of your work?
[Bo Wu] Thanks for pointing this out. And I need more guidance on this
issue. The definition of percentage in this draft is the same as that in RFC
8532, which is also for "loss-ratio". Actually, this value may derive from
the mechanism of RFC 8532. We have imported "ietf-lime-time-types" from RFC
8532. But "percentage" is defined in "ietf-connectionless-oam". As
"ietf-connectionless-oam" is a device model, I'm not sure if a network
configuration model could import "ietf-connectionless-oam".

---

5.

vpn-pm-type has a case for inter-vpn-access-interface that is empty and
described as a placeholder. And that is all good.

But I expected some text (not a lot) explaining:
- why this is empty
- how/why it might be used in future (presumably through augmentation)

I suspect this belongs in the "VPN PM type" hanging text in Section 4.4 [Bo
Wu] Our consideration is inter-vpn-access-interface PM is VPN-specific
measurement, compared with the tunnel PM that may be shared by multiple
VPNs. And based on this, the measurement could be CE-PE-PE-CE or PE-PE or
other combination. The empty leaf is defined to specify the basic VPN
specific measurement, and allow extension for other measurement
combinations.
Please see whether the new text helps. Here is the text proposed:
"This is a placeholder for inter-vpn-access-interface PM, which is not bound
to a specific VPN access interface. The source or destination VPN access
interface of the measurement can be augmented as needed."

---

OLD
   Appendix A.  Illustrating Examples
NEW
   Appendix A.  Illustrative Examples
OR
   Appendix A.  Examples
END
[Bo Wu] Fixed.

_______________________________________________
OPSAWG mailing list
[e-mail]
https://www.ietf.org/mailman/listinfo/opsawg

_______________________________________________
OPSAWG mailing list
[e-mail]
https://www.ietf.org/mailman/listinfo/opsawg
=