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

When to ACK on new CE? #307

Open
martinduke opened this issue Dec 20, 2024 · 7 comments · May be fixed by #313
Open

When to ACK on new CE? #307

martinduke opened this issue Dec 20, 2024 · 7 comments · May be fixed by #313

Comments

@martinduke
Copy link
Contributor

Section 6.4 says a new CE mark should be immediately acked "if the Ack-Eliciting Threshold is larger than 1." But a threshold of 1 indicates that some packets are not being acked! Is this intentional, or should it be "larger than zero?"

@ianswett
Copy link
Collaborator

ianswett commented Dec 20, 2024

As you said, everything gets acked with a Reordering Threshold of 1, so technically CE only changes behavior for values greater than 1. But that's probably thinking about it in an overly clever way and we should say zero or state it differently.

@martinduke
Copy link
Contributor Author

No! ack-eliciting-threshold = 0 means everything gets acked! 1 means we skip packets.

@martinduke
Copy link
Contributor Author

So I think this is the current literal interpretation of the draft:

  • ack-eliciting-threshold = 0: ack every packet, the CE marking is neither here nor there
  • ack-eliciting-threshold = 1: per RFC 9000, SHOULD ack any CE packet, period.
  • ack-eliciting-threshold > 1: per the draft, SHOULD ack the first CE only.

So the main question, is this the intent?

If not the intent (i.e. the actual intent is to ack the first CE only, even when threshold=1), then s/larger than 1/larger than 0, and we're done.

If it is the intent, a sentence like "When the ack-eliciting threshold is 1, an ENDPOINT SHOULD ack every CE-marked packet, even if consecutive" would make it clearer.

ianswett added a commit that referenced this issue Dec 20, 2024
@ianswett
Copy link
Collaborator

My mistake, I was thinking of Reordering Threshold, not Ack-Eliciting Threshold.

My intent was to ack only the first CE, so changing it to larger than 0 seems correct. If you're acking every other packet anyway, you'll be getting plenty of feedback.

I wrote #308 to fix this.

@mirjak
Copy link
Contributor

mirjak commented Dec 23, 2024

So I think this is the current literal interpretation of the draft:

  • ack-eliciting-threshold = 0: ack every packet, the CE marking is neither here nor there
  • ack-eliciting-threshold = 1: per RFC 9000, SHOULD ack any CE packet, period.
  • ack-eliciting-threshold > 1: per the draft, SHOULD ack the first CE only.

So the main question, is this the intent?

Yes this interpretation is correct as also discussed in issue #253.

If it is the intent, a sentence like "When the ack-eliciting threshold is 1, an ENDPOINT SHOULD ack every CE-marked packet, even if consecutive" would make it clearer.

I believe we also decided to not add any additional normative language here because as you write above this is simply the behaviour as defined normatively already in RFC9000 and we shouldn't define it twice because that make it harder to change things. I guess we could add a non-normative clarification if that really helps but as you actually interpreted the doc correctly, maybe that's not needed?

@ianswett
Copy link
Collaborator

I closed my first PR, but I do think editorial clarification would be helpful here, since I already forgot what we had agreed upon.

mirjak added a commit that referenced this issue Jan 27, 2025
@mirjak mirjak linked a pull request Jan 27, 2025 that will close this issue
@mirjak
Copy link
Contributor

mirjak commented Jan 27, 2025

I created PR #313

@mirjak mirjak added the has-pr label Jan 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants