Re: [quicwg/base-drafts] Closing and draining tidy (#3988)

[ianswett <[e-mail]>]

@ianswett commented on this pull request.

This is a big change, but seems good.  Some specific suggestions on improving the text.

> +
+An immediate close can be used after an application protocol has arranged to
+close a connection.  This might be after the application protocols negotiates a
+graceful shutdown.  The application protocol exchanges whatever messages that
+are needed to cause both endpoints to agree to close the connection, after which
+the application requests that the connection be closed.  When the application
+closes the connection, a CONNECTION_CLOSE frame with an appropriate error code
+will be used to signal closure.
+
+The closing and draining connection states exist to ensure that connections
+close cleanly and that delayed or reordered packets are properly discarded.
+These states SHOULD persist for at least three times the current Probe Timeout
+(PTO) interval as defined in {{QUIC-RECOVERY}}.
+
+Disposing of connection state prior to the exiting the closing or draining state
+could cause delayed or reordered packets to generate an unnecessary stateless

Given the situation, I believe reordered is correct.
```suggestion
could cause reordered packets to generate an unnecessary stateless
```

> +close a connection.  This might be after the application protocols negotiates a
+graceful shutdown.  The application protocol exchanges whatever messages that
+are needed to cause both endpoints to agree to close the connection, after which
+the application requests that the connection be closed.  When the application
+closes the connection, a CONNECTION_CLOSE frame with an appropriate error code
+will be used to signal closure.
+
+The closing and draining connection states exist to ensure that connections
+close cleanly and that delayed or reordered packets are properly discarded.
+These states SHOULD persist for at least three times the current Probe Timeout
+(PTO) interval as defined in {{QUIC-RECOVERY}}.
+
+Disposing of connection state prior to the exiting the closing or draining state
+could cause delayed or reordered packets to generate an unnecessary stateless
+reset. Endpoints that have some alternative means to ensure that late-arriving
+packets on the connection do not induce a response, such as those that are able

```suggestion
packets do not induce a response, such as those that are able
```

> +The closing and draining connection states exist to ensure that connections
+close cleanly and that delayed or reordered packets are properly discarded.
+These states SHOULD persist for at least three times the current Probe Timeout
+(PTO) interval as defined in {{QUIC-RECOVERY}}.
+
+Disposing of connection state prior to the exiting the closing or draining state
+could cause delayed or reordered packets to generate an unnecessary stateless
+reset. Endpoints that have some alternative means to ensure that late-arriving
+packets on the connection do not induce a response, such as those that are able
+to close the UDP socket, MAY end these states earlier to allow
+for faster resource recovery.  Servers that retain an open socket for accepting
+new connections SHOULD NOT end the closing or draining states early.
+
+Once the closing or draining state ends, an endpoint SHOULD discard all
+connection state.  This results in new packets on the connection being handled
+generically.  For instance, an endpoint MAY send a stateless reset in response

This sentence is not very helpful, so I'd remove it unless you have a better suggestion.

> +connection state.  This results in new packets on the connection being handled
+generically.  For instance, an endpoint MAY send a stateless reset in response

This sentence is not very helpful, so I'd remove it unless you have a better suggestion.
```suggestion
connection state.  An endpoint MAY send a stateless reset in response
```

> +While in the closing state, an endpoint could receive packets from a new
+source address, indicating a connection migration; see {{migration}}. An
+endpoint that discards information about validated addresses might be used to
+create an amplification attack; see {{address-validation}}. To avoid being used
+for an amplication attack, an endpoint in the closing state MUST either discard
+packets received from unvalidated addresses or limit the cumulative size of
+packets it sends to unvalidated addresses to 3 times the size of packets it
+receives from the address.

Given other path migration constraints, this text seems odd.  It's possible it could be fully removed.

```suggestion
While in the closing state, an endpoint could receive packets from a new
source address, possibly indicating a connection migration; see {{migration}}.
An endpoint in the closing state MUST either discard packets received from
unvalidated addresses or limit the cumulative size of packets it sends to
unvalidated addresses to 3 times the size of packets it receives from the address.
```

-- 
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/3988#pullrequestreview-464677566