When a sender has requested a stream to be reset, and the last sender
assigned TSN hasn't been received yet, the receiver will enter deferred
reset mode, where it will store any data chunks received after that
given TSN, and replay those later, when the stream has been reset.
Before this CL, leaving deferred mode was done as soon as the sender's
last assigned TSN was received. That's actually not how the RFC
describes the process[1], but was done that way to properly handle some
sequences of RE-CONFIG and FORWARD-TSN. But after having read the RFCs
again, and realizing that whenever RFC6525 mention "any data arriving",
this also applies to any FORWARD-TSN[2] - it's better to reset streams
synchronously with the incoming requests, and defer not just DATA past
the sender last assigned TSN, but also any FORWARD-TSN after that TSN.
This mostly simplifies the code and is mostly a refactoring, but most
importantly aligns it with how the resetting procedure is explained in
the RFC. It also fixes two bugs:
* It defers FORWARD-TSN *as well as* DATA chunks with a TSN later
than the sender's last assigned TSN - see test case. The old
implementation tried to handle that by exiting the deferred reset
processing as soon as it reached the sender's last assigned TSN, but
it didn't manage to do that in all cases.
* It only defers DATA chunks for streams that are to be reset, not
all DATA chunks with a TSN > sender's last assigned TSN. This was
missed in the old implementation, but as it's now implemented
strictly according to the RFC, this was now done.
[1] https://datatracker.ietf.org/doc/html/rfc6525#section-5.2.2
[2] RFC6525 cover stream resetting, and RFC3758 cover FORWARD-TSN, and
the combination of these is not covered in the RFCs.
Bug: webrtc:14600
Change-Id: Ief878b755291b9c923aa6fb4317b0f5c00231df4
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/322623
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#40889}
Similar to change I602a8552a9a4c853684fcf105309ec3d8073f2c2, which
ensured that only new DATA chunks would be processed by the reassembly
queue by utilizing the data tracker, the same is done for FORWARD-TSN
chunks.
By having the data tracker gate keeping what is provided to the
reassembly queue, the reassembly queue can be simplified as well, which
is an added bonus, by removing last_assembled_tsn_watermark_ and
reassembled_messages_ as those were protecting the queue from
re-delivering messages it had already delivered, but as now the data
tracker would ensure that it wouldn't re-process DATA/FORWARD-TSNs, that
would have the same effect. In this CL, we will still update those
variables and save to the handover state, but not actually read from
them, and then when this change has been rolled out on the servers, I
can remove the variables as well.
The core change is to move validation from ReassemblyQueue::Handle
to DataTracker::HandleForwardTsn.
Some tests have been moved/replicated into data_tracker_test.cc to
ensure that it catches the issues that the reassembly queue did earlier.
Bug: webrtc:14600
Change-Id: I75c1d5911185d594f73c8b1e6bcf776e88f5b7c7
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/321603
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#40856}
The log_prefix frequently used in dcSCTP is intended to be used
to separate logs from different sockets within the same log output,
typically in unit tests. Every log entry always has the file and
line, so it's not important to add more information to the log prefix
that indicates _where_ it's logged. So those have been removed.
Also, since log_prefix is a string (typically 32 bytes) and it's
never changing during the lifetime of the socket, pass and store it
as a absl::string_view to save memory.
Bug: None
Change-Id: I10466710ca6c2badfcd3adc5630426a90ca74204
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/274704
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39571}
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 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}
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}
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}
These are some fixes that were added after submission of
https://webrtc-review.googlesource.com/c/src/+/213664
Mainly:
* Don't accept TSNs that have a too large difference from expected
* Renaming of member variable (to confirm to style guidelines)
Bug: webrtc:12614
Change-Id: I06e11ab2acf5d307b68c3cbc135fde2c038ee690
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/215070
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Reviewed-by: Tommi <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#33721}
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}
