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

PATH_* frames should not be ACK-eliciting #2372

Closed
rpaulo opened this issue Jan 28, 2019 · 12 comments
Closed

PATH_* frames should not be ACK-eliciting #2372

rpaulo opened this issue Jan 28, 2019 · 12 comments
Labels
-recovery 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

@rpaulo
Copy link
Contributor

rpaulo commented Jan 28, 2019

Recovery draft assumes only PADDING and ACK are not ACK-eliciting. However, PATH_CHALLENGE is not ACK-eliciting since it's ACK'ed by PATH_RESPONSE.

@ianswett
Copy link
Contributor

I'm not finding the text in transport that says they're not ack-eliciting, and I'd rather make them ack-eliciting unless there's an important reason they can't be. I did find text that says acknowledgement is not a substitute for PATH_RESPONSE:
"Receipt of an acknowledgment for a packet containing a
PATH_CHALLENGE frame is not adequate validation, since the
acknowledgment can be spoofed by a malicious peer."

@rpaulo
Copy link
Contributor Author

rpaulo commented Jan 28, 2019

https://quicwg.org/base-drafts/draft-ietf-quic-recovery.html#rfc.section.2

Ack-eliciting Frames: All frames besides ACK or PADDING are considered ack-eliciting

I don't know how connection migration would work if PATH_CHALLENGE triggers PATH_RESPONSE+ACK since ACK is a non-probing frame (i.e., it triggers migration).

Or does this mean the PATH_CHALLENGE will be ACK'ed on the "main" path and the PATH_RESPONSE will be sent on the alternative path?

@ianswett
Copy link
Contributor

I think you're supposed to send the ACK on the non-probing path, but I'm not the connection migration expert.

The main benefit of them not being ack-eliciting in my mind is it makes it clear that their loss should not cause a congestion control reaction.

@rpaulo
Copy link
Contributor Author

rpaulo commented Jan 28, 2019

I think it would greatly simplify things if they were made non ack-eliciting. Like you said, if they are not ACK'ed then it doesn't change the congestion window.

@ianswett ianswett changed the title PATH_* frames are not ACK-eliciting PATH_* frames should not be ACK-eliciting Jan 28, 2019
@ianswett ianswett added the design An issue that affects the design of the protocol; resolution requires consensus. label Jan 28, 2019
@ianswett ianswett added this to Connection ID / Migration in Transport / Recovery / TLS Jan 28, 2019
@erickinnear
Copy link
Contributor

I think the original intent was that they would be acked on the main path, but making them non ack-eliciting would also remove a (somewhat minor) linkability concern.
Having them not influence congestion control seems like a strong reason to make them non ack-eliciting.
Either way, seems like clarification would be helpful here.

@huitema
Copy link
Contributor

huitema commented Jan 28, 2019

I implemented these frames as non ack eliciting. Trying to manage ACK for them would be weird.

@ianswett
Copy link
Contributor

I'm happy to make these non ACK-eliciting now that I think about it more.

If anyone objects to this change, I'd like to hear why.

@kazuho
Copy link
Member

kazuho commented Jan 28, 2019

Does making it non ACK-eliciting imply that we would no longer be allowed to bundle other types of frame in a path probing packet?

I think I am fine with that, but I would like to confirm what the impact is.

@erickinnear
Copy link
Contributor

One potential thought: anything else that you bundle that would be ack-eliciting should also be considered an immediate migration and would cause the ack to come back on the new path, which means there's no issue.
I think the new requirement would be: any frames considered probing packets that don't trigger migration should also be non ack-eliciting.

@kazuho
Copy link
Member

kazuho commented Jan 28, 2019

I think the issue is if we want probing packet to not be ACK-eliciting.

If the answer is yes, then we should consider what to do with other types of frames that are allowed to be included in a path probing packet: i.e., PATH_RESPONSE, NEW_CONNECTION_ID. I'd assume that we at least want NEW_CONNECTION_ID frame to be ACK-eliciting.

(thanks to @rpaulo for the discussion).

@martinthomson
Copy link
Member

Tokyo conclusion: this is not easy, but what we have is tolerable. If we obtain more data, then we will reopen the issue.

@mikkelfj
Copy link
Contributor

If you consider future multi-path where you might want to ACK on specific paths for RTT reasons, it might simplify things to not mix in PATH_* in the equation.

If you consider RTT today, it might be misleading to ACK on a different path that could have a very different RTT measure.

@mnot mnot added the has-consensus An issue that the Chairs have determined has consensus, by canvassing the mailing list. label Feb 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
-recovery 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
No open projects
Transport / Recovery / TLS
Connection ID / Migration
Development

No branches or pull requests

8 participants