From 4387ad6cdc559659875e384fc67772629b673f87 Mon Sep 17 00:00:00 2001 From: Evan Shrubsole Date: Mon, 9 Jan 2023 13:28:53 +0000 Subject: [PATCH] [Unwrap] Migrate dcsctp sequence numbers to SeqNumUnwrapper Bug: webrtc:13982 Change-Id: Ic900a967d1b8e96a2b1ec99424674ccb33eb7165 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/288940 Commit-Queue: Harald Alvestrand Reviewed-by: Victor Boivie Reviewed-by: Harald Alvestrand Auto-Submit: Evan Shrubsole Cr-Commit-Position: refs/heads/main@{#39084} --- net/dcsctp/common/BUILD.gn | 5 ++- net/dcsctp/common/sequence_numbers.h | 32 ++++--------------- rtc_base/BUILD.gn | 1 - rtc_base/DEPS | 3 -- .../sequence_numbers_conformance_test.cc | 29 ----------------- 5 files changed, 11 insertions(+), 59 deletions(-) diff --git a/net/dcsctp/common/BUILD.gn b/net/dcsctp/common/BUILD.gn index 251ebaaf91..78fa0d307e 100644 --- a/net/dcsctp/common/BUILD.gn +++ b/net/dcsctp/common/BUILD.gn @@ -22,7 +22,10 @@ rtc_source_set("math") { } rtc_source_set("sequence_numbers") { - deps = [ ":internal_types" ] + deps = [ + ":internal_types", + "../../../rtc_base:rtc_numerics", + ] sources = [ "sequence_numbers.h" ] } diff --git a/net/dcsctp/common/sequence_numbers.h b/net/dcsctp/common/sequence_numbers.h index c3422c2ccd..d324fb223a 100644 --- a/net/dcsctp/common/sequence_numbers.h +++ b/net/dcsctp/common/sequence_numbers.h @@ -15,6 +15,7 @@ #include #include "net/dcsctp/common/internal_types.h" +#include "rtc_base/numerics/sequence_number_unwrapper.h" namespace dcsctp { @@ -42,7 +43,7 @@ class UnwrappedSequenceNumber { // unwrapped ones. class Unwrapper { public: - Unwrapper() : largest_(kValueLimit) {} + Unwrapper() = default; Unwrapper(const Unwrapper&) = default; Unwrapper& operator=(const Unwrapper&) = default; @@ -53,40 +54,21 @@ class UnwrappedSequenceNumber { // This will also update the Unwrapper's state, to track the last seen // largest value. UnwrappedSequenceNumber Unwrap(WrappedType value) { - WrappedType wrapped_largest = - static_cast(largest_ % kValueLimit); - int64_t result = largest_ + Delta(value, wrapped_largest); - if (largest_ < result) { - largest_ = result; - } - return UnwrappedSequenceNumber(result); + return UnwrappedSequenceNumber(unwrapper_.Unwrap(*value)); } // Similar to `Unwrap`, but will not update the Unwrappers's internal state. UnwrappedSequenceNumber PeekUnwrap(WrappedType value) const { - WrappedType uint32_largest = - static_cast(largest_ % kValueLimit); - int64_t result = largest_ + Delta(value, uint32_largest); - return UnwrappedSequenceNumber(result); + return UnwrappedSequenceNumber( + unwrapper_.PeekUnwrap(*value)); } // Resets the Unwrapper to its pristine state. Used when a sequence number // is to be reset to zero. - void Reset() { largest_ = kValueLimit; } + void Reset() { unwrapper_.Reset(); } private: - static int64_t Delta(WrappedType value, WrappedType prev_value) { - static constexpr typename WrappedType::UnderlyingType kBreakpoint = - kValueLimit / 2; - typename WrappedType::UnderlyingType diff = *value - *prev_value; - diff %= kValueLimit; - if (diff < kBreakpoint) { - return static_cast(diff); - } - return static_cast(diff) - kValueLimit; - } - - int64_t largest_; + webrtc::SeqNumUnwrapper unwrapper_; }; // Returns the wrapped value this type represents. diff --git a/rtc_base/BUILD.gn b/rtc_base/BUILD.gn index 8b1a0c8460..1fa3eaa0cf 100644 --- a/rtc_base/BUILD.gn +++ b/rtc_base/BUILD.gn @@ -2140,7 +2140,6 @@ if (rtc_include_tests) { ":strong_alias", ":timeutils", "../modules:module_api_public", - "../net/dcsctp/common:sequence_numbers", "../test:test_main", "../test:test_support", ] diff --git a/rtc_base/DEPS b/rtc_base/DEPS index 3882f5acb5..3a77b5502a 100644 --- a/rtc_base/DEPS +++ b/rtc_base/DEPS @@ -12,7 +12,4 @@ specific_include_rules = { "gunit\.h": [ "+testing/base/public/gunit.h" ], - "sequence_numbers_conformance_test\.cc": [ - "+net/dcsctp/common/sequence_numbers.h", - ], } diff --git a/rtc_base/numerics/sequence_numbers_conformance_test.cc b/rtc_base/numerics/sequence_numbers_conformance_test.cc index 62f7eb7b1d..e3a0061de8 100644 --- a/rtc_base/numerics/sequence_numbers_conformance_test.cc +++ b/rtc_base/numerics/sequence_numbers_conformance_test.cc @@ -13,9 +13,7 @@ #include #include "modules/include/module_common_types_public.h" -#include "net/dcsctp/common/sequence_numbers.h" #include "rtc_base/numerics/sequence_number_unwrapper.h" -#include "rtc_base/strong_alias.h" #include "rtc_base/time_utils.h" #include "test/gmock.h" #include "test/gtest.h" @@ -25,27 +23,6 @@ namespace { using ::testing::Test; -using dcsctp::UnwrappedSequenceNumber; -using Wrapped = StrongAlias; -using TestSequence = UnwrappedSequenceNumber; - -template -class UnwrapperHelper; - -template <> -class UnwrapperHelper { - public: - int64_t Unwrap(uint32_t val) { - TestSequence s = unwrapper_.Unwrap(Wrapped(val)); - // UnwrappedSequenceNumber starts counting at 2^32. - constexpr int64_t kDcsctpUnwrapStart = int64_t{1} << 32; - return s.value() - kDcsctpUnwrapStart; - } - - private: - TestSequence::Unwrapper unwrapper_; -}; - // MaxVal is the max of the wrapped space, ie MaxVal + 1 = 0 when wrapped. template ::max()> struct FixtureParams { @@ -138,8 +115,6 @@ TYPED_TEST_P(UnwrapperConformanceFixture, MultipleNegativeWrapArounds) { // SequenceNumberUnwrapper can only wrap negative once. // rtc::TimestampWrapAroundHandler does not wrap around correctly. if constexpr (std::is_same() || - std::is_same>() || std::is_same()) { return; } @@ -166,7 +141,6 @@ using UnwrapperTypes = ::testing::Types< FixtureParams, FixtureParams, FixtureParams, - FixtureParams>, // SeqNumUnwrapper supports arbitrary limits. FixtureParams, k15BitMax>>; @@ -185,9 +159,6 @@ class TestNames { if constexpr (std::is_same>()) return "SeqNumUnwrapper15bit"; - if constexpr (std::is_same>()) - return "UnwrappedSequenceNumber"; } };