It's useful for other parts of WebRTC and there is no real reason why
it should be located in net/dcsctp.
Bug: None
Change-Id: Iccaed4e943e21ddaea8603182d693114b2da9f6b
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/232606
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#35055}
This allows build targets that need only HandoverReadinessStatus
to depend only on public:socket, and not on socket:dcsctp_socket.
Bug: webrtc:13154
Change-Id: I29f41910cdb5baed96b57fd7284b96fc50a56ba4
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/232331
Reviewed-by: Victor Boivie <boivie@webrtc.org>
Commit-Queue: Sergey Sukhanov <sergeysu@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#35037}
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}
The buffer of reassembled messages in ReassemblyQueue is only to be
used while processing a DATA/I-DATA or FORWARD-TSN as processing these
chunks may result in assembling messages.
When the socket is idle - between API calls - it's supposed to be empty.
Instead of having it as a member in ReassemblyQueue, it could be
provided as an argument to ReassemblyQueue::Add and
ReassemblyQueue::Handle(ForwardTSN), but that would be a quite big
refactoring. That will be investigated separately.
Bug: None
Change-Id: I41238de28f32f2a622c1d045debe3ea11e7c23f6
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/232000
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#35014}
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}
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}
The previous limits were taken from Oracles SCTP stack[1] as they were
more up-to-date than the suggested ones in RFC4960. However, after
having evaluated them for a while, it's evident that they are a bit too
aggressive and likely have their origin from a wired LAN network.
Let's do a re-take. These values have been taken from Solaris TCP
stack[2]. They are even less aggressive than Linux defaults. This can be
iterated even more, and is always possible to override by the client.
It's generally the increase of rto_min that is helping here, as the
delayed SACK and RTT jitter require that the RTO.min is quite much
higher than the delayed SACK timeout of the peer (which isn't in control
by us, but one can assume it's 200ms or less). And with a too low
RTO.min, it's increased risk of getting spurious retransmissions and
decreasing the congestion window.
[1] https://docs.oracle.com/cd/E93309_01/docs.466/SIGTRAN/GUID-2136614F-4BED-407C-87B0-7EE10E0FF534.htm
[2] https://docs.oracle.com/cd/E19120-01/open.solaris/819-2724/chapter4-69/index.html
Bug: webrtc:12943
Change-Id: I9678ac4396286a55c251c5f57589379da70fd27d
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/231139
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#34927}
By allowing the max timer backoff duration to be limited, a socket can
fast recover in case of intermittent network issues. Before this CL, the
exponential backoff algorithm could result in very long retry durations
(in the order of minutes), when connection has been lost or been flaky
for a long while.
Note that limiting the maximum backoff duration might require
compensating the maximum retransmission limit to avoid closing the
socket prematurely due to reaching the maximum retransmission limit much
faster than previously.
Bug: webrtc:13129
Change-Id: Ib94030d666433e3fa1a2c8ef69750a1afab8ef94
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/230702
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#34913}
The restart limit for timers can already be limitless, but the
RetransmissionErrorCounter didn't support this. With this change, the
max_retransmissions and max_init_retransmits can be absl::nullopt to
indicate that there should be infinite retries.
Bug: webrtc:13129
Change-Id: Ia6e91cccbc2e1bb77b3fdd7f37436290adc2f483
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/230701
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#34882}
dcSCTP seems to be able to provoke usrsctp to send ABORT in some
situations, as described in
https://github.com/sctplab/usrsctp/issues/597. Using a packetdrill
script, it seems as a contributing factor to this behavior is when a
FORWARD-TSN chunk is bundled with a DATA chunk. This is a valid and
recommended pattern in RFC3758:
"F2) The data sender SHOULD always attempt to bundle an outgoing
FORWARD TSN with outbound DATA chunks for efficiency."
However, that seems to be a rare event in usrsctp, which generally sends
each FORWARD-TSN in a separate packet.
By doing the same, the assumption is that this scenario will generally
be avoided.
With many browsers and other clients using usrsctp, and which will not
be upgraded for a long time, there is an advantage of avoiding the issue
even if it is according to specification.
Before this change, a FORWARD-TSN was bundled with outgoing DATA and due
to this, it wasn't rate limited as the overhead was very little. With
this change, a rate limiting behavior has been added to avoid sending
too many FORWARD-TSN in small packets. It will be sent every RTT, or
200 ms, whichever is smallest. This is also described in the RFC.
Bug: webrtc:12961
Change-Id: I3d8036a34f999f405958982534bfa0e99e330da3
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/229101
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#34801}
The congestion window is unlikely to be even divisible by the size
of a packet, so when the congestion window is almost full, there is
often just a few bytes remaining in it. Before this change, a small
packet was created to fill the remaining bytes in the congestion window,
to make it really full.
Small packets don't add much. The cost of sending a small packet is
often the same as sending a large one, and you usually get lower
throughput sending many small packets compared to few larger ones.'
This mode will only be enabled when the congestion window is large, so
if the congestion window is small - e.g. due to poor network conditions,
it will allow packets to become fragmented into small parts, in order to
fully utilize the congestion window.
Bug: webrtc:12943
Change-Id: I8522459174bc72df569edd57f5cc4a494a4b93a8
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/228526
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#34778}
For symmetry, as the outstanding_bytes is increased/decreased by
the serialized chunk size (not just the payload) - which is compared
to the congestion window, the congestion window should be increased
by the serialized size of chunks acked - not just their payload.
Bug: webrtc:12943
Change-Id: I0a06033e8ca0d58433138df6442ca80494918cf2
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/228525
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#34775}
The same code was done for both acking chunks due to moving the
cum-ack-tsn and when acking gap-ack-blocks. Unify them completely
to have a single code path.
Bug: webrtc:12943
Change-Id: I3b864e41cc2ec674460517660c23b72a70edf720
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/228521
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#34773}
This is mainly a refactoring commit, to break out packet sending to a
dedicated component.
Bug: webrtc:12943
Change-Id: I78f18933776518caf49737d3952bda97f19ef335
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/228565
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#34772}
Before this change, there was no way for a client to indicate to the
dcSCTP library if a packet that was supposed to be sent, was actually
sent. It was assumed that it always was.
To handle temporary failures better, such as retrying to send packets
that failed to be sent when the send buffer was full, this information
is propagated to the library.
Note that this change only covers the API and adaptations to clients.
The actual implementation to make use of this information is done as a
follow-up change.
Bug: webrtc:12943
Change-Id: I8f9c62e17f1de1566fa6b0f13a57a3db9f4e7684
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/228563
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Reviewed-by: Niels Moller <nisse@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#34767}
Some deployments, e.g. Chromium, has a limited send buffer. It's
reasonable that it's quite small, as it avoids queuing too much, which
typically results in increased latency for real-time communication. To
avoid SCTP to fill up the entire buffer at once - especially when doing
fast retransmissions - limit the amount of packets that are sent in one
go.
In a typical scenario, SCTP will not send more than three packets for
each incoming packet, which is is the case when a SACK is received which
has acknowledged two large packets, and which also adds the MTU to the
congestion window (due to in slow-start mode), which then may result in
sending three packets. So setting this value to four makes any
retransmission not use that much more of the send buffer.
This is analogous to usrsctp_sysctl_set_sctp_fr_max_burst_default in
usrsctp, which also has the default value of four (4).
Bug: webrtc:12943
Change-Id: Iff76a1668beadc8776fab10312ef9ee26f24e442
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/228480
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#34744}
To support implementing RTCSctpTransportStats, a few metrics are needed.
Some more were added that are useful for metric collection in SFUs.
Bug: webrtc:13052
Change-Id: Idafd49e1084922d01d3e6c5860715f63aea08b7d
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/228243
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#34708}
This corresponds to one part of sstat_unackdata in RFC6458. The
remaining part is the data in the send queue, which isn't packetized
yet, so it must be estimated. But the DATA items in the retransmission
queue is already determined, so it can be easily tracked and retrieved.
Bug: webrtc:13052
Change-Id: I16c3b5b61eb6b3022d7104e6457d943d5df3d6b9
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/228240
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#34706}
As part of go/coil update code search links to not point to the
"master" branch.
Bug: chromium:1226942
Change-Id: I0ae9e84ecc660f789a69fe0b226f93bbc39a8a66
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/226081
Commit-Queue: Tony Herre <toprice@chromium.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#34531}
The previous logic to abandon chunks when partial reliability was used
was a bit too eager and trigger happy.
* Chunks with limited retransmissions should only be abandoned when a
chunk is really considered lost. It should follow the same rules as
for retransmitting chunks - that it must be nacked three times or
due to a T3-RTX expiration. Before this change, a single SACK not
referencing it would be enough to abandon it. This resulted in a lot
of unnecessary sent FORWARD-TSN and undelivered messages - especially
if running with zero retransmissions.
The logic to expire chunks by limited retransmissions will now only
be applied when a chunk is actually nacked.
* The second partial reliability trigger - expiration time - wasn't
evaluated when producing a middle chunk of a larger message.
A number of test cases were added and updated as chunks will now be
abandoned immediately instead of first scheduled for retransmission and
later abandoned.
Bug: webrtc:12961
Change-Id: I0ae17b2672568bdbdc32073a99d4c24b09ff5fe9
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/225548
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#34458}
CL partially auto-generated with:
git grep -l "\bassert(" | grep "\.[c|h]" | \
xargs sed -i 's/\bassert(/RTC_DCHECK(/g'
And with:
git grep -l "RTC_DCHECK(false)" | \
xargs sed -i 's/RTC_DCHECK(false)/RTC_NOTREACHED()/g'
With some manual changes to include "rtc_base/checks.h" where
needed.
A follow-up CL will remove assert() from Obj-C code as well
and remove the #include of <assert.h>.
The choice to replace with RTC_DCHECK is because assert()
is because RTC_DCHECK has similar behavior as assert()
based on NDEBUG.
This CL also contains manual changes to switch from
basic RTC_DCHECK to other (preferred) versions like
RTC_DCHECK_GT (and similar).
Bug: webrtc:6779
Change-Id: I00bed8886e03d685a2f42324e34aef2c9b7a63b0
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/224846
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Mirko Bonadei <mbonadei@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#34442}
When a single stream is reset, and an outgoing SSN reset request is sent
and later acked by the peer sending a reconfiguration response with
status=Performed, the sender should unpause the paused stream and reset
the SSNs of that (ordered) stream. But only the single stream that was
paused, and not all streams. In this scenario, dcSCTP would - when the
peer acked the SSN reset request - reset the SSN of all streams.
This was found by orphis@webrtc.org using a data channel test
application. The peer, if it's a usrsctp client, will ABORT with
PROTOCOL_VIOLATION as it has already seen that SSN on that stream but
with a different TSN.
This bug was introduced when implementing the Round Robin scheduler in
https://webrtc-review.googlesource.com/c/src/+/219682. The FCFS
scheduler prior to this change was implemented correctly.
Bug: webrtc:12952
Change-Id: I3ea144a1df303145f69a5b03aada7f448c8c8163
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/225266
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#34436}
This is for convenience to the users of dcSCTP, which may want to have
unit tests where the socket is mocked. And since it's best practice not
to mock other teams' or project's classes, a mock will be provided by
the upstream project - this one.
Bug: webrtc:12614
Change-Id: I65d5d21097e7feda9162567560d3838759c962fc
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/224161
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#34385}
The factory allows us to isolate the implementation from users who only
need to depend directly on the public folder now.
Bug: webrtc:12614
Change-Id: Ied09cf772ed427eaf17a7b5705f587da57405640
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/220939
Commit-Queue: Florent Castelli <orphis@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#34330}
While in the COOKIE ECHO state, there is a TCB and there might be data
in the send buffer, and RFC4960 allows the COOKIE ECHO chunk to bundle
additional DATA chunks in the same packet, but there mustn't be more
than one such packet sent, and that packet must have a COOKIE ECHO chunk
as the first chunk in it.
When the COOKIE ACK chunk has been received, the socket is allowed to
send multiple packets.
Previously, this was state managed by the socket and not the TCB, as
the socket is responsible for moving between the different states. And
when the COOKIE ECHO chunk was sent, the TCB was instructed to only send
a single packet by the socket.
However, if there were retransmissions or anything else that could
result in calling TransmissionControlBlock::SendBufferedChunks, it would
do as instructed and send those, even if the socket was in a state where
that wasn't allowed.
When the peer was dcSCTP, this didn't cause any issues as dcSCTP tries
to be tolerant in what it receives (but strict in what it sends, except
for when there are bugs). When the peer was usrsctp, it would send an
ABORT for each received packet that didn't have a COOKIE ECHO as the
first chunk, and then restart the handshake (sending an INIT). So this
resulted in a longer handshake, but the connection would eventually be
correctly established and any DATA chunks that resulted in the ABORTs
would've been retransmitted.
By making the TCB aware of that particular state, and to make it
responsible for creating the SCTP packet with the COOKIE ECHO chunk
first, and also to only send a single packet when it is in that state,
there will not be any way to bypass this limitation.
Also, while not explicitly mentioned in the RFC, the retransmission
timer will not affect resending any outstanding DATA chunks that were
bundled together with the COOKIE ECHO chunk, as then there would be two
timers that both would drive resending COOKIE ECHO and DATA chunks.
Bug: webrtc:12880
Change-Id: I76f215a03cceab5bafe9f16eb4775f3dc68a6f05
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/222645
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#34329}
The previous approach was that the caller was responsible for ensuring
that any buffer passed in to the Bounded IO wrappers, and that any
offset from where sub-readers were created were valid. The called would
always do a validation of the data and return proper error messages
if they were not.
This didn't pan out. https://crbug.com/1216758 found an overflow that
fooled the validation logic and the fuzzer could read out-of-bounds,
although it would always crash in that particular case.
There was already bounds checking, but under DCHECKs. This CL changes
that so that any bounds checking is done with CHECKS, as would've been
done in Rust. It's better to crash than to read arbitrary memory.
Bug: chromium:1216758
Change-Id: I89b52f0758495b5fe46f926c142870a263b96314
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/221743
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#34303}
This was found when fuzzing. If the specified number of parameter count
was larger than std::numeric_limits<size_t>::max()/2, the comparison
would overflow and read out-of-bounds. This would only apply to 32-bit
platforms and it would lead to a crash as it would access all of the
virtual memory range, and more.
Fixed: chromium:1216758
Change-Id: I2193d3ed078120b6c3e4645c0b16b9f230055e8d
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/221742
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#34256}
If there is only little space left in a packet, and the remaining data
for a partially sent message is much larger, it will not generate a
small fragment for this message. This is to avoid fragmenting a message
into too many packets, as that increases the risk of losing messages
when partial reliability is enabled.
And when a stream doesn't want to generate a too small fragment, the
scheduler should _not_ switch streams. It should only switch streams
when a message has been fully sent. Previously, it would switch stream
when a stream doesn't want to produce a message, but as noted above,
that could happen for other reasons.
This required some refactoring, which also increased its robustness by
now only doing explicit stream switching on fully produced messages.
Bug: webrtc:12832
Change-Id: Icb213774fd0d26fba5640b00aac0407d393e4bfc
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/220937
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#34197}
The way that the "next stream" was picked when round-robin cycling was
flawed. When a message was produced in its entirety, the "next stream"
would be put at a stream identifier value that was just larger than what
was previously used. And then, for each fragment that was to be created,
it would try to resolve the nearest stream (above or equal to that
number) that had messages to send - always starting from that stream id
that didn't necessarily point to the stream for which fragments were
actually produced.
For example, if the previous stream ID for which a message was fully
produced on was 5, then the next_stream_id would be set to 6, and then
when producing next fragment, it might have produced something from
stream_id=1, because that was the only stream with messages in it. It
wouldn't update next_stream_id at this time; it would still be 6.
After a single fragment had been produced from that stream, a message
was queued on stream_id=6. The next time a fragment was to be produced,
it would not continue one stream_id=1, but instead pick the new stream,
which would suddenly produce a new fragment (with B flag set) while the
previous message (from stream_id=1) wasn't finished yet.
The fix is simple; Just ensure that we continue iterating from where we
ever produce a fragment from.
Bug: webrtc:12832
Change-Id: Icc761c572ed200db607a7609dab1ac6a8aeb2f04
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/220938
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#34190}
Before this CL, before sending out any chunk, all inflight data chunks
were inspected to find out if they were supposed to be retransmitted.
When the congestion window is large, this is a lot of data chunks to
inspect, which takes time.
By having a separate collection for chunks to be retransmitted, this
becomes a much faster operation. In most cases, constant in time.
Bug: webrtc:12799
Change-Id: I0d43ba7a88656eead26d5e0b9c4735622a8d080e
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/219626
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#34178}
There is no need to iterate through all outstanding data chunks to know
if a FORWARD-TSN can be sent. As the FORWARD-TSN will just move the
cumulative TSN ack, if a chunk is found that is not to be expired,
there is no need to continue any further. This makes it much faster
to know if to send a FORWARD-TSN when the congestion window is large.
Bug: webrtc:12799
Change-Id: I58bce408ae9814c8d3d7bbb480b0037a2cf88dd7
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/219625
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#34176}
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}
