Review of draft-ietf-teas-yang-path-computation-18
Minor
Abstract
Abstract only mentions "intra-domain paths", whereas the documents do talk about inter-layer and inter-domain path computation as well.
IBSB> Ok, Fixed
Section 1.3
Remove inet from the table
IBSB> Ok, Fixed
Section 2
You state "Use cases d) and e)..." without stating what d) and e) is. Perhaps use section numbers instead?
IBSB> Ok, Fixed
Section 2.1
Figure 1 and Figure 2 are completely different network topologies, but the text gives an impression that they are related. I suggest making it clear that Figure 2 shows a different network topology.
IBSB> Ok, Fixed
Section 2.4
Please update "[RFC5441] has defined the Virtual Source Path Tree (VSPT) TLV within PCE Reply Object..." to "[RFC5441] has defined the Virtual Source Path Tree (VSPT) flag within RP (Request Parameters) object..."
IBSB> Ok, Fixed
Section 5.3.1
Could you explain the rationale for the fact that the requested path can only be a primary path for non-transit cases?
IBSB> Rephrased to clarify that in non-transit case the only path must be a primary path.
Section 5.3.2
The phrase "empty ERO" makes sense for PCEP, but not for YANG. Please rephrase.
IBSB> Ok, Fixed
Section 6.2 (YANG Model)
WG Web: should point to datatracker instead of tools page
IBSB> Ok, Fixed
Authors/Editors mismatch between the front page and YANG
IBSB> Our understanding is that there are no requirements for an exact match between the editors of YANG and the front page editors of the RFC
Reference for svec-metric-cumul-hop should be RFC5541
IBSB> Ok, Fixed
Is the use of the term "configuration" inside grouping requested-state correct? After all, this is part of the RPC input, can that be called configuration?
IBSB> We think it is correct since the grouping contains the attributes to configure how the transient state should be reported (e.g., the expiration timer)
For objective-function-type in grouping synchronization-optimization, the use of "default "te-types:of-minimize-cost-path" is a problem, because this OF is not applicable to SVEC. I don't think you need a default here!
IBSB> We need to further investigate whether the SVEC objective functions the same or different from the path objective functions: see issue #107
I am not able to make sense of this when statement. Is this when the statement suppose to tell it is transit? How?
choice path-role {
when 'not (./source) and not (./destination) and
not (./src-tunnel-tp-id) and
not (./dst-tunnel-tp-id)' {
IBSB> The choice applies only to transit tunnels (i.e., when the source, src-tp-id, destination and dst-tp-id attributes are empty). See description in section 5.3.1 of this draft and in section 5.2.1 of draft-ietf-ccamp-transport-nbi-app-statement-15. Added some more details to the YANG description.
This description is incorrect in the context of the container, it is the description of the leaf primary-path-name that follows -
container secondary-path {
presence
"Indicates that the requested path is a secondary
path.";
description
"The name of the primary path which the requested
primary reverse path belongs to.";
The IP address in the example should be from the range reserved for documentation. See RFC5737. I see this issue exists in draft-ietf-teas-yang-te-29 as well.
Review of draft-ietf-teas-yang-path-computation-18
Minor
Abstract
IBSB> Ok, Fixed
Section 1.3
IBSB> Ok, Fixed
Section 2
IBSB> Ok, Fixed
Section 2.1
IBSB> Ok, Fixed
Section 2.4
IBSB> Ok, Fixed
Section 5.3.1
IBSB> Rephrased to clarify that in non-transit case the only path must be a primary path.
Section 5.3.2
IBSB> Ok, Fixed
Section 6.2 (YANG Model)
IBSB> Ok, Fixed
IBSB> Our understanding is that there are no requirements for an exact match between the editors of YANG and the front page editors of the RFC
IBSB> Ok, Fixed
IBSB> Ok, Fixed
IBSB> We think it is correct since the grouping contains the attributes to configure how the transient state should be reported (e.g., the expiration timer)
IBSB> We need to further investigate whether the SVEC objective functions the same or different from the path objective functions: see issue #107
IBSB> The choice applies only to transit tunnels (i.e., when the source, src-tp-id, destination and dst-tp-id attributes are empty). See description in section 5.3.1 of this draft and in section 5.2.1 of draft-ietf-ccamp-transport-nbi-app-statement-15. Added some more details to the YANG description.
IBSB> OK: fixed c/primary reverse path/secondary-path/
Appendix A
IBSB> Ok, to be fixed in https://github.com/rvilalta/ietf-te-path-computation/issues/108 (aligning the changes to the IP address within the TE tunnel draft)
Nits
IBSB> Ok, fixed