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

Explicitly allow discard of packets <40 bytes #2864

Closed
wants to merge 1 commit into from
Closed

Conversation

martinthomson
Copy link
Member

The stateless reset text implied a lot, but never really explicitly said
what to do when going below the minimum recommended size.

This is a design change to an issue marked editorial. Enjoy.

Closes #2153.

The stateless reset text implied a lot, but never really explicitly said
what to do when going below the minimum recommended size.

This is a design change to an issue marked editorial.  Enjoy.

Closes #2153.
@martinthomson martinthomson added design An issue that affects the design of the protocol; resolution requires consensus. -transport labels Jul 1, 2019
@@ -2525,9 +2526,6 @@ a small packet might result in Stateless Reset not being useful in detecting
cases of broken connections where only very small packets are sent; such
failures might only be detected by other means, such as timers.

An endpoint can increase the odds that a packet will trigger a Stateless Reset
Copy link
Contributor

Choose a reason for hiding this comment

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

Why did you remove this sentence?

Copy link
Member Author

Choose a reason for hiding this comment

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

See #2153. Or were you asking for it to be retained?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes. I think it's a good idea to always send a packet padded to > 40 bytes after quiescence, so you'd be able to quickly recover by receiving the stateless reset.

Copy link
Contributor

Choose a reason for hiding this comment

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

It might be nice to explicitly mention the packet after quiescence as part of this advice, and/or advise sending at least one 40+ byte packet in each flight.

@DavidSchinazi
Copy link
Contributor

This PR adds a recommendation involving the number 40 without explaining where that number comes from. It would be nice to include a reference to the section that explains the math.

That said, I don't think this number should depend on the maximum connection ID length, as discussed here. But feel free to tell me that this PR is not the place to discuss that.

@martinthomson
Copy link
Member Author

If you want to talk about the idea that the connection ID might change in length, that's a separate issue, which you should feel free to open. I am fairly sure that I can show that this isn't a meaningful insight, but happy to discuss it there.

@DavidSchinazi
Copy link
Contributor

@martinthomson filed #2869.

@@ -2510,7 +2510,8 @@ Reset in response, which could lead to an infinite exchange.
An endpoint MUST ensure that every Stateless Reset that it sends is smaller than
the packet which triggered it, unless it maintains state sufficient to prevent
looping. In the event of a loop, this results in packets eventually being too
small to trigger a response.
small to trigger a response. An endpoint MAY choose not to send a Stateless
Reset in response to a packet that is smaller than 40 bytes.
Copy link
Contributor

Choose a reason for hiding this comment

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

The existing text was confusing, but I don'd see how this text is relevant to looping?

Copy link
Contributor

Choose a reason for hiding this comment

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

This provides the end condition to the loop.

@@ -2510,7 +2510,8 @@ Reset in response, which could lead to an infinite exchange.
An endpoint MUST ensure that every Stateless Reset that it sends is smaller than
the packet which triggered it, unless it maintains state sufficient to prevent
looping. In the event of a loop, this results in packets eventually being too
small to trigger a response.
small to trigger a response. An endpoint MAY choose not to send a Stateless
Reset in response to a packet that is smaller than 40 bytes.
Copy link
Contributor

Choose a reason for hiding this comment

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

This provides the end condition to the loop.

@@ -2525,9 +2526,6 @@ a small packet might result in Stateless Reset not being useful in detecting
cases of broken connections where only very small packets are sent; such
failures might only be detected by other means, such as timers.

An endpoint can increase the odds that a packet will trigger a Stateless Reset
Copy link
Contributor

Choose a reason for hiding this comment

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

It might be nice to explicitly mention the packet after quiescence as part of this advice, and/or advise sending at least one 40+ byte packet in each flight.

@martinthomson
Copy link
Member Author

We've moved on here, allowing smaller outbound packets.

@martinthomson martinthomson deleted the issue2153 branch August 27, 2019 05:22
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Increase the odds of a stateless reset?
6 participants