Re: [ippm] Éric Vyncke's Discuss on draft-ietf-ippm-ioam-flags-09: (with DISCUSS and COMMENT)

[Tal Mizrahi <[e-mail]>]

Dear Eric,

Many thanks for the feedback.

Please see the responses to the DISCUSS issues below, marked [TM].

Cheers,
Tal.


On Wed, Jun 29, 2022 at 8:42 AM Éric Vyncke via Datatracker
<[e-mail]> wrote:
>
> Éric Vyncke has entered the following ballot position for
> draft-ietf-ippm-ioam-flags-09: Discuss
>
> When responding, please keep the subject line intact and reply to all
> email addresses included in the To and CC lines. (Feel free to cut this
> introductory paragraph, however.)
>
>
> Please refer to https://www.ietf.org/about/groups/iesg/statements/handling-ballot-positions/
> for more information about how to handle DISCUSS and COMMENT positions.
>
>
> The document, along with other ballot positions, can be found here:
> https://datatracker.ietf.org/doc/draft-ietf-ippm-ioam-flags/
>
>
>
> ----------------------------------------------------------------------
> DISCUSS:
> ----------------------------------------------------------------------
>
> # Éric Vyncke, INT AD, comments for draft-ietf-ippm-ioam-flags-09
> CC @evyncke
>
> Thank you for the work put into this document.
>
> Please find below some blocking DISCUSS points (easy to address as it is about
> clarifications), some non-blocking COMMENT points (but replies would be
> appreciated even if only for my own education), and some nits.
>
> Thanks to Pascal Thubert for his internet directorate review at:
> https://datatracker.ietf.org/doc/review-ietf-ippm-ioam-flags-09-intdir-telechat-thubert-2022-06-28/
> (please consider Pascal's comments as mine).
>
> Special thanks to Tommy Pauly for the shepherd's detailed write-up including
> the WG consensus and the justification of the intended status.
>
> I hope that this helps to improve the document,
>
> Regards,
>
> -éric
>
> ## DISCUSS
>
> As noted in https://www.ietf.org/blog/handling-iesg-ballot-positions/, a
> DISCUSS ballot is a request to have a discussion on the following topics:
>
> ### Section 4.2 which address
>
> `The address of the node performing the copy operation` is confusing in the
> case of multiple interfaces (typical for a transit device BTW)... Which address
> should be used ? If the packet was received through an interface with a global
> address, then this should be the obvious choice or a loopback interface or ???
>

[TM] The current draft, much like RFC 9197, is intended to be
encapsulation-agnostic, while there are encapsulation-specific
documents, such as draft-ietf-ippm-ioam-ipv6-options, intended to
define how IOAM works over a specific encapsulation such as an IPv6
option.
That said, I agree that the sentence you quoted might be
misunderstood, and I suggest to remove it, i.e. remove:
OLD:
   The address of the node performing the copy operation is used as the
   source address.
NEW:
---NA

> ### Section 4.2 just truncation ?
>
> ```
>    The copy is also truncated, i.e., any payload that
>    resides after the IOAM option(s) is removed before transmitting the
>    looped back packet back towards the encapsulating node.
> ```
> It is unclear what happens to the IPv6 Next header field... Should the IP
> header length field be modified ?

[TM] Specifically, in IOAM over IPv6 (which is not within the scope of
the current draft), the IOAM option resides in a hop-by-hop options
header or in a destination options header. Either way, the Next Header
of this option header is available after the truncation. As a result
of the truncation there may be a need to modify some
encapsulation-specific fields, such as the IPv6 Payload Length.

However, I agree that some clarification would help here.
I would suggest the following text update:

OLD:
   The copy is also truncated, i.e., any payload that
   resides after the IOAM option(s) is removed before transmitting the
   looped back packet back towards the encapsulating node.
NEW:
   The copy is also truncated, i.e., any payload that
   resides after the IOAM option(s) is removed before transmitting the
   looped back packet back towards the encapsulating node. The truncation
   may require some encapsulation-specific updates in the encapsulation header.


>
> ### Section 4.2 forwarding ?
>
> It is unclear whether the packet is sent back to the source via the received
> interface or whether the packet is forwarded based on the FIB.
>

[TM] The draft mentions IOAM loopback as an alternative to Traceroute.
Following this analogy, the question you raised would also arise when
a router sends an ICMP TTL Exceeded back to the source that runs
Traceroute. RFC 792 (ICMP) does not go into this issue, and I would
suggest avoiding this issue in the current draft as well, especially
since we are doing our best to be encapsulation agnostic.

> ### IANA considerations conflicting text ?
> In section 4.1:
> ```
>    An IOAM trace option that has the Loopback flag set MUST have the
>    value '1' in the most significant bit of IOAM-Trace-Type, and '0' in
>    the rest of the bits of IOAM-Trace-Type.
> ```
> but in section 6:
> ```
>    IANA is requested to allocate the following bits in the "IOAM Trace
>    Flags Registry" as follows:
>
>    Bit 1  "Loopback" (L-bit)
>
>    Bit 2  "Active" (A-bit)
>
>    Note that bit 0 is the most significant bit in the Flags Registry.
> ```
>
> Is it bit 0 or bit 1 ?

[TM] The sentence from Section 4.1 refers to the IOAM-Trace-Type,
while the sentence from Section 6 refers to the IOAM Trace Flags. It
is a different field and a different registry, so there does not seem
to be a conflict.
Nevertheless, I suggest the following clarification:

OLD:
Note that bit 0 is the most significant bit in the Flags Registry.
NEW:
Note that bit 0 is the most significant bit in the Flags Registry.
This bit was allocated by [RFC 9197] as the 'Overflow' bit.


>
>
> ----------------------------------------------------------------------
> COMMENT:
> ----------------------------------------------------------------------
>
> ## COMMENTS
>
> ### "loopback"
>
> No need to reply, but every time I read "loopback", I think of the local
> "loopback interface". The use of "echo" would probably have made my reading
> easier ;-)
>
> ### Section 4.1
>
> ```
>    An IOAM trace option that has the Loopback flag set MUST have the
>    value '1' in the most significant bit of IOAM-Trace-Type, and '0' in
>    the rest of the bits of IOAM-Trace-Type.
> ```
> Does it prevent further enhancements to Trace types ?
>
> ### Section 4.1.1
>
> "SHOULD NOT exceed 1/N of the interface capacity" but this recommendation is
> only for the encapsulating node while there are nodes / links on the path that
> may have much more constrained capacity. I suggest to remove this part and
> replace it by text not refering to encapsulating node interface.
>
> ### Section 5
>
> ```
>    The
>    IOAM options are encapsulated in one of the IOAM encapsulation types,
>    e.g., [I-D.ietf-sfc-ioam-nsh], or [I-D.ietf-ippm-ioam-ipv6-options].
> ```
> Should this text also appear in the section 4?
>
> ### Section 5 capacity
>
> In
> ```
>    Thus, the rate of the traffic that
>    includes the Active flag rate SHOULD NOT exceed 1/N of the interface
>    capacity on any of the IOAM node's interfaces.
> ```
> Should thie be "IOAM nodes' interfaces" to take into account all IOAM nodes
> (including transit).
>
> ## NITS
>
> ### Section 5
> s/This draft focuses on three possible use cases/This document focuses on three
> possible use cases/
>
> ## 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
>
>
>