alibaba / xquic

XQUIC Library released by Alibaba is a cross-platform implementation of QUIC and HTTP/3 protocol.
Apache License 2.0
1.65k stars 326 forks source link

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

Open Luffbee opened 2 years ago

Luffbee commented 2 years ago

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