Ballot for draft-ietf-ippm-ioam-deployment
Yes
No Objection
Note: This ballot was opened for revision 02 and is now closed.
# Éric Vyncke, INT AD, comments for draft-ietf-ippm-ioam-deployment-02 CC @evyncke Thank you for the work put into this document. Please find below some non-blocking COMMENT points (but replies would be appreciated even if only for my own education). Special thanks to Tommy Pauly for the shepherd's detailed write-up including the WG consensus *and* a partial justification of the intended status. Other thanks to Juan-Carlos Zúñiga, the Internet directorate reviewer (at my request), please consider this int-dir review: https://datatracker.ietf.org/doc/review-ietf-ippm-ioam-deployment-02-intdir-telechat-zuniga-2022-12-11/ I hope that this review helps to improve the document, Regards, -éric ## COMMENTS ### Section 2 Should there be references to BIER and GRE as references are given for Geneve and others? ### Section 5.4 Why is the GRE encapsulated described in more details than the other encapsulations ? The encapsulation is also an individual draft, what will happen to this document if the individual draft is changed heavily ? ## Notes This review is in the ["IETF Comments" Markdown format][ICMF], You can use the [`ietf-comments` tool][ICT] to automatically convert this review into individual GitHub issues. [ICMF]: https://github.com/mnot/ietf-comments/blob/main/format.md [ICT]: https://github.com/mnot/ietf-comments
I concur with the shepherd suggestion that the BCP 14 references can be removed. Section 2 defines "E2E", "SFC", "SID", and "SR", but this document doesn't use those terms anywhere.
Proof-of-transit uses methods like nested hashing or nested encryption of the IOAM data or mechanisms such as Shamir's Secret Sharing Schema (SSSS). is there a draft or RFC reference for SSSS? It seems like a the main example of a Proof-of-transit as it is the only mechanism named, but it has no informative reference? NITS: node A is IOAM encapsulating node Node A is the IOAM .... ? (more occurances of missing "the" in this section. that addes that adds ? A deployment can leverage IOAM profiles is to limit the scope of IOAM features Doesn't parse, perhaps "is" should be removed?
Thank you Brian Weis for the SECDIR review. Please review his feedback to improve the clarity of the Security Considerations.
Thanks for addressing my discussion points and comments.
# GEN AD review of draft-ietf-ippm-ioam-deployment-02 CC @larseggert Thanks to Maria Ines Robles for the General Area Review Team (Gen-ART) review (https://mailarchive.ietf.org/arch/msg/gen-art/A39Rwii1B-f8X3VfnjmdQ2Iz_6k). ## Comments ### "Abstract", paragraph 1 ``` In-situ Operations, Administration, and Maintenance (IOAM) collects operational and telemetry information in the packet while the packet traverses a path between two points in the network. This document provides a framework for IOAM deployment and discusses best current practices. ``` Since this document is not a BCP, it might be better to rephrase "discusses best current practices" as "gives deployment guidance" or something like that. ### Section 3, paragraph 4 ``` An "IOAM transit node" updates one or more of the IOAM-Data-Fields. If both the Pre-allocated and the Incremental Trace Option-Types are present in the packet, each IOAM transit node will update at most one of these Option-Types. A transit node does not add new IOAM-Option- Types to a packet, and does not change the IOAM-Data-Fields of an IOAM Edge-to-Edge Option-Type. ``` If each node can unilaterally determine which of the two option types to update, how will the eventual receiver of this information be able to come to a consistent view of he signaled information, if two different values are present in a packet? ## Nits All comments below are about very minor potential issues that you may choose to address in some way - or ignore - as you see fit. Some were flagged by automated tools (via https://github.com/larseggert/ietf-reviewtool), so there will likely be some false positives. There is no need to let me know what you did with these suggestions. ### Typos #### Section 1, paragraph 1 ``` - leveraged where mechanisms using e.g. ICMP do not apply or do not - ^ + leveraged where mechanisms using, e.g., ICMP, do not apply or do not + + ^ + ``` #### Section 3, paragraph 1 ``` - an IOAM domain, e.g. using for example packet filtering methods. The - ------------ + an IOAM domain, e.g., using packet filtering methods. The + + ``` #### Section 7.2, paragraph 1 ``` - encapsulation mechanisms. Similarly, the sematics of the IOAM-Data- + encapsulation mechanisms. Similarly, the semantics of the IOAM-Data- + + ``` #### Section 7.3, paragraph 1 ``` - is expanded at every IOAM node that addes IOAM-Data-Fields. "Pre- - - ``` #### Section 10, paragraph 5 ``` - are subject to IOAM encpasulation and the rate of exported traffic, - - - it is possible to cofine the impact of such attacks. + are subject to IOAM encapsulation and the rate of exported traffic, + + + it is possible to confine the impact of such attacks. + + ``` ### Boilerplate Document still refers to the "Simplified BSD License", which was corrected in the TLP on September 21, 2021. It should instead refer to the "Revised BSD License". ### Outdated references Document references `draft-ietf-ippm-ioam-flags`, but that has been published as `RFC9322`. Document references `draft-ietf-ippm-ioam-conf-state-07`, but `-10` is the latest available revision. Document references `draft-gandhi-spring-ioam-sr-mpls-02`, but `-08` is the latest available revision. Document references `draft-ietf-ippm-ioam-direct-export`, but that has been published as `RFC9326`. ### Grammar/style #### Section 4.1, paragraph 2 ``` s for generic IOAM data include geo-location information (location of the no ^^^^^^^^^^^^ ``` This word is normally spelled as one. #### Section 6, paragraph 2 ``` elds. IOAM-Namespaces support several different uses: o IOAM-Namespaces can ^^^^^^^^^^^^^^^^^ ``` Consider using "several". #### Section 7.1, paragraph 3 ``` -Data-Fields in one layer are independent from IOAM-Data-Fields in another l ^^^^^^^^^^^^^^^^ ``` The usual collocation for "independent" is "of", not "from". Did you mean "independent of"? #### Section 7.1, paragraph 3 ``` tionship, in IOAM, layers are independent from each other and don't assume a ^^^^^^^^^^^^^^^^ ``` The usual collocation for "independent" is "of", not "from". Did you mean "independent of"? #### Section 7.2, paragraph 3 ``` ta-Fields does not exceed the MTU of any of the links in the IOAM domain. In ^^^^^^^^^ ``` Consider simply using "of" instead. #### Section 7.3, paragraph 1 ``` tays within the bounds of the MTU of any of the links in the IOAM domain. Th ^^^^^^^^^ ``` Consider simply using "of" instead. #### Section 7.3, paragraph 1 ``` as to ensure that the minimum MTU of any of the links in the IOAM domain is k ^^^^^^^^^ ``` Consider simply using "of" instead. #### Section 7.6, paragraph 3 ``` Proof of Transit Option-Type (Section Section 4.2) is used for verifying the ^^^^^^^^^^^^^^^ ``` Possible typo: you repeated a word. #### Section 7.6, paragraph 5 ``` single LAN, but span multiple inter-connected sites (for example, using an o ^^^^^^^^^^^^^^^ ``` This word is normally spelled as one. #### Section 8, paragraph 2 ``` revent IOAM traffic from leaking outside of the IOAM domain, and prevent IOA ^^^^^^^^^^ ``` This phrase is redundant. Consider using "outside". ## Notes This review is in the ["IETF Comments" Markdown format][ICMF], You can use the [`ietf-comments` tool][ICT] to automatically convert this review into individual GitHub issues. Review generated by the [`ietf-reviewtool`][IRT]. [ICMF]: https://github.com/mnot/ietf-comments/blob/main/format.md [ICT]: https://github.com/mnot/ietf-comments [IRT]: https://github.com/larseggert/ietf-reviewtool