The RTT will quickly be updated whenever a DATA chunk is acked, but
the initial value was set to zero. In unit tests, which often are
short and rarely increase the simulated time between a DATA is sent,
and acked, the smoothed RTT would usually stay at 0, which causes
e.g. the rate limiting of FORWARD not to work.
Bug: None
Change-Id: Ieb515fe875ce88d001777b00d6efd9762565a09d
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/273900
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38013}
This adds the final piece, which makes the socket and the retransmission
queue generate the callbacks.
Bug: webrtc:5696
Change-Id: I1e28c98e9660bd018e817a3ba0fa6b03940fcd33
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/264125
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#37455}
For all messages where the last fragment was _not_ put on the wire, the
send queue is responsible for generating lifecycle events, but once all
fragments have been put on the wire, it's the retransmission queue that
is responsible. It does that by marking the final fragment of a message
with the lifecycle identifier, and once that message has been fully
acked by the cumulative ack TSN, it's clear that the peer has fully
received all fragments of the message, as the last fragment was acked.
For abandoned messages - where FORWARD-TSNs are sent, those will be
replied to by a SACK as well, and then we report abandoned messages
separately, to later trigger `OnLifecycleMessageExpired`.
This CL adds support in OutstandingData, which doesn't generate the
callbacks itself, but just reports them in the AckInfo.
Bug: webrtc:5696
Change-Id: I64092f13bcfda685443b7df9967b04d54aedd36a
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/264124
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#37448}
The send queue is responsible for generating lifecycle events for all
messages that are still in the queue. Because, if they are still in the
queue, that means that the last fragment of the message hasn't been sent
yet (because then it would have been in the retransmission queue
instead). And if the last fragment hasn't been sent, the send queue is
responsible for generating the
`OnLifecycleMessageExpired(/*maybe_sent=*/false)` event.
Bug: webrtc:5696
Change-Id: Icd5956d6aa0f392cae54f2a05bd20728d9f7f0a6
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/264144
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#37419}
Minor refactoring of the API, to put optional arguments last. Also
changed internal structures to reflect that order, for consistency.
Also reduced size of Item from 88 to 72 bytes, by packing fields better.
Bug: webrtc:5696
Change-Id: I1b9d50831a8e9a358224682d06a782a3269b8416
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/264123
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#37403}
Let the OutgoingStream reference the parent instead of passing
references to individual items it needs, as follow-up CLs will add even
more items.
No functional change - pure refactoring.
Bug: webrtc:5696
Change-Id: I914e590c0d90e898d7d230a16170cf4faff2338c
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/264142
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#37398}
Before this CL, some components, e.g. the SendQueue, was first created
and then later restored from handover state, while some were created from
the handover state, as an optional parameter to their constructors.
This CL will make it consistent, by always creating the components in a
pristine state, and then modifying it when restoring them from handover
state. The name "RestoreFromState" was used to be consistent with SendQueue
and the socket.
This is just refactoring.
Bug: None
Change-Id: Ifad2d2e84a74a12a93abbfb0fe1027ebb9580e73
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/267006
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#37384}
This adds support to enable message interleaving in the stream scheduler
from the socket, proxied by the send queue.
It also adds socket unit tests to ensure that prioritization and
interleaving works. Also, send queue test has been added to validate the
integration of the stream scheduler. But the actual scheduling parts of
it will be tested in the stream scheduler unit tests.
Bug: webrtc:5696
Change-Id: Ic7d3d2dc28405c77a107f0148f0096882961eec7
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/262248
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#37355}
This mainly modifies the stream scheduler to add a weighted fair queuing
algorithm in addition to its round robin algorithm. The WFQ algorithm is
selected whenever interleaving is enabled, to ensure that the socket
stays backwards compatible in the normal (non-interleaved) scenario.
Adaptation to send queue and socket comes in a follow-up CL.
Bug: webrtc:5696
Change-Id: I8f0dbfa8c2f40f2e84cee536ea821e7ef4af6310
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/261947
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#37330}
This is a revert of the revert of commit d729d12454906d924d5a142deb3432
which was reverted because it caused upstream test failures.
Contains fix in StreamScheduler::GetActiveStreamsForTesting.
This reverts commit 5df960d3073630c5619e00d79f89937bf6fabd69.
Bug: webrtc:5696
Change-Id: I89dada257a6fb1f149f50067ab66b17e24a7c01a
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/266368
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#37299}
RFC 4960, 7.2.4. Fast Retransmit on Gap Reports
4) Restart the T3-rtx timer only if the last SACK acknowledged the
lowest outstanding TSN number sent to that address, or the
endpoint is retransmitting the first outstanding DATA chunk sent
to that address.
The second part of this sentence, "or the endpoint is retransmitting the
first outstanding DATA chunk sent to that address."
This means that on fast retransmit, and if the retransmitted chunks
weren't quickly acknowledged by the peer, the retransmission timer would
expire too quickly. With this CL, it will restart, as it should.
Bug: webrtc:12943
Change-Id: I7627253718530876acc516460562e66bfcc79533
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/265620
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#37193}
This reverts commit d729d12454906d924d5a142deb3432e2d5fa97ae.
Reason for revert: Breaks downstream project.
Original change's description:
> dcsctp: Use stream scheduler in send queue
>
> Changing the currently embedded scheduler that was implemented using a
> revolving pointer, to the parameterized stream scheduler that is
> implemented using a "virtual finish time" approach.
>
> Also renamed StreamCallback to StreamProducer, per review comments.
>
> Bug: webrtc:5696
> Change-Id: I7719678776ddbe05b688ada1b52887e5ca2fb206
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/262160
> Reviewed-by: Harald Alvestrand <hta@webrtc.org>
> Commit-Queue: Victor Boivie <boivie@webrtc.org>
> Cr-Commit-Position: refs/heads/main@{#37170}
Bug: webrtc:5696
Change-Id: Iaf3608b52a31eb31b4ca604539edb2e8ca89399b
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/265389
Auto-Submit: Victor Boivie <boivie@webrtc.org>
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Bot-Commit: rubber-stamper@appspot.gserviceaccount.com <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/heads/main@{#37172}
Changing the currently embedded scheduler that was implemented using a
revolving pointer, to the parameterized stream scheduler that is
implemented using a "virtual finish time" approach.
Also renamed StreamCallback to StreamProducer, per review comments.
Bug: webrtc:5696
Change-Id: I7719678776ddbe05b688ada1b52887e5ca2fb206
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/262160
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#37170}
This adds a stream scheduler using virtual finish time (as defined in
e.g. many Fair Queuing scheduler implementations), which indicates when
a stream's next sent packet is supposed to be sent.
In the initial version, this will be used to implement a round robin
scheduler, by emulating that a stream's virtual finish time - when
scheduled - is the "one more" than all existing virtual finish times.
That will make the scheduler simply iterate between the streams in
round robin order.
The stream scheduler component is tested in isolation, and follow-up
CLs will integrate it into the send queue.
Bug: webrtc:5696
Change-Id: Iaa2c204f9b9a00517f55355cb11cfd25bb415f9e
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/261946
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#37157}
Make HasDataToSend not mutate any state, and let the Produce method do
all state mutation and possibly indicate if there is nothing that can be
sent. This is helpful preparation for extracting the scheduling part of
the send queue to a separate component.
Bug: webrtc:5696
Change-Id: I132779e77d3ce6a41e5fcf4432140d3728d03cdc
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/261945
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#37141}
The current code assumed that chunks that were scheduled for fast
retransmission would never be abandoned, as chunks marked for fast
retransmission would be immediately sent after the SACK has been
processed, giving no time for them to be abandoned.
But fuzzers keep on fuzzing, and can craft a sequence of chunks that
result in a SACK that both marks the chunks for fast retransmission and
later (while processing the same SACK) abandons them.
Bug: chromium:1331087
Change-Id: Id218607e18a6f3a9d6d51044dccb920e1e77372a
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/264960
Commit-Queue: Florent Castelli <orphis@webrtc.org>
Auto-Submit: Victor Boivie <boivie@webrtc.org>
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#37108}
This is a reland of commit 17a02a31d7d2897b75ad69fdac5d10e7475a5865.
This is the first part of supporting stream priorities, and adds the API
and very basic support for setting and retrieving the stream priority.
This commit doesn't in any way change the actual packet sending - the
specified priority values are stored, but not acted on.
This is all that is client visible, so clients can start using the API
as written, and they would never notice that things are missing.
Bug: webrtc:5696
Change-Id: I04d64a63cbaec67568496ad99667e14eba85f2e0
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/264424
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#37081}
This reverts commit 17a02a31d7d2897b75ad69fdac5d10e7475a5865.
Reason for revert: Breaks downstream test
Original change's description:
> dcsctp: Add public API for setting priorities
>
> This is the first part of supporting stream priorities, and adds the API
> and very basic support for setting and retrieving the stream priority.
>
> This commit doesn't in any way change the actual packet sending - the
> specified priority values are stored, but not acted on.
>
> This is all that is client visible, so clients can start using the API
> as written, and they would never notice that things are missing.
>
> Bug: webrtc:5696
> Change-Id: I24fce8cbb6f3cba187df99d1d3f45e73621c93c6
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/261943
> Reviewed-by: Harald Alvestrand <hta@webrtc.org>
> Commit-Queue: Victor Boivie <boivie@webrtc.org>
> Cr-Commit-Position: refs/heads/main@{#37034}
Bug: webrtc:5696
Change-Id: If172d9c9dbce7aae72152abbbae1ccc77340bbc1
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/264444
Owners-Override: Björn Terelius <terelius@webrtc.org>
Bot-Commit: rubber-stamper@appspot.gserviceaccount.com <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Björn Terelius <terelius@webrtc.org>
Auto-Submit: Björn Terelius <terelius@webrtc.org>
Reviewed-by: Victor Boivie <boivie@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#37039}
This is the first part of supporting stream priorities, and adds the API
and very basic support for setting and retrieving the stream priority.
This commit doesn't in any way change the actual packet sending - the
specified priority values are stored, but not acted on.
This is all that is client visible, so clients can start using the API
as written, and they would never notice that things are missing.
Bug: webrtc:5696
Change-Id: I24fce8cbb6f3cba187df99d1d3f45e73621c93c6
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/261943
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#37034}
When streams were to be reset, but there was already an ongoing
stream reset command in-flight, those streams wouldn't be properly
reset. When multiple streams were reset close to each other (within
an RTT), some streams would not have their SSNs reset, which resulted
in the stream resuming the SSN sequence. This could result in ordered
streams not delivering all messages as the receiver wouldn't deliver any
messages with SSN different from the expected SSN=0.
In WebRTC data channels, this would be triggered if multiple channels
were closed at roughly the same time, then re-opened, and continued
to be used in ordered mode. Unordered messages would still be delivered,
but the stream state could be wrong as the DATA_CHANNEL_ACK message is
sent ordered, and possibly not delivered.
There were unit tests for this, but not on the socket level using
real components, but just on the stream reset handler using mocks,
where this issue wasn't found. Also, those mocks didn't validate that
the correct parameters were provided, so that's fixed now.
The root cause was the PrepareResetStreams was only called if there
wasn't an ongoing stream reset operation in progress. One may try to
solve it by always calling PrepareResetStreams also when there is an
ongoing request, or to call it when the request has finished. One would
then realize that when the response of the outgoing stream request is
received, and CommitResetStreams is called, it would reset all paused
and (prepared) to-be-reset streams - not just the ones in the outgoing
stream request.
One cause of this was the lack of a single source of truth of the stream
states. The SendQueue kept track of which streams that were paused, but
the stream reset handler kept track of which streams that were
resetting. As that's error prone, this CL moves the source of truth
completely to the SendQueue and defining explicit stream pause states. A
stream can be in one of these possible states:
* Not paused. This is the default for an active stream.
* Pending to be paused. This is when it's about to be reset, but
there is a message that has been partly sent, with fragments
remaining to be sent before it can be paused.
* Paused, with no partly sent message. In this state, it's ready to
be reset.
* Resetting. A stream transitions into this state when it has been
paused and has been included in an outgoing stream reset request.
When this request has been responded to, the stream can really be
reset (SSN=0, MID=0).
This CL also improves logging, and adds socket tests to catch this
issue.
Bug: webrtc:13994, chromium:1320194
Change-Id: I883570d1f277bc01e52b1afad62d6be2aca930a2
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/261180
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#36771}
Before this CL, fast retransmission didn't follow the SHOULDs:
https://datatracker.ietf.org/doc/html/rfc4960#section-7.2.4
* "the sender SHOULD ignore the value of cwnd (...)"
* "(...) and SHOULD NOT delay retransmission for this single
packet."
With this CL, chunks that are eligible for fast retransmission (limited
to what can fit in a single packet) will be sent just after having
received the SACK that reported them missing and transitioned the socket
into fast recovery, and they will be sent even if the congestion window
is full.
Bug: webrtc:13969
Change-Id: I12c7e191a8ffd67973db7f083bad8a6061549fa2
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/259866
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#36724}
This CL makes OutstandingData keep track of chunks that are eligible for
fast retransmission. When the socket goes into fast recovery, the
reported missing chunks can be retransmitted quickly (ignoring the
congestion window) according to
https://datatracker.ietf.org/doc/html/rfc4960#section-7.2.4.
The CL also adds the new API to OutstandingData to retrieve only the
chunks that are eligible for fast retransmission, and moves the
remaining chunks to the ordinary list of chunks to be retransmitted
later.
This solves an issue where the retransmission timer wouldn't start if
there wouldn't be any chunks to fast-retransmit.
It doesn't, however, make sure that chunks that should be fast
retransmitted can send even when the congestion window is full. That
will be solved in the follow-up CL.
Bug: webrtc:13969
Change-Id: If4012d1cb284ef4a2d815683ed60cbbbff5b3c3b
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/259865
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#36721}
This CL replaces two booleans, that could never be active at the same
time (there is no such thing as an abandoned chunk that is scheduled
for retransmission), with a single enum.
Just for increased readability, and to understand that there is no such
thing as an abandoned chunk that will be retransmitted.
Bug: None
Change-Id: I1682c383aed692db07fd4ae1f84c0166db86c062
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/259864
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#36707}
This is mainly an issue when sending items with partial reliability,
with high bandwidth on a link with packet loss.
In SCTP, when a fragment isn't included in the SACK a number of times,
it's scheduled to be retransmitted or abandoned, if it has been
retransmitted too many times already (depending configuration). Before
this CL, if a fragment was NACKed and scheduled for retransmission, but
couldn't be retransmitted immediately (due to congestion window not
allowing it), future received SACKs - that would still indicate that the
fragment hasn't been received yet - would still increment the
retransmission counter. Which wasn't fair, because this fragment hasn't
had a chance to be retransmitted yet.
With this CL, the fragment will only have its retransmission counter
increased when it is not already scheduled to be retransmitted, but
actually sent on the wire and considered in-flight again.
Bug: webrtc:12943
Change-Id: I2af08255650221c044cc14591a5835c885e94c58
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/259825
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#36683}
This was available in the beginning, as a way to increase the chance of
a message sent with partial reliability to be delivered, by avoiding it
to be fragmented in too small fragments.
This however added a few downsides:
* Packet efficiency goes down, as the entire MTU isn't always used
* Complexity increases when adding message interleaving, since if one
stream refuses to produce messages, but there is another stream with
a very small message that could fit in its place, it should be used
instead.
Removing this feature altogether is much easier. It's hard to defend.
Bug: webrtc:5696
Change-Id: Ie2f296e052f4a32a281497d379c0d528a2df3308
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/257168
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#36396}
RFC8831, section 6.7 states that closing a data channel MUST be signaled
as resetting an outgoing stream, and that will ensure that all messages
are either delivered or abandoned before the stream is reset. In the
current implementation, dcSCTP has opted to abandoned any queued
messages that haven't been partially sent.
And this CL simply adds more documentation around this choice. It's
subject to change and a client implementation shouldn't depend on any
such behavior as the RFC allows the implementation to decide.
Bug: None
Change-Id: I60305fe396a6a3f494d823c71e092acfeb6075b7
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/257167
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#36395}
This use case was missing test coverage and sufficient comments in the
code.
Bug: None
Change-Id: I95b54a64f714b68a347fdbeef79eb38e715adbc3
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/257166
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#36393}
It's not used as a constructor parameter, but instead invoked by calling
RestoreFromState. Removing this parameter avoids confusion.
Bug: None
Change-Id: I3bb8a0135808e3ae010be6d37439513517f71832
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/254820
Auto-Submit: Victor Boivie <boivie@webrtc.org>
Reviewed-by: Sergey Sukhanov <sergeysu@webrtc.org>
Commit-Queue: Sergey Sukhanov <sergeysu@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#36188}
Context: The timer precision of PostDelayedTask() is about to be lowered
to include up to 17 ms leeway. In order not to break use cases that
require high precision timers, PostDelayedHighPrecisionTask() will
continue to have the same precision that PostDelayedTask() has today.
webrtc::TaskQueueBase has an enum (kLow, kHigh) to decide which
precision to use when calling PostDelayedTaskWithPrecision().
See go/postdelayedtask-precision-in-webrtc for motivation and a table of
delayed task use cases in WebRTC that are "high" or "low" precision.
Most timers in DCSCTP are believed to only be needing low precision (see
table), but the delayed_ack_timer_ of DataTracker[1] is an example of a
use case that is likely to break if the timer precision is lowered (if
ACK is sent too late, retransmissions may occur). So this is considered
a high precision use case.
This CL makes it possible to specify the precision of dcsctp::Timer.
In a follow-up CL we will update delayed_ack_timer_ to kHigh precision.
[1] https://source.chromium.org/chromium/chromium/src/+/main:third_party/webrtc/net/dcsctp/rx/data_tracker.cc;l=340
Bug: webrtc:13604
Change-Id: I8eec5ce37044096978b5dd1985fbb00bc0d8fb7e
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/249081
Reviewed-by: Victor Boivie <boivie@webrtc.org>
Reviewed-by: Tomas Gunnarsson <tommi@webrtc.org>
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#35809}
As WebRTC now supports C++17, simplify the code of dcSCTP by binding
return values from std::pair or std::tuple to separate names.
Bug: webrtc:13220
Change-Id: Ie49154ff4c823e1528deaef7e372cbc550923bc2
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/246442
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#35773}
This avoids copying the payload at all. Future CL will change the
transport.
In performance tests, memcpy was visible in the performance profiles
prior to this change.
Bug: webrtc:12943
Change-Id: I507a1a316165db748e73cf0d58c1be62cc76a2d2
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/236346
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#35428}
It's put in the public folder since the intention is to expose it in
SendOptions.
Additionally, use TimeMs::InfiniteFuture() to represent sending a
message with no limited lifetime (i.e. to send it reliably).
One benefit for these two is avoiding using absl::optional more than
necessary, as it results in larger struct sizes for the outstanding
data chunks.
Bug: webrtc:12943
Change-Id: I87a340f0e0905342878fe9d2a74869bfcd6b0076
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/235984
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#35323}
RetransmissionQueue was growing too long (almost 1000 lines), and as
there is reason to believe that more changes are necessary in it for
performance reasons, the data structure that handles managing the
in-flight outstanding data has been extracted as a separate class with
its own test cases. What remains in RetransmissionQueue is that it holds
OutstandingData, fetch data from the SendQueue and manage all congestion
control variables and algorithms.
Bug: webrtc:12943
Change-Id: I46062a774e0e76b44e36c66f836b7d992508bf5f
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/235980
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#35279}
A vector of which TSNs that were acked for each received SACK was
created, but only used in debug logs, which aren't enabled by default.
Removing them, as they don't add that much value and cost a bit
of performance.
Bug: webrtc:12943
Change-Id: Ice323cf46ca6e469fbbcf2a268ad67ca883bb2f5
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/235985
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#35265}
This is explained in RFC 4960, section 6.1, A.
... However, regardless of the value of rwnd (including if it
is 0), the data sender can always have one DATA chunk in flight to
the receiver if allowed by cwnd ... This rule
allows the sender to probe for a change in rwnd that the sender
missed due to the SACK's having been lost in transit from the data
receiver to the data sender.
Before this change, when a receiver has advertised a zero receiver
window size (a_rwnd=0) and a subsequent SACK advertising a non-zero
receiver window was lost, the sender was blocked from sending and since
SACKs are only sent when a DATA chunk is received, it would be
deadlocked. The retransmission timer would fire, but nothing would be
retransmitted (as it respected the zero receiver window).
With this change, when the retransmission timer fires (after RTO), it
would send a single packet with DATA chunk(s) and then SACKs would
eventually be received, with the non-zero receiver window and the socket
would recover.
Bug: chromium:1258225
Change-Id: I1ea62fb3c002150eeada28d3e703dbc09cfd038e
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/235280
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#35215}
This is mentioned in
https://datatracker.ietf.org/doc/html/rfc4960#section-6.3.1 and further
described in https://datatracker.ietf.org/doc/html/rfc6298#section-4.
The TCP RFCs mentioned G as the clock granularity, but in SCTP it should
be set much higher, to account for the delayed ack timeout (ATO) of the
peer (as that can be seen as a very high clock granularity). That one is
set to 200ms by default in many clients, so a reasonable default limit
could be set to 220 ms.
If the measured variance is higher, it will be used instead.
Bug: webrtc:12943
Change-Id: Ifc217daa390850520da8b3beb0ef214181ff8c4e
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/232614
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#35068}
This was caused by change 231229 and was later caught when reviewing the
code. The rtt variable was accidentally re-used for another purpose, and
then assumed to still be used to represent the rtt.
There have been no issues found with this re-use, but it was wrong.
Bug: webrtc:12614
Change-Id: If1a180315cf833e293f78c54c3c3b29394a19a20
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/232610
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#35064}
dcSCTP library users can set their custom
g_handover_state_transformer_for_test that can serialize and
deserialize the state. All dcSCTP handover tests call
g_handover_state_transformer_for_test. If some part of the state is
serialized incorrectly or is forgotten, high chance that it will
fail the tests.
Bug: webrtc:13154
Change-Id: I251a099be04dda7611e9df868d36e3a76dc7d1e1
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/232325
Commit-Queue: Sergey Sukhanov <sergeysu@webrtc.org>
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Reviewed-by: Victor Boivie <boivie@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#35035}
When there is no outstanding data, then next TSN to allocate should
always be one more than what the client has last ACKed.
Bug: None
Change-Id: Ieb8b5b23912d77d96fe3749fb53fd53652d97066
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/232002
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#35016}
Following Congestion avoidance and control by V. Jacobson at
https://dl.acm.org/doi/10.1145/52324.52356, use integer math instead
of floating point. Not that it matters, but it results in some code size
savings, and is more efficient. Due to not using floating point math,
some golden values in test cases were rounded a bit differently.
Bug: webrtc:12614
Change-Id: I0b7d54b8fd9ce7156e6b2582437ef5720f8838ea
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/231229
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#34956}
