Re: [ippm] Benjamin Kaduk's Discuss on draft-ietf-ippm-initial-registry-14: (with DISCUSS and COMMENT)

["MORTON, ALFRED C (AL)" <[e-mail]>]

Hi Ben, 
I think we have reached a point of agreement
on all your comments now, see below.

Al

> -----Original Message-----
> From: Benjamin Kaduk [mailto:[e-mail]]
> Sent: Thursday, December 12, 2019 1:32 AM
> To: MORTON, ALFRED C (AL) <[e-mail]>
> Cc: The IESG <[e-mail]>; [e-mail];
> Brian Trammell <[e-mail]>; [e-mail]; [e-mail]
> Subject: Re: Benjamin Kaduk's Discuss on draft-ietf-ippm-initial-registry-
> 14: (with DISCUSS and COMMENT)
> 
> On Tue, Dec 10, 2019 at 04:37:02PM +0000, MORTON, ALFRED C (AL) wrote:
> > Hi Benjamin,
> > Thanks for your review, please see replies below,
> > Al
> >
> > > -----Original Message-----
> > > From: Benjamin Kaduk via Datatracker [mailto:[e-mail]]
> > > Sent: Thursday, December 5, 2019 3:33 PM
> > > To: The IESG <[e-mail]>
> > > Cc: [e-mail]; Brian Trammell
> > > <[e-mail]>; [e-mail]; [e-mail];
> [e-mail]
> > > Subject: Benjamin Kaduk's Discuss on draft-ietf-ippm-initial-registry-
> 14:
> > > (with DISCUSS and COMMENT)
> > >
> > > Benjamin Kaduk has entered the following ballot position for
> > > draft-ietf-ippm-initial-registry-14: 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://urldefense.proofpoint.com/v2/url?u=https-
> > > 3A__www.ietf.org_iesg_statement_discuss-
> 2Dcriteria.html&d=DwIDaQ&c=LFYZ-
> > > o9_HUMeMTSQicvjIg&r=OfsSu8kTIltVyD1oL72cBw&m=ppr2cZUpYOwZNFNM-
> > > AVYIIbZmprIcWRkpil0txFbqNw&s=9uZ1zYhbpUMnkI-
> > > PrgJpngXD1cCYGXGrRYLlJKYaWxA&e=
> > > for more information about IESG DISCUSS and COMMENT positions.
> > >
> > >
> > > The document, along with other ballot positions, can be found here:
> > > https://urldefense.proofpoint.com/v2/url?u=https-
> > > 3A__datatracker.ietf.org_doc_draft-2Dietf-2Dippm-2Dinitial-
> > > 2Dregistry_&d=DwIDaQ&c=LFYZ-
> > > o9_HUMeMTSQicvjIg&r=OfsSu8kTIltVyD1oL72cBw&m=ppr2cZUpYOwZNFNM-
> > > AVYIIbZmprIcWRkpil0txFbqNw&s=JX-
> > > Y6Mc8UNYVvJqdYM35CcHEEf58UUAWy7q2ZmBAsEA&e=
> > >
> > >
> > >
> > > ----------------------------------------------------------------------
> > > DISCUSS:
> > > ----------------------------------------------------------------------
> > >
> > > Quite minor points, really, but they do need to be resolved before
> > > publication.  (Slightly more details/locations in the COMMENT
> section.)
> > >
> > > We don't actually provide a definition (whether directly or by
> > > reference) for the "classic calculation for standard deviation of a
> > > population".
> > [acm]
> > This would be ambiguous if it left-out "of a population",
> > the alternative being sample standard deviation.
> > Millions of references and search results will
> > produce the right formula.  But I have provided the formula
> > in Sections 7 and 8.
> >
> > >
> > > We don't actually provide a definition (whether directly or by
> > > reference) for what the "time_offset" calibration value records.
> > [acm]
> > I supplied the NTP reference for the term, which is RFC 5905.
> 
> Thanks!
> I see the -15 has landed already, so I'll go clear after writing this
> message.
[acm] 

Thank you!
> 
> > >
> > >
> > > ----------------------------------------------------------------------
> > > COMMENT:
> > > ----------------------------------------------------------------------
> > >
> > > Section 2
> > >
> > >    This document defines the initial set of Performance Metrics
> Registry
> > >    entries, for which IETF approval (following development in the IP
> > >    Performance Metrics (IPPM) Working Group) will satisfy the
> > >    requirement for Expert Review.  Most are Active Performance
> Metrics,
> > >
> > > That's not really how Expert Review works, though as is being
> discussed
> > > in other ballot threads, we have other options for getting these
> > > registered.
> > [acm]
> > We tried to cover this special case when both drafts
> > seek approval at once.  Now it says:
> >
> > This document defines a set of initial Performance Metrics Registry
> entries.
> >
> >
> > >
> > > Section 4
> > >
> > >    Note: Each Registry entry only produces a "raw" output or a
> > >    statistical summary.  To describe both "raw" and one or more
> > >    statistics efficiently, the Identifier, Name, and Output Categories
> > >    can be split and a single section can specify two or more closely-
> > >    related metrics.  This section specifies two Registry entries with
> > >    many common columns.  See Section 7 for an example specifying
> > >    multiple Registry entries with many common columns.
> > >
> > > I'm not sure I understand the intended scope/audience of this remark.
> > > It is discussing something that we are doing for these registrations
> or
> > > something that might be done in the future?
> > [acm]
> > Authors of the future Registry entries will likely review
> > the current entries for guidance (and copying, which is fine).
> > This is the first registry entry, most-likely to be read that way,
> > and we provide a little extra guidance to start.
> 
> I'd suggest making the connection more explicit, as in "can specify two or
> more closely-related metrics, as is done here", or "For example, this
> section specifies two Registry entries with many common columns".
[acm] 
The later sounds good to me, thanks.

> 
> > >
> > > Section 4.2.2
> > >
> > >    o  UDP Payload
> > >
> > >       *  total of 100 bytes
> > >
> > > In the vein of "clear and reproducible metric definition", should we
> say
> > > something about the content of the payload (even if it's just "the
> > > content is arbitrary")?  Per Section 4.3.1 we could even note that a
> > > sequence number could be used as the payload.
> > > (I won't mention a similar consideration for the later metrics.)
> > [acm]
> > The bits in the payload are determined by the measurement protocol,
> > as you point-out the sequence number is part, and the rest of the
> > payload details are determined by that protocol. IF we said,
> > 100 bytes, of which the first 24 are the TWAMP-SENDER format, etc.
> > then we take-away choice of measurement protocol for this metric.
> 
> It sounds like you're stance is that the reader will understand that this
> payload is to be used by the measurement protocol in use, and we don't need
> to hit them over the head with it?  That's probably reasonable, given the
> target audience...
[acm] 
Good.

> 
> > >
> > >    o  Tmax: a loss threshold waiting time
> > >
> > >       *  3.0, expressed in units of seconds, as a positive value of
> type
> > >          decimal64 with fraction digits = 4 (see section 9.3 of
> > >          [RFC6020]) and with resolution of 0.0001 seconds (0.1 ms),
> with
> > >          lossless conversion to/from the 32-bit NTP timestamp as per
> > >          section 6 of [RFC5905].
> > >
> > > Am I reading this right that the Tmax is a fixed parameter with value
> > > 3.0 seconds?  If so, I'm confused why the extra specification on its
> > > representation is needed (and why we write only "3.0" and not "3.0000"
> > > to get the 4 fraction digits, though per RFC 6020 the fraction digits
> is
> > > just a maximum).
> > > (I won't mention a similar consideration for the later metrics.)
> > [acm]
> > I don't think this is a significant issue, and others
> > seem to agree (we're in the late stages of review here).
> 
> I agree that having it is not harmful, and any change comes with its own
> risk.
> 
> > >
> > > Section 4.3.1
> > >
> > >    The calculations on the delay (RTT) SHALL be performed on the
> > >    conditional distribution, conditioned on successful packet arrival
> > >    within Tmax.  Also, when all packet delays are stored, the process
> > >    which calculates the RTT value MAY enforce the Tmax threshold on
> > >    stored values before calculations.  See section 4.1 of [RFC3393]
> for
> > >
> > > Why is this a MAY and not a MUST?  Wouldn't it make the results not
> > > necessarily reproducible/transferrable?
> > > (I won't mention a similar consideration for the later metrics.)
> > [acm]
> > Good catch, fixed for all relevant sections.
> 
> It's a tough one, since it's very tempting to say "but in this case I have
> the complete picture of data, so I can get more accurate numbers" ... but
> my sense is that the goal here is to focus on
> reproducibility/interoperability, so the latter concern trumps the desire
> for accuracy.
[acm] 
Right.

> 
> > > Section 4.3.2
> > >
> > >    dT the duration of the interval for allowed sample start times,
> with
> > >       value 1.0, expressed in units of seconds, as a positive value of
> > >       type decimal64 with fraction digits = 4 (see section 9.3 of
> > >       [RFC6020]) and with resolution of 0.0001 seconds (0.1 ms).
> > >
> > > [1.0 does not have 4 fraction digits; though we did use 0.0200 for
> incT]
> > [acm]
> > we've skipped the extra digits for the values with all-zero-fractions...
> >
> > >
> > > Section 4.3.5
> > >
> > > There are potentially privacy considerations (albeit likely to be
> > > minimal) with requiring the reporting of Src and Dst addresses as part
> > > of the measurement results.
> > [acm]
> > Please see the many privacy considerations identified in the
> > LMAP framework (which we've referenced).
> > https://urldefense.proofpoint.com/v2/url?u=https-
> 3A__tools.ietf.org_html_rfc7594-23section-2D8&d=DwIDAw&c=LFYZ-
> o9_HUMeMTSQicvjIg&r=OfsSu8kTIltVyD1oL72cBw&m=HdXvuit3d69xOSjmJ-
> 7aH3W8CQjEShVuhPZPFH-bEQE&s=6AKDwaabdK7TfL0flV2s72LYDtY_czbfXLGBs0QOSAA&e=
> 
> Thanks for adding the reference in the security considerations section;
> that's a big help.
> 
> > >
> > > Section 4.4.2
> > >
> > >    95Percentile  The time value of the result is expressed in units of
> > >       seconds, as a positive value of type decimal64 with fraction
> > >       digits = 9 (see section 9.3 of [RFC6020]) with resolution of
> > >       0.000000001 seconds (1.0 ns), and with lossless conversion
> to/from
> > >       the 64-bit NTP timestamp as
> > >
> > > Is there a missing end of the sentence?
> > [acm]
> > Yes, will fix.
> > >
> > > Section 4.4.4
> > >
> > >    When a measurement controller requests a calibration measurement,
> the
> > >    loopback is applied and the result is output in the same format as
> a
> > >    normal measurement with additional indication that it is a
> > >    calibration result.
> > >
> > > If the goal of the calibration is intended in part to quantify the
> > > random errors of a measurement, shouldn't we also report something
> akin
> > > to a variance or standard distribution, as opposed to just the same
> > > summary results that we would get from a non-calibration measurement?
> > > My understanding of what this says to do will only give you the
> > > "average offset" due to the random+systematic error (for the
> appropriate
> > > definition of "average") but not give you a sense for whether the
> > > effective precision is reduced by those random errors.
> > > (I won't mention a similar consideration for the later metrics.)
> > [acm]
> > I see your point, but it doesn't seem correct to add measurements
> > during calibration. The user could repeat the measurements many times
> > and review the reported results, or conduct a calibration with a
> > different metric.
> >
> > >
> > > Section 5.4.4
> > >
> > > Does this really give me a clear definition of what the time_offset is
> > > measuring (as opposed to just how to represent it)?
> > > (I won't mention a similar consideration for the later metrics.)
> > >
> > [acm]
> > We are using the RFC 5905 definition of time offset.
> > I'll add the references where they are needed.
> 
> Thanks.
> 
> > > Section 6.3.1
> > >
> > >    DNS Messages bearing Queries provide for random ID Numbers in the
> > >    Identification header field, so more than one query may be launched
> > >    while a previous request is outstanding when the ID Number is used.
> > >    Therefore, the ID Number MUST be retained at the Src or included
> with
> > >    each response packet to disambiguate packet reordering if it
> occurs.
> > >
> > > Is this an "or" or an "and"?  Also, the DNS protocol is pretty well
> > > specified about this already, right, so we're just noting the
> properties
> > > that is has?
> > [acm]
> > Yes, and is correct. We're adding a consideration for the measurement
> > which isn't the emphasis of the DNS specs.
> >
> > >
> > >    In addition to operations described in [RFC2681], the Src MUST
> parse
> > >    the DNS headers of the reply and prepare the information for
> > >    subsequent reporting as a measured result, along with the Round-
> Trip
> > >    Delay.
> > >
> > > Since we only use the Rcode, maybe we could be more specific than "the
> > > information"?
> > [acm]
> > ok
> >
> > >
> > > Section 6.3.2
> > >
> > >    Section 11.1.3 of RFC 2681 [RFC2330] provides three methods to
> > >    generate Poisson sampling intervals.  The reciprocal of lambda is
> the
> > >    average packet rate, thus the Run-time Parameter is
> Reciprocal_lambda
> > >    = 1/lambda, in seconds.
> > >
> > > Wouldn't something measured in seconds be an inter-packet interval,
> not
> > > a "packet rate"?  (Hmm, § 7.3.2 calls it the "average packet
> spacing".)
> > [acm]
> > it should be s/in seconds/in packets per second/
> 
> I thought that lambda was in pps but 1/lambda would be seconds (as
> written).  My complaint here was about "reciprocal of lambda is the
> average
> packet rate", which I thought should be "reciprocal of lambda is the
> average packet spacing" to match § 7.3.2.
[acm] 
Yes, I have sometimes mixed these up, and I didn't have reference
material to help on the plane. I wish I read 7.3.2 first, because that
one was correct.  I made both 6.3.2 and 7.3.2 consistent with 
reference definitions (again, in the working version).


> 
> >
> > >
> > > Section 6.3.5
> > >
> > >    Trunc  Upper limit on Poisson distribution expressed in units of
> > >       seconds, as a positive value of type decimal64 with fraction
> > >       digits = 4 (see section 9.3 of [RFC6020]) with resolution of
> > >       0.0001 seconds (0.1 ms), and with lossless conversion to/from
> the
> > >       32-bit NTP timestamp as per section 6 of [RFC5905] (values above
> > >       this limit will be clipped and set to the limit value). (if
> fixed,
> > >       Trunc = 30.0000 seconds.)
> > >
> > > For a registered matric, a parameter is either Fixed or Run-time;
> there
> > > is no option.  This last parenthetical needs to be removed.
> > [acm]
> > Done.
> > >
> > >    ID The 16-bit identifier assigned by the program that generates the
> > >       query, and which must vary in successive queries, see
> > >       Section 4.1.1 of [RFC1035].  This identifier is copied into the
> > >       corresponding reply and can be used by the requester (Src) to
> > >       match-up replies to outstanding queries.
> > >
> > > If it must vary in successive queries (which are still part of the
> same
> > > Stream), that's not a single Run-time parameter anymore, is it?
> > [acm]
> > I see, if one query fails, we need more than one ID to get a successful
> response.
> > So
> > ... and which must vary in successive queries (a list of IDs is needed),
> 
> I think so, yes.
> 
> > >
> > >    QTYPE  The Query Type, which will correspond to the IP address
> family
> > >       of the query (decimal 1 for IPv4 or 28 for IPv6, formatted as a
> > >       uint16, as per section 9.2 of [RFC6020].
> > >
> > > Does this implicitly exclude non-IP-address queries?
> > [acm]
> > Yes, this is the simple case.
> >
> > >
> > > Section 7.2.2
> > >
> > > Do we need to report or specify which TWAMP security features are in
> > > use, in case that has an effect on processing times and the
> > > corresponding measurements?
> > > (I won't mention a similar consideration for the later metrics.)
> > [acm]
> > yes, let's say:
> >  * 250 octets total, including the TWAMP format type, which MUST be
> reported.
> >
> > >
> > > Section 7.3.1
> > >
> > >    The reference method requires some way to distinguish between
> > >    different packets in a stream to establish correspondence between
> > >    sending times and receiving times for each successfully-arriving
> > >    packet.  Sequence numbers or other send-order identification MUST
> be
> > >    retained at the Src or included with each packet to disambiguate
> > >    packet reordering if it occurs.
> > >
> > >    Since a standard measurement protocol is employed [RFC5357], then
> the
> > >    measurement process will determine the sequence numbers or
> timestamps
> > >    applied to test packets after the Fixed and Runtime parameters are
> > >    passed to that process.  The measurement protocol dictates the
> format
> > >    of sequence numbers and time-stamps conveyed in the TWAMP-Test
> packet
> > >    payload.
> > >
> > > I'm not sure whether it makes sense to keep the end of the first
> > > paragraph given that the second paragraph covers the specifics of how
> > > the requirement is satisfied.
> > > (I won't mention a similar consideration for the later metrics.)
> > [acm]
> > Right, last sentence deleted in first paragraph.
> >
> > >
> > > Section 7.4.2.5
> > >
> > >    See section 4.3.2 of [RFC6049] for a closely related method for
> > >    calculating this statistic, and 4.3.3 of [RFC6049].  The formula is
> > >    the classic calculation for standard deviation of a population.
> > >
> > > Please provide the actual formula, whether directly or by reference.
> A
> > > "closely related method" implies that it is not the actual method to
> > > use, leaving the actual method to use underspecified here.
> > [acm]
> > I think the references provides the most of the formula,
> > but I have provided the complete formula here and in section 8.
> 
> Thanks.  (To be clear, I was complaining that we reference 6049 merely for
> "a closely related method" -- if we referenced it for "how to calculate
> this statistic" I would not be concerned.  If we only say "closely-
> related"
> that implies that it's not the same, but we didn't say what was
> different.)
[acm] 
ok

> 
> > >
> > > Section 9
> > >
> > > Interesting to see that we don't include 95thpercentile or StdDev for
> > > ICMP, though this is of course not actually problematic.
> > >
> > >    All column entries beside the ID, Name, Description, and Output
> > >    Reference Method categories are the same, thus this section
> proposes
> > >    two closely-related registry entries.  As a result, IANA is also
> > >    asked to assign four corresponding URLs to each Named Metric.
> > >
> > > nit: s/four//
> > [acm]
> > Done.
> >
> > >
> > > Section 9.2.2
> > >
> > >    o  ICMP Payload
> > >
> > >       *  total of 32 bytes of random info
> > >
> > > Randomly selected when -- per-test?  per-packet?
> > > I note that this is the "fixed parameters" section, and the above
> seems
> > > to be anything but *fixed*.
> > [acm]
> > Let's add
> >
> > *  total of 32 bytes of random info, constant per test
> >
> > >
> > > Section 10
> > >
> > >    All column entries beside the ID, Name, Description, and Output
> > >    Reference Method categories are the same, thus this section
> proposes
> > >    four closely-related registry entries.  As a result, IANA is also
> > >    asked to assign four corresponding URLs to each Named Metric.
> > >
> > > nit: s/four//
> > [acm]
> > Done
> >
> > >
> > > Section 10.1.2
> > >
> > > nit: s/opportuinty/opportunity/
> > [acm]
> > Done
> > >
> > > Section 10.2.1
> > >
> > >
> > >    Traffic filters reduce the packet stream at the OP to a Qualified
> > >    bidirectional flow packets.
> > >
> > > nit: singular/plural mistmatch "a"/"packets"
> > [acm]
> > should be
> > Traffic filters reduce the packet stream at the OP to
> > a Qualified bidirectional flow of packets.
> >
> > >
> > >    For a real number, RTD_fwd, >> the Round-trip Delay in the Forward
> > >    direction from OP to host B at time T' is RTD_fwd << REQUIRES that
> OP
> > >    observed a Qualified Packet to host B at wire-time T', that host B
> > >    received that packet and sent a Corresponding Packet back to host
> A,
> > >    and OP observed the Corresponding Packet at wire-time T' + RTD_fwd.
> > >
> > > I'm either failing to parse this or confused by the notation (or
> both).
> > > Are the doubled angle brackets intended to enclose a parenthetical
> > > remark defining RTD_fwd?  (If so, why would traditional parentheses
> not
> > > work?)
> > [acm]
> > This is the same symbolism used in the earliest IPPM Metric Definitions,
> > simply enclosing the defined phrase within the >> ... << for emphasis.
> > We were forced to provide formal definitions here, references were
> > short on details and the same is true for many passive metrics.
> 
> Okay.  If I in effect treat the >>definition<< grammatically as a
> parenthetical, the sentence still flows oddly to me, though.  (Consider
> "for a real number, X (defined as <definition>), requires that
> <procedure>".  There's no subject for the verb "requires".)
> 
> If I were writing this (which I'm not), I'd try:
> 
> """The real-valued number RTD_fwd represents >>definition<< .  Measuring
> this
> value requires that [...]"""
> 
> RTD_rev would get a similar treatment, of course.
> 
> (Also, it looks like I failed to note that REQUIRES is not a BCP 14
> keyword, only REQUIRED.)
[acm] 
I fixed the requirement term and grammar to allow it.
(trying to minimize the changes in critical paragraphs now).

> 
> > >
> > > Section 11
> > >
> > > Please mention the potential privacy considerations for observed
> > > traffic, particularly for passive metrics.
> 
> I think I was hoping for a little more detail, but this will suffice to
> get
> the reader thinking about it.
> 
> > > An attacker that knows that its TCP connection is being measured can
> > > modify its behavior to skew the measurement results.
> >
> > >    Security.  Each referenced Metric contains a Security
> Considerations
> > >    section.
> > >
> > > I do not see any such sections.
> > [acm]
> > The References (like RFC7680) have the Security Considerations sections.
> > changed the text to
> > Each IETF Metric definition referenced above contains a Security
> Considerations section.
> 
> Hmm, maybe I'm (still) misreading this, then.  I was inferring that the
> "referenced Metric"s (or "Metric definition[s] referenced above") meant
> the
> specific metrics being defined in this document as the initial registry
> contents.  With that assumption, this seems to be saying that each metric
> gets its own paragraph of security considerations, but neither the text
> nor
> the registration template makes a provision for that.  Is this rather
> intended to be interpreted as more like "RFC 2681 defines a round-trip
> delay metric, and we're using that procedure for the registry entries we
> define in this document, so go read the security considerations of RFC
> 2681"?  In that case maybe it's better to talk about IETF documents rather
> than "metric definitions".
[acm] 
ok, really it's IETF RFCs, but I think some SDOs have adopted
IETF's outline in that they request a discussion of security
considerations.

> 
> > >
> > > Section 14.2
> > >
> > > Shouldn't RFC 5481 be normative, given that in Section 5.3.1 we see
> that
> > > "this metric entry requires implementation of the PDV form defined in
> > > section 4.2 of [RFC5481]"?
> 
> [This didn't change in the -15, though I'm not sure whether that was a
> conscious choice or not.]
[acm] 
Nope, I missed it and I agree, thanks!

> 
> 
> One final comment looking at the changes in the -15:
> 
> The new Section 1 text says "Specification Required policy, which includes
> Expert Review", but RFC 8126 is pretty adamant that you should not include
> the second clause -- see the penultimate paragraph of Section 4.6 thereof.
[acm] 
Guilty.  thanks!
Al

> 
> 
> Thanks for all the updates!
> 
> -Ben