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

[Giuseppe Fioccola <[e-mail]>]

Hi Lars,
Thank you for your review.
Please see my replies inline tagged as [GF].
Note that I will address your comments in the next revision.

Best Regards,

Giuseppe

-----Original Message-----
From: Lars Eggert via Datatracker <[e-mail]> 
Sent: Tuesday, July 12, 2022 8:37 PM
To: The IESG <[e-mail]>
Cc: [e-mail]; [e-mail]; [e-mail]; [e-mail]; [e-mail]
Subject: Lars Eggert's Discuss on draft-ietf-ippm-rfc8889bis-02: (with DISCUSS and COMMENT)

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:
https://datatracker.ietf.org/doc/draft-ietf-ippm-rfc8889bis/



----------------------------------------------------------------------
DISCUSS:
----------------------------------------------------------------------

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

CC @larseggert

Thanks to Russ Housley for the General Area Review Team (Gen-ART) review
(https://mailarchive.ietf.org/arch/msg/gen-art/OTAnYgNGwDIRaQu_9IDo1QN1KHM).

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

[GF]: I think I can simplify this sentence and only refer to RFC8321bis where in section 6 it is provided detailed guidance. I would delete the last part of the paragraph and revise as follows:
"Note that the fragmented packets case can be managed with the Alternate-Marking methodology and the same guidance provided in section 6 of [I-D.ietf-ippm-rfc8321bis] apply also in the case of Multipoint Alternate Marking. "

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


----------------------------------------------------------------------
COMMENT:
----------------------------------------------------------------------

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

[GF]: Sure, I will revise the sentence

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

[GF]: Yes, I will clarify the assumption

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

[GF]: I will revise this part. I meant a unidirectional flow.

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

[GF]: Ok, maybe the example can be moved to an Appendix and keep only the two-step algorithm in section 5.1.

### 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
     endpoints.
```
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??

[GF]: Yes the weight is exactly the number of packets. I will revise the sentence accordingly.

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

[GF]: I can also omit RFC2119 wording here.

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

[GF]: Yes I can replace it.

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

[GF]: Regarding the packet loss measurement SHOULD can be replaced with MUST since it is the only option. For delay measurements, I will also replace SHOULD/MAY with MUST for the cases with single option, otherwise I will leave the two possibilities with SHOULD for the primary option and MAY for the secondary option.

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

[GF]: I will fix.

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

[GF]: Ok

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

[GF]: Ok

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

[GF]: Ok 

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

[GF]: Ok

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

[GF]: Ok

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

[GF]: Ok

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

[GF]: Ok

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

[GF]: Ok

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