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

[core] Revise SND DROP logic #1910

Open
maxsharabayko opened this issue Apr 1, 2021 · 1 comment
Open

[core] Revise SND DROP logic #1910

maxsharabayko opened this issue Apr 1, 2021 · 1 comment
Labels
[core] Area: Changes in SRT library core Epic Type: Bug Indicates an unexpected problem or unintended behavior
Milestone

Comments

@maxsharabayko
Copy link
Collaborator

maxsharabayko commented Apr 1, 2021

CUDT::checkNeedDrop(..) function.

Some contansts for further reference:

const int drop_delay_ms = max(SRTO_PEERLATENCY + SRTO_SNDDROPDELAY, 1s);
const int drop_threshold_ms = drop_delay_ms  + 20ms;

1. Dropping on Buffer Timespan?

Sender dropping logic is applied if the sender buffer timespan exceeds drop_threshold_ms.
Instead, the delay of the oldest packet in the buffer should be used (Tnow - OldestPacketTS).

At the same time, only packets older than Tnow - drop_threshold_ms are dropped by sender.

See issue #898.

2. Congestion if timespan > 0.5 x latency

Sending is forced (ignores the output pacing) if the buffer timespan exceeds 0.5 x SRTO_PEERLATENCY.
If there is no packet loss, SRT sender holds a packet in its buffer for at least (RTT + 10ms).
If SRTO_LATENCY is configured below 2xRTT, SRT sender will always think there is congestion and will tend to ignore SRTO_MAXBW option (or any other output pacing).

3. Dropping too early

It takes ~RTT/2 for a packet to reach the receiver. Then receiver may request a retransmission for SRTO_RCVLATENCY.
Retransmission takes ~1xRTT.
The latest reasonable time for a receiver to request a retransmission would be (from sender's clock):

RTT0/2 + SRTO_RCVLATENCY - RTT

where RTT0/2 is the sender->receiver network delay at the time of connection.

Sender receives this request at

RTT0/2 + SRTO_RCVLATENCY - RTT/2

If RTT0 == RTT, dropping after SRTO_RCVLATENCY + 20ms is ok-ish.

If RTT0 > RTT, then sender drops too early.

4. Receiver Handling of Drop Request

The drop request from the sender does not always have the message number, but most of the time it has the range of sequence numbers to drop. However, the receiver buffer drops packets only by message number.

See CUDT::processCtrlDropReq(..).

Related Issues

#713, #898, #1519. #1738, #2003

@maxsharabayko maxsharabayko added Type: Bug Indicates an unexpected problem or unintended behavior [core] Area: Changes in SRT library core Epic labels Apr 1, 2021
@maxsharabayko maxsharabayko added this to the v1.4.4 milestone Apr 1, 2021
@maxsharabayko maxsharabayko modified the milestones: v1.4.4, v1.4.5 Aug 12, 2021
@maxsharabayko maxsharabayko modified the milestones: v1.4.5, Next Release Apr 25, 2022
@maxsharabayko maxsharabayko modified the milestones: v1.5.1, Next release Sep 12, 2022
@maxsharabayko maxsharabayko modified the milestones: v1.5.2, Backlog Dec 8, 2022
@maxsharabayko
Copy link
Collaborator Author

Some results related to SRTO_SNDDROPDELAY.

Ran tests to determine the RCV buffer size leading to a 0% packet drop. See PR #2815.

50 Mbps, 100 ms RTT, 25% packet loss, SRT latency 1.0 seconds.

The target RCV buffer size recommended by the Configuration Guidelines is 7339392 bytes.

The value to recommend based on the results below is 10339392 (+40%).

SRT v1.5.3

RCVBUF:  7339392 pktRcvUnique 70568, pktRcvDrop: 20988 (29.74%)
RCVBUF:  7839392 pktRcvUnique 72596, pktRcvDrop: 20726 (28.55%)
RCVBUF:  8339392 pktRcvUnique 71079, pktRcvDrop: 20844 (29.33%)
RCVBUF:  8839392 pktRcvUnique 94360, pktRcvDrop:    92 (0.1%)
RCVBUF:  9339392 pktRcvUnique 94674, pktRcvDrop:    33 (0.03%)
RCVBUF:  9839392 pktRcvUnique 94463, pktRcvDrop:     3 (0%)
RCVBUF: 10339392 pktRcvUnique 94710, pktRcvDrop:     2 (0%)
RCVBUF: 10839392 pktRcvUnique 94752, pktRcvDrop:     2 (0%)
RCVBUF: 11339392 pktRcvUnique 94522, pktRcvDrop:     1 (0%)
RCVBUF: 11839392 pktRcvUnique 94652, pktRcvDrop:     3 (0%)

SRT v1.5.3 with PR #2815

RCVBUF:  7339392 pktRcvUnique 91678, pktRcvDrop: 2682 (2.93%)
RCVBUF:  7839392 pktRcvUnique 92193, pktRcvDrop: 1925 (2.09%)
RCVBUF:  8339392 pktRcvUnique 94336, pktRcvDrop:  312 (0.33%)
RCVBUF:  8839392 pktRcvUnique 94530, pktRcvDrop:   69 (0.07%)
RCVBUF:  9339392 pktRcvUnique 94437, pktRcvDrop:    8 (0.01%)
RCVBUF:  9839392 pktRcvUnique 94499, pktRcvDrop:   13 (0.01%)
RCVBUF: 10339392 pktRcvUnique 94560, pktRcvDrop:    1 (0%)
RCVBUF: 10839392 pktRcvUnique 94552, pktRcvDrop:    1 (0%)
RCVBUF: 11339392 pktRcvUnique 94737, pktRcvDrop:    0 (0%)
RCVBUF: 11839392 pktRcvUnique 94500, pktRcvDrop:    3 (0%)

SRT v1.5.3 with PR #2815 and SRTO_SNDDROPDELAY=-1

RCVBUF:  7339392 pktRcvUnique 72603, pktRcvDrop: 20784 (28.63%)
RCVBUF:  7839392 pktRcvUnique 86328, pktRcvDrop:  7383 (8.55%)
RCVBUF:  8339392 pktRcvUnique 93663, pktRcvDrop:   128 (0.14%)
RCVBUF:  8839392 pktRcvUnique 94510, pktRcvDrop:   174 (0.18%)
RCVBUF:  9339392 pktRcvUnique 94515, pktRcvDrop:    29 (0.03%)
RCVBUF:  9839392 pktRcvUnique 94607, pktRcvDrop:     6 (0.01%)
RCVBUF: 10339392 pktRcvUnique 94403, pktRcvDrop:     0 (0%)
RCVBUF: 10839392 pktRcvUnique 94771, pktRcvDrop:     2 (0%)
RCVBUF: 11339392 pktRcvUnique 94710, pktRcvDrop:     2 (0%)
RCVBUF: 11839392 pktRcvUnique 94790, pktRcvDrop:     0 (0%)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[core] Area: Changes in SRT library core Epic Type: Bug Indicates an unexpected problem or unintended behavior
Projects
None yet
Development

No branches or pull requests

1 participant