Echo Request/Reply for Enabled In Situ OAM (IOAM) Capabilities
draft-ietf-ippm-ioam-conf-state-10

# Internet AD comments for draft-ietf-ippm-ioam-conf-state-07
CC @ekline

I support John's request for clarification.

## Comments

### S3.1

* "MUST send an echo reply without IOAM capabilities or no echo reply"

  I think I understand the intent, and overall I think this text is likely
  to not be an issue in practice.  But I'll note that anything "MUST" where
  ICMP is concerned may not exactly be enforceable.

## Nits

### S2.2

* Maybe consider adding DEX to this list (it's also fully expanded on first
  use in S1, but it might be nice for ease of reference).

### S3.1, S3.2

* /depends on the specific environment it is applied at/ ->
  /depends on the specific deployment environment/ maybe?

Thanks for the revisions. I have one further comment that occurred to me
in looking at the revised text:

                                                Note that since the
   Default-Namespace-ID of 0x0000 is mandated to appear first in the
   list, if it appears any trailing 0x0000 octets must therefore be
   padding and MUST be disregarded.
   
it seems to me it can be tightened up a little more, as in

   Since the Default-Namespace-ID of 0x0000 is mandated to appear first in
   the list, any other occurrences of 0x0000 MUST be disregarded. 
   
That is, unless you want to mandate some kind of error response to
0x0000 appearing in a non-first, non-trailing position.

I think the two IANA registries being created in this document are under-specified.  I suggest enumerating the fields in each entry, describing what they're for, and what their valid values are, before providing the initial values.  As it stands, IANA has to read other parts of the document to infer valid values for the first column in both cases.

I also concur with Lars' point about the limited set of available values in each registry, but I see the latest version dealt with that.

Thanks to Chris Lonvick for the SecDir review. He raised a few points that are worth addressing: https://datatracker.ietf.org/doc/review-ietf-ippm-ioam-conf-state-05-secdir-lc-lonvick-2022-09-30/


   Reserved field is reserved for future use and MUST be set to zero.

Can this be extended to say "and MUST be ignored when non-zero" ? There is more than one of these.


Why is the SoP field specified as two bits from a reserved field, when it only uses one bit? Why not just take one bit from the reserved field?

Thank you to Chris Lonvick for the SECDIR review.

Thank you for addressing my DISCUSS and COMMENT feedback.

Thank you for your reply, addressing my DISCUSS (https://mailarchive.ietf.org/arch/msg/ippm/93f6B7Xb3zwj-WVqhe_ygbhAjKk/)

I am still concerned about what feels like an increase in things like filtering requirements in documents, and what is actually implementable on network devices - see https://mailarchive.ietf.org/arch/msg/ippm/1qNDrh-AiYY4a17KEqn2bEWNfLI/ for more background. I understand that it is not feasible to stop developing new protocols until all routers support $whatever, but it is still a topic which causes my discomfort. 

However, this is a larger issue than just this document, and so I've cleared my DISCUSS and changed to NoObjection...

Thank you again for addressing my ballot, and doing it so quickly.

I support Roman's and John's Discuss.

# Éric Vyncke, INT AD, comments for draft-ietf-ippm-ioam-conf-state-07
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 Marcus Ihlar for the shepherd's detailed write-up including the WG consensus and the justification of the intended status. 

I hope that this review helps to improve the document,

Regards,

-éric

## COMMENTS

### Abstract

This is a very long sentence... Consider shortening it.

The `echo request/reply mechanisms used in IPv6 ...` would benefit of being qualified (perhaps with 'generic' or 'specified' ?).

### Section 1

The IPv6 node information query, while sent over ICMPv6, is not really an 'echo/request' exchange, hence the title of this RFC is not really adequate. I have no suggestion though.

### Section 2.2

When defining "TTL" please also add that this is the hop-limit field in the IPv6 header (which has no TTL field).

### Section 3.2.1 and other sections including 3.2.6 "Must be zero"

Please do not forget to specify the receiver's behavior when receiving reserved bits that are no all 0.

### Section 3.2.3

Explaining what "SoP" is would ease the reading.


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

The Abstract (and the corresponding text in the Introduction) is misleading:

   This document describes an extension to the echo request/reply
   mechanisms used in IPv6 (including Segment Routing with IPv6 data
   plane (SRv6)), MPLS (including Segment Routing with MPLS data plane
   (SR-MPLS)), Service Function Chain (SFC) and Bit Index Explicit
   Replication (BIER) environments, which can be used within the In situ
   Operations, Administration, and Maintenance (IOAM) domain, allowing
   the IOAM encapsulating node to discover the enabled IOAM capabilities
   of each IOAM transit and IOAM decapsulating node.

This document describes an extension that *could be used* as mentioned above, but it doesn't specify "an extension to the echo request/reply mechanisms used in..."  Please clarify that the document describes a generic format.

The Introduction later clarifies that "specification details for these different echo request/reply protocols are outside the scope of this document".  However, if the mentioned applications are expected to be the users of this specification, the corresponding WGs (6man, spring, mpls, sfc, and bier) should have been consulted.  I couldn't find anything in the archives to show interaction.

# GEN AD review of draft-ietf-ippm-ioam-conf-state-07

CC @larseggert

Thanks to Gyan S. Mishra for the General Area Review Team (Gen-ART) review
(https://mailarchive.ietf.org/arch/msg/gen-art/wA_8u8-xjQ7sl29P3Qul3W_U_aw).

## Comments

### Section 5.1, paragraph 3
```
     0b01 - 0b11 are available for assignment via RFC Required process as
     per [RFC8126].
```
Since there are only three codepoints remaining, would a stricter assignment
policy not make sense?

### Section 5.2, paragraph 3
```
     0b11 is available for assignment via RFC Required process as per
     [RFC8126].
```
Since there is only a single codepoint remaining, would a stricter assignment
policy not make sense?

## 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 3.1, paragraph 4
```
-    begining of the list of IOAM Namespace-IDs.  The IOAM encapsulating
+    beginning of the list of IOAM Namespace-IDs.  The IOAM encapsulating
+        +
```

#### Section 6, paragraph 9
```
-    Except for what's described above, the securiy issues discussed in
+    Except for what's described above, the security issues discussed in
+                                                 +
```

### Grammar/style

#### Section 3.2.2, paragraph 9
```
C9197]. This document defines SoP as follow: 0b00 means 64-bit "PktID" and 6
                                  ^^^^^^^^^
```
Did you mean "as follows"?

#### Section 3.2.6, paragraph 1
```
th TTL equal to 2 to reach the second nearest node, which also may be an IOAM
                               ^^^^^^^^^^^^^^
```
It appears that a hyphen is missing.

#### Section 4, paragraph 1
```
s per [RFC8126]. The subsequent sub-sections detail the registries herein co
                                ^^^^^^^^^^^^
```
This word is normally spelled as one.

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

Discuss cleared.