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.

Sign up for GitHub

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

Jump to bottom

Reduced flow control limits #447

Closed
martinthomson opened this issue Apr 20, 2017 · 4 comments
Closed

Reduced flow control limits #447

martinthomson opened this issue Apr 20, 2017 · 4 comments
Labels
-transport design An issue that affects the design of the protocol; resolution requires consensus. has-consensus An issue that the Chairs have determined has consensus, by canvassing the mailing list.

Comments

@martinthomson
Copy link
Member

We currently say that a sender MUST NOT reduce these limits, but don't really require or permit a receiver to enforce this. Now, it's true that reordering might cause limits to be received out of order, and it's probably best that we not require a receiver to check that the limit never reduces. However, we might want to allow a receiver to look at packet numbers, determine that the number has been reduced, and kill the connection with a flow control error.
@martinthomson martinthomson added design An issue that affects the design of the protocol; resolution requires consensus. -transport labels Apr 20, 2017
@marten-seemann
Copy link
Contributor

An old WINDOW_UPDATE (with a lower offset) might end up in a packet with a higher packet number if it is a retransmission, and the sender doesn't regenerate the frame.

@mikkelfj
Copy link
Contributor

Is there are more general control frame flooding attack concern? For example starving stream 0.

@martinthomson
Copy link
Member Author

@marten-seemann, you aren't supposed to retransmit WINDOW_UPDATE verbatim like that. A consequence of making a change like this would be a stronger prohibition on dumb retransmission strategies.

@mnot mnot added this to Streams in QUIC Apr 28, 2017
@mnot mnot changed the title Rejecting reduced flow control limits Reduced flow control limits Jun 21, 2017
@martinthomson
Copy link
Member Author

I don't see much point in policing this, or even typing words for it, at this point.

@mnot mnot removed this from Streams in QUIC Mar 15, 2018
@martinthomson martinthomson mentioned this issue Jul 31, 2018
Closed
martinthomson added a commit that referenced this issue Aug 1, 2018
@martinthomson
An endpoint cannot renege on advertised limits
2bff0a9 
For MAX_DATA, MAX_STREAM_DATA, and MAX_STREAM_ID equally, having a MUST
NOT on reneging isn't aligned with practice.  Some implementations
retransmit frame contents without updating the contents of these frames
and that isn't strictly a problem.

Some would have like to police this, but we seem to have consensus to
disallow that.  It creates a coupling between frame processing and
packet numbers that we don't currently have, and it seems that we don't
really want to have either.  More to the point, it prevents the sorts of
optimizations that some people really want.

So, rather than use a MUST NOT renege, observe that it is really just a
an axiom that arises as a consequence of how processing these frames
works: if the value is reduced, then ignore the value.  Whether that is
as a result of out of order arrival, or an optimization, that's OK.

Closes #1612, #447 (the latter for real this time).
@martinthomson martinthomson mentioned this issue Aug 1, 2018
Merged
@mnot mnot added the has-consensus An issue that the Chairs have determined has consensus, by canvassing the mailing list. label Mar 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
-transport design An issue that affects the design of the protocol; resolution requires consensus. has-consensus An issue that the Chairs have determined has consensus, by canvassing the mailing list.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@martinthomson @mnot @mikkelfj @marten-seemann