Re: [RTG-DIR] Should JSON code examples start with <CODE BEGINS>? (was RE: Rtgdir early review of draft-ietf-ccamp-transport-nbi-app-statement-12)

[tom petch <[e-mail]>]

From: CCAMP <[e-mail]> on behalf of Acee Lindem (acee) <acee=[e-mail]>
Sent: 29 September 2021 19:42
Cc: [e-mail]; [e-mail]; 'Dhruv Dhody'

Hi Italo,

From: rtg-dir <[e-mail]> on behalf of Italo Busi <[e-mail]>
Date: Wednesday, September 29, 2021 at 1:23 PM
To: 'CCAMP' <[e-mail]>
Cc: Dhruv Dhody <[e-mail]>, Routing Directorate <[e-mail]>, "[e-mail]" <[e-mail]>, "'[e-mail]'" <[e-mail]>, 'Dhruv Dhody' <[e-mail]>
Subject: [RTG-DIR] Should JSON code examples start with <CODE BEGINS>? (was RE: Rtgdir early review of draft-ietf-ccamp-transport-nbi-app-statement-12)

Hi CCAMP WG,

We are addressing Dhruv’s comments on the TNBI draft but we think this comment requires further discussion from the WG:

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

For what concerns us, we are ok to accept the comment/suggestion.

However, I have seen a mail thread on Netmod WG which seems not in favor of adding these markers for instance examples:

https://mailarchive.ietf.org/arch/msg/netmod/FzeQUo4Vr9I1qXDiQflLdUokR98/

We would therefore appreciate any feedbacks from the WG about this issue before updating the draft.

The <CODE BEGINS>/<CODE ENDS> markers are to be used for Normative YANG modules, not examples. See https://datatracker.ietf.org/doc/html/rfc8407#section-3.11

<tp>
That may be the case for YANG but the general case, as for JSON, is different.

CODE BEGINS and CODE ENDS enclose material that is subject to different legal constraints as per the licence on the IETF Trust website.  It allows the material to be used more widely but at the same time imposes constraints on how the material is presented.  In general the Revised BSD licence applies.

My cut and paste will likely fail but the details are on
https://trustee.ietf.org/documents/trust-legal-provisions/tlp-5/
or something close thereto.

So the question is, what licence conditions do you wish to apply to the text in question?

Tom Petch

Thanks,
Acee


Thanks, Italo (on behalf of co-authors)

From: Daniel King [mailto:[e-mail]] On Behalf Of [e-mail]
Sent: giovedì 13 maggio 2021 14:16
To: 'Dhruv Dhody' <[e-mail]>; 'Dhruv Dhody' <[e-mail]>
Cc: [e-mail]; 'CCAMP' <[e-mail]>; [e-mail]
Subject: RE: [RTG-DIR] Rtgdir early review of draft-ietf-ccamp-transport-nbi-app-statement-12

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 <[e-mail]<mailto:[e-mail]>>
Sent: 11 May 2021 12:38
To: Dhruv Dhody <[e-mail]<mailto:[e-mail]>>
Cc: [e-mail]<mailto:[e-mail]>; CCAMP ([e-mail]<mailto:[e-mail]>) <[e-mail]<mailto:[e-mail]>>; [e-mail]<mailto:[e-mail]>
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!

I am pasting the review again, hoping this works!

=====

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