Subject: Re: [quicwg/base-drafts] Rework draining period into two (or three) states (#899)

From: Mike Bishop <[email-masked]>
Date: Tue, 14 Nov 2017 02:06:18 +0000 (UTC)

MikeBishop approved this pull request.

Good; minor nits, some structural feedback.

-Different treatment is given to packets that are received while a connection is
-in the draining period depending on how the connection was closed.  An endpoint
-that is in a draining period MUST NOT send packets unless they contain a
-CONNECTION_CLOSE or APPLICATION_CLOSE frame.
+Disposing of connection state prior to the end of the draining period could
+result in delayed or reordered packets to be handled poorly.  Endpoints that

Nit:  Awkward sentence structure.  How about "result in" => "cause"?

-Different treatment is given to packets that are received while a connection is
-in the draining period depending on how the connection was closed.  An endpoint
-that is in a draining period MUST NOT send packets unless they contain a
-CONNECTION_CLOSE or APPLICATION_CLOSE frame.
+Disposing of connection state prior to the end of the draining period could
+result in delayed or reordered packets to be handled poorly.  Endpoints that
+have some alternative means to ensure that late-arriving packets on the
+connection do not create QUIC state, such as by closing the UDP socket, MAY use

This avoids creating QUIC state, but might generate some ICMP spam (port unreachable).  Have we agreed that's okay?

 Once the draining period has ended, an endpoint SHOULD discard per-connection
-state.  This results in new packets on the connection being discarded.  An
-endpoint MAY send a stateless reset in response to any further incoming packets.
+state.  This results in new packets on the connection being handled generically.
+For instance, an endpoint MAY send a stateless reset in response to any further
+incoming packets.

The ordering feels odd here.  If you go to "closing" first and then "draining," why do we talk about "draining" first?  Wouldn't it make more sense to talk about closing, then transition to draining?

 The time at which an idle timeout takes effect won't be perfectly synchronized
-on peers.  A connection enters the draining period when the idle timeout
-expires.  During this time, an endpoint that receives new packets MAY choose to
-restore the connection.  Alternatively, an endpoint that receives packets MAY
-signal the timeout using an immediate close.
+on peers.  An endpoint MAY enter a third state where receipt of new packets
+causes the connection to be restored to normal operation.

"A third state" in a second subsection at the same level feels a little off.  If you want to talk about three closing states, I'd make closing/draining/idle three subsections in one place (or just merge this text into the same section above and don't rely on section headers to demarcate).

Also, these sections no longer line up with the three bullet points immediately before them.  Maybe we need to talk about "closing states" and "ways connections close."

 ### Immediate Close
 
-An endpoint sends a CONNECTION_CLOSE or APPLICATION_CLOSE frame to terminate the
-connection immediately.  Either frame causes all open streams to immediately
-become closed; open streams can be assumed to be implicitly reset.  After
-sending or receiving a CONNECTION_CLOSE frame, endpoints immediately enter a
-draining period.
-
-During the draining period, an endpoint that sends a CONNECTION_CLOSE or
-APPLICATION_CLOSE frame SHOULD respond to any subsequent packet that it receives
-with another packet containing either close frame.  To reduce the state that an
-endpoint maintains in this case, it MAY send the exact same packet.  However,
-endpoints SHOULD limit the number of packets they generate containing either
-close frame.  For instance, an endpoint could progressively increase the number
-of packets that it receives before sending additional packets.
+An endpoint sends a closing frame, either CONNECTION_CLOSE or APPLICATION_CLOSE,
+to terminate the connection immediately.  Either closing frame causes all open

Nit:  You don't need "open" here.  All streams are immediately closed; open streams get reset.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/899#pullrequestreview-76308681

---

From: Martin Thomson <[email-masked]>
Date: Wed, 29 Nov 2017 04:35:31 +0000 (UTC)

@martinthomson pushed 1 commit.

60bc004  Updates based on review feedback

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/899/files/0b05b99661c01e881f5e317ce22f4369224b0589..60bc0046a2efe716be401a08bb590220a331484a

---

From: Martin Thomson <[email-masked]>
Date: Wed, 29 Nov 2017 04:36:03 +0000 (UTC)

martinthomson commented on this pull request.

There's a question about 5-tuple and migration after closing/draining.  Also about key update.  I'm explicitly not addressing that here.  I'll file a follow-up with a short paragraph on those points.

You would have to assume that that is acceptable.  Probably not worth mentioning though (to avoid a serious digression).

-Different treatment is given to packets that are received while a connection is
-in the draining period depending on how the connection was closed.  An endpoint
-that is in a draining period MUST NOT send packets unless they contain a
-CONNECTION_CLOSE or APPLICATION_CLOSE frame.
+Disposing of connection state prior to the end of the draining period could
+result in delayed or reordered packets to be handled poorly.  Endpoints that
+have some alternative means to ensure that late-arriving packets on the
+connection do not create QUIC state, such as by closing the UDP socket, MAY use
+an abbreviated draining period which can allow for faster resource recovery.
+Servers that retain an open socket for accepting new connections SHOULD NOT exit
+the draining period early.

I've taken a stab at this.  connection ID is pretty much it.  I will use MAY for the packet protection keys.  I'll observe here (but not in the text) that a key update will be difficult to manage, as much as a path migration is difficult.  Let's not over-optimize for those cases.

This is a great suggestion.  I've taken the suggestion.

I think that I might remove this third state thing and not mention revivification.

-endpoints SHOULD limit the number of packets they generate containing either
-close frame.  For instance, an endpoint could progressively increase the number
-of packets that it receives before sending additional packets.
+An endpoint sends a closing frame, either CONNECTION_CLOSE or APPLICATION_CLOSE,
+to terminate the connection immediately.  Either closing frame causes all open
+streams to immediately become closed; open streams can be assumed to be
+implicitly reset.
+
+After sending a closing frame, endpoints immediately enter a closing period.
+After receiving a closing frame, endpoints enter a draining period.  An endpoint
+that receives a closing frame MAY send a single closing frame before entering a
+draining period, but MUST NOT send further packets, which could result in a
+constant exchange of closing frames until the closing period on either peer
+ended.
+

I considered that, but a strict prohibition is easier.  It would be possible to reserve a codepoint for this, but we would also have to mandate decryption and I don't want to do that.

> +implicitly reset.
+
+After sending a closing frame, endpoints immediately enter a closing period.
+After receiving a closing frame, endpoints enter a draining period.  An endpoint
+that receives a closing frame MAY send a single closing frame before entering a
+draining period, but MUST NOT send further packets, which could result in a
+constant exchange of closing frames until the closing period on either peer
+ended.
+
+During the draining period, an endpoint that sends a closing frame SHOULD
+respond to any subsequent packet that it receives with another packet containing
+either closing frame.  To reduce the state that an endpoint maintains in this
+case, it MAY send the exact same packet.  However, endpoints SHOULD limit the
+number of packets they generate containing a closing frame.  For instance, an
+endpoint could progressively increase the number of packets that it receives
+before sending additional packets.

Yep, that's a straight error on my part.  I've rearranged this a little so that the distinction is clearer.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/899#pullrequestreview-79733130

---

From: janaiyengar <[email-masked]>
Date: Wed, 29 Nov 2017 15:58:24 -0800

janaiyengar approved this pull request.

Looks great!

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/899#pullrequestreview-80040128

---

From: janaiyengar <[email-masked]>
Date: Wed, 29 Nov 2017 23:59:55 +0000 (UTC)

Merged #899.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/899#event-1364671105