306 Commits

Author SHA1 Message Date
Evan Shrubsole
0ebd67f89d Move string_builder.h to webrtc namespace
Bug: webrtc:42232595
Change-Id: Iad12b11767c3bbaddcf0e87357e8e6037608defb
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/377740
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Evan Shrubsole <eshr@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#43926}
2025-02-19 06:30:53 -08:00
Evan Shrubsole
418a8c2c83 Move string_format.h to webrtc namespace
Bug: webrtc:42232595
Change-Id: I208257358150eeb97304946929649414af5eb2ca
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/377542
Commit-Queue: Evan Shrubsole <eshr@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#43915}
2025-02-18 06:34:45 -08:00
Victor Boivie
b9f8636c5a dcsctp: Fix incorrect merge conflict from last CL
A refactoring was lost after revision 3 of
https://webrtc-review.googlesource.com/c/src/+/377122
due to an incorrect merging/cherry-picking.

Reapplied it.

Bug: webrtc:396373001
Change-Id: Ice7a8e94ad984cb308eb9cab83df2e9ecca3d53c
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/377283
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#43912}
2025-02-18 03:42:45 -08:00
Victor Boivie
bcf588da8f dcsctp: Estimate rwnd by payload bytes
The dcSCTP receiver was advertising available window space (arwnd) based
solely on payload bytes, while the sender's rwnd estimation included
packet headers. This mismatch caused the sender to underestimate the
receiver's available buffer, potentially leading to reduced throughput.

This commit resolves the issue by ensuring both sender and receiver use
payload bytes, as headers have been removed on the receiver side while
in the reassembly queue.

Bug: webrtc:396373001
Change-Id: I508419efb09cabf2fb011f952f5f4a06586a4019
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/377122
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#43899}
2025-02-17 02:36:07 -08:00
Victor Boivie
f6902aff95 dcsctp: Set I-SACK bit when cwnd is low
To reduce latency when delivering messages on channel with low traffic
volume and with packet loss, where retransmissions are not driven by
fast retransmit but by T3-RTX timer, set the I-SACK bit (RFC7053) when
the congestion window is low.

Note that RFC7053 doesn't have to be negotiated, as is explained in
https://www.rfc-editor.org/rfc/rfc7053.html#section-6, and if the
receiver doesn't support it, it will delay SACKs as is done today.

When T3-RTX fires, the congestion window will be set to one MTU and any
future sent message will only send one MTU's worth of data before waiting
for the receiver's SACK until more data is sent. Delayed SACK, which is
normally used, could delay the next packet from being sent unecessarily
long. Setting I-SACK when the congestion window is small will make the
receiver always send a SACK for every received packet with a DATA chunk
in it. By not setting it always, it will not affect the packet rate when
the channel is fully utilized.

This modification improves latency in the aforementioned situations
without significantly affecting bandwidth. While this change increases
SACK frequency in specific scenarios, the impact on overall network load
is expected to be minimal.

Bug: webrtc:396080535
Change-Id: If4a5aa960969f1385c9ea59baa7e2d52caf25626
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/377140
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#43897}
2025-02-14 09:22:03 -08:00
Victor Boivie
625884e7ca dcsctp: Refactor rr_send_queue_test
This test used a fixture to create the send queue, but that makes it
hard to construct them with different parameters in some tests.

This refactoring removes the test fixture and creates the queue in each
test, which improves test readability instead, as there will be no need
for remembering how it was created - that's given by each test now.

Bug: webrtc:393540127
Change-Id: I8d158b6ff57fe9cb03b2762d736cf79afbbb8283
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/376100
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#43880}
2025-02-12 06:09:39 -08:00
Boris Tsirkin
6b6ebf3689 Format /net folder
Formatting done via:

git ls-files | grep -E '^net\/.*\.(h|cc|mm)' | xargs clang-format -i

No-Iwyu: Includes didn't change and it isn't related to formatting
Bug: webrtc:42225392
Change-Id: I5c9daed6c83459ace469dd2820e1cc4565dad8f6
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/373882
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#43683}
2025-01-08 08:39:49 -08:00
Victor Boivie
b0acde349c dcsctp: Add handover test for interleaved streams
This test was missing, which made me believe that it wasn't supported as
the handover state only included SSN and not MID. But when adding tests,
I saw that the current implementation used the SSN field to handover the
MID information for ordered streams which is sufficient given the 32 bit
type used for that (SSNs are only 16 bits).

For unordered streams, there is no need to handover any state there are
no expected next MID for unordered streams (they can be received in any
order).

So, adding tests and removing the handover state I just added.

Bug: webrtc:41481008
Change-Id: If1799cb1def5bd9f585a87cff6d835f4a9053b4f
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/370121
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#43495}
2024-12-04 14:10:32 +00:00
Victor Boivie
a79c709ed3 dcsctp: Rename test module
A few tests in dcsctp_socket_test was named DcSctpSocketResendInitTest
instead of DcSctpSocketTest. There is no reason for that.

Bug: None
Change-Id: I845eb0ab6150c4e5d457307e12f056486f86e468
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/369820
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#43470}
2024-11-29 09:48:24 +00:00
Victor Boivie
58562a8229 dcsctp: Add handover state for received MIDs
The next expected MID to use (which applies to both ordered and
unordered streams, in contrast to SSNs) was properly handed over for
streams this socket sends on, but not for streams this socket receives
on.

Adding handover state first.

Bug: webrtc:41481008
Change-Id: Ib3941f0ee1a34c24605792d9f0b658bb6a9ade4a
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/369821
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#43469}
2024-11-29 09:46:02 +00:00
Dor Hen
da7b7ca1c1 Comment unused variables in implemented functions 15\n
Bug: webrtc:370878648
Change-Id: I4529c17f54c653864cca27097e44c843210b9c52
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/368061
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Commit-Queue: Dor Hen <dorhen@meta.com>
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#43429}
2024-11-20 11:50:20 +00:00
Tom Sepez
7085a884aa Avoid string duplication when returning StringBuilder strings
The const-ref result of .str() must be copied into the returned
value, whereas the result of .Release() can be moved.

Bug: webrtc:374845009
Change-Id: I3abc98be30ce9947127c7664f5ffa6846b772ea2
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/366480
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Reviewed-by: Tomas Gunnarsson <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#43288}
2024-10-23 07:54:18 +00:00
Victor Boivie
db54ea73e3 dcsctp: Add a fastpath for interleaved reassembly
The same as https://webrtc-review.googlesource.com/c/src/+/331340, but
for interleaved messages.

This avoids inserting into maps where possible, and also fixes a bug
when the payload was accidentally copied unintentionally -
crbug.com/365594101.

Bug: chromium:365594101
Change-Id: Iaeaa97b0cf3a26ada9afc61f2545760b7ab4c731
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/363960
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#43099}
2024-09-28 07:08:43 +00:00
Dor Hen
65b59a9c2d Prepend webrtc ns to StrJoin calls in dcsctp ns
Bug: webrtc:365299886
Change-Id: I4eb87a2b116c3e18b1a84865fab0a22a6084912c
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/361980
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Dor Hen <dorhen@meta.com>
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#42980}
2024-09-09 11:16:56 +00:00
Victor Boivie
65b46da1f8 dcsctp: Don't send FORWARD-TSN in its own chunk
This is a rollback of a change list [1] that was an attempt to avoid a
bug in usrsctp[2], but it wasn't very successful. But with the usrsctp
bug fixed, we can revert to the desired state; that we bundle
FORWARD-TSN with other control and data chunks in a SCTP packet.

[1] https://webrtc-review.googlesource.com/c/src/+/229101
[2] https://github.com/sctplab/usrsctp/issues/597

Bug: webrtc:42223134
Change-Id: I2f3b511c91639e6b9516160190600beb0c04b5fa
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/361862
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#42976}
2024-09-06 13:29:52 +00:00
Victor Boivie
7929ef578a dcsctp: Add test for stream reset pending
Bug: None
Change-Id: Ida040461dda53b438d88df4a38518f334cadc379
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/361861
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Auto-Submit: Victor Boivie <boivie@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#42975}
2024-09-06 13:20:58 +00:00
Florent Castelli
8037fc6ffa Migrate absl::optional to std::optional
Bug: webrtc:342905193
No-Try: True
Change-Id: Icc968be43b8830038ea9a1f5f604307220457807
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/361021
Auto-Submit: Florent Castelli <orphis@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Florent Castelli <orphis@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#42911}
2024-09-02 12:16:47 +00:00
Victor Boivie
7348f820a9 dcsctp: Re-add lost validating in test case
This was unintentionally removed in change
https://webrtc-review.googlesource.com/c/src/+/359681 due to a dirty
workspace.

Re-adding it.

Bug: None
Change-Id: Icff55b7a656ed9b504b0e10e7aeb947e8df78e85
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/360540
Auto-Submit: Victor Boivie <boivie@webrtc.org>
Commit-Queue: Florent Castelli <orphis@webrtc.org>
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#42846}
2024-08-26 09:22:13 +00:00
Victor Boivie
adb224b3e9 dcsctp: Simplify congestion control algorithm
There was a feature in the retransmission queue to avoid fragmenting
large messages when the congestion window was large. This was a feature
that intended to improve data channel performance under Chrome, where
communication with the network process (over MOJO) was lossy and losing
messages with small fragments could result in unnecessary
retransmissions. But back then, the implementation for fast retransmit
wasn't implemented correctly, so the benchmarking result don't
reproduce any longer.

So just improve the algorithm by removing this code. This aligns it with
the RFC and makes it easier to implement pluggable congestion control
algorithms (that wouldn't want this feature).

Bug: webrtc:42223116
Change-Id: Ifaaa82dac4b8fe2f55418158ae8b3da97199212f
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/359681
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#42797}
2024-08-18 15:20:35 +00:00
Dor Hen
1921fa5ea1 Apply include-cleaner to api/test/[^/]*
e.g all files in the api/test folder not including subdirectories

Bug: webrtc:42226242
Change-Id: I18d74a18f8feec41eb252faa9acfffd1d6f45ce4
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/359420
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Commit-Queue: Dor Hen <dorhen@meta.com>
Cr-Commit-Position: refs/heads/main@{#42773}
2024-08-13 15:28:34 +00:00
Victor Boivie
135f781b94 dcsctp: Pack state cookie
The elements in it can be packet better. This saves one byte.

The state cookie is only used from one peer to itself, so there is no
considerations around backwards or forwards compatibility.

Bug: None
Change-Id: I4f9a44f4c825626c7ed84bff52ed3155f5025a69
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/353142
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Commit-Queue: Florent Castelli <orphis@webrtc.org>
Auto-Submit: Victor Boivie <boivie@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#42421}
2024-05-31 13:20:31 +00:00
Victor Boivie
b206ab1f81 dcsctp: Restart heartbeat timer when sending DATA
Before this change, the heartbeat timer was restarted every time a
packet was sent on the socket. On an idle connection, if the peer is
sending heartbeats, just responding to those heartbeats (with a
HEARTBEAT-ACK) would restart the timer, and then this socket wouldn't
do any heartbeating itself because the next hearbeat by the peer would
be received before the timer expires.

This is not according to the specification, where
https://datatracker.ietf.org/doc/html/rfc9260#section-8.3 states that
"A destination transport address is considered "idle" if no new chunk
 that can be used for updating path RTT (usually including first
 transmission DATA, INIT, COOKIE ECHO, or HEARTBEAT chunks, etc.)"

There are already timers running when INIT, and COOKIE-ECHO are sent
and not acked, so the heartbeat shouldn't be sent then. This is further
confirmed in the same section in the RFC which says that "The sending of
HEARTBEAT chunks MAY begin upon reaching the ESTABLISHED state". And
when INIT and COOKIE-ECHO are sent, the connection is not yet
established.

This CL changes so that the heartbeat timer is only restarted when any
DATA or I-DATA chunk is sent. This will make both sides send heartbeats
on an idle connection.

Bug: webrtc:343600379
Change-Id: I5ab159b7901e2ec9d37b24aaf845891b60a53c13
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/352841
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#42409}
2024-05-30 10:50:30 +00:00
Florent Castelli
99c519b3fd Mass removal of absl_deps in all BUILD.gn files
Bug: webrtc:341803749
Change-Id: Id73844ba8d63b9f2f2c9391d8d8116ad0864c36d
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/351540
Commit-Queue: Florent Castelli <orphis@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#42372}
2024-05-23 15:09:46 +00:00
Per K
5566b91356 Reland "Replace usage of link_capacity_kbps with DataRate link_capacity"
This reverts commit ff2dd50fd88e07affc4b070ce535935409f6673a.

Reason for revert: Temporary fix for downstream breakage in patch 2

Original change's description:
> Revert "Replace usage of link_capacity_kbps with DataRate link_capacity"
>
> This reverts commit 6186c0226e41dabbfc5cc8527e6b350b62f39f02.
>
> Reason for revert: Breaks downstream project.
>
> Original change's description:
> > Replace usage of link_capacity_kbps with DataRate link_capacity
> >
> > Replace usage of BuiltInNetworkBehaviorConfig.link_capacity_kbps in tests with  DataRate link_capacity.
> >
> > Bug: webrtc:14525
> > Change-Id: Id1c1b8d20eb2be5e9d1461704bb7c37c61c491f0
> > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/350300
> > Reviewed-by: Erik Språng <sprang@webrtc.org>
> > Reviewed-by: Harald Alvestrand <hta@webrtc.org>
> > Commit-Queue: Per Kjellander <perkj@webrtc.org>
> > Reviewed-by: Björn Terelius <terelius@webrtc.org>
> > Cr-Commit-Position: refs/heads/main@{#42306}
>
> Bug: webrtc:14525
> Change-Id: I09ede3e89d065061cb4133bff96dbf242ff70832
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/350621
> Commit-Queue: Mirko Bonadei <mbonadei@webrtc.org>
> Bot-Commit: rubber-stamper@appspot.gserviceaccount.com <rubber-stamper@appspot.gserviceaccount.com>
> Owners-Override: Mirko Bonadei <mbonadei@webrtc.org>
> Cr-Commit-Position: refs/heads/main@{#42309}

Bug: webrtc:14525
Change-Id: Ie35cd97a158d008a80ed007b27d2c6b1a9affff9
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/350541
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Reviewed-by: Erik Språng <sprang@webrtc.org>
Commit-Queue: Per Kjellander <perkj@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Björn Terelius <terelius@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#42320}
2024-05-16 10:39:10 +00:00
Mirko Bonadei
ff2dd50fd8 Revert "Replace usage of link_capacity_kbps with DataRate link_capacity"
This reverts commit 6186c0226e41dabbfc5cc8527e6b350b62f39f02.

Reason for revert: Breaks downstream project.

Original change's description:
> Replace usage of link_capacity_kbps with DataRate link_capacity
>
> Replace usage of BuiltInNetworkBehaviorConfig.link_capacity_kbps in tests with  DataRate link_capacity.
>
> Bug: webrtc:14525
> Change-Id: Id1c1b8d20eb2be5e9d1461704bb7c37c61c491f0
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/350300
> Reviewed-by: Erik Språng <sprang@webrtc.org>
> Reviewed-by: Harald Alvestrand <hta@webrtc.org>
> Commit-Queue: Per Kjellander <perkj@webrtc.org>
> Reviewed-by: Björn Terelius <terelius@webrtc.org>
> Cr-Commit-Position: refs/heads/main@{#42306}

Bug: webrtc:14525
Change-Id: I09ede3e89d065061cb4133bff96dbf242ff70832
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/350621
Commit-Queue: Mirko Bonadei <mbonadei@webrtc.org>
Bot-Commit: rubber-stamper@appspot.gserviceaccount.com <rubber-stamper@appspot.gserviceaccount.com>
Owners-Override: Mirko Bonadei <mbonadei@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#42309}
2024-05-15 11:09:33 +00:00
Per K
6186c0226e Replace usage of link_capacity_kbps with DataRate link_capacity
Replace usage of BuiltInNetworkBehaviorConfig.link_capacity_kbps in tests with  DataRate link_capacity.

Bug: webrtc:14525
Change-Id: Id1c1b8d20eb2be5e9d1461704bb7c37c61c491f0
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/350300
Reviewed-by: Erik Språng <sprang@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Per Kjellander <perkj@webrtc.org>
Reviewed-by: Björn Terelius <terelius@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#42306}
2024-05-15 08:44:20 +00:00
Sergey Sukhanov
26a082ce36 Introduce a mode that lets NetworkEmulationManager ignore DTLS handshake sizes.
Bug: b/169531206
Change-Id: I02c19385ff7078944f7509ecc07358b4315f7b08
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/350181
Commit-Queue: Sergey Sukhanov <sergeysu@webrtc.org>
Reviewed-by: Victor Boivie <boivie@webrtc.org>
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Reviewed-by: Per Kjellander <perkj@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#42261}
2024-05-08 13:20:20 +00:00
Per K
569849e885 Move call/simulated_network to test/network
Old target and call/simulated.h exist but refer to new target in test/network.

Bug: webrtc:14525
Change-Id: Ida04cef17913f2f829d7e925ae454dc40d5e8240
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/349264
Commit-Queue: Per Kjellander <perkj@webrtc.org>
Reviewed-by: Björn Terelius <terelius@webrtc.org>
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Owners-Override: Per Kjellander <perkj@webrtc.org>
Reviewed-by: Erik Språng <sprang@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#42191}
2024-04-29 09:55:06 +00:00
Victor Boivie
b5f2442275 dcsctp: Remove dead code
This was only used for handover state, and not updated at all after
https://webrtc-review.googlesource.com/c/src/+/322623.

Bug: None
Change-Id: I5005902486d1fae76badd9f196e0e3525fe573de
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/349163
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#42175}
2024-04-25 12:39:48 +00:00
Victor Boivie
28d07ddbfd dcsctp: Compute RTO with higher precision
Since the code measuring the RTT has been converted to using TimeDelta
which internally stores the duration in microseconds, from DurationMs
which uses milliseconds, the RTO calculation can use the higher
precision to calculate lower non-zero durations on really fast networks
such within a data center.

Before this CL, which is from the initial drop of dcSCTP, the RTO
calculation was done using the algorithm from the paper "V. Jacobson:
Congestion avoidance and control", but now we're using the original
algorith from https://tools.ietf.org/html/rfc4960#section-6.3.1, which
comes from https://datatracker.ietf.org/doc/html/rfc6298#section-2.

Two issues were found and corrected:
 1. The min RTT variance that is specified in the config file was
    previously incorrectly divided by 8. That was not its intention,
    but we're keeping that behaviour as other clients have actually
    measured a good value to put there. This represents "G" in
    the "basic algorithm" above, and since that is multiplied with K,
    which is four, the default value of 220 wouldn't make sense if it
    wasn't scaled down, as that would make the RTO easily saturate to
    the RTO_max (800ms).
 2. The previous algorithm had large round-off errors (probably because
    the code used milliseconds), which makes fairly big changes to the
    calculated RTO in some situations.

Bug: webrtc:15593
Change-Id: I95a3e137c2bbbe7bf8b99c016381e9e63fd01d87
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/349000
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#42170}
2024-04-25 07:53:24 +00:00
Victor Boivie
de276cf049 dcsctp: Remove initial TSN from reassembly queue
With a previous refactoring, which made the data tracker responsible for
ensuring that the reassembly queue doesn't see any duplicate received
chunks, it no longer needs to know the initial peer's TSN. Removing.

Bug: None
Change-Id: I0e2aef1de0293f1860b46dee0089757c9c300aea
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/345701
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#41997}
2024-04-04 19:19:47 +00:00
Victor Boivie
0b83b2cbb4 dcsctp: Remove unreferenced reassembly_streams.cc
This code was moved to ReassemblyQueue::AddReassembledMessage, the build
file was updated to remove the source file, but the source file was
never actually deleted. Dead code.

Bug: None
Change-Id: Iafb9bb276ff870398a76737ceb16ffc50a91738e
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/345620
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#41994}
2024-04-04 10:44:11 +00:00
Victor Boivie
2fc097ea83 Reapply "dcsctp: Add per-stream-limit, refactor limits."
Keeping the old setting for the total queue size
limit, which avoids breaking a downstream.

This reverts commit 47ce449afaf9ba38785437fdd338630cad24a77b
and relands commit 4c990e2e56157175324e651f95f3d8c6a0e5c030.

Bug: chromium:40072842
Change-Id: I1e7d14b5d0026232d1fc9277172b6947b8be3490
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/343120
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#41907}
2024-03-15 13:27:37 +00:00
Björn Terelius
47ce449afa Revert "dcsctp: Add per-stream-limit, refactor limits."
This reverts commit 4c990e2e56157175324e651f95f3d8c6a0e5c030.

Reason for revert: Breaks downstream build.

Original change's description:
> dcsctp: Add per-stream-limit, refactor limits.
>
> The limits have been moved out from the Send Queue as they were enforced
> outside the queue anyway (in the socket). That was a preparation for
> adding even more limits; There is now also a per-stream limit, allowing
> individual streams to have one (global) limit, and the entire socket to
> have another limit.
>
> These limits are very small in the default options. In Chrome, the limit
> is 16MB per stream, so expect the defaults to be updated when the
> additional buffering outside dcSCTP is removed.
>
> Bug: chromium:41221056
> Change-Id: I9f835be05d349cbfce3e9235d34b5ea0e2fe87d1
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/342481
> Reviewed-by: Florent Castelli <orphis@webrtc.org>
> Commit-Queue: Victor Boivie <boivie@webrtc.org>
> Cr-Commit-Position: refs/heads/main@{#41895}

Bug: chromium:41221056
Change-Id: Icd57fbfca87d6b512cfc7f7682ae709000c2bcad
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/343080
Commit-Queue: Björn Terelius <terelius@webrtc.org>
Reviewed-by: Victor Boivie <boivie@webrtc.org>
Bot-Commit: rubber-stamper@appspot.gserviceaccount.com <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/heads/main@{#41901}
2024-03-14 16:47:45 +00:00
Victor Boivie
4c990e2e56 dcsctp: Add per-stream-limit, refactor limits.
The limits have been moved out from the Send Queue as they were enforced
outside the queue anyway (in the socket). That was a preparation for
adding even more limits; There is now also a per-stream limit, allowing
individual streams to have one (global) limit, and the entire socket to
have another limit.

These limits are very small in the default options. In Chrome, the limit
is 16MB per stream, so expect the defaults to be updated when the
additional buffering outside dcSCTP is removed.

Bug: chromium:41221056
Change-Id: I9f835be05d349cbfce3e9235d34b5ea0e2fe87d1
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/342481
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#41895}
2024-03-13 11:13:56 +00:00
Victor Boivie
2bfb5db548 dcsctp: Update zero checksum option to v-06 draft
https://datatracker.ietf.org/doc/draft-ietf-tsvwg-sctp-zero-checksum/06/

The previous implementation was for version 00, and since then changes
have been made. The chunk that is used to negotiate this capability has
now grown to include an additional property - the sender's alternate
error detection method.

Bug: webrtc:14997
Change-Id: I78043d187b79f40bbadbcba02eae6eedf54f30f9
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/336380
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#41759}
2024-02-19 10:25:17 +00:00
Tal Benesh
e126e45403 Fixing unspecified evaluation order of std:move(), to avoid future issues.
This will be done by splitting the use of variables values prior to performing std:move

Bug: webrtc:15771
Change-Id: Ia88e733c3a4edf729e440295ae271d3cd9926ec5
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/334461
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#41532}
2024-01-16 08:53:28 +00:00
Victor Boivie
6791c9d17e dcsctp: Relax thread sequence checker
The DcSctpSocket is thread compatible. As long as you serialize accesses
to it - either by calling it from the same thread, or using some kind of
concurrency primitive (e.g. mutex) to avoid calling the API methods from
different threads concurrently, it's fine.

Using the sequence checker, we can verify that the socket is called from
the thread it was created on, or from the same task queue. This provided
a more strict verification, as it didn't allow e.g. creating a socket on
one thread, and then handing it to a different thread where it was used.
Nor did it allow having multiple threads use it, protecting any calls to
it using an external mutex.

One can avoid these checks using webrtc::CurrentTaskQueueSetter to allow
the sequence checker to believe it's running where it's not running, but
this is a hack.

This CL removes the sequence checker in the socket, to simplify using it
in environments that don't use task queues for synchronization. Since it
is still kept in dcsctp::TaskQueueTimeoutFactory, it's still used in all
environments where the task queue is used (e.g. Chrome).

This makes it easier to use dcSCTP without WebRTC.

Bug: None
Change-Id: I2674d7cd902bad45ed3d0816c908ecf3ee971727
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/333801
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#41482}
2024-01-09 11:50:44 +00:00
Daniel Collins
c9d44b3fb9 Add SendMany method to dcsctp socket
Bug: webrtc:15724
Change-Id: Ib1689cd46395e2315803714ef50c009580fd71bb
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/331021
Reviewed-by: Victor Boivie <boivie@webrtc.org>
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#41397}
2023-12-15 21:35:14 +00:00
Daniel Collins
d093d0db7f Change CallbackDeferrer to pre-reserve the deferred vector.
The push_back pattern results in frequent vector growth which has performance overhead. This is .5% of our server's CPU

Bug: webrtc:15723
Change-Id: Ic151c81a4b49a7d48a354b75f62694bc6f9a1ee4
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/331440
Reviewed-by: Victor Boivie <boivie@webrtc.org>
Commit-Queue: Daniel Collins <dpcollins@google.com>
Cr-Commit-Position: refs/heads/main@{#41388}
2023-12-14 21:08:58 +00:00
Daniel Collins
042e57deea Add a fastpath for message reassembly that avoids map insertion
Change-Id: I50204915f4857f22e091fb9d88b4111e40d64227
Bug: webrtc:15724
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/331340
Commit-Queue: Daniel Collins <dpcollins@google.com>
Reviewed-by: Victor Boivie <boivie@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#41379}
2023-12-13 16:46:50 +00:00
Victor Boivie
161d2c8452 dcsctp: Fix not using iteraters after NackItem
OutstandingData::NackItem nacks a chunk, and if that chunk reaches its
partial reliability critera, it will abandon the entire message. If that
message hasn't been sent in full, a placeholder "end" message is
inserted (see https://crbug.com/webrtc/12812). And when the message is
inserted, any iterators may be invalidated (if e.g. std::deque would
want to grow the deque).

So ensure that there are no iterators used after having called NackItem.
By changing the interface of NackItem, and not passing an Item, but just
the TSN, this is encouraged. NackAll was rewritten as a two-pass
algorithm to first collect TSNs, then iterating that list, looking up
the items in the second pass (constant complexity).

Bug: chromium:1510364
Change-Id: I5156b6d6a683184f290e71c98f16bc68ea2a562f
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/331320
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Reviewed-by: Sam Zackrisson <saza@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#41374}
2023-12-13 14:21:12 +00:00
Daniel Collins
f418f48702 Change CallbackDeferrer to use a variant and callback pointer instead of std::function
This should substantially reduce the overhead due to deferred callbacks in profiles.

Bug: webrtc:15723
Change-Id: I4c52beb91eb08c9b0ac2d1ce9a4e11839aa35e38
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/331020
Reviewed-by: Victor Boivie <boivie@webrtc.org>
Commit-Queue: Daniel Collins <dpcollins@google.com>
Cr-Commit-Position: refs/heads/main@{#41363}
2023-12-12 13:15:12 +00:00
Victor Boivie
63e273ad4b dcsctp: Persist all state in state cookie
In the example below, the association is being established between peer
A and Z, and A is the initiating party.

Before this CL, when an association was about to be established, Z would
after having received the INIT chunk, persist state in the socket about
which verification tag and initial TSN that was picked. These would be
re-generated on every incoming INIT (that's fine), but when A had
extracted the cookie from INIT_ACK and sent a reply (COOKIE_ECHO) with
the state cookie, that could fail validation when it's received by Z, if
the sent cookie was not the most recent one or if the COOKIE_ECHO had a
verification tag coming not from the most recent INIT_ACK, because Z had
replaced the state in the socket with the one generated when the second
INIT_ACK chunk was generated - state it used for validation of future
received data.

In other words:
A -> INIT 1
<timeout>
A -> INIT 2 (retransmission of INIT 1)
INIT 1 -> Z - sends INIT_ACK 1 with verification_tag=1, initial_tsn=1,
              cookie 1 (and records these to socket state)
INIT 2 -> Z - sends INIT_ACK 2 with verification_tag=2, initial_tsn=2,
              cookie 2 (replaces socket state with the new data)
INIT_ACK 1 -> A -> sends COOKIE_ECHO with verification_tag=1, cookie 1
COOKIE_ECHO (cookie 1) -> Z <FAILS, as the state isn't as expected>.

The solution is really to do what RFC4960 says, to not maintain any
state as the receiving peer until COOKIE_ECHO has been received. This
was initially not done because the underlying reason why this is
important in SCTP is to avoid denial of service, and this is why SCTP
has the four-way handshake. But for Data Channels - SCTP over DTLS -
this attack vector isn't available. So the implementation was
"simplified" by keeping socket state instead of encoding it in the
state cookie, but that obviously had downsides.

So with this CL, the non-initiating peer in connection establishment
doesn't keep any socket state, and puts all that state in the state
cookie instead. This allows any COOKIE_ECHO to be received by Z.

Bug: webrtc:15712
Change-Id: I596c7330ce27292612d3c9f86b21c712f6f4e408
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/330440
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#41340}
2023-12-08 10:54:42 +00:00
Victor Boivie
9a2e32b9f2 dcsctp: Rename outstanding bytes to unacked bytes
And the same for outstanding items, which become unacked items. The old
names were unfortunate - especially since they were managed by a class
called OutstandingData.

To make this less complicated, these variables have been renamed to
something that is easier to understand; "Unacked bytes/items". Simply
what has been sent but hasn't been acked or nacked yet. So likely what's
in-flight, but could possibly be lost and not found to be lost yet.

Bug: None
Change-Id: I877d7f2cac5d164bf2f9f66cb32ae1f6d850ad2c
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/329761
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#41313}
2023-12-04 19:10:17 +00:00
Victor Boivie
9abc4865a4 dcsctp: Use std::deque for outstanding_data
A std::map is a fairly inefficient data structure. Accessing an item
is O(log(N)), but as every item is a separate allocation, iterating it
and searching for items is not very kind to the data caches.

As the outstanding data is a contiguous list (no gaps) where you only
append to the end and remove from the front, use a std::deque instead.

Bug: webrtc:15699
Co-authored-by: Daniel Collins <dpcollins@google.com>
Change-Id: I1f5fe97d06204c75b2b9553856af24e50f2ce715
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/329422
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#41310}
2023-12-04 15:10:15 +00:00
Victor Boivie
032805068c dcsctp: Calculate next_tsn
Before this CL, the next_tsn was stored as a variable, but that was
not needed as it can be calculated from the higest outstanding TSN. The
next TSN is simply the value after the highest outstanding TSN.

The highest outstanding TSN calculation could be simplified as well,
as the outstanding_data_ is contiguous list of TSNs counted from
last_cumulative_tsn_ack_.

Bug: None
Change-Id: Iafe188683427b5f2959d5ce2b19b5943d4760791
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/329421
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#41303}
2023-12-04 10:10:47 +00:00
Victor Boivie
b506d68f2a dcsctp: Remove deprecated delivery checks
With https://webrtc-review.googlesource.com/c/src/+/321603, the
responsibility to not ingest duplicate received chunks was moved from
the reassembly queue to the data tracker. But in that CL, we couldn't
remove updating the internal variables in the reassembly queue, because
those were included in the handover state. Now that time has passed,
we can remove this code altogether as nothing was ever reading from
these variables - only writing to them.

Bug: webrtc:14600
Change-Id: Icf958c75f74974be6cad7cd827cf49b3ab2f5412
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/329300
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#41291}
2023-11-30 17:27:52 +00:00
Sergey Silkin
ebc4d3edd5 Move StrJoin to rtc_base/strings
A similar function was defined in rtc_base/openssl_adapter. Moving it from net/dcsctp/common/ to rtc_base/strings/. I'm planning to use StrJoin in a video codec test (a follow-up change).

Bug: webrtc:14852
Change-Id: Ie657c03e7f9fb52c189c127af6f66ec505b512ae
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/327322
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Commit-Queue: Sergey Silkin <ssilkin@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#41166}
2023-11-15 12:10:28 +00:00
Victor Boivie
82cbbcc179 dcsctp: Convert use of TimeMs to webrtc::Timestamp
While this is a fairly big CL, it's fairly straightforward. It replaces
uses of TimeMs with webrtc::Timestamp, and additionally does some
cleanup of DurationMs to webrtc::TimeDelta that are now easier to do.

Bug: webrtc:15593
Change-Id: Id0e3edcba0533e0e8df3358b1778b6995c344243
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/326560
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#41138}
2023-11-12 21:12:29 +00:00