Commit Graph

16 Commits

Author SHA1 Message Date
c8680c5dc6 dcsctp: Generate lifecycle events
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}
2022-07-06 08:04:15 +00:00
2cffde72b8 dcsctp: Restore from handover as separate methods
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}
2022-06-30 22:09:04 +00:00
d7fd0f9744 dcsctp: Handle rapid closing of streams
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}
2022-05-05 07:30:58 +00:00
5e354d9969 dcsctp: Improve fast retransmission support
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}
2022-05-02 08:29:52 +00:00
a5fecb3917 dcsctp: Add proper fast retransmission support
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}
2022-05-02 07:03:42 +00:00
71604ebf64 dcsctp: Refactor out OutstandingData
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}
2021-10-29 09:03:49 +00:00
7bb853f549 dcsctp: Remove debug-log-only TSN collection
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}
2021-10-26 15:11:51 +00:00
8f486f94e6 dcsctp: support socket handover in RetransmissionQueue
Bug: webrtc:13154
Change-Id: I9c73b1153b65409eb026e015804c22f3e874ff82
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/232022
Reviewed-by: Victor Boivie <boivie@webrtc.org>
Commit-Queue: Sergey Sukhanov <sergeysu@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#35009}
2021-09-16 05:18:29 +00:00
14ef6338b0 dcsctp: Don't send small packets when cwnd full
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}
2021-08-17 09:03:36 +00:00
d912446f52 dcsctp: Refactor chunk acking
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}
2021-08-16 20:20:55 +00:00
82ea522b27 dcsctp: Track the number of inflight DATA items
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}
2021-08-10 18:00:26 +00:00
bf15e567e8 dcsctp: Abandon chunks consistently
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}
2021-07-12 22:47:51 +00:00
7b4fd5ca59 dcsctp: Determine chunks to be retransmitted fast
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}
2021-05-31 21:07:17 +00:00
236ac50628 dcsctp: Add public API for BufferedAmountLow
This adds native support for the RTCDataChannel properties:
https://developer.mozilla.org/en-US/docs/Web/API/RTCDataChannel/bufferedAmount
https://developer.mozilla.org/en-US/docs/Web/API/RTCDataChannel/bufferedAmountLowThreshold

And the RTCDataChannel event:
https://developer.mozilla.org/en-US/docs/Web/API/RTCDataChannel/onbufferedamountlow

The old callback, NotifyOutgoingMessageBufferEmpty, is deprecated as it
didn't work very well. It will not be triggered and will be removed
as soon as all users of it are gone. There is a new callback,
OnTotalBufferedAmountLow, that serves the same purpose but also allows
setting an arbitrary limit when it should be triggered (See
DcSctpOptions::total_buffered_amount_low_threshold).

Bug: webrtc:12794
Change-Id: Ic1c92f174eff8a1acda0b5fd3dcc45bd1cfa2704
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/219691
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#34144}
2021-05-27 15:27:27 +00:00
9700d88b1a dcsctp: Avoid recalculation of outstanding bytes
Recalculating outstanding bytes is expensive when the congestion window
is large, as it iterates over all inflight data chunks. By doing it
incrementally, it will be a constant operation in most cases, and
in the remaining cases, a function of the number of chunks acked in a
single SACK, which is typically just a few chunks.

Implementing this fix required some refactoring to calculate it
correctly (and to be honest, it was likely done incorrectly previously).

Previously, the state of an item in the retransmission queue was
simplified as "in flight", "acked", "nacked", "abandoned", but these
were not completely orthogonal. A chunk could be abandoned while it was
in-flight or it could be abandoned because it was lost. The difference
between these if that chunk should be accounted for in
outstanding_bytes() or not.

Unit tests have been added to verify this.

Bug: webrtc:12799
Change-Id: I72341538bb0c4f8f89555b08f0c8a28815f0f828
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/219623
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#34139}
2021-05-27 07:40:11 +00:00
03e912abaf dcsctp: Add Retransmission Queue
The Retransmission Queue contain all message fragments (DATA chunks)
that have once been sent, but not yet ACKed by the receiver. It will
process incoming SACK chunks, which informs it which chunks that the
receiver has seen (ACKed) and which that are lost (NACKed), and will
retransmit chunks when it's time.

If a message has been sent with partial reliability, e.g. to have a
limited number of retransmissions or a limited lifetime, the
Retransmission Queue may discard a partially sent and expired message
and will instruct the receiver that "don't expect this message - it's
expired" by sending a FORWARD-TSN chunk.

This currently also includes the congestion control algorithm as it's
tightly coupled with the state of the retransmission queue. This is
a fairly complicated piece of logic which decides how much data that
can be in-flight, depending on the available bandwidth. This is not done
by any bandwidth estimation, but similar to TCP, where data is sent
until it's lost, and then "we dial down a knob" and take it more
carefully from here on.

Future refactoring will try to separate the logic regarding fragment
retransmission and the congestion control algorithm.

Bug: webrtc:12614
Change-Id: I8678250abb766e567c3450634686919936ea077b
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/214046
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#33833}
2021-04-26 14:58:21 +00:00