Subject: Re: [quicwg/base-drafts] Updated spinbit text (#2564)

From: mirjak <[email-masked]>
Date: Mon, 01 Apr 2019 08:12:00 -0700

mirjak requested changes on this pull request.

> +is available after version negotiation and connection establishment are
+completed. On-path measurement and use of the latency spin bit is further
+discussed in {{QUIC-MANAGEABILITY}}.
+
+The spin bit is an OPTIONAL feature of QUIC. A QUIC stack that chooses to
+support the spin bit MUST implement it as specified in this document.
+
+Each endpoint unilaterally decides if the spin bit is enabled or disabled for a
+connection. Implementations MUST allow administrators of clients and servers
+to disable the spin bit either globally or on a per-connection basis. Even when
+the spin bit is not disabled by the administrator implementations MUST disable
+the spin bit on a randomly chosen fraction of connections. The random selection
+process SHOULD be designed such that on average the spin bit is disabled for at
+least one eighth of network paths. The selection process should be externally
+unpredictable but consistent for any given combination of source and destination
+address and port. The selection process performed at the beginning of the

Should it be consistent for address/port forever? Is this restriction actually needed...? What's the reasoning for this? 

> +the spin bit on a randomly chosen fraction of connections. The random selection
+process SHOULD be designed such that on average the spin bit is disabled for at
+least one eighth of network paths. The selection process should be externally
+unpredictable but consistent for any given combination of source and destination
+address and port. The selection process performed at the beginning of the
+connection SHOULD be applied for all paths used by the connection.
+
+In case multiple connections share the same five-tuple, i.e. same source and
+destination IP address and UDP port the setting of the spin bit needs to be
+coordinated across all connections to ensure a clear signal to any on path
+measurement point, however that might not be feasible.
+
+When the spin bit is disabled, endpoints MAY set the spin bit to any value, and
+MUST accept any incoming value. It is RECOMMENDED that they set the spin bit to
+a random value either chosen independently for each packet, or chosen
+independently for each path and kept constant for that path.

I this we should recommend to set to the same value for the whole connection. That's easier to implement, easier to cope with in the network, and I don't see a benefit of randomly setting it on a per-packet basis for the endpoint.

> +independently for each path and kept constant for that path.
+
+If the spin bit is enabled for the connection, the endpoint maintains a spin
+value and sets the spin bit in the short header to the currently stored
+value when a packet with a short header is sent out. The spin value is
+initialized to 0 in the endpoint at connection start.  Each endpoint also
+remembers the highest packet number seen from its peer on the connection.
+
+When a server receives a short header packet that increments the highest
+packet number seen by the server from the client, it sets the spin value to be
+equal to the spin bit in the received packet.
+
+When a client receives a short header packet that increments the highest
+packet number seen by the client from the server, it sets the spin value to the
+inverse of the spin bit in the received packet.
+

Maybe add a sentence, explaining why this is done, e.g. "With this mechanism, the server reflects the spin value received, while the client 'spins' it after one RTT. On network observers can measure the time between two spin bit toggle events and thereby estimate the end-to-end RTT of a connection, e.g. for network monitoring purposes."

-- 
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/2564#pullrequestreview-221170219

---

From: Martin Thomson <[email-masked]>
Date: Tue, 02 Apr 2019 04:49:17 -0700

martinthomson approved this pull request.

Thanks for writing this up.  Just a few tiny tweaks to recommend.

```suggestion
In case multiple connections share the same five-tuple, that is, have the same source and
```

```suggestion
destination IP address and UDP ports, the setting of the spin bit needs to be
```

That creates a different signal.  Besides, we already discussed this point and (I believe, chairs can correct me), that we agreed to allow this flexibility.

-- 
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/2564#pullrequestreview-221603507

---

From: mirjak <[email-masked]>
Date: Tue, 02 Apr 2019 05:12:19 -0700

mirjak commented on this pull request.

Yes, we should allow for this flexibility. In fact there is no way to not allow for it. I'm just saying we should recommend per-connection random values as the preferred way to do it.

-- 
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/2564#discussion_r271269907

---

From: Jana Iyengar <[email-masked]>
Date: Mon, 08 Apr 2019 22:28:17 -0700

janaiyengar commented on this pull request.

```suggestion
destination IP address and UDP ports, endpoints should try to co-ordinate across all connections to ensure a clear signal to any on-path measurement points.
```

-- 
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/2564#pullrequestreview-224197967

---

From: Jana Iyengar <[email-masked]>
Date: Mon, 08 Apr 2019 22:28:23 -0700

+

```suggestion
coordinated across all connections to ensure a clear signal to any on path
```

-- 
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/2564#pullrequestreview-224197996

---

From: Jana Iyengar <[email-masked]>
Date: Mon, 08 Apr 2019 22:28:38 -0700

```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/2564#pullrequestreview-224198044

---

From: Jana Iyengar <[email-masked]>
Date: Mon, 08 Apr 2019 22:30:06 -0700

```suggestion
MUST accept any incoming value. It is RECOMMENDED that endpoints set the spin bit to
```

-- 
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/2564#pullrequestreview-224198176

---

From: Jana Iyengar <[email-masked]>
Date: Mon, 08 Apr 2019 22:31:03 -0700

janaiyengar approved this pull request.

I'd like to make some editorial changes, but let's get this in and we can iron it out later.

-- 
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/2564#pullrequestreview-224198491

---

From: Lars Eggert <[email-masked]>
Date: Mon, 08 Apr 2019 23:20:47 -0700

larseggert requested changes on this pull request.

> @@ -3932,6 +3938,58 @@ version-independent.  The remaining fields are specific to the selected QUIC
 version.  See {{QUIC-INVARIANTS}} for details on how packets from different
 versions of QUIC are interpreted.
 
+### Latency Spin Bit {#spin-bit}
+
+The latency spin bit enables passive latency monitoring from observation points
+on the network path throughout the duration of a connection. The spin bit is
+only present in the short packet header, since it is possible to measure the
+initial RTT of a connection by observing the handshake. Therefore, the spin bit
+is available after version negotiation and connection establishment are
+completed. On-path measurement and use of the latency spin bit is further
+discussed in {{QUIC-MANAGEABILITY}}.
+
+The spin bit is an OPTIONAL feature of QUIC. A QUIC stack that chooses to
+support the spin bit MUST implement it as specified in this document.

document -> section

> +The latency spin bit enables passive latency monitoring from observation points
+on the network path throughout the duration of a connection. The spin bit is
+only present in the short packet header, since it is possible to measure the
+initial RTT of a connection by observing the handshake. Therefore, the spin bit
+is available after version negotiation and connection establishment are
+completed. On-path measurement and use of the latency spin bit is further
+discussed in {{QUIC-MANAGEABILITY}}.
+
+The spin bit is an OPTIONAL feature of QUIC. A QUIC stack that chooses to
+support the spin bit MUST implement it as specified in this document.
+
+Each endpoint unilaterally decides if the spin bit is enabled or disabled for a
+connection. Implementations MUST allow administrators of clients and servers
+to disable the spin bit either globally or on a per-connection basis. Even when
+the spin bit is not disabled by the administrator implementations MUST disable
+the spin bit on a randomly chosen fraction of connections. The random selection

on a randomly chosen fraction of connections -> for a given connection with a certain likelihood

> +only present in the short packet header, since it is possible to measure the
+initial RTT of a connection by observing the handshake. Therefore, the spin bit

Is it 1/8th of *paths*? I thought it was supposed to be 1/8th of *connections*? (Related to @mirjak's comment below.)

accept -> ignore

-- 
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/2564#pullrequestreview-224208609

---

From: Marcus Ihlar <[email-masked]>
Date: Tue, 09 Apr 2019 06:43:52 -0700

ihlar commented on this pull request.

I do agree with Mirja that it would be nice to recommend per-connection random, but I don't have a strong opinion as I'll have to expect per packet random regardless.

-- 
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/2564#discussion_r273498143

---

From: Marcus Ihlar <[email-masked]>
Date: Tue, 09 Apr 2019 07:02:55 -0700

There was a suggestion to keep decisions consistent for a given path (client - server connection). However, re-reading that discussion, it looks like it's both over engineering and a potential privacy issue. 
I'm fine with simply stating that spinning is disabled for 1/8th of connections. 

-- 
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/2564#discussion_r273507944

---

From: Marcus Ihlar <[email-masked]>
Date: Tue, 09 Apr 2019 14:03:36 +0000 (UTC)

Agree, will change to connections. 

-- 
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/2564#discussion_r273508263

---

From: Marcus Ihlar <[email-masked]>
Date: Tue, 09 Apr 2019 07:04:09 -0700

@ihlar pushed 1 commit.

43f90c22fe7df66c7b3036e0a80d997b83aa4cc7  Apply suggestions from code review

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2564/files/04ef1408fded666cc5f9540ed5cc2a2332a9a9c6..43f90c22fe7df66c7b3036e0a80d997b83aa4cc7

---

From: mirjak <[email-masked]>
Date: Tue, 09 Apr 2019 14:21:29 +0000 (UTC)

In the network you have to handle both but per-packet randomness may be harder to filter ou, while always 1 or 0 is a clear signal. Especially if you look at aggregates too much per-packet randomness could render the whole signal useless, therefore I think recommending to do the right thing is important here, given this document is probably what implementers read.

-- 
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/2564#discussion_r273517181

---

From: Marcus Ihlar <[email-masked]>
Date: Tue, 09 Apr 2019 14:34:37 +0000 (UTC)

@ihlar pushed 1 commit.

3535f5de16aec7ee857ae35c6c17b482ee35ec0f  Editorial changes and clarifications

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2564/files/43f90c22fe7df66c7b3036e0a80d997b83aa4cc7..3535f5de16aec7ee857ae35c6c17b482ee35ec0f

---

From: Marcus Ihlar <[email-masked]>
Date: Tue, 09 Apr 2019 14:38:31 +0000 (UTC)

@ihlar pushed 1 commit.

0558b1717bf5e5fe7bc0a1d41693a8860d01a37b  accept->ignore

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2564/files/3535f5de16aec7ee857ae35c6c17b482ee35ec0f..0558b1717bf5e5fe7bc0a1d41693a8860d01a37b

---

From: MikkelFJ <[email-masked]>
Date: Tue, 09 Apr 2019 07:53:16 -0700

mikkelfj commented on this pull request.

> I'm fine with simply stating that spinning is disabled for 1/8th of connections.

I know why it is there, but does the text have any motivation for having this 1/8 rule?

-- 
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/2564#discussion_r273533560

---

From: Marcus Ihlar <[email-masked]>
Date: Tue, 09 Apr 2019 14:54:57 +0000 (UTC)

Good point, what about:
"When the spin bit is disabled, endpoints MAY set the spin bit to any value, and MUST ignore any incoming value. It is RECOMMENDED that endpoints set the spin bit to a random value chosen independently for each path for a connection."

-- 
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/2564#discussion_r273534561

---

From: Marcus Ihlar <[email-masked]>
Date: Tue, 09 Apr 2019 15:10:04 +0000 (UTC)

> I know why it is there, but does the text have any motivation for having this 1/8 rule?
Good point, could add something like this:
"... The random selection process SHOULD be designed such that on average the spin bit is disabled for at least one eighth of connections. Disabling the spin bit for a large enough fraction of connections makes it difficult for an observer to distinguish between connections that disable the spin bit explicitly or by random selection. "

-- 
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/2564#discussion_r273542272

---

From: MikkelFJ <[email-masked]>
Date: Tue, 09 Apr 2019 15:14:21 +0000 (UTC)

I'd suggest also adding "for privacy reasons"

-- 
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/2564#discussion_r273544208

---

From: Martin Thomson <[email-masked]>
Date: Tue, 09 Apr 2019 17:36:24 -0700

I'm going to merge this soon, with one very important change and one editorial one.

> +the spin bit is not disabled by the administrator implementations MUST disable
+the spin bit for a given connection with a certain likelihood. The random
+selection process SHOULD be designed such that on average the spin bit is
+disabled for at least one eighth of connections. The selection process performed
+at the beginning of the connection SHOULD be applied for all paths used by the
+connection.
+
+In case multiple connections share the same five-tuple, that is, have the same
+source and destination IP address and UDP ports, endpoints should try to
+co-ordinate across all connections to ensure a clear signal to any on-path
+measurement points.
+
+When the spin bit is disabled, endpoints MAY set the spin bit to any value, and
+MUST ignore any incoming value. It is RECOMMENDED that endpoints set the spin
+bit to a random value either chosen independently for each packet or chosen
+independently for each connection.

```suggestion
independently for each connection ID.
```

+support the spin bit MUST implement it as specified in this section.

```suggestion
the spin bit is not disabled by the administrator, implementations MUST disable
```

-- 
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/2564#pullrequestreview-224715115

---

From: Marcus Ihlar <[email-masked]>
Date: Wed, 10 Apr 2019 04:27:36 -0700

@ihlar pushed 1 commit.

c27bf5318d698698a40f464b2edaf106f63aef9e  Final edits

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2564/files/0558b1717bf5e5fe7bc0a1d41693a8860d01a37b..c27bf5318d698698a40f464b2edaf106f63aef9e

---

From: Martin Thomson <[email-masked]>
Date: Wed, 10 Apr 2019 08:29:15 -0700

Merged #2564 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/2564#event-2266490421