dcsctp: Limit the size of generated SACK chunks
Today, there is no actual limit on how large a SACK chunk can be. And having limits is good to be able to stay within the MTU. This commit adds a limit to the number of reported duplicate TSNs as well as the number of reported gap-ack-blocks in a SACK chunk. These limits are never expected to be reached in a real-life situation. Bug: webrtc:12614 Change-Id: Ib2c143714a214cd3d961e8a52dac26a04b909b80 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/219464 Reviewed-by: Harald Alvestrand <hta@webrtc.org> Commit-Queue: Victor Boivie <boivie@webrtc.org> Cr-Commit-Position: refs/heads/master@{#34108}
This commit is contained in:
committed by
WebRTC LUCI CQ
parent
e4adedc0cd
commit
c09c58134a
@ -26,6 +26,9 @@
|
||||
|
||||
namespace dcsctp {
|
||||
|
||||
constexpr size_t DataTracker::kMaxDuplicateTsnReported;
|
||||
constexpr size_t DataTracker::kMaxGapAckBlocksReported;
|
||||
|
||||
bool DataTracker::IsTSNValid(TSN tsn) const {
|
||||
UnwrappedTSN unwrapped_tsn = tsn_unwrapper_.PeekUnwrap(tsn);
|
||||
|
||||
@ -52,7 +55,9 @@ void DataTracker::Observe(TSN tsn,
|
||||
|
||||
// Old chunk already seen before?
|
||||
if (unwrapped_tsn <= last_cumulative_acked_tsn_) {
|
||||
duplicate_tsns_.insert(unwrapped_tsn.Wrap());
|
||||
if (duplicate_tsns_.size() < kMaxDuplicateTsnReported) {
|
||||
duplicate_tsns_.insert(unwrapped_tsn.Wrap());
|
||||
}
|
||||
return;
|
||||
}
|
||||
|
||||
@ -69,7 +74,9 @@ void DataTracker::Observe(TSN tsn,
|
||||
bool inserted = additional_tsns_.insert(unwrapped_tsn).second;
|
||||
if (!inserted) {
|
||||
// Already seen before.
|
||||
duplicate_tsns_.insert(unwrapped_tsn.Wrap());
|
||||
if (duplicate_tsns_.size() < kMaxDuplicateTsnReported) {
|
||||
duplicate_tsns_.insert(unwrapped_tsn.Wrap());
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@ -200,12 +207,14 @@ std::vector<SackChunk::GapAckBlock> DataTracker::CreateGapAckBlocks() const {
|
||||
|
||||
auto flush = [&]() {
|
||||
if (first_tsn_in_block.has_value()) {
|
||||
auto start_diff = UnwrappedTSN::Difference(*first_tsn_in_block,
|
||||
if (gap_ack_blocks.size() < kMaxGapAckBlocksReported) {
|
||||
auto start_diff = UnwrappedTSN::Difference(*first_tsn_in_block,
|
||||
last_cumulative_acked_tsn_);
|
||||
auto end_diff = UnwrappedTSN::Difference(*last_tsn_in_block,
|
||||
last_cumulative_acked_tsn_);
|
||||
auto end_diff = UnwrappedTSN::Difference(*last_tsn_in_block,
|
||||
last_cumulative_acked_tsn_);
|
||||
gap_ack_blocks.emplace_back(static_cast<uint16_t>(start_diff),
|
||||
static_cast<uint16_t>(end_diff));
|
||||
gap_ack_blocks.emplace_back(static_cast<uint16_t>(start_diff),
|
||||
static_cast<uint16_t>(end_diff));
|
||||
}
|
||||
first_tsn_in_block = absl::nullopt;
|
||||
last_tsn_in_block = absl::nullopt;
|
||||
}
|
||||
|
||||
@ -38,6 +38,11 @@ namespace dcsctp {
|
||||
// 200ms, whatever is smallest).
|
||||
class DataTracker {
|
||||
public:
|
||||
// The maximum number of duplicate TSNs that will be reported in a SACK.
|
||||
static constexpr size_t kMaxDuplicateTsnReported = 20;
|
||||
// The maximum number of gap-ack-blocks that will be reported in a SACK.
|
||||
static constexpr size_t kMaxGapAckBlocksReported = 20;
|
||||
|
||||
// The maximum number of accepted in-flight DATA chunks. This indicates the
|
||||
// maximum difference from this buffer's last cumulative ack TSN, and any
|
||||
// received data. Data received beyond this limit will be dropped, which will
|
||||
|
||||
@ -25,6 +25,7 @@ namespace dcsctp {
|
||||
namespace {
|
||||
using ::testing::ElementsAre;
|
||||
using ::testing::IsEmpty;
|
||||
using ::testing::SizeIs;
|
||||
using ::testing::UnorderedElementsAre;
|
||||
|
||||
constexpr size_t kArwnd = 10000;
|
||||
@ -269,5 +270,30 @@ TEST_F(DataTrackerTest, ClearsDuplicateTsnsAfterCreatingSack) {
|
||||
EXPECT_THAT(sack2.duplicate_tsns(), IsEmpty());
|
||||
}
|
||||
|
||||
TEST_F(DataTrackerTest, LimitsNumberOfDuplicatesReported) {
|
||||
for (size_t i = 0; i < DataTracker::kMaxDuplicateTsnReported + 10; ++i) {
|
||||
TSN tsn(11 + i);
|
||||
buf_.Observe(tsn, AnyDataChunk::ImmediateAckFlag(false));
|
||||
buf_.Observe(tsn, AnyDataChunk::ImmediateAckFlag(false));
|
||||
}
|
||||
|
||||
SackChunk sack = buf_.CreateSelectiveAck(kArwnd);
|
||||
EXPECT_THAT(sack.gap_ack_blocks(), IsEmpty());
|
||||
EXPECT_THAT(sack.duplicate_tsns(),
|
||||
SizeIs(DataTracker::kMaxDuplicateTsnReported));
|
||||
}
|
||||
|
||||
TEST_F(DataTrackerTest, LimitsNumberOfGapAckBlocksReported) {
|
||||
for (size_t i = 0; i < DataTracker::kMaxGapAckBlocksReported + 10; ++i) {
|
||||
TSN tsn(11 + i * 2);
|
||||
buf_.Observe(tsn, AnyDataChunk::ImmediateAckFlag(false));
|
||||
}
|
||||
|
||||
SackChunk sack = buf_.CreateSelectiveAck(kArwnd);
|
||||
EXPECT_EQ(sack.cumulative_tsn_ack(), TSN(11));
|
||||
EXPECT_THAT(sack.gap_ack_blocks(),
|
||||
SizeIs(DataTracker::kMaxGapAckBlocksReported));
|
||||
}
|
||||
|
||||
} // namespace
|
||||
} // namespace dcsctp
|
||||
|
||||
Reference in New Issue
Block a user