In Situ Operations, Administration, and Maintenance (IOAM) Deployment
draft-ietf-ippm-ioam-deployment-05

Thank you Brian Weis for the SECDIR review.  Please review his feedback to improve the clarity of the Security Considerations.

# 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

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?

Thanks for addressing my discussion points and comments.