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

retransmit path_abandon on another path #473

Closed
wants to merge 2 commits into from
Closed

Conversation

mirjak
Copy link
Collaborator

@mirjak mirjak commented Dec 3, 2024

fixes #470

Copy link
Contributor

@qdeconinck qdeconinck left a comment

Choose a reason for hiding this comment

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

I am not sure if this text is really needed, but I am not against some examples.

draft-ietf-quic-multipath.md Outdated Show resolved Hide resolved
draft-ietf-quic-multipath.md Outdated Show resolved Hide resolved
Thanks

Co-authored-by: Quentin De Coninck <[email protected]>
Copy link
Contributor

@yfmascgy yfmascgy left a comment

Choose a reason for hiding this comment

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

My feeling is that It may not be necessary to include this level of detail here. multipath QUIC, as a reliable protocol, will inherently address frame loss, and the decision on how best to handle it—including on which path to retransmit—should be guided by the path scheduler. But I do like the fact that the loss of path_abandon could hurt performance and is called out here.

@huitema
Copy link
Contributor

huitema commented Dec 8, 2024

I agree with @yfmascgy. This is a general issue, not specific to path abandon. We don't need more text.

@mirjak
Copy link
Collaborator Author

mirjak commented Dec 10, 2024

For sure this text is not necessarily needed but I think having a PTO for the path_abandon frame is an important case and therefore pointing this out could people help to get at least this case right. But I have no strong opinion. I think the question is would it potentially help implementors?

@qdeconinck
Copy link
Contributor

qdeconinck commented Dec 13, 2024

I agree with the majority here and still stick with my original opinion that this text is not really needed (or at least not there). We may add something in the implementation considerations regarding the scheduling and the potential performance issues we may have -- though again, I would not fight to have such text in the draft.

@huitema
Copy link
Contributor

huitema commented Dec 14, 2024

I do not think that additional text would help implementers. We have to assume that people implementing a complex real-time transport protocol have sufficient skills and exercise judgment. Piling up extra text leads to generally increasing the size of the document -- maybe one paragraph at a time, but it does add up. And then we have the next issue: making the document so large that implementers shudder at the idea of reading it all.

Also, let me reiterate that we have very little operational experience. We can make recommendations, but they are based on that limited experience. When there are lots of possible valid strategies, it might be better to say nothing.

@mirjak
Copy link
Collaborator Author

mirjak commented Dec 16, 2024

Again, we don't need this text and I'm fine close this PR, however, I want to comment on two points above:

I think one of the few things that we actually know is exactly this advise above, that if you have a PTO for an PATH_ABANDON frame, there is really no good reason to not retransmit it on another path that you know is working for sure. I guess you could even always transmit the first PATH_ABANDON on a path that you know is working for sure but I guess that's something we didn't really experiment with yet. So I think this specific case is really more than a random recommendation.

Second, you mention that the draft gets too long. a) these are really the final edits here, so I actually think it a bit too late to worry about this now. However, more importantly b) we need to do another editorial path. I think there are a bunch of things in the intro part that we can remove or move to the appendix. Maybe there are also a few things from the "normative" part (section 2, 3, 4) that we can move into the implementation guidance. However that part is only 10 pages and I don't think overly lengthly. Again an editorial pass might still help to tighten the language up more but that's on my todo list for after this draft revision.

@huitema
Copy link
Contributor

huitema commented Dec 17, 2024

My code will always send the PATH_ABANDON on another path, will not wait the PTO to do that. This should be the preferred behavior, especially if the path is abandoned because its quality is poor.

@mirjak
Copy link
Collaborator Author

mirjak commented Dec 17, 2024

Should we maybe then change the previous sentence the following way maybe:

OLD:
This, even if connectivity on that path is already broken
but there is still another usable path, it is RECOMMENDED to
send the PATH_ABANDON frames on another path.

NEW
It is RECOMMENDED to
send the PATH_ABANDON frames on another path,
especially if connectivity on the to-be-abandoned path
is expected to be broken.

?

@huitema
Copy link
Contributor

huitema commented Dec 17, 2024

@mirjak we already have that text, "PATH_ABANDON frames can be sent on any path,
not only the path that is intended to be closed. Thus,
even if connectivity on that path is already broken
but there is still another usable path, it is RECOMMENDED to
send the PATH_ABANDON frames on another path."

I think that's sufficient. We might want to tune it a little, but the basic idea is that PATH_ABANDON should be sent on a path that the node wants to keep, not on the path that the node wants to abandon. The handling of repetition is just a consequence of that preference, and does not need to be explicit -- there is no difference there between path abandon and any other frame.

@mirjak
Copy link
Collaborator Author

mirjak commented Dec 20, 2024

@huitema yes, I cited the sentence already above (please note the OLD/NEW style). My proposal was simply to slightly rephrase this sentence to make it more clear.

For me the way I interpret the current sentence is that gives the recommendation to send on another path only if you know that the current path is broken (which is nearly obvious). I think what we want to say instead is that is it always recommend to send the path abandon frame on another path. Right? That is to my interpretation not what the current sentence says (or actually wasn't my intention when I was writing it because I believe I wrote this at some point).

@mirjak mirjak closed this Dec 20, 2024
mirjak added a commit that referenced this pull request Dec 20, 2024
I created this PR based on the discussion in (the now-closed) PR #473.

This is a slight change of the normative recommendation as it generally recommends to send the PATH_ABANDON on another path.

I understood that this is what we want to recommend but if not, we cannot also not apply this PR. Please comment!
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Recommend that PATH_ABANDON should be retransmitted on another path
5 participants