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

[Bug]: recv_record list length need to be limited #200

Open
Luffbee opened this issue Jul 1, 2022 · 0 comments
Open

[Bug]: recv_record list length need to be limited #200

Luffbee opened this issue Jul 1, 2022 · 0 comments
Assignees
Labels
💡 enhancement New feature or request help wanted The suggestion or request has been accepted, we need you to help us by sending a pull request.

Comments

@Luffbee
Copy link

Luffbee commented Jul 1, 2022

What happened?

Problem 1(BUG)

Currently, the recv_record list has no length limit, so this list may become very long in some cases, and this will lead to problems like OOM (with 16M memory, 1% loss rate, 500 connections trigger OOM kill after half a day).

One case leads to long recv_record is unidirectional connection, i.e., only one side send data. The receiver in this case has no data to send, therefore, may never send ack elicit packets, so the function xqc_recv_record_del will never be called, the recv_record grows long.

Problem 2(Feature Request)

Even if the recv_record length get limited to XQC_MAX_ACK_RANGE_CNT(64), the above case still leads to performance problem: the receiver always send a large ack frame with 64 ranges, increase the overhead of ack parsing, what's more, the function xqc_send_ctl_on_ack_received always iterate all ranges.

I didn't see the standard mention this problem, so I think this is not a bug. However, I suggest to fix this performance problem by:

adding a ping frame (to make it ack elicit) when sending an ack only packet if the range number is higher than a threshold.

This will make the receiver has opportunity to call xqc_recv_record_del.

Steps To Reproduce

The unidirectional connection case with a link with some loss rate.

Relevant log output

No response

@ruiqizhou ruiqizhou added 💡 enhancement New feature or request help wanted The suggestion or request has been accepted, we need you to help us by sending a pull request. labels Sep 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
💡 enhancement New feature or request help wanted The suggestion or request has been accepted, we need you to help us by sending a pull request.
Projects
None yet
Development

No branches or pull requests

3 participants