Re: [ippm] John Scudder's No Objection on draft-ietf-ippm-rfc8889bis-03: (with COMMENT)

[Giuseppe Fioccola <[e-mail]>]

Hi John,
Thank you for reviewing this new version of the draft.

Best Regards,

Giuseppe


________________________________

Giuseppe Fioccola
Mobile: [phone]<tel:[phone]>
Email: [e-mail]<mailto:[e-mail]>

From: John Scudder via Datatracker<[e-mail]<mailto:[e-mail]>>
To: The IESG<[e-mail]<mailto:[e-mail]>>
Cc: draft-ietf-ippm-rfc8889bis<[e-mail]<mailto:[e-mail]>>;ippm-chairs<[e-mail]<mailto:[e-mail]>>;ippm<[e-mail]<mailto:[e-mail]>>;tpauly<[e-mail]<mailto:[e-mail]>>;tpauly<[e-mail]<mailto:[e-mail]>>
Subject: John Scudder's No Objection on draft-ietf-ippm-rfc8889bis-03: (with COMMENT)
Time: 2022-08-18 21:28:19

John Scudder has entered the following ballot position for
draft-ietf-ippm-rfc8889bis-03: No Objection

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/



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

Thanks for addressing my DISCUSS, I've cleared it and retained the text below
for posterity.

Previous DISCUSS:

Thanks for this document. As you may have noticed I had considerable difficulty
with the definition of "cluster". Once I completed an end-to-end read-through,
this was resolved (good!) but because RFCs are often consumed piecemeal (e.g.
someone may just dip into a portion of a document rather than settling down
with a nice cup of tea to read it end-to-end), I think it's important to fix
this problem, on the assumption I'm not the only person who might be thrown off.

I'll leave the details in the COMMENT, but I will repeat one observation from
my comment #3, which is that I count at least four separate (re-)definitions of
"cluster" in the document. With so many, it's no wonder that they're
inconsistent, and quite possibly the simplest solution would involve cutting
the number of definitions down to as close to 1 as possible.

COMMENT:

1. In §1, this

                                  While this document and its Clustered
   Alternate-Marking method is valid for multipoint-to-multipoint
   unicast flows, anycast, and ECMP flows.

This is a sentence fragment (a naked subordinate clause). I'd suggest a rewrite
but I can't make out what you are trying to say.

2. You have multiple definitions of "cluster" in the document, it seems. In §2
you have,

      Cluster: Smallest identifiable subnetwork of the entire monitoring
      network graph that still satisfies the condition that the number
      of packets that go in is the same as the number that go out.  A
      cluster partition algorithm can be applied to split the monitoring
      network into clusters.

As we have discussed in the past, according to this definition, a cluster is
always a single router. I think in an earlier discussion you suggested that it
could be corrected by saying something like "smallest subnetwork larger than a
singleton router", which I think would work, "smallest non-trivial" would too
(although it's less explicit). But then in §5 you have,

   In addition, it is also possible to leverage the data provided by the
   other counters in the network to converge on the smallest
   identifiable subnetworks where the losses occur.  These subnetworks
   are named "clusters".

As written, this makes nonzero losses a definitional attribute of a cluster.
Now that I've taken in the entire document, I guess this is the incorrect
definition. Possibly a fix would be to just drop the final sentence, which
(incorrectly?) implies that losses are an essential element of the definition.

For what it's worth, the algorithm of Section 5.1 is perfectly clear. I see the
desire to have a prose description of what a cluster is trying to do, but I
wonder if in the end it would be best to make the algorithm the canonical
definition, referenced from the prose definition as in

      Cluster: Smallest identifiable non-trivial
      subnetwork of the entire monitoring
      network graph that still satisfies the condition that the number
      of packets that go in is the same as the number that go out.  A
      cluster partition algorithm, such as that found in Section 5.1,
      can be applied to split the monitoring
      network into clusters.

3. In §5, you have

   A cluster graph is a subnetwork of the entire monitoring network
   graph that still satisfies the packet loss equation (introduced in
   the previous section), where PL in this case is the number of packets
   lost in the cluster.

I'd previously pointed out that this is problematic since the PL equation you
mention is in the nature of a definition; since you don't supply a condition
there's no way of saying whether it's "satisfied" or not. In response you said
(https://mailarchive.ietf.org/arch/msg/ippm/iXSBuXrQaETl6MyeDH8DBaPiJ9Q/),

```
We can modify the wording in Section 5 accordingly:

"A cluster graph is a subnetwork of the entire monitoring network graph that
still satisfies the condition that the number of packets that go in is the same
as the number that go out, if no packet loss occurs."

```

If indeed that is the right definition (see my earlier point, so probably
inserting "non-trivial" or "with more than a single router") then I do think
you should make that change, or in any case you should correct the current text
somehow.

By the way, do you mean something subtly different by "cluster graph" than you
do by "cluster"? Or do you just mean "the graph that represents the cluster"? I
think my confusion over just what a "cluster" is would have been mitigated by
fewer re-definitions of it. If "cluster graph" is indeed just another way of
saying "cluster" it's the third of four definitions! (The two I mention in my
point #2, this one, and the algorithm in Section 5.1.) With four definitions of
the same thing, in different words, no wonder some inconsistency crept in!

4. In §5.1,

   In summary, once a flow is defined, the algorithm to build the
   clusters partition is based on topological information; therefore, it
   considers all the possible links and nodes crossed by the given flow,
   even if there is no traffic.

If a flow has no traffic, can we call it a flow? It seems counter to the normal
English meaning of the word. Perhaps you mean possible links and nodes that
could potentially be crossed by the given flow?

5. Thanks for reporting the outcome of the earlier experiment, in §9. As with
my review of rfc8321bis, I think the RFC 2119 keywords need to be in a
"Deployment Considerations" section or similar, which could be achieved by
retitling this section or by separating the material into reporting the
experiment outcome (this section) and a new subsection for deployment advice.

6. Again similarly to my review of the companion document, I think §9's "the
Multipoint Alternate Marking Method is RECOMMENDED only for controlled domains"
needs to be fixed. Please bring it into line with whatever language you choose
for rfc8321bis.

Nits:

7. While the reference tags are in principle arbitrary strings, I wondered if
it was a typo that you used "PNPM" to tag a paper titled "AM-PM"?

8. In §7.1.1,

                 This means that, in the calculation, it is possible to
   weigh the timestamps by considering the number of packets for each
   endpoints.

s/endpoints/endpoint/