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 is the receive-side part of supporting what is frequently called
"ndata", but actually RFC8260 - "User Message Interleaving".
This CL adds a new ReassemblyStreams implementation that can assemble
I-DATA chunks and process I-FORWARD-TSN for partial reliability.
Bug: webrtc:5696
Change-Id: I3cfbea62e7b6c02fbd3f51b43ba3fb7863cf0f88
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/218506
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#37128}
This is a solution to some problems that have been found locally when
running the fuzzer for a long time. The fuzzer keeps on fuzzing, and has
found a way to trigger a consistency check to fail when a client
intentionally sends different messages - unordered and ordered - using
the same TSNs. As the reassembly queue has different handling of ordered
and unordered chunks due to how they are reassembled, it will not notice
if it receives two different chunks with the same TSN. They will both go
to their respective reassembly streams, as those are separate by design.
The data tracker - which keeps track of all received DATA chunks as it
needs to generate SACKs, has a global understanding of all received
chunks. By having it indicate if this is a duplicate received chunk, the
socket can avoid forwarding both chunks to the reassembly queue; only
one chunk will get there.
Bug: None
Change-Id: I602a8552a9a4c853684fcf105309ec3d8073f2c2
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/256110
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#36316}
If a FORWARD-TSN contains an ordered skipped stream with a large TSN
but with a too small SSN, it can result in messages being assembled
that should've been skipped. Typically:
Receive DATA, ordered, complete, TSN=10, SID=1, SSN=0
- will be delivered.
Receive DATA, ordered, complete, TSN=43, SID=1, SSN=7
- will stay in queue, due to missing SSN=1,2,3,4,5,6.
Receive FORWARD-TSN, TSN=44, SSN=6
- is invalid, as the SSN should've been 7 or higher.
However, as the TSN isn't used for removing messages in ordered streams,
but just the SSN, the SSN=7 isn't removed but instead will be delivered
as it's the next following SSN after 6. This will trigger internal
consistency checks as a chunk with TSN=43 will be delivered when the
current cumulative TSN is set to 44, which is greater.
This was found when fuzzing, and can only be provoked by a client that
is intentionally misbehaving. Before this fix, there was no harm done,
but it failed consistency checks which fuzzers have enabled. When
bug 13799 was fixed (in a previous commit), this allowed the fuzzers to
find it faster.
Bug: webrtc:13799
Change-Id: I830ef189476e227e1dbe08157d34f96ad6453e30
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/254240
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#36157}
When a FORWARD-TSN is received as the first chunk on an ordered stream,
it will fail to set the new "next expected SSN" that is present in the
FORWARD-TSN as that stream hasn't been allocated yet. It's allocated
when the first DATA is received on that stream.
This is a non-issue for ordinary data channels as the first message on
any stream will be the "Data Channel Establishment Protocol" messages,
which are always sent reliably. But if prenegotiated channels are used,
and the very first packet received on an ordered data channel is lost
_and_ signaled to the receiver as lost _before_ the receiver has
received any other fragments on that data channel, future messages will
not be delivered on that channel.
Bug: webrtc:13799
Change-Id: Ide5c656243b3a51a2ed9d76615cfc3631cfe900c
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/253902
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#36155}
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}
This is a safe cleanup change since top-level const applied to
parameters in function declarations (that are not also
definitions) are ignored by the compiler. Hence, such changes do
not change the type of the declared functions and are simply
no-ops.
Bug: webrtc:13610
Change-Id: Ibafb92c45119a6d8bdb6f9109aa8dad6385163a9
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/249086
Reviewed-by: Niels Moller <nisse@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Ali Tofigh <alito@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#35802}
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}
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}
The ReassemblyQueue will need to track which messages that have already
been delivered to the client so that they are not re-delivered on e.g.
retransmissions. It does that by tracking which TSNs that those messages
were built from. It tracks that in two variables,
`last_assembled_tsn_watermark` and `delivered_tsns_`, where the first
one represent that all TSNs including and prior this one have been
delivered and `delivered_tsns` contain additional ones when there are
gaps.
When receiving a FORWARD-TSN and asked to forget about some partially
received messages, these two variables were updated correctly, but the
`delivered_tsns_` were left in a state where it could be adjacent to the
`last_assembled_tsn_watermark` - when `last_assembled_tsn_watermark`
could actually have been moved further.
Added consistency check (that would trigger in existing tests) and
fixing the issue.
This bug is quite benign, as any received chunk would've corrected the
problem, and even at this faulty state, the ReassemblyQueue would
function completely fine.
Bug: webrtc:13154
Change-Id: Iaa7c612999c9dc609fc6e2fb3be2d0bd04534c90
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/232124
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Reviewed-by: Sergey Sukhanov <sergeysu@webrtc.org>
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#35013}
Before this CL, a SACK was generated from scratch based on information
about each received fragment, to generate correct gap-ack-blocks.
When there was a lot of data in the data tracker (due to packet loss),
this took considerate time, as generating a SACK was O(N), where N is
the amount of fragments in the data tracker.
By instead having precomputed gap-ack-blocks that are continuously
updated, generating a SACK is much faster and the memory usage goes down
a bit as well.
Bug: webrtc:12799
Change-Id: I924752c1d6d31f06d27246e10b595e9ccb19320f
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/220763
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#34171}
There limit that decides if an incoming TSN should be accepted or not
was decided based on very small transfers with no packet loss. But in
simulations where a socket tries to send a lot of data and when there
is moderate packet loss, the number of tracker data chunks on the
receive side will be considerably higher than what the limit was.
Set the limit to allow high data rate also on moderate packet loss.
Bug: webrtc:12799
Change-Id: I6ca237e5609d8b511e9b10c919da33dca7420c01
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/220761
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#34169}
The receive buffer mustn't be full; If it's full, and a message can't be
assembled, the socket can't accept more data. To avoid this, there is
a high watermark limit that, when reached, will make the socket only
accept chunks that advance the cumulative ack TSN.
Before this CL, the announced receiver window size in every sent SACK
was based on what the receive buffer could maximally be, which means
that in really high data rate applications, the amount of outstanding
data could actually fill the receive buffer (due to packet loss, that
prevents messages from being reassembled). As the socket started
behaving more conservatively when the high watermark limit was reached,
this resulted in unnecessary T3-RTXes. But by announcing the high
watermark limit instead, the sender will stay within it, and will have
a peer socket that behaves as expected.
Bug: webrtc:12799
Change-Id: Ife2f409914a230640217553c54f60d05843efc70
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/220762
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#34168}
There were some missing unit tests that are now written. When doing
this, it was found that SACKs weren't sent for duplicate received
chunks, which they should be according to the spec.
Bug: webrtc:12614
Change-Id: I8296473c0c8cbaf0329785de95e9b9945f254339
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/220607
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#34165}
Today, there is no actual limit on how large a SACK chunk can be. And
having limits is good to be able to stay within the MTU.
This commit adds a limit to the number of reported duplicate TSNs as
well as the number of reported gap-ack-blocks in a SACK chunk. These
limits are never expected to be reached in a real-life situation.
Bug: webrtc:12614
Change-Id: Ib2c143714a214cd3d961e8a52dac26a04b909b80
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/219464
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#34108}
Reporting the duplicate TSNs is a SHOULD in the RFC, and using the
duplicate TNSs is a MAY, and in reality I haven't seen an implementation
use it yet. However, it's good for debugging and for stats generation.
Bug: webrtc:12614
Change-Id: I1cc3f86961a8d289708cbf50d98dedfd25077955
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/219462
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#34053}
This should avoid the situation where WebRTC's GN check is green and
Chromium (which turns it ON for //third_party/webrtc) fails.
Bug: webrtc:12614
Change-Id: Id4c06ac57e9faa07c5e43491a61fbc093c68a40d
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/217221
Commit-Queue: Mirko Bonadei <mbonadei@webrtc.org>
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Reviewed-by: Victor Boivie <boivie@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#33900}
Those were found when trying to build within Chromium's codebase.
Bug: webrtc:12614
Change-Id: Ic3f7a266ad4b5d816a693645e1e909fc39d513c3
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/217220
Reviewed-by: Victor Boivie <boivie@webrtc.org>
Commit-Queue: Florent Castelli <orphis@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#33896}
* `AddTo` and `Difference` are made into static methods, as one may have
believed that these modified the current object previously. The
`Increment` method is kept, as it's obvious that it modifies the
current object as it doesn't have a return value, and `next_value` is
kept, as its naming (lower-case, snake) indicates that it's a simple
accessor.
* Difference will return the absolute difference. This is actually the
only reasonable choice, as the return value was unsigned and any
negative value would just wrap.
Bug: webrtc:12614
Change-Id: If14a71636e67fc612d12759dc80a9c2518c85281
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/215069
Reviewed-by: Tommi <tommi@webrtc.org>
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#33714}
The Reassembly Queue receives fragmented messages (DATA or I-DATA
chunks) and - with help of stream reassemblers - will reassemble these
fragments into messages, which will be delivered to the client.
It also handle partial reliability (FORWARD-TSN) and stream resetting.
To avoid a DoS attack vector, where a sender can send fragments in a way
that the reassembly queue will never succeed to reassemble a message and
use all available memory, the ReassemblyQueue has a maximum size.
Bug: webrtc:12614
Change-Id: Ibb084fecd240d4c414e096579244f8f5ee46914e
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/214043
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Reviewed-by: Tommi <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#33678}
This class handles the assembly of fragmented received messages (as DATA
chunks) and manage per-stream queues. This class only handles
non-interleaved messages as described in RFC4960, and is not used when
message interleaving is enabled on the association, as described in
RFC8260.
This is also only part of the reassembly - a follow-up change will add
the ReassemblyQueue that handle the other part as well. And an even
further follow-up change will add a "interleaved reassembly stream".
Bug: webrtc:12614
Change-Id: Iaf339fa215a2b14926f5cb74f15528392e273f99
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/214042
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Reviewed-by: Tommi <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#33677}
The Data Tracker's purpose is to keep track of all received DATA chunks
and to ACK/NACK that data, by generating SACK chunks reflecting its view
of what has been received and what has been lost.
It also contains logic for _when_ to send the SACKs, as that's different
depending on e.g. packet loss. Generally, SACKs are sent every second
packet on a connection with no packet loss, and can also be sent on a
delayed timer.
In case partial reliability is used, and the transmitter has decided
that some data shouldn't be retransmitted, it will send a FORWARD-TSN
chunk, which this class also handles, by "forgetting" about those
chunks.
Bug: webrtc:12614
Change-Id: Ifafb0c211f6a47872e81830165ab5fc43ee7f366
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/213664
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Reviewed-by: Tommi <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#33676}
