Commit graph

8 commits

Author SHA1 Message Date
Victor Boivie
3c6b46fc16 Reland "dcsctp: Support zero checksum packets"
This reverts commit 45eae34693.

It was found not to be the root cause of the performance
regression, so it's safe to reland.

Bug: webrtc:14997
Change-Id: I67c90752875bf4071cbdd5adfa462a37f4d4ceab
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/302162
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Bot-Commit: rubber-stamper@appspot.gserviceaccount.com <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/heads/main@{#39910}
2023-04-20 20:32:01 +00:00
Victor Boivie
45eae34693 Revert "dcsctp: Support zero checksum packets"
This reverts commit bd46bb7660.

Reason for revert: There is a slight performance degradation
pointing to this CL, so revert this to be able to confirm if
it is the culprit.


Original change's description:
> dcsctp: Support zero checksum packets
>
> If configured, the packet parser will allow packets with
> a set checksum of zero. In that case, the correct checksum
> will not even be calculated, avoiding a CPU intensive
> calculation.
>
> Also, if specified when building a packet, the checksum can
> be opted to be not calculated and written to the packet.
> This is to be used when draft-tuexen-tsvwg-sctp-zero-checksum
> has been negotiated, except for some packets during association
> establishment.
>
> This is mainly a preparation CL and follow-up CL will enable
> this feature.
>
> Low-Coverage-Reason: Affects debug logging code not run in tests
> Bug: webrtc:14997
> Change-Id: I3207ac3a626df039ee2990403c2edd6429f17617
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/298481
> Reviewed-by: Harald Alvestrand <hta@webrtc.org>
> Commit-Queue: Victor Boivie <boivie@webrtc.org>
> Cr-Commit-Position: refs/heads/main@{#39737}

Bug: webrtc:14997
Change-Id: Ie22267abb4bcd25d5af07875eb933c51ed5be853
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/301580
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39878}
2023-04-17 19:29:55 +00:00
Victor Boivie
bd46bb7660 dcsctp: Support zero checksum packets
If configured, the packet parser will allow packets with
a set checksum of zero. In that case, the correct checksum
will not even be calculated, avoiding a CPU intensive
calculation.

Also, if specified when building a packet, the checksum can
be opted to be not calculated and written to the packet.
This is to be used when draft-tuexen-tsvwg-sctp-zero-checksum
has been negotiated, except for some packets during association
establishment.

This is mainly a preparation CL and follow-up CL will enable
this feature.

Low-Coverage-Reason: Affects debug logging code not run in tests
Bug: webrtc:14997
Change-Id: I3207ac3a626df039ee2990403c2edd6429f17617
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/298481
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39737}
2023-04-02 21:38:00 +00:00
Henrik Boström
b951dc6f4c Allow specifying delayed task precision of dcsctp::Timer.
Context: The timer precision of PostDelayedTask() is about to be lowered
to include up to 17 ms leeway. In order not to break use cases that
require high precision timers, PostDelayedHighPrecisionTask() will
continue to have the same precision that PostDelayedTask() has today.
webrtc::TaskQueueBase has an enum (kLow, kHigh) to decide which
precision to use when calling PostDelayedTaskWithPrecision().

See go/postdelayedtask-precision-in-webrtc for motivation and a table of
delayed task use cases in WebRTC that are "high" or "low" precision.

Most timers in DCSCTP are believed to only be needing low precision (see
table), but the delayed_ack_timer_ of DataTracker[1] is an example of a
use case that is likely to break if the timer precision is lowered (if
ACK is sent too late, retransmissions may occur). So this is considered
a high precision use case.

This CL makes it possible to specify the precision of dcsctp::Timer.
In a follow-up CL we will update delayed_ack_timer_ to kHigh precision.

[1] https://source.chromium.org/chromium/chromium/src/+/main:third_party/webrtc/net/dcsctp/rx/data_tracker.cc;l=340

Bug: webrtc:13604
Change-Id: I8eec5ce37044096978b5dd1985fbb00bc0d8fb7e
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/249081
Reviewed-by: Victor Boivie <boivie@webrtc.org>
Reviewed-by: Tomas Gunnarsson <tommi@webrtc.org>
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#35809}
2022-01-26 18:40:24 +00:00
Victor Boivie
f75f9c2b29 dcsctp: Avoid integer overflow in HEARTBEAT-ACK
When a HEARTBEAT is sent out, the current timestamp is stored in the
parameter that will be returned by the HEARTBEAT-ACK. If the timestamp
from the HEARTBEAT-ACK would be from the future (either by jumping
clocks, bit errors, exploiting peer or a fuzzer), the measured RTT would
become really large, and when it was calculated, it would result in an
integer overflow; a wrapping subtraction.

This isn't a problem, as RetransmissionTimeout::ObserveRTT method would
discard measurements that were negative or too large, but it would
trigger an UBSAN violation.

Adding an additional check so that the subtraction doesn't ever wrap.

Bug: chromium:1252515
Change-Id: I1f97b1e773a4639b8193a528716ebd6a27fcb740
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/232904
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#35095}
2021-09-27 10:52:46 +00:00
Victor Boivie
5429d71022 dcsctp: Allow heartbeats to be disabled
This is useful in tests and in scenarios where the connection is
monitored externally and the heartbeat monitoring would be of no use.

Bug: webrtc:12614
Change-Id: Ida4f4e2e40fc4d2aa0c27ae9431f434da4cc8313
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/220766
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#34164}
2021-05-31 12:19:38 +00:00
Victor Boivie
1d2fa9a1c3 dcsctp: Expire timers just before triggering them
In real life, when a Timeout expires, the caller is supposed to call
DcSctpSocket::HandleTimeout directly, as the Timeout that just expired
is stopped (it just expired), but the Timer still believes it's running.
The system is not in a consistent state.

In tests, all timeouts were evaluated at the same time, which, if two
timeouts expired at the same time, would put them both as "not running",
and with their timers believing they were running. So if you would do
any operation on a timer whose timeout had just expired, the timeout
would assert saying that "you can't stop a stopped timeout" or similar.

This isn't relevant in non-test scenarios.

Solved by expiring timeouts one by one.

Bug: webrtc:12614
Change-Id: I79d006f4d3e96854d77cec3eb0080aa23b8569cb
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/217560
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#33925}
2021-05-05 12:40:21 +00:00
Victor Boivie
b9bdf64b92 dcsctp: Add Heartbeat Handler
It's responsible for answering incoming Heartbeat Requests, and to
send requests itself when a connection is idle. When it receives
a response, it will measure the RTT and if it doesn't receive a response
in time, that will result in a TX error, which will eventually close
the connection.

Bug: webrtc:12614
Change-Id: I08371d9072ff0461f60e0a2f7696c0fd7ccb57c5
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/214129
Reviewed-by: Tommi <tommi@webrtc.org>
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#33828}
2021-04-25 20:40:18 +00:00