10 Commits

Author SHA1 Message Date
Victor Boivie
c20f1563b6 dcsctp: Don't sent more packets before COOKIE ACK
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}
2021-06-18 08:50:59 +00:00
Victor Boivie
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
Victor Boivie
bd9031bf22 dcsctp: Add OnTotalBufferedAmountLow in Send Queue
This is similar to Change-Id: I12a16f44f775da3711f3aa52a68a0bf24f70d2f8
but with the entire send buffer as scope, not a single stream.

This can be used by clients to take alternate action (such as delaying
transmission or using other buffering) if the send buffer ever becomes
full, as they can now be notified when the send buffer is no longer
full.

Bug: webrtc:12794
Change-Id: Icf3be3b118888ffb5ced955fd7ba4826a37140f9
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/220360
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#34143}
2021-05-27 14:38:18 +00:00
Victor Boivie
791adafa09 dcsctp: Add OnBufferedAmountLow in Send Queue
This adds the necessary properties and callback to the Send Queue to
support the bufferedAmount & bufferedAmountLowThreshold properties and
the bufferedamountlow event in RTCDataChannel.

The public API changes and socket support comes in a follow-up CL.

Bug: webrtc:12794
Change-Id: I12a16f44f775da3711f3aa52a68a0bf24f70d2f8
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/219690
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#34142}
2021-05-27 13:16:28 +00:00
Victor Boivie
50a0b1219e dcsctp: Avoid transition back to ShutdownPending
When a socket is shutting down, either explicitly by the ULP calling
Shutdown(), or when the socket receives a SHUTDOWN chunk, the socket
should send all outstanding data and when all is sent and acked,
_then_ it should continue the shutdown protocol.

As it currently doesn't calculate correctly when all data has been sent,
as NACKED chunks are not included in what it believes is remaining in
the retransmission queue, it will shut down prematurely and may go back
to a previous state (ShutdownPending) from ShutdownSent or
ShutdownAckSent.

This is a workaround that just avoids the illegal state transition as
that puts the socket in an inconsistent state. The bug is merely
theoretical as WebRTC doesn't currently gracefully shut down a SCTP
socket, but just terminates the DTLS transport.

As TODOs mention, this will be fixed correctly a bit later.

Bug: webrtc:12739
Change-Id: Ibde2acc3a6aca701ac178d6181028404d470a5d5
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/218340
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#33982}
2021-05-11 16:04:42 +00:00
Victor Boivie
d3b186e3d6 dcsctp: Support message with low lifetime
While it's not strictly defined, the expectation is that sending a
message with a lifetime parameter set to zero (0) ms should allow it to
be sent if it can be sent without being buffered. If it can't be
directly sent, it should be discarded.

This is initial support for it. Small messages can now be delivered fine
if they are not to be buffered, but fragmented messages could be partly
sent (if this fills up the congestion window), which means that the
message will then fail to be sent whenever the congestion window frees
up again. It would be better to - at a higher level - realize early that
the message can't be sent in full, and discard it without sending
anything. But that's an optimization that can be done later.

A few off-by-one errors were found when strictly defining that the
message is alive during its entire lifetime. It will expire just _after_
its lifetime.

Sending messages with a lifetime of zero may not supported in all
libraries, so a workaround would be to set a very small timeout instead,
which is tested as well.

Bug: webrtc:12614
Change-Id: I9a00bedb639ad7b3b565b750ef2a49c9020745f1
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/217562
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#33977}
2021-05-11 08:44:14 +00:00
Victor Boivie
914925f51e dcsctp: Don't access TCB when the socket is closed
When the shutdown timer has expired, the socket will abort/close and the
TCB is not valid after InternalClose.

Bug: webrtc:12614
Change-Id: I09a94a049f0cda4577225dd9c80a92a8ec7e0423
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/217767
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#33956}
2021-05-07 19:04:49 +00:00
Victor Boivie
f95536dd5a dcsctp: Stop connection timers during shutdown
If Shutdown is called when the socket is being established and while the
connection timers are running, it will put the socket in an inconsistent
state, which is verified in debug builds.

Bug: webrtc:12614
Change-Id: I66f07d1170ac8f0ad9fd485d77d6aef4c365f150
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/217765
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#33949}
2021-05-07 13:51:57 +00:00
Florent Castelli
0810b05104 dcsctp: Add SetMaxMessageSize() to socket
An SCTP transport for Data Channels allows changing the maximum
message size through SDP.
See https://w3c.github.io/webrtc-pc/#sctp-transport-update-mms

Bug: webrtc:12614
Change-Id: I8cff33c5f9c1d60934a726c546bc9cbdcd9e22d9
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/217387
Reviewed-by: Victor Boivie <boivie@webrtc.org>
Commit-Queue: Florent Castelli <orphis@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#33920}
2021-05-04 21:43:24 +00:00
Victor Boivie
b6580ccb29 dcsctp: Add Socket
This completes the basic implementation of the dcSCTP library. There
are a few remaining commits to e.g. add compatibility tests and
benchmarks, as well as more support for e.g. RFC8260, but those are not
strictly vital for evaluation of the library.

The Socket contains the connection establishment and teardown sequences
as well as the general chunk dispatcher.

Bug: webrtc:12614
Change-Id: I313b6c8f4accc144e3bb88ddba22269ebb8eb3cd
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/214342
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Reviewed-by: Tommi <tommi@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#33890}
2021-05-01 07:16:21 +00:00