Subject: [CCAMP] Rtgdir early review of draft-ietf-ccamp-transport-nbi-app-statement-12

From: Dhruv Dhody via Datatracker <[email-masked]>
Date: Tue, 11 May 2021 04:31:18 -0700

Reviewer: Dhruv Dhody
Review result: Has Issues

Subject:RtgDir Early review: draft-ietf-ccamp-transport-nbi-app-statement

Hello

I have been selected to do a routing directorate “early” review of this draft.
https://datatracker.ietf.org/doc/draft-ietf-ccamp-transport-nbi-app-statement/

The routing directorate will, on request from the working group chair, perform
an “early” review of a draft before it is submitted for publication to the
IESG. The early review can be performed at any time during the draft’s lifetime
as a working group document. The purpose of the early review depends on the
stage that the document has reached.

This review request might have been marked as an Early review by mistake. The
review stands nevertheless.

For more information about the Routing Directorate, please see
http://trac.tools.ietf.org/area/rtg/trac/wiki/RtgDir

Document: draft-ietf-ccamp-transport-nbi-app-statement
Reviewer: Dhruv Dhody
Review Date: 2021-05-10
Intended Status: Informational

Summary:
I have some minor concerns about this document that I think should be resolved
before it is submitted to the IESG.

Overview:
I have done a review of this I-D based on my understanding of ACTN and the
related YANG models. Overall, I found the document difficult to review. I was
scrolling up and down the document and finally had to keep the document open on
multiple screens to cross-reference various figures to understand the text. I
wonder if there is a way to improve readability; can't think of anything
obvious...

Given the complexity of the subject matter, I do believe that the authors have
done a good job. I hope the document would be useful for the implementors.

I have some concerns about the JSON Code which I have listed first, followed by
other minor comments and nits!

JSON Code Issues:
- This document provides a 'non-standard' JSON Code in the appendix and
provides personal GitHub links on how to validate that JSON code. While the
JSON code is common in the YANG documents, this document introduces what looks
like a new way to add comments. Question to John (as responsible AD): Does this
approach has any IETF process issue? The authors have done a decent job in
explaining the need for it and how to handle it. - I tried to set up the docker
validate tool as per https://github.com/GianmarcoBruno/json-yang ; I was not
able to make it run and gave up after a while. :( The instruction on GitHub
README.md does not match with what's in the docker image include the options. -
JSON code is based on an older version of YANG models. It would be a good idea
to update it to the latest. Related question: YANG models are normative
references (which seems the right thing to do), so this document should be
published together or after those YANG models are ready and thus some
coordination across WG is required. Again something for John to consider. -
JSON code should start with <CODE BEGINS>. Example - <CODE BEGINS> file
"mpi1-otn-topology.json", so that the json code can be stripped from the
document. - Should the GitHub repo mentioned in the appendix moved under CCAMP
WG's GitHub? That would be giving some control over the longtime validity of
these URLs. Should they be added as references in the document as well. -
Update this -
  ========== NOTE: '\\' line wrapping per BCP XXX (RFC XXXX) ==========
  It was published as an informational RFC 8792, so remove the BCP and update
  the RFC number!
- On first reading I did not understand the TTP ID naming convention used as per
  "// __COMMENT__ tunnel-tp-id": "AN1-1 TTP-ID (1 ->\
   \ 0x01 -> 'AQ==')"
  Perhaps it could be mentioned somewhere that AQ== is the base64
  representation of 0x01.
- Appendix B.1.1, Figure 3 - "OTN Abstract Topology exposed at MPI1 (MPI1 OTN
Topology)" does not have AN1-8 but the JSON does. I think that might be a
mistake?

Others:
- Title, "Transport Northbound Interface Applicability Statement" - not sure
about the title, it does not match with the content of the I-D. - Section 2,
the definition of connection is not clear. The use of connection/LSP multiple
times is also distracting. Similarly "Link: A link, or a physical link, ..."
reads weird.  The note in section 2 needs to be handled at this stage. Note
that [TE-TUTORIAL] is already marked informative in the -12 version! - Section
3.1, the notations need to explain what {} means. - Section 3.2 is better
suited for the appendix as the JSON code exists only in the appendix. - Section
4.6 is not clear to me. Should one refer to RFC 8640, 8641, and 8650 as far as
the datastore update is concerned? Clarify the term 'alarm', do you mean YANG
notification or RFC 8623? This section needs some work. - Section 5.1.1, the
text at the very end of this section about JSON code is much useful closer to
the code in the appendix. - Section 5.1.4, we need to all add some text to
describe for merging of multiple instances of TE topology from the same PNC.
For example, the merging of OTN (Figure 3) and ETH (Figure 4) should lead to
Network domain 1 topology in Figure 6. - Section 5.2.1, is there any reference
that can be provided about the handling of TTP and
route-object-include-exclude? Or is this document specifying how they should be
handled? This is applicable in other instances as well. - Section 5.2.2,
"...abstracting S3-1 and S18-3 TTPs", please show S18-3 in the figure,
otherwise, it is not clear which TTP it is!

Nits:
- Authors address at the end of the document do not match the front
- Please make abstract as the first section as per the style guide
https://www.rfc-editor.org/rfc/rfc7322.html#section-4 - It is expected to use
https in the status of the memo - Expand on first use: ODU, EPL, EVPL, OTN,
LSP, FC, STM-n, L1CSM, L2SM, VN, - Section 2, s/swith/switch/ ;
s/failurs/failures/ - Section 2, adding a reference to documents where these
terms come from would help. It's already done for some terms! - Section 4.2,
s/[RFC8453] Provides/[RFC8453] provides/ ; s/PNcs/PNCs/ - Section 4.3, Ri (PKT
-> foo) and Rj (foo -> PKT) does not align to the format in Section 3.1. Should
this be Ri [(PKT) -> foo] and Rj [foo -> (PKT)] instead? - Section 4.7, order
of closing brackets is incorrect OLD
      R1 [(PKT) -> ODU2], S3[(ODU2]), S1 [(ODU2]), S2[(ODU2]),
      S8 [(ODU2]), S12[(ODU2]), S15 [(ODU2]), S18[(ODU2]), R8 [ODU2 ->
      (PKT)]
NEW
      R1 [(PKT) -> ODU2], S3[(ODU2)], S1 [(ODU2)], S2 [(ODU2)],
      S8 [(ODU2)], S12[(ODU2)], S15 [(ODU2)], S18[(ODU2)], R8 [ODU2 ->
      (PKT)]
- Section 5.2, s/models is (e.g., L1CSM, L2SM, VN) is outside/models (e.g.,
L1CSM, L2SM, VN) is outside/ - Section 5.2, The text says ".. (steps 2.1, 2.2
and 2.3 in Figure 7)". There are no steps 2.1, 2.2, and 2.3 in the figure. -
Section 5.2.1, s/OUD2/ODU2/ - Section 5.2.1.1, s/terminating on AN-1 and AN1-2
LTPs/terminating on AN1-1 and AN1-2 LTPs/ - Section 5.2.2, s/Appendix Error!
Reference source not found./Appendix B.2.3/; s/OUD2/ODU2/; s/[ETH ->
(ODU)]/[ETH -> (ODU2)]/g; - Section 5.2.2.1, s/Tunnel between the AN1-1 and
AN1-2/Tunnel between the AN1-1 and AN1-8/ - Section 5.2.3, s/[STM-64 ->
(ODU)]/[STM-64 -> (ODU2)]/ - Section 5.2.4, "..physical nodes S3, S1, S2, S31,
S33, S15 and S18.." -> S34 is missing!!! - Section 5.3.2, s/S31 and D34/S31 and
S34/ ; s/lin/link/ ; - Appendix A, s/an Internet-Draft/this document/

Thanks!
Dhruv

---

From: Dhruv Dhody <[email-masked]>
Date: Tue, 11 May 2021 17:08:06 +0530

Hi,

The formatting seems to be off when I uploaded the text file in the
review tool.
It looks okay in the datatracker :
https://datatracker.ietf.org/doc/review-ietf-ccamp-transport-nbi-app-statement-12-rtgdir-early-dhody-2021-05-11/
; seems to be an issue with the email then!

I am pasting the review again, hoping this works!

=====

Hello

I have been selected to do a routing directorate “early” review of this
draft.
https://datatracker.ietf.org/doc/draft-ietf-ccamp-transport-nbi-app-statement/

The routing directorate will, on request from the working group chair,
perform an “early” review of a draft before it is submitted for publication
to the IESG. The early review can be performed at any time during the
draft’s lifetime as a working group document. The purpose of the early
review depends on the stage that the document has reached.

This review request might have been marked as an Early review by mistake.
The review stands nevertheless.

Summary:
I have some minor concerns about this document that I think should be
resolved before it is submitted to the IESG.

Overview:
I have done a review of this I-D based on my understanding of ACTN and the
related YANG models. Overall, I found the document difficult to review. I
was scrolling up and down the document and finally had to keep the document
open on multiple screens to cross-reference various figures to understand
the text. I wonder if there is a way to improve readability; can't think of
anything obvious...

Given the complexity of the subject matter, I do believe that the authors
have done a good job. I hope the document would be useful for the
implementors.

I have some concerns about the JSON Code which I have listed first,
followed by other minor comments and nits!

JSON Code Issues:
- This document provides a 'non-standard' JSON Code in the appendix and
provides personal GitHub links on how to validate that JSON code. While the
JSON code is common in the YANG documents, this document introduces what
looks like a new way to add comments. Question to John (as responsible AD):
Does this approach has any IETF process issue? The authors have done a
decent job in explaining the need for it and how to handle it.
- I tried to set up the docker validate tool as per
https://github.com/GianmarcoBruno/json-yang ; I was not able to make it run
and gave up after a while. :( The instruction on GitHub README.md does not
match with what's in the docker image include the options.
- JSON code is based on an older version of YANG models. It would be a good
idea to update it to the latest. Related question: YANG models are
normative references (which seems the right thing to do), so this document
should be published together or after those YANG models are ready and thus
some coordination across WG is required. Again something for John to
consider.
- JSON code should start with <CODE BEGINS>. Example - <CODE BEGINS> file
"mpi1-otn-topology.json", so that the json code can be stripped from the
document.
- Should the GitHub repo mentioned in the appendix moved under CCAMP WG's
GitHub? That would be giving some control over the longtime validity of
these URLs. Should they be added as references in the document as well.
- Update this -
  ========== NOTE: '\\' line wrapping per BCP XXX (RFC XXXX) ==========
  It was published as an informational RFC 8792, so remove the BCP and
update the RFC number!
- On first reading I did not understand the TTP ID naming convention used
as per
  "// __COMMENT__ tunnel-tp-id": "AN1-1 TTP-ID (1 ->\
   \ 0x01 -> 'AQ==')"
  Perhaps it could be mentioned somewhere that AQ== is the base64
representation of 0x01.
- Appendix B.1.1, Figure 3 - "OTN Abstract Topology exposed at MPI1 (MPI1
OTN Topology)" does not have AN1-8 but the JSON does. I think that might be
a mistake?

Others:
- Title, "Transport Northbound Interface Applicability Statement" - not
sure about the title, it does not match with the content of the I-D.
- Section 2, the definition of connection is not clear. The use of
connection/LSP multiple times is also distracting. Similarly "Link: A link,
or a physical link, ..." reads weird.  The note in section 2 needs to be
handled at this stage. Note that [TE-TUTORIAL] is already marked
informative in the -12 version!
- Section 3.1, the notations need to explain what {} means.
- Section 3.2 is better suited for the appendix as the JSON code exists
only in the appendix.
- Section 4.6 is not clear to me. Should one refer to RFC 8640, 8641, and
8650 as far as the datastore update is concerned? Clarify the term 'alarm',
do you mean YANG notification or RFC 8623? This section needs some work.
- Section 5.1.1, the text at the very end of this section about JSON code
is much useful closer to the code in the appendix.
- Section 5.1.4, we need to all add some text to describe for merging of
multiple instances of TE topology from the same PNC. For example, the
merging of OTN (Figure 3) and ETH (Figure 4) should lead to Network domain
1 topology in Figure 6.
- Section 5.2.1, is there any reference that can be provided about the
handling of TTP and route-object-include-exclude? Or is this document
specifying how they should be handled? This is applicable in other
instances as well.
- Section 5.2.2, "...abstracting S3-1 and S18-3 TTPs", please show S18-3 in
the figure, otherwise, it is not clear which TTP it is!

Nits:
- Authors address at the end of the document do not match the front
- Please make abstract as the first section as per the style guide
https://www.rfc-editor.org/rfc/rfc7322.html#section-4
- It is expected to use https in the status of the memo
- Expand on first use: ODU, EPL, EVPL, OTN, LSP, FC, STM-n, L1CSM, L2SM, VN,
- Section 2, s/swith/switch/ ; s/failurs/failures/
- Section 2, adding a reference to documents where these terms come from
would help. It's already done for some terms!
- Section 4.2, s/[RFC8453] Provides/[RFC8453] provides/ ; s/PNcs/PNCs/
- Section 4.3, Ri (PKT -> foo) and Rj (foo -> PKT) does not align to the
format in Section 3.1. Should this be Ri [(PKT) -> foo] and Rj [foo ->
(PKT)] instead?
- Section 4.7, order of closing brackets is incorrect
OLD
      R1 [(PKT) -> ODU2], S3[(ODU2]), S1 [(ODU2]), S2[(ODU2]),
      S8 [(ODU2]), S12[(ODU2]), S15 [(ODU2]), S18[(ODU2]), R8 [ODU2 ->
      (PKT)]
NEW
      R1 [(PKT) -> ODU2], S3[(ODU2)], S1 [(ODU2)], S2 [(ODU2)],
      S8 [(ODU2)], S12[(ODU2)], S15 [(ODU2)], S18[(ODU2)], R8 [ODU2 ->
      (PKT)]
- Section 5.2, s/models is (e.g., L1CSM, L2SM, VN) is outside/models (e.g.,
L1CSM, L2SM, VN) is outside/
- Section 5.2, The text says ".. (steps 2.1, 2.2 and 2.3 in Figure 7)".
There are no steps 2.1, 2.2, and 2.3 in the figure.
- Section 5.2.1, s/OUD2/ODU2/
- Section 5.2.1.1, s/terminating on AN-1 and AN1-2 LTPs/terminating on
AN1-1 and AN1-2 LTPs/
- Section 5.2.2, s/Appendix Error! Reference source not found./Appendix
B.2.3/; s/OUD2/ODU2/; s/[ETH -> (ODU)]/[ETH -> (ODU2)]/g;
- Section 5.2.2.1, s/Tunnel between the AN1-1 and AN1-2/Tunnel between the
AN1-1 and AN1-8/
- Section 5.2.3, s/[STM-64 -> (ODU)]/[STM-64 -> (ODU2)]/
- Section 5.2.4, "..physical nodes S3, S1, S2, S31, S33, S15 and S18.." ->
S34 is missing!!!
- Section 5.3.2, s/S31 and D34/S31 and S34/ ; s/lin/link/ ;
- Appendix A, s/an Internet-Draft/this document/

Thanks!
Dhruv

---

From: <[email-masked]>
Date: Thu, 13 May 2021 13:15:49 +0100

Hi Dhruv, 

Thanks very much for the detailed review! The comments and suggestions are most welcome. 

Re: Draft Naming 

Let me chat with the co-authors and get back to you and the WG with a response. 

For the other items, we will address in the next version. 

BR, Dan. 

From: Dhruv Dhody <[email-masked]> 
Sent: 11 May 2021 12:38
To: Dhruv Dhody <[email-masked]>
Cc: [email-masked]; CCAMP ([email-masked]) <[email-masked]>; [email-masked]
Subject: Re: [RTG-DIR] Rtgdir early review of draft-ietf-ccamp-transport-nbi-app-statement-12

Hi, 

The formatting seems to be off when I uploaded the text file in the review tool. It looks okay in the datatracker : https://datatracker.ietf.org/doc/review-ietf-ccamp-transport-nbi-app-statement-12-rtgdir-early-dhody-2021-05-11/ ; seems to be an issue with the email then! 

===== 

Hello

The routing directorate will, on request from the working group chair, perform an “early” review of a draft before it is submitted for publication to the IESG. The early review can be performed at any time during the draft’s lifetime as a working group document. The purpose of the early review depends on the stage that the document has reached.

This review request might have been marked as an Early review by mistake. The review stands nevertheless.

For more information about the Routing Directorate, please see http://trac.tools.ietf.org/area/rtg/trac/wiki/RtgDir

Summary:
I have some minor concerns about this document that I think should be resolved before it is submitted to the IESG.

Overview:
I have done a review of this I-D based on my understanding of ACTN and the related YANG models. Overall, I found the document difficult to review. I was scrolling up and down the document and finally had to keep the document open on multiple screens to cross-reference various figures to understand the text. I wonder if there is a way to improve readability; can't think of anything obvious...  

Given the complexity of the subject matter, I do believe that the authors have done a good job. I hope the document would be useful for the implementors.

I have some concerns about the JSON Code which I have listed first, followed by other minor comments and nits!  

JSON Code Issues:
- This document provides a 'non-standard' JSON Code in the appendix and provides personal GitHub links on how to validate that JSON code. While the JSON code is common in the YANG documents, this document introduces what looks like a new way to add comments. Question to John (as responsible AD): Does this approach has any IETF process issue? The authors have done a decent job in explaining the need for it and how to handle it.   
- I tried to set up the docker validate tool as per https://github.com/GianmarcoBruno/json-yang ; I was not able to make it run and gave up after a while. :( The instruction on GitHub README.md does not match with what's in the docker image include the options. 
- JSON code is based on an older version of YANG models. It would be a good idea to update it to the latest. Related question: YANG models are normative references (which seems the right thing to do), so this document should be published together or after those YANG models are ready and thus some coordination across WG is required. Again something for John to consider.
- JSON code should start with <CODE BEGINS>. Example - <CODE BEGINS> file "mpi1-otn-topology.json", so that the json code can be stripped from the document.  
- Should the GitHub repo mentioned in the appendix moved under CCAMP WG's GitHub? That would be giving some control over the longtime validity of these URLs. Should they be added as references in the document as well.  
- Update this -
  ========== NOTE: '\\' line wrapping per BCP XXX (RFC XXXX) ==========
  It was published as an informational RFC 8792, so remove the BCP and update the RFC number!
- On first reading I did not understand the TTP ID naming convention used as per
  "// __COMMENT__ tunnel-tp-id": "AN1-1 TTP-ID (1 ->\
   \ 0x01 -> 'AQ==')"
  Perhaps it could be mentioned somewhere that AQ== is the base64 representation of 0x01.  
- Appendix B.1.1, Figure 3 - "OTN Abstract Topology exposed at MPI1 (MPI1 OTN Topology)" does not have AN1-8 but the JSON does. I think that might be a mistake?

Others:
- Title, "Transport Northbound Interface Applicability Statement" - not sure about the title, it does not match with the content of the I-D.  
- Section 2, the definition of connection is not clear. The use of connection/LSP multiple times is also distracting. Similarly "Link: A link, or a physical link, ..." reads weird.  The note in section 2 needs to be handled at this stage. Note that [TE-TUTORIAL] is already marked informative in the -12 version!  
- Section 3.1, the notations need to explain what {} means.
- Section 3.2 is better suited for the appendix as the JSON code exists only in the appendix.
- Section 4.6 is not clear to me. Should one refer to RFC 8640, 8641, and 8650 as far as the datastore update is concerned? Clarify the term 'alarm', do you mean YANG notification or RFC 8623? This section needs some work.
- Section 5.1.1, the text at the very end of this section about JSON code is much useful closer to the code in the appendix.
- Section 5.1.4, we need to all add some text to describe for merging of multiple instances of TE topology from the same PNC. For example, the merging of OTN (Figure 3) and ETH (Figure 4) should lead to Network domain 1 topology in Figure 6.
- Section 5.2.1, is there any reference that can be provided about the handling of TTP and route-object-include-exclude? Or is this document specifying how they should be handled? This is applicable in other instances as well.
- Section 5.2.2, "...abstracting S3-1 and S18-3 TTPs", please show S18-3 in the figure, otherwise, it is not clear which TTP it is!

Nits:
- Authors address at the end of the document do not match the front
- Please make abstract as the first section as per the style guide https://www.rfc-editor.org/rfc/rfc7322.html#section-4
- It is expected to use https in the status of the memo
- Expand on first use: ODU, EPL, EVPL, OTN, LSP, FC, STM-n, L1CSM, L2SM, VN,
- Section 2, s/swith/switch/ ; s/failurs/failures/
- Section 2, adding a reference to documents where these terms come from would help. It's already done for some terms!
- Section 4.2, s/[RFC8453] Provides/[RFC8453] provides/ ; s/PNcs/PNCs/
- Section 4.3, Ri (PKT -> foo) and Rj (foo -> PKT) does not align to the format in Section 3.1. Should this be Ri [(PKT) -> foo] and Rj [foo -> (PKT)] instead?
- Section 4.7, order of closing brackets is incorrect
OLD
      R1 [(PKT) -> ODU2], S3[(ODU2]), S1 [(ODU2]), S2[(ODU2]),
      S8 [(ODU2]), S12[(ODU2]), S15 [(ODU2]), S18[(ODU2]), R8 [ODU2 ->
      (PKT)]
NEW
      R1 [(PKT) -> ODU2], S3[(ODU2)], S1 [(ODU2)], S2 [(ODU2)],
      S8 [(ODU2)], S12[(ODU2)], S15 [(ODU2)], S18[(ODU2)], R8 [ODU2 ->
      (PKT)]
- Section 5.2, s/models is (e.g., L1CSM, L2SM, VN) is outside/models (e.g., L1CSM, L2SM, VN) is outside/
- Section 5.2, The text says ".. (steps 2.1, 2.2 and 2.3 in Figure 7)". There are no steps 2.1, 2.2, and 2.3 in the figure.
- Section 5.2.1, s/OUD2/ODU2/
- Section 5.2.1.1, s/terminating on AN-1 and AN1-2 LTPs/terminating on AN1-1 and AN1-2 LTPs/  
- Section 5.2.2, s/Appendix Error! Reference source not found./Appendix B.2.3/; s/OUD2/ODU2/; s/[ETH -> (ODU)]/[ETH -> (ODU2)]/g;
- Section 5.2.2.1, s/Tunnel between the AN1-1 and AN1-2/Tunnel between the AN1-1 and AN1-8/
- Section 5.2.3, s/[STM-64 -> (ODU)]/[STM-64 -> (ODU2)]/
- Section 5.2.4, "..physical nodes S3, S1, S2, S31, S33, S15 and S18.." -> S34 is missing!!!
- Section 5.3.2, s/S31 and D34/S31 and S34/ ; s/lin/link/ ;
- Appendix A, s/an Internet-Draft/this document/

Thanks! 
Dhruv