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

Clarify conditions for disabling spin bit #3296

Merged
merged 3 commits into from Jan 7, 2020
Merged

Conversation

martinthomson
Copy link
Member

This builds on #3270 to address the linkability issues that it added. It recommends 1/16 of network paths are disabled randomly as suggested by @kazuho. It also allows the determination to be made per connection ID rather than per-path (which is more granular, but I expect it will have a nearly identical effect). The net effect is that 1/8 paths will not spin as originally intended.

Closes #3270.
Closes #3257.
Closes #2628.

igorlord and others added 2 commits December 11, 2019 11:02
This addresses linkability concerns.

Builds on #3270 and includes suggested improvements.

Closes #3270.
Closes #3257.
Closes #2628.
@martinthomson martinthomson added editorial An issue that does not affect the design of the protocol; does not require consensus. -transport labels Dec 11, 2019
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
performed at the beginning of the connection SHOULD be applied for all paths
used by the connection.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see the last sentence being dropped in the updated text. Is that intentional?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, it seems to be duplicative as the text now talks about paths rather than connections.

Copy link
Contributor

@janaiyengar janaiyengar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One suggestion

used by the connection.
the spin bit is not disabled by the administrator, endpoints MUST disable their
use of the spin bit for a random selection of at least one in every 16 network
paths, or for one in every 16 connection IDs. This ensures that the spin bit
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
paths, or for one in every 16 connection IDs. This ensures that the spin bit
paths, or for one in every 16 connection IDs. Since both endpoints in a connection disable the spin bit independently, this ensures that the spin bit

Copy link
Contributor

@igorlord igorlord left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good technically. It may be slightly confusing, though, that the last sentence only talks about paths, while a sentence before talks about "paths or connection IDs". In any case, the behavior ensures a sufficient amount of traffic without the spin bit to prevent ossification of the bit.

@martinthomson martinthomson merged commit efedc1c into master Jan 7, 2020
@martinthomson martinthomson deleted the spin-per-path branch January 7, 2020 04:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
-transport editorial An issue that does not affect the design of the protocol; does not require consensus.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Disabling Spin bit for what percentage of connections?
6 participants