Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

You don't always need the draining period #871

Closed
ekr opened this issue Oct 13, 2017 · 3 comments · Fixed by #899
Closed

You don't always need the draining period #871

ekr opened this issue Oct 13, 2017 · 3 comments · Fixed by #899
Labels
-transport design An issue that affects the design of the protocol; resolution requires consensus. has-consensus An issue that the Chairs have determined has consensus, by canvassing the mailing list.

Comments

@ekr
Copy link
Collaborator

ekr commented Oct 13, 2017

PR#868 doubled down on the draining and the proposal for PR#870 is about creating an exception to the draining period.

The basic problem here is spec confusion about the draining period, specifically, whether it's some sort "do nothing state" or whether it's a state where you're supposed to send packets. So, we have:

The draining period persists for three times the current Retransmission Timeout
(RTO) interval as defined in {{QUIC-RECOVERY}}.  During this period, new packets
can be acknowledged, but no new application data can be sent on the connection.

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.

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.

However, in the specific case of stateless reset, the only thing that the connection needs to do is the thing in the fourth paragraph, which is after the draining period, namely discard packets (note that it can't send a stateless reset because that's forbidden).

The right fix for this is to have two states:

  1. A state where you're expected to occasionally respond to new packets with something that says "go away" (e.g., CONNECTION_CLOSE) and where we're prescriptive about you doing that.

  2. A state where you're expected to silently discard packets (perhaps with a stateless reset).

@huitema
Copy link
Contributor

huitema commented Oct 20, 2017

I agree, the text is confusing. From what I understand, the spec is to absorb packets and occasionally repeat the CONNECTION_CLOSE (or APPLICATION_CLOSE) for 3* RTO. The peer is not supposed to do anything with the packets, just count them and send a repeat if the count reaches a threshold. Arguably, if the peer receives a CONNECTION CLOSE, it does not even need repeating its own close again. The only real requirement is to NOT send a stateless reset during that time.

There is a related issue about entering a draining period after idle timeout. How is that a different behavior from adding 3*RTO to the idle timeout period?

@huitema
Copy link
Contributor

huitema commented Oct 20, 2017

Also, the text does not explicitly say what the receiver of the CONNECTION_CLOSE shall do. From what I read, it is OK to just go away, no draining period required.

martinthomson added a commit that referenced this issue Oct 25, 2017
As @ekr points out, there was definitely more than one closing state.  I've
modified this to the following three:

1. draining - when you know that the other side is closing, this is just to
   absorb stray packets

2. closing - when you have initiated the close and you need to re-send the
   close in addition to absorbing stray packets (this could be longer than the
   3RTO period if we like, but I've opted for keeping a shared timer)

3. timed out - when you timed out, and you are willing to revive the connection
   if new packets arrive; this one being optional

In the process I realized that we needed greater clarity about how to respond
to a closing frame (thanks to @huitema for the prompting).  I allow the
recipient to generate one closing frame, but no more than that.  They enter the
draining period thereafter and can't send any more packets.  Having two peers
in the closing state could result in a constant back-and-forth.

Editorial: I added the term "closing frame" to cover CONNECTION_CLOSE and
APPLICATION_CLOSE collectively, which were getting cumbersome.

Closes #871.
martinthomson added a commit that referenced this issue Oct 25, 2017
As @ekr points out, there was definitely more than one closing state.  I've
modified this to the following three:

1. draining - when you know that the other side is closing, this is just to
   absorb stray packets

2. closing - when you have initiated the close and you need to re-send the
   close in addition to absorbing stray packets (this could be longer than the
   3RTO period if we like, but I've opted for keeping a shared timer)

3. timed out - when you timed out, and you are willing to revive the connection
   if new packets arrive; this one being optional

In the process I realized that we needed greater clarity about how to respond
to a closing frame (thanks to @huitema for the prompting).  I allow the
recipient to generate one closing frame, but no more than that.  They enter the
draining period thereafter and can't send any more packets.  Having two peers
in the closing state could result in a constant back-and-forth.

Editorial: I added the term "closing frame" to cover CONNECTION_CLOSE and
APPLICATION_CLOSE collectively, which were getting cumbersome.

Closes #871.
martinthomson added a commit that referenced this issue Oct 25, 2017
As @ekr points out, there was definitely more than one closing state.  I've
modified this to the following three:

1. draining - when you know that the other side is closing, this is just to
   absorb stray packets

2. closing - when you have initiated the close and you need to re-send the
   close in addition to absorbing stray packets (this could be longer than the
   3RTO period if we like, but I've opted for keeping a shared timer)

3. timed out - when you timed out, and you are willing to revive the connection
   if new packets arrive; this one being optional

In the process I realized that we needed greater clarity about how to respond
to a closing frame (thanks to @huitema for the prompting).  I allow the
recipient to generate one closing frame, but no more than that.  They enter the
draining period thereafter and can't send any more packets.  Having two peers
in the closing state could result in a constant back-and-forth.

Editorial: I added the term "closing frame" to cover CONNECTION_CLOSE and
APPLICATION_CLOSE collectively, which were getting cumbersome.

Closes #871.
martinthomson added a commit that referenced this issue Oct 25, 2017
As @ekr points out, there was definitely more than one closing state.  I've
modified this to the following three:

1. draining - when you know that the other side is closing, this is just to
   absorb stray packets

2. closing - when you have initiated the close and you need to re-send the
   close in addition to absorbing stray packets (this could be longer than the
   3RTO period if we like, but I've opted for keeping a shared timer)

3. timed out - when you timed out, and you are willing to revive the connection
   if new packets arrive; this one being optional

In the process I realized that we needed greater clarity about how to respond
to a closing frame (thanks to @huitema for the prompting).  I allow the
recipient to generate one closing frame, but no more than that.  They enter the
draining period thereafter and can't send any more packets.  Having two peers
in the closing state could result in a constant back-and-forth.

Editorial: I added the term "closing frame" to cover CONNECTION_CLOSE and
APPLICATION_CLOSE collectively, which were getting cumbersome.

Closes #871.
@martinthomson
Copy link
Member

I think that I've addressed these comments in a new PR. I agree with @ekr that splitting the states makes things clearer. I've also tried to address the confusion that @huitema had.

To answer the questions:

There is a related issue about entering a draining period after idle timeout. How is that a different behavior from adding 3*RTO to the idle timeout period?

The difference would be that you tell the application that the connection is done with after the idle timeout runs out, which might result in the packets being poorly handled otherwise. The draining period would remain to ensure that the other side didn't somehow send just before their timer ran down. #899 allows a bit of flexibility here, and likely could be made more concrete in future, but all possibilities are allowed (drop the packets, send a CONNECTION_CLOSE, or even revive the connection).

the text does not explicitly say what the receiver of the CONNECTION_CLOSE shall do

It sort of did say that you can just go away if you string things together, but I think that was a mistake. You might receive reordered packets after the close, so you should hang around still.

@martinthomson martinthomson added -transport design An issue that affects the design of the protocol; resolution requires consensus. labels Oct 25, 2017
@mnot mnot added this to Reliability in QUIC Nov 7, 2017
janaiyengar pushed a commit that referenced this issue Nov 29, 2017
* Rework draining period into two (or three) states

As @ekr points out, there was definitely more than one closing state.  I've
modified this to the following three:

1. draining - when you know that the other side is closing, this is just to
   absorb stray packets

2. closing - when you have initiated the close and you need to re-send the
   close in addition to absorbing stray packets (this could be longer than the
   3RTO period if we like, but I've opted for keeping a shared timer)

3. timed out - when you timed out, and you are willing to revive the connection
   if new packets arrive; this one being optional

In the process I realized that we needed greater clarity about how to respond
to a closing frame (thanks to @huitema for the prompting).  I allow the
recipient to generate one closing frame, but no more than that.  They enter the
draining period thereafter and can't send any more packets.  Having two peers
in the closing state could result in a constant back-and-forth.

Editorial: I added the term "closing frame" to cover CONNECTION_CLOSE and
APPLICATION_CLOSE collectively, which were getting cumbersome.

Closes #871.

* Updates based on review feedback
@mnot mnot removed this from Reliability in QUIC Mar 6, 2018
@mnot mnot added the has-consensus An issue that the Chairs have determined has consensus, by canvassing the mailing list. label Mar 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
-transport design An issue that affects the design of the protocol; resolution requires consensus. has-consensus An issue that the Chairs have determined has consensus, by canvassing the mailing list.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants