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

More text on ACK rate in the applicability draft #386

Merged
merged 4 commits into from Jun 30, 2021
Merged

More text on ACK rate in the applicability draft #386

merged 4 commits into from Jun 30, 2021

Conversation

mirjak
Copy link
Contributor

@mirjak mirjak commented Jun 2, 2021 •
edited

fixes #330

@mirjak mirjak mentioned this pull request Jun 2, 2021
Closed
@mirjak mirjak linked an issue Jun 2, 2021 that may be closed by this pull request
ACK generation #330
Closed
ianswett
ianswett reviewed Jun 2, 2021
View reviewed changes
draft-ietf-quic-applicability.md Outdated Show resolved Hide resolved
draft-ietf-quic-applicability.md Outdated Show resolved Hide resolved
@mirjak @ianswett
Apply suggestions from Ian
36fcd33 
Co-authored-by: ianswett <ianswett@users.noreply.github.com>
ianswett
ianswett reviewed Jun 2, 2021
View reviewed changes
draft-ietf-quic-applicability.md Outdated
Comment on lines 575 to 576
for QUIC traffic. Instead QUIC endpoint need to control the ACK rate
appropriately and some implenentation may even provide an interface to
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
for QUIC traffic. Instead QUIC endpoint need to control the ACK rate
appropriately and some implenentation may even provide an interface to
for QUIC traffic. Instead QUIC endpoints need to control the ACK rate
appropriately and some implenentations may even provide an interface to

martinthomson
martinthomson reviewed Jun 3, 2021
View reviewed changes
Copy link
Member

@martinthomson martinthomson left a comment

Choose a reason for hiding this comment

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

This seems like it is in the right direction. I don't like how much text it adds though.

draft-ietf-quic-applicability.md Outdated
Comment on lines 566 to 580
# ACK-only packets on constrained links

The stragegy on sending acknowlegdements in QUIC version 1 is adopted from
TCP, meaning that usually an QUIC implementation is expected to send one
packet containing an ACK frame for each two packets received.
The cost of sending acknowledgments - in processing cost or link
utilization - could be a significant proportion of available resources if
these resources are constrained. Reducing the rate at which acknowledgments
are generated can preserve these resources and improve overall performance,
for both network processing as well as application-relevant metrics.
these resources are constrained. "ACK-only" packets are not reliably
detectable in QUIC, so network ACK thinning done for TCP is not suitable
for QUIC traffic. Instead QUIC endpoint need to control the ACK rate
appropriately and some implenentation may even provide an interface to
impact the amount of ACK traffic sent. Reducing the rate at which
acknowledgments are generated in constrained situations can preserve resources
and improve overall performance, for both network processing as well as
application-relevant metrics.
Copy link
Member

Choose a reason for hiding this comment

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

I can't suggest over the diff in the way I'd like, but here is my take on this:

# Acknowledgment Efficiency

QUIC version 1 without extensions uses an acknowledgment strategy based on TCP.
That is, every other packet is acknowledged.  However, generating and processing
QUIC acknowledgments can consume significant resources, both in terms of link
utilization and processing time.  Some applications might be able to improve
overall performance by using alternative strategies that reduce the rate of
acknowledgments.

This drops the "thinning" text (that's for the other doc).

There is a lot more to unpack here, but the transport doc already talks about this topic in more precise terms.

Copy link
Contributor

Choose a reason for hiding this comment

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

That looks good, OK also to me.

gorryfair
gorryfair reviewed Jun 3, 2021
View reviewed changes
Copy link
Contributor

@gorryfair gorryfair left a comment

Choose a reason for hiding this comment

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

For me, I think we are now so close ... I'd kind of like to see two paras, since these are two concepts, and it would be great to see both paras in the same section.

draft-ietf-quic-applicability.md Outdated
for both network processing as well as application-relevant metrics.
these resources are constrained. "ACK-only" packets are not reliably
detectable in QUIC, so network ACK thinning done for TCP is not suitable
for QUIC traffic. Instead QUIC endpoint need to control the ACK rate
Copy link
Contributor

Choose a reason for hiding this comment

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

/instead QUIC endpoint need/instead a QUIC endpoint needs/

draft-ietf-quic-applicability.md Outdated
these resources are constrained. "ACK-only" packets are not reliably
detectable in QUIC, so network ACK thinning done for TCP is not suitable
for QUIC traffic. Instead QUIC endpoint need to control the ACK rate
appropriately and some implenentation may even provide an interface to
Copy link
Contributor

Choose a reason for hiding this comment

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

/some implementation may even provide an interface to impact/implementations provide an interface that can reduce/ ??? ... although I like MT's comment also which avoids this.

draft-ietf-quic-applicability.md Outdated
Comment on lines 566 to 580
# ACK-only packets on constrained links

The stragegy on sending acknowlegdements in QUIC version 1 is adopted from
TCP, meaning that usually an QUIC implementation is expected to send one
packet containing an ACK frame for each two packets received.
The cost of sending acknowledgments - in processing cost or link
utilization - could be a significant proportion of available resources if
these resources are constrained. Reducing the rate at which acknowledgments
are generated can preserve these resources and improve overall performance,
for both network processing as well as application-relevant metrics.
these resources are constrained. "ACK-only" packets are not reliably
detectable in QUIC, so network ACK thinning done for TCP is not suitable
for QUIC traffic. Instead QUIC endpoint need to control the ACK rate
appropriately and some implenentation may even provide an interface to
impact the amount of ACK traffic sent. Reducing the rate at which
acknowledgments are generated in constrained situations can preserve resources
and improve overall performance, for both network processing as well as
application-relevant metrics.
Copy link
Contributor

Choose a reason for hiding this comment

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

That looks good, OK also to me.

@mirjak
More or less proposed text by MT
2ac301e 
but added mentioning of constraint networks
@britram britram merged commit 6e51169 into master Jun 30, 2021
@britram britram deleted the mirjak-patch-8 branch June 30, 2021 11:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@martinthomson martinthomson martinthomson left review comments

@gorryfair gorryfair gorryfair left review comments

@ianswett ianswett ianswett approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

ACK generation
5 participants
@mirjak @martinthomson @gorryfair @ianswett @britram