Re: Erik Kline's Yes on draft-ietf-quic-transport-33: (with COMMENT)

[Lucas Pardue <[e-mail]>]

Hi Erik,

Thanks for the review. I've captured your comments as issues on the QUIC WG
GItHub repository. Links to each are provided as in-line responses.

On Tue, Jan 5, 2021 at 6:18 AM Erik Kline via Datatracker <[e-mail]>
wrote:

> Erik Kline has entered the following ballot position for
> draft-ietf-quic-transport-33: Yes
>
> 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-quic-transport/
>
>
>
> ----------------------------------------------------------------------
> COMMENT:
> ----------------------------------------------------------------------
>
> [[ comments ]]
>
> [ section 8.2.3 ]
>
> * I found this wording a tad odd:
>
>     If the PATH_CHALLENGE frame that resulted in successful path
>     validation was sent in a datagram that was not expanded to at least
>     1200 bytes, the endpoint can regard the address as valid.
>
>   It seems like whether the frame was padded to 1200 or not, if the
> response
>   data matches the challenge data the address can be considered validated.
>
>   I think the point at the end of the sentence is to say that *only* the
>   address, but not the MTU, can be taken as validated.
>

https://github.com/quicwg/base-drafts/issues/4513


> [ section 9.6.3 ]
>
> * Entirely optional, but I wonder if it's worth noting that in certain
>   situations, for example an IPv6-only client and IPv4-only server, the
>   client might be required to evaluate use of an alternate address family
>   address if, for example, some transition mechanism (a la NAT64) was in
>   use.
>

https://github.com/quicwg/base-drafts/issues/4514


> [ section 9.7 ]
>
> * "as this would enable..." reads to me like the opposite of what's
> intended.
>   Perhaps: "as failure to do so would enable..."?
>

https://github.com/quicwg/base-drafts/issues/4515


> [ section 14.1 ]
>
> * I think it might be important to note that this strategy places some
>   restrictions on the use of things like IPv6 extension headers that can be
>   used in these packets.
>
>   For example, on an IPv6-only link with a 1280 MTU, enforcing a 1200 byte
>   UDP payload in these packets leaves only 32 bytes of space for any
>   extension headers.
>
>   I think this is likely fine for these initial packets (vis. section 8.1
> and
>   so on ), but as a general requirement for all packets this could
>   artificially constrain use of new extension headers.
>
>
https://github.com/quicwg/base-drafts/issues/4516

[ section 19.3.1 ]
>
> * This seems intricate enough that it might be nice if there were an
>   Appendix (A.5?) section walking through an example computation or two.
>
>
https://github.com/quicwg/base-drafts/issues/4517

[ section 19.18 ]
>
> * I'm idly wondering if there would be any debugging value in the response
>   including the IP & port to which the response is being sent (i.e. from
>   which the path challenge was received) ... assuming the packet with the
>   PATH_RESPONSE frame is protected.
>
>   Not important though, and perhaps it was already discussed and rejected.
>   (or maybe it's better as some future, entirely separate PATH_INFO frame)
>
>
https://github.com/quicwg/base-drafts/issues/4518


> [[ questions ]]
>
> [ section 8.2.4 ]
>
> * To be clear, this document is effectively saying that it takes no
> position
>   on the interpretation of any ICMP errors received?  Is it up to the
>   implementer to decide if "validated" (in as much as ICMP messages can be
>   validated) Admin Prohibited messages, for example, should constitute a
>   positive confirmation of path failure?  Or is there some very specific
>   stance that should be taken ("never trust that lyin' ICMP!")?
>
>
https://github.com/quicwg/base-drafts/issues/4519

[ section 10.3 ]
>
> * Does this "datagram ends with stateless reset token" scheme mean that
>   implementations must check the output of every packet, including post
>   encryption, and take some action if a (very low probability) collision
>   occurs (meaning the output accidentally produces this 16 byte value
>   but the packet is not intended to be a stateless reset)?
>
>
https://github.com/quicwg/base-drafts/issues/4520


> [[ nits ]]
>
> [ section 7 ]
>
> * There seem to be two paragraphs with the same text about how an endpoint
>   validates ECN support.  Seems like maybe only the first paragraph is
> really
>   necessary (or, put another way: I can't see what new information the
> second
>   paragraph adds).
>
>   (the paragraph below Figure 4 seems to be repeated information)
>

https://github.com/quicwg/base-drafts/issues/4521


> [ section 8.1.1 ]
>
> * "a NEW_TOKEN frames" -> "a NEW_TOKEN frame" or "NEW_TOKEN frames"
>

https://github.com/quicwg/base-drafts/issues/4522


> [ section 17.2.3 ]
>
> * ", as defined Section" -> ", as defined in Section"
>
>
https://github.com/quicwg/base-drafts/issues/4523

Cheers,
Lucas
On behalf of QUIC WG Chairs