Re: [tram] Benjamin Kaduk's Discuss on draft-ietf-tram-stun-pmtud-10: (with DISCUSS and COMMENT)

Benjamin Kaduk <kaduk@mit.edu> Mon, 19 August 2019 23:50 UTC

Return-Path: <kaduk@mit.edu>
X-Original-To: tram@ietfa.amsl.com
Delivered-To: tram@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id E3684120088; Mon, 19 Aug 2019 16:50:20 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -4.2
X-Spam-Level:
X-Spam-Status: No, score=-4.2 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] 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 N_mO402vWnDA; Mon, 19 Aug 2019 16:50:18 -0700 (PDT)
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 ECDCC120116; Mon, 19 Aug 2019 16:50:14 -0700 (PDT)
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 x7JNo6Us026862 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Mon, 19 Aug 2019 19:50:09 -0400
Date: Mon, 19 Aug 2019 18:50:06 -0500
From: Benjamin Kaduk <kaduk@mit.edu>
To: "Felipe Garrido (fegarrid)" <fegarrid@cisco.com>
Cc: "tram-chairs@ietf.org" <tram-chairs@ietf.org>, "draft-ietf-tram-stun-pmtud@ietf.org" <draft-ietf-tram-stun-pmtud@ietf.org>, "iesg@ietf.org" <iesg@ietf.org>, "tasveren@rbbn.com" <tasveren@rbbn.com>, "tram@ietf.org" <tram@ietf.org>
Message-ID: <20190819234944.GC60855@kduck.mit.edu>
References: <D4026211-D94F-4BE9-BF3E-0B655AF4E83C@cisco.com>
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <D4026211-D94F-4BE9-BF3E-0B655AF4E83C@cisco.com>
User-Agent: Mutt/1.10.1 (2018-07-13)
Archived-At: <https://mailarchive.ietf.org/arch/msg/tram/6aSN206wguJRm4z9KygbpUlrWYo>
Subject: Re: [tram] Benjamin Kaduk's Discuss on draft-ietf-tram-stun-pmtud-10: (with DISCUSS and COMMENT)
X-BeenThere: tram@ietf.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: "Discussing the creation of a Turn Revised And Modernized \(TRAM\) WG, which goal is to consolidate the various initiatives to update TURN and STUN." <tram.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/tram>, <mailto:tram-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/tram/>
List-Post: <mailto:tram@ietf.org>
List-Help: <mailto:tram-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/tram>, <mailto:tram-request@ietf.org?subject=subscribe>
X-List-Received-Date: Mon, 19 Aug 2019 23:50:21 -0000

Hi Felipe,

The new wording is at least enough for me to come up with what seems like a
plausible guess for what the identifier (and algorithm thereto) is supposed
to be, but I do still feel like I'm guessing.  If it's the path identifier
used to associate stored (PL)PMTU state across different flows (which is my
guess), then maybe "associated to a given path's identifier" would be a
further improvement?

Thanks,

Ben

On Mon, Aug 19, 2019 at 02:13:13PM +0000, Felipe Garrido (fegarrid) wrote:
> Hi Ben, 
> 
> Do you agree with the latest wording changes? A new draft version has been published with them.
> 
> Thanks,
> -Felipe
> 
> On 7/8/19, 10:43 AM, "tram on behalf of Felipe Garrido (fegarrid)" <tram-bounces@ietf.org on behalf of fegarrid@cisco.com> wrote:
> 
>     Hi Ben, 
>     
>     Totally agree now that I've re-read it. 
>     
>     Here's the new proposed wording. 
>     
>     "The packets that are to be associated to an identifier are selected according to Section 5.2 of [RFC4821]."
>     
>     Here's the full text. 
>     
>     A server supporting this specification will keep the identifiers of
>     all packets received in a chronologically ordered list.  The
>     packets that are to be associated to an identifier are selected 
>     according to Section 5.2 of [RFC4821]."
>     
>     Thanks,
>     -Felipe
>     
>     On 6/27/19, 12:00 AM, "Benjamin Kaduk" <kaduk@mit.edu> wrote:
>     
>         Hi Felipe,
>         
>         Thanks for following up -- it looks like I filed away the original mail
>         without responding to it, somehow(!)
>         The Discuss point's resolution is fine; I just have one more question
>         (inline).
>         
>         -Ben
>         
>         On Wed, Jun 26, 2019 at 04:01:58AM +0000, Felipe Garrido (fegarrid) wrote:
>         > Hi Benjamin,
>         > 
>         > Just following up on my previous email. Let me know if the below response satisfies your comments.
>         > 
>         > Thanks,
>         > -Felipe
>         > 
>         > From: "Felipe Garrido (fegarrid)" <fegarrid@cisco.com>
>         > Date: Tuesday, May 21, 2019 at 11:59 AM
>         > To: "kaduk@mit.edu" <kaduk@mit.edu>, "iesg@ietf.org" <iesg@ietf.org>
>         > Cc: "draft-ietf-tram-stun-pmtud@ietf.org" <draft-ietf-tram-stun-pmtud@ietf.org>, "tram-chairs@ietf.org" <tram-chairs@ietf.org>, "tasveren@rbbn.com" <tasveren@rbbn.com>, "tram@ietf.org" <tram@ietf.org>
>         > Subject: Benjamin Kaduk's Discuss on draft-ietf-tram-stun-pmtud-10: (with DISCUSS and COMMENT)
>         > 
>         > 
>         > Hi Benjamin,
>         > 
>         > Apologies for the delay in responding, the current authors are having scheduling conflicts and have added me to address the current concerns. Please see my responses inline.
>         > 
>         > thanks
>         > -Felipe
>         > 
>         > 
>         > Benjamin Kaduk has entered the following ballot position for
>         > draft-ietf-tram-stun-pmtud-10: 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/iesg/statement/discuss-criteria.html
>         > for more information about IESG DISCUSS and COMMENT positions.
>         > 
>         > 
>         > The document, along with other ballot positions, can be found here:
>         > https://datatracker.ietf.org/doc/draft-ietf-tram-stun-pmtud/
>         > 
>         > 
>         > 
>         > ----------------------------------------------------------------------
>         > DISCUSS:
>         > ----------------------------------------------------------------------
>         > 
>         > I was going to report the same thing as Adam, but will just say that I support his Discuss.
>         > [FG]: I’ll be addressing this Discuss in Adam’s feedback.
>         > 
>         > I also have one other (also minor and easy to resolve) Discuss point:  Section 4.2.6 needs
>         > to state what the Length field is measuring the length of.
>         > [FG]: Agree that this is required. Adding the following text to Section 4.2.6.
>         > “The Length field specifies the length in bytes of the sequence number and application data fields.”
>         >
>         > ----------------------------------------------------------------------
>         > COMMENT:
>         > ----------------------------------------------------------------------
>         > 
>         > I understand that this document inherently has to be incomplete and "vague",
>         > since the procedure specified within is only meaningful in the context of a
>         > STUN usage or other protocol.  But in general it seems like there could be
>         > greater clarity even within the constraints that we must work under.  My
>         > points are probably less interesting than the ones Adam raised already, though.
>         > The only general observation in this space that I can offer is that some parts of
>         > the text read as if only the Probe packets are going to be monitored for the
>         > report (but this is clearly not the case given the document as a whole).
>         > 
>         > Section 4.2
>         > 
>         >   The Complete Probing mechanism is implemented by sending one or more
>         >   Probe Indications with a PADDING attribute over UDP with the DF bit
>         >   set in the IP header followed by a Report Request to the same server.
>         >   A router on the path to the server can reject this Indication with an
>         >   ICMP message or drop it.
>         > 
>         > nit: I don't think "this" is the right word; perhaps "each" would be
>         > better.
>         > 
>         > 
>         > 
>         > [FG]: Agree, updates will be made.
>         > 
>         > Section 4.2.3
>         > 
>         >   A server supporting this specification will keep the identifiers of
>         >   all packets received in a chronologically ordered list.  The packets
>         >   that are to be associated to a list are selected according to
>         >   Section 5.2 of [RFC4821].  [...]
>         > 
>         > 4821 doesn't talk about "list"s at all, and in fact the indicated section
>         > seems to be talking more about where to store a PMTU value after it has
>         > been determined, rather than what packets to be considering for a report.
>         > So I'm pretty confused about what this sentence is trying to say.
>         > 
>         > 
>         > 
>         > [FG]: Agree. Updated wording to make the statement easier to read.
>         > “The selection process specified in Section 5.2 of [RFC4821] is to be used to determine whether a packet is added with a list.”
>         
>         I still don't understand what "the selection process specified in Section
>         5.2 of [RFC4821]" is -- can you point me to the text from RFC 4821
>         describing the process in question?
>         
>         > Section 4.2.4
>         > 
>         > nit: I think that all instances of "the Probe Indication" should be
>         > replaced with "a Probe Indication", in this section.
>         > 
>         > 
>         > 
>         > [FG]: Agree, updates will be made.
>         > 
>         > Section 4.2.5
>         > 
>         >   When using a checksum as a packet identifier, the client calculates
>         >   the checksum for each packet sent over UDP that is not a STUN Probe
>         >   Indication or Request and keeps this checksum in a chronologically
>         >   ordered list.  The client also keeps the checksum of the STUN Probe
>         >   Indication or Request sent in that same chronologically ordered list.
>         >   The algorithm used to calculate the checksum is similar to the
>         >   algorithm used for the FINGERPRINT attribute (i.e., the CRC-32 of the
>         >   payload XOR'ed with the 32-bit value 0x5354554e [ITU.V42.2002]).
>         > 
>         > (editorial) It's pretty confusing to start out with the split between STUN
>         > and non-STUN messages, only later to clarify that this is because the
>         > FINGERPRINT is used for STUN messages.  So maybe:
>         > 
>         >  When using a checksum as a packet identifier, the client keeps a
>         >  chronologically ordered list of the packets it transmits, along with an
>         >  associated checksum value.  For STUN Probe Indication or Request packets,
>         >  the associated checksum value is the FINGERPRINT value from the packet; for
>         >  other packets a checksum value is computed using a similar algorithm to the
>         >  FINGERPRINT calculation.
>         > 
>         > 
>         > 
>         > [FG]: Agree with changing of the language. It doesn’t change the content and easier to read.
>         > 
>         > Section 4.2.6
>         > 
>         >   When using sequence numbers, a small header similar to the TURN
>         >   ChannelData header [...]
>         > 
>         > Probably want an informative reference for this header.
>         > 
>         > 
>         > 
>         > [FG]: Agree, updates will be made to reference.
>         > Section 6.2
>         > 
>         > 6.2.  PMTUD-SUPPORTED
>         > 
>         >   The PMTUD-SUPPORTED attribute indicates that its sender supports this
>         >   specification.  This attribute has no value part and thus the
>         >   attribute length field is 0.
>         > 
>         > "this specification" is not sufficiently detailed to interoperate, so I
>         > think this needs to be qualified as more like "supports this mechanism, as
>         > incorporated into the STUN usage or protocol being used".
>         > 
>         > 
>         > 
>         > [FG]: Agree, updates will be made.
>         > 
>         > Section 7
>         > 
>         > The contents of the PADDING do not seem to be specified anywhere, so it
>         > could in theory be used as a side channel to convey other information,
>         > which has some potential privacy considerations.  Nowadays we tend to ask
>         > for the value of the padding bytes to be deterministic (but validation
>         > remains optional); I forget if there are STUN-specific considerations that
>         > would discourage just setting them all to zero.
>         > 
>         > 
>         > 
>         > [FG]: Agree.  Adding language to state contents of PADDING.
>         > “The padding bits MUST be set to zero on sending and MUST be ignored by the receiver.”
>         > 
>         > 
>         > 
>         > 
>         > 
>         
>     
>     _______________________________________________
>     tram mailing list
>     tram@ietf.org
>     https://www.ietf.org/mailman/listinfo/tram
>     
>