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.
---
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
---
2.1
OLD
SLA Service Level Agreements
NEW
SLA Service Level Agreement
END
---
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.
---
3.1
s/dynamic-changing/dynamic/
---
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
---
4.
Would it be more consistent if the box on the right of Figure 2 showed
"ietf-network-vpn-pm"?
---
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
---
4.1
s/topologies are both built/topologies are built/
---
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.
---
4.1
s/VPN PM can provides/VPN PM can provide/
---
4.2
s/[RFC9181])./[RFC9181]./
---
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.
---
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.
---
4.4
The performance data of a link is a collection of counters that
report the performance status.
Perhaps "counters and gauges"?
---
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.
---
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?
---
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
---
OLD
Appendix A. Illustrating Examples
NEW
Appendix A. Illustrative Examples
OR
Appendix A. Examples
END
_______________________________________________
OPSAWG mailing list
[email protected]
https://www.ietf.org/mailman/listinfo/opsawg
