Subject: Re: [quicwg/base-drafts] Limit RCID state (#3547)

From: ekr <[email-masked]>
Date: Fri, 08 May 2020 13:34:26 -0700

@ekr requested changes on this pull request.

> @@ -1069,6 +1069,15 @@ to cease using the connection IDs when requested can result in connection
 failures, as the issuing endpoint might be unable to continue using the
 connection IDs with the active connection.
 
+An endpoint SHOULD limit the state it commits to retiring connection IDs using
+RETIRE_CONNECTION_ID by limiting the number of connection IDs that it tracks for
+retirement to at least twice the active_connection_id_limit. An endpoint MUST
+NOT forget a connection ID without retiring it, though MAY choose to treat
+having too many connection IDs in need of retirement as a connection error of
+type CONNECTION_ID_LIMIT_ERROR.
+
+Endpoints SHOULD NOT issue updates of the Retire Prior To field before receiving
+RETIRE_CONNECTION_ID frames for the previous update of Retire Prior To.

```suggestion
RETIRE_CONNECTION_ID frames that retire all connection IDs indicated by the previous Retire Prior To value.
```

> +An endpoint SHOULD limit the state it commits to retiring connection IDs using
+RETIRE_CONNECTION_ID by limiting the number of connection IDs that it tracks for
+retirement to at least twice the active_connection_id_limit. An endpoint MUST

```suggestion
An endpoint SHOULD limit the number of connection IDs for which it has issued an outstanding RETIRE_CONNECTION_ID to at least twice the active_connection_id limit.
```

> +An endpoint SHOULD limit the state it commits to retiring connection IDs using
+RETIRE_CONNECTION_ID by limiting the number of connection IDs that it tracks for
+retirement to at least twice the active_connection_id_limit. An endpoint MUST

I tried to rewrite the text to make it clearer, but in doing so, I realize that the original text is unclear.

Ignoring the details it says: "It should limit value X to no less than Y."

But I can achieve this by not limiting it at all, which is not the advice we seem to want to give. Is what you are trying to say:

1. There should be a fixed limit.
1. It should be no lower than Y.

What's the rationale for > 2xactive_connection_id.

> @@ -1069,6 +1069,15 @@ to cease using the connection IDs when requested can result in connection
 failures, as the issuing endpoint might be unable to continue using the
 connection IDs with the active connection.
 
+An endpoint SHOULD limit the state it commits to retiring connection IDs using
+RETIRE_CONNECTION_ID by limiting the number of connection IDs that it tracks for
+retirement to at least twice the active_connection_id_limit. An endpoint MUST
+NOT forget a connection ID without retiring it, though MAY choose to treat
+having too many connection IDs in need of retirement as a connection error of

"too many" seems non-specific here. I think we need more detail.

-- 
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/3547#pullrequestreview-408507339

---

From: Martin Thomson <[email-masked]>
Date: Sun, 10 May 2020 16:39:58 -0700

@martinthomson commented on this pull request.

Yes, I think that this is a clearer formulation.  I've taken this and rewritten to:

> An endpoint SHOULD limit the state it commits to retiring connection IDs.  An endpoint SHOULD allow for sending and tracking a number of RETIRE_CONNECTION_ID frames of at least twice the active_connection_id_limit.

These are both recommendations only.  There are reasons why you might not care for these limits.

The 2x is to allow for a combination of you deciding to retire all active connection IDs and the peer asking you to retire all active connection IDs.  If you allow for that much, you will never encounter a situation where the combination of both peers asking for retirement will cause RETIRE_CONNECTION_ID to be dropped or delayed.

That said, this works if you only track 1 RETIRE_CONNECTION_ID at a time, it just might be slow.

-- 
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/3547#discussion_r422717607

---

From: martinduke <[email-masked]>
Date: Mon, 11 May 2020 10:41:33 -0700

@martinduke pushed 1 commit.

017c103440f89092773ab11edd6529972448f2bf  Update draft-ietf-quic-transport.md

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3547/files/f436727acd7dcd49d613cac8ec032a9934c3f6ef..017c103440f89092773ab11edd6529972448f2bf

---

From: Eric Kinnear <[email-masked]>
Date: Mon, 11 May 2020 10:49:23 -0700

@erickinnear approved this pull request.

> @@ -1077,7 +1077,7 @@ having too many connection IDs in need of retirement as a connection error of
 type CONNECTION_ID_LIMIT_ERROR.
 
 Endpoints SHOULD NOT issue updates of the Retire Prior To field before receiving
-RETIRE_CONNECTION_ID frames for the previous update of Retire Prior To.
+RETIRE_CONNECTION_ID frames that retire all connection IDs indicated by the previous Retire Prior To value.

Just wrapping:
```suggestion
RETIRE_CONNECTION_ID frames that retire all connection IDs indicated by the 
previous Retire Prior To value.
```

-- 
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/3547#pullrequestreview-409394322

---

From: martinduke <[email-masked]>
Date: Mon, 11 May 2020 10:50:39 -0700

5b9205c5c06f3b3a06dde340d9418d81f9e64a68  Update draft-ietf-quic-transport.md

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3547/files/017c103440f89092773ab11edd6529972448f2bf..5b9205c5c06f3b3a06dde340d9418d81f9e64a68

---

From: martinduke <[email-masked]>
Date: Mon, 11 May 2020 10:52:42 -0700

@martinduke commented on this pull request.

How about "though it MAY choose to treat having more connection IDs in need of retirement than is consistent with a manageable amount of states as a connection error..."?

A number doesn't feel right here, but I'm open to wording suggestions.

-- 
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/3547#discussion_r423215145

---

From: martinduke <[email-masked]>
Date: Mon, 11 May 2020 10:56:38 -0700

@ekr, "outstanding RETIRE_CONNECTION_ID" implies the frame is actually in flight, which may not be the case for state management reasons. I think @martinthomson 's edit resolves this problem, so I'm going to commit that one. Hopefully it addresses your concerns.

-- 
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/3547#discussion_r423217364

---

From: martinduke <[email-masked]>
Date: Mon, 11 May 2020 18:00:41 +0000 (UTC)

51e52b27a79adc4f85472fcc30cd5de04b28b356  MT's suggestion for ekr

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3547/files/5b9205c5c06f3b3a06dde340d9418d81f9e64a68..51e52b27a79adc4f85472fcc30cd5de04b28b356

---

From: martinduke <[email-masked]>
Date: Mon, 11 May 2020 18:01:24 +0000 (UTC)

If @ekr is happy I'll resolve this sub-thread.

-- 
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/3547#discussion_r423220179

---

From: martinduke <[email-masked]>
Date: Mon, 11 May 2020 12:42:25 -0700

ff574d568b039e4b4e3f691a0d88e2d2ad18a3e9  typos/whitespace

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3547/files/51e52b27a79adc4f85472fcc30cd5de04b28b356..ff574d568b039e4b4e3f691a0d88e2d2ad18a3e9

---

From: ekr <[email-masked]>
Date: Mon, 11 May 2020 15:09:48 -0700

@ekr commented on this pull request.

I think that first sentence needs *some* clarification, though perhaps you don't like mine.

> @@ -1069,6 +1069,16 @@ to cease using the connection IDs when requested can result in connection
 
+An endpoint SHOULD limit the state it commits to retiring connection IDs. An

I feel like "commits to" is unclear. 

Perhaps
"An endpoint should limit the number of connection IDs that it has retired locally and have not yet been acknowledged."

> @@ -1069,6 +1069,16 @@ to cease using the connection IDs when requested can result in connection
 
+An endpoint SHOULD limit the state it commits to retiring connection IDs. An
+endpoint SHOULD allow for sending and trackign a number of RETIRE_CONNECTION_ID

```suggestion
endpoint SHOULD allow for sending and tracking a number of RETIRE_CONNECTION_ID
```

-- 
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/3547#pullrequestreview-409566191

---

From: martinduke <[email-masked]>
Date: Mon, 11 May 2020 16:06:27 -0700

442f5774a16899063d2f3fbae6c46802a6ec86b9  Update draft-ietf-quic-transport.md

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3547/files/ff574d568b039e4b4e3f691a0d88e2d2ad18a3e9..442f5774a16899063d2f3fbae6c46802a6ec86b9

---

From: ekr <[email-masked]>
Date: Mon, 11 May 2020 18:35:30 -0700

Instead of "too many" how about "having connection IDs in need of retirement that exceed this limit..."  or "in excess of this limit". That way we don't have ambiguity about the referent of "too many"

-- 
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/3547#discussion_r423412817

---

From: ianswett <[email-masked]>
Date: Tue, 12 May 2020 04:52:15 -0700

@ianswett approved this pull request.

Still LG, but I like ekr's suggestion

-- 
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/3547#pullrequestreview-409962703

---

From: martinduke <[email-masked]>
Date: Tue, 12 May 2020 07:01:04 -0700

done

-- 
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/3547#discussion_r423757308

---

From: martinduke <[email-masked]>
Date: Tue, 12 May 2020 07:08:48 -0700

47424338c35fc15cc7e022f8077887b45a7ade6c  ekr's suggestions

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3547/files/442f5774a16899063d2f3fbae6c46802a6ec86b9..47424338c35fc15cc7e022f8077887b45a7ade6c

---

From: ekr <[email-masked]>
Date: Fri, 15 May 2020 13:18:11 -0700

LGTM.

-- 
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/3547#issuecomment-629461603

---

From: Eric Kinnear <[email-masked]>
Date: Fri, 15 May 2020 13:32:01 -0700

Latest text looks good 👍 

-- 
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/3547#pullrequestreview-412948279

---

From: Martin Thomson <[email-masked]>
Date: Sun, 17 May 2020 16:57:43 -0700

Merged #3547 into master.

-- 
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/3547#event-3345068977