[RTG-DIR] Rtgdir early review of draft-ietf-ccamp-transport-nbi-app-statement-12

Dhruv Dhody via Datatracker <noreply@ietf.org> Tue, 11 May 2021 11:31 UTC

Return-Path: <noreply@ietf.org>
X-Original-To: rtg-dir@ietf.org
Delivered-To: rtg-dir@ietfa.amsl.com
Received: from ietfa.amsl.com (localhost [IPv6:::1]) by ietfa.amsl.com (Postfix) with ESMTP id 3F5E43A11A9; Tue, 11 May 2021 04:31:18 -0700 (PDT)
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 8bit
From: Dhruv Dhody via Datatracker <noreply@ietf.org>
To: rtg-dir@ietf.org
Cc: ccamp@ietf.org, draft-ietf-ccamp-transport-nbi-app-statement.all@ietf.org
X-Test-IDTracker: no
X-IETF-IDTracker: 7.28.0
Auto-Submitted: auto-generated
Precedence: bulk
Message-ID: <162073267819.27623.14933803198192582361@ietfa.amsl.com>
Reply-To: Dhruv Dhody <dd@dhruvdhody.com>
Date: Tue, 11 May 2021 04:31:18 -0700
Archived-At: <https://mailarchive.ietf.org/arch/msg/rtg-dir/KmrYf7QRni5Bl3M0fzwKGg2uFmE>
Subject: [RTG-DIR] Rtgdir early review of draft-ietf-ccamp-transport-nbi-app-statement-12
X-BeenThere: rtg-dir@ietf.org
X-Mailman-Version: 2.1.29
List-Id: Routing Area Directorate <rtg-dir.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/rtg-dir>, <mailto:rtg-dir-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/rtg-dir/>
List-Post: <mailto:rtg-dir@ietf.org>
List-Help: <mailto:rtg-dir-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/rtg-dir>, <mailto:rtg-dir-request@ietf.org?subject=subscribe>
X-List-Received-Date: Tue, 11 May 2021 11:31:18 -0000

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