One-way/Two-way Active Measurement Protocol Extensions for Performance Measurement on LAG
draft-ietf-ippm-otwamp-on-lag-08

Thank you for the work on this document.

Many thanks to Jean Mahoney for her ART ART review: https://mailarchive.ietf.org/arch/msg/art/MzzadDve7VnngXF61Rt-a2KTY-I/.

One minor comment:

        the hash-based mechanism is used

Maybe a reference would be good here?

Thank you to Steve Hanna for the SECDIR review.

** Section 2.
   All micro sessions of a LAG share the same Sender IP Address and
   Receiver IP Address of the LAG.  As for the UDP layer, the micro
   sessions may share the same Sender Port and Receiver Port pair, or
   each micro session is configured with a different Sender Port and
   Receiver Port pair.  But from the operational point of view, the
   former is simpler and is RECOMMENDED.

Is there a reason to provide both options?  Why would one not choose the RECOMMENDED approach?  Could the reason for choosing one configuration over the other be documented?

# Éric Vyncke, INT AD, comments for draft-ietf-ippm-otwamp-on-lag-07

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.

Other thanks to Tim Winters, the Internet directorate reviewer (at my request), please consider this int-dir review:
https://datatracker.ietf.org/doc/review-ietf-ippm-otwamp-on-lag-07-intdir-telechat-winters-2023-11-20/

I hope that this review helps to improve the document,

Regards,

-éric

# COMMENTS (non-blocking)

## LAG specific

I agree with the shepherd's comment: this I-D could be made more generic than plain LAG.

## Section 1 & 2 vs. draft-ietf-ippm-stamp-on-lag-05

See my IESG on draft-ietf-ippm-stamp-on-lag-05 for comments as those two documents share the same introduction. Could a single I-D cover STAMP & OTWAMP ?

## Section 3.1

About `the OWAMP Server MUST build a set of micro sessions for all the member links of the LAG from which the Request-OW-Micro-Sessions message is received` what happens when a link is added/removed from the LAG group ?

## Section 3.2

Unsure I can parse `When receives a test packet` (a subject appears to be missing).

# GEN AD review of draft-ietf-ippm-otwamp-on-lag-07

CC @larseggert

Thanks to Stewart Bryant for the General Area Review Team (Gen-ART) review
(https://mailarchive.ietf.org/arch/msg/gen-art/1ps0WlynMuNXtHU73X9ulB3K_Ws).

## Comments

### Section 5, paragraph 3
```
     For micro TWAMP sessions, the similar set up procedure as micro OWAMP
     sessions is used.  Then the micro TWAMP Session-Sender sends micro
     Session-Sender packets with the Sender Micro-session ID and the
     Reflector Micro-session ID.  The micro Session-Reflector checks
     whether a test packet is received from the member link associated
     with the correct micro TWAMP session, if the Reflector Micro-session
     ID field is set.  When reflecting, the micro TWAMP Session-Reflector
     copies the Sender Micro-session ID from the received micro Session-
     Sender packet to the micro Session-Reflector packet, and sets the
     Reflector Micro-session ID field with the member link identifier that
     is associated with the micro TWAMP session.  When receiving the micro
     TWAMP Session-Reflector packet, the micro Session-Sender uses the the
     Sender Micro-session ID to check whether the packet is received from
     the member link associated with the correct micro TWAMP session.  The
     micro Session-Sender also use the Reflector Micro-session ID to
     validate the Reflector's behavior.
```
Some critical pieces of the process seem to not be described here. At
the very least, I would have expected to see some text requiring at
least one session needing to be established per member link to
utilize the LAG fully. Are there other such considerations?

### Boilerplate

This document uses the RFC2119 keywords "SHALL", "MAY", "SHOULD",
"RECOMMENDED", "MUST", "NOT RECOMMENDED", "MUST NOT", "SHOULD NOT", "OPTIONAL",
"SHALL NOT", and "REQUIRED", but does not contain the recommended RFC8174
boilerplate. (It contains some text with a similar beginning.)

## 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 2, paragraph 4
```
-    sessions estabilished on member links of a LAG, test packets of micro
-                  -
```

#### Section 6.1, paragraph 0
```
- 6.1.  Mico OWAMP-Control Command
+ 6.1.  Micro OWAMP-Control Command
+          +
```

#### Section 6.2, paragraph 0
```
- 6.2.  Mico TWAMP-Control Command
+ 6.2.  Micro TWAMP-Control Command
+          +
```

### Grammar/style

#### Section 1, paragraph 1
```
ics of every member link of a LAG. Hence the measured performance metrics ca
                                   ^^^^^
```
A comma may be missing after the conjunctive/linking adverb "Hence".


#### Section 4.2.3, paragraph 9
```
 micro TWAMP sessions, the similar set up procedure as micro OWAMP sessions
                                   ^^^^^^
```
When "set-up" is used as a noun or modifier, it needs to be hyphenated.

#### Section 4.2.3, paragraph 12
```
ket, the micro Session-Sender uses the the Sender Micro-session ID to check w
                                   ^^^^^^^
```
Possible typo: you repeated a word.

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

Hi, 

The same comment that applies to the stamp document equally applies here (repeated below):

I have to confess that this whole approach feels somewhat like a layer violation to me.  I perceive that the benefit of LAG is to increase link bandwidth and add some level of redundancy without the higher layers needing to be aware that the LAG interface is composed of separate physical Ethernet interfaces (in the same way, that most protocols don't need to know that a 400G Ethernet interface may be spread over multiple lambdas at the physical layer because that abstraction is hidden to the layers above).  I would argue that at the point that we are starting to steer traffic onto particular LAG members (beyond a local passive load-balancing operation) and to monitor and report the performance characteristics of individual members then perhaps the LAG abstraction is somewhat breaking down, and perhaps a cleaner solution would be to not have the interfaces in a LAG and to rely on something like ECMP instead.

However, I appreciate that my previous comment is not directly actionable by the authors and arguably this ship has already sailed with RFC 8668.  Hence, a potentially more actionable suggestion would be:

Should the document have any text regarding how these measurements should be used when individual LAG members fail, and I presume that the pinned member traffic is hashed to different LAG members instead?  Or to phrase this in an alternative way, are their operational deployment considerations about when and how this technology should be deployed and what other LAG configuration should or should not be used at the same time to result in a sane robust solution.

Regards,
Rob