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

Benjamin Kaduk <kaduk@mit.edu> Thu, 12 December 2019 06:31 UTC

Return-Path: <kaduk@mit.edu>
X-Original-To: ippm@ietfa.amsl.com
Delivered-To: ippm@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id 0B41D1200FF; Wed, 11 Dec 2019 22:31:53 -0800 (PST)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -4.199
X-Spam-Level:
X-Spam-Status: No, score=-4.199 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, RCVD_IN_DNSWL_MED=-2.3, SPF_HELO_NONE=0.001, SPF_PASS=-0.001, URIBL_BLOCKED=0.001] autolearn=ham autolearn_force=no
Received: from mail.ietf.org ([4.31.198.44]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 1sM9Exg95Mqz; Wed, 11 Dec 2019 22:31:49 -0800 (PST)
Received: from outgoing.mit.edu (outgoing-auth-1.mit.edu [18.9.28.11]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id 6415B1200FD; Wed, 11 Dec 2019 22:31:49 -0800 (PST)
Received: from kduck.mit.edu ([24.16.140.251]) (authenticated bits=56) (User authenticated as kaduk@ATHENA.MIT.EDU) by outgoing.mit.edu (8.14.7/8.12.4) with ESMTP id xBC6VhhV011478 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Thu, 12 Dec 2019 01:31:46 -0500
Date: Wed, 11 Dec 2019 22:31:43 -0800
From: Benjamin Kaduk <kaduk@mit.edu>
To: "MORTON, ALFRED C (AL)" <acm@research.att.com>
Cc: The IESG <iesg@ietf.org>, "draft-ietf-ippm-initial-registry@ietf.org" <draft-ietf-ippm-initial-registry@ietf.org>, Brian Trammell <ietf@trammell.ch>, "ippm-chairs@ietf.org" <ippm-chairs@ietf.org>, "ippm@ietf.org" <ippm@ietf.org>
Message-ID: <20191212063143.GV13890@kduck.mit.edu>
References: <157557798354.16382.453970072175034852.idtracker@ietfa.amsl.com> <4D7F4AD313D3FC43A053B309F97543CFA6F09758@njmtexg5.research.att.com>
MIME-Version: 1.0
Content-Type: text/plain; charset="iso-8859-1"
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <4D7F4AD313D3FC43A053B309F97543CFA6F09758@njmtexg5.research.att.com>
User-Agent: Mutt/1.12.1 (2019-06-15)
Archived-At: <https://mailarchive.ietf.org/arch/msg/ippm/Vl0vs1g4z5EERHaFjc-1XW331Kk>
Subject: Re: [ippm] Benjamin Kaduk's Discuss on draft-ietf-ippm-initial-registry-14: (with DISCUSS and COMMENT)
X-BeenThere: ippm@ietf.org
X-Mailman-Version: 2.1.29
Precedence: list
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: Thu, 12 Dec 2019 06:31:53 -0000

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:noreply@ietf.org]
> > Sent: Thursday, December 5, 2019 3:33 PM
> > To: The IESG <iesg@ietf.org>
> > Cc: draft-ietf-ippm-initial-registry@ietf.org; Brian Trammell
> > <ietf@trammell.ch>; ippm-chairs@ietf.org; ietf@trammell.ch; ippm@ietf.org
> > 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.

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

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

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

> > 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://tools.ietf.org/html/rfc7594#section-8

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.

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

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

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

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


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.


Thanks for all the updates!

-Ben