[ippm] Lars Eggert's Discuss on draft-ietf-ippm-rfc8889bis-02: (with DISCUSS and COMMENT)

Lars Eggert via Datatracker <noreply@ietf.org> Tue, 12 July 2022 18:37 UTC

Return-Path: <noreply@ietf.org>
X-Original-To: ippm@ietf.org
Delivered-To: ippm@ietfa.amsl.com
Received: from ietfa.amsl.com (localhost [IPv6:::1]) by ietfa.amsl.com (Postfix) with ESMTP id 618B8C14F6EB; Tue, 12 Jul 2022 11:37:23 -0700 (PDT)
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit
From: Lars Eggert via Datatracker <noreply@ietf.org>
To: The IESG <iesg@ietf.org>
Cc: draft-ietf-ippm-rfc8889bis@ietf.org, ippm-chairs@ietf.org, ippm@ietf.org, tpauly@apple.com, tpauly@apple.com
X-Test-IDTracker: no
X-IETF-IDTracker: 8.6.0
Auto-Submitted: auto-generated
Precedence: bulk
Reply-To: Lars Eggert <lars@eggert.org>
Message-ID: <165765104339.6369.12188473702871581484@ietfa.amsl.com>
Date: Tue, 12 Jul 2022 11:37:23 -0700
Archived-At: <https://mailarchive.ietf.org/arch/msg/ippm/KC_whiyW7SwEh8nwNXTT72ZhF-o>
Subject: [ippm] Lars Eggert's Discuss on draft-ietf-ippm-rfc8889bis-02: (with DISCUSS and COMMENT)
X-BeenThere: ippm@ietf.org
X-Mailman-Version: 2.1.39
List-Id: IETF IP Performance Metrics Working Group <ippm.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/ippm>, <mailto:ippm-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/ippm/>
List-Post: <mailto:ippm@ietf.org>
List-Help: <mailto:ippm-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/ippm>, <mailto:ippm-request@ietf.org?subject=subscribe>
X-List-Received-Date: Tue, 12 Jul 2022 18:37:23 -0000

Lars Eggert has entered the following ballot position for
draft-ietf-ippm-rfc8889bis-02: 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:


# GEN AD review of draft-ietf-ippm-rfc8889bis-02

CC @larseggert

Thanks to Russ Housley for the General Area Review Team (Gen-ART) review

## Discuss

### Section 1, paragraph 11
     Note that the fragmented packets case can be managed with the
     Alternate-Marking methodology.  The same considerations of
     [I-D.ietf-ippm-rfc8321bis] apply also in the case of Multipoint
     Alternate Marking.  As defined in [I-D.ietf-ippm-rfc8321bis] the
     marking node MUST mark all the fragments except in the case of
     fragmentation within the network domain, in that event it is
     suggested to mark only the first fragment.
"MUST mark ... except" is not clear enough. In the case where there is
fragmentation, what is the defined behavior? "Suggest to mark" leaves a lot
open to interpretation.

In general, the use of RFC2119 language in this document should be checked. See comments below.


## Comments

### Section 3, paragraph 15
     The case of unicast flow is considered in Figure 1.  The anycast flow
     is also in scope, because there is no replication and only a single
     node from the anycast group receives the traffic, so it can be viewed
     as a special case of unicast flow.
Anycast is only a special case of a unicast flow is routing is stable throughout
the measurement period.

### Section 4.2, paragraph 0
  4.2.  Network Packet Loss
The definitions in this section seem to assume that routing is stable during the
measurement period (because otherwise the loss definitions don't work in case of
route changes or loops). That assumption should probably be made more explicit.

### Section 5, paragraph 5
     In a completely monitored unidirectional network (a network where
     every network interface is monitored), each network device
     corresponds to a cluster, and each physical link corresponds to two
     clusters (one for each device).
What is a "unidirectional network"?

### Section 5.1, paragraph 0
  5.1.  Algorithm for Clusters Partition
It would be helpful if this algorithm was also specified as pseudo code, rather
than just by example.

### Section 7.1.1, paragraph 2
     The average latency can be measured as the difference between the
     weighted averages of the mean timestamps of the sets of output and
     input nodes.  This means that, in the calculation, it is possible to
     weigh the timestamps by considering the number of packets for each
Wouldn't you need to require that the weight is exactly the number of packets to
make the math work, and not just a weight that "considers" the number of
packets, i.e., is an unspecified function of it??

### Section 7.2.2, paragraph 2
     The hash-based selection methodologies for delay measurement can work
     in a multipoint-to-multipoint path and MAY be used either coupled to
     mean delay or stand-alone.
I think the MAY needs to be a MUST, because otherwise unspecified alternatives
are permitted?

### Section 7.2.2, paragraph 4
     In a multipoint environment, the hashing selection MAY be the
     solution for performing delay measurements on specific packets and
     overcoming the single- and double-marking limitations.
I don't understand why this uses an RFC2119 MAY?

### Section 9, paragraph 2
     Either one or two flag bits might be available for marking in
     different deployments:
The use of SHOULD and MAY in the three sections following this is leaving things
underspecified. What happens if SHOULDs and MAYs are not followed?

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

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

### Grammar/style

#### Section 1, paragraph 6
 of problems (packet loss is measured or the delay is too high), the filterin
Use a comma before "or" if it connects two independent clauses (unless they are
closely connected and short).

#### Section 1, paragraph 8
DISCUSS: Note that the fragmented packets case can be managed with the Alter
An apostrophe may be missing.

#### Section 4.2, paragraph 3
 algorithm was also specified as pseudo code, rather than just by example. A
This word is normally spelled as one.

#### Section 5.1, paragraph 13
 defined in Section 4.2, valid for the an entire monitored flow, can easily
Two determiners in a row. Choose either "the" or "an".

#### Section 5.1, paragraph 25
ng the number of packets for each endpoints. Wouldn't you need to require th
The noun should probably be in the singular form.

#### Section 5.1, paragraph 32
 can do the calculation. If we would perform a delay measurement for more th
Consider removing "would". (Usually, "would" does not occur in a conditional
clause, unless to make a request or give a polite order.).

Also, the document uses first person plural ("we") in several places, which is
unusual in an RFC.

#### Section 7, paragraph 1
with the hashing selection allows to choose a simplified hash function since
Did you mean "choosing"? Or maybe you should add a pronoun? In active voice,
"allow" + "to" takes an object, usually a pronoun.

#### Section 7, paragraph 3
along the path. For example, in case an hashed sample is lost, it is confined
Use "a" instead of "an" if the following word doesn't start with a vowel sound,
e.g. "a sentence", "a university".

#### Section 8, paragraph 4
of necessity (packet loss is measured or the delay is too high), the filterin
Use a comma before "or" if it connects two independent clauses (unless they are
closely connected and short).

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