From ea9255e972dd335c0498cbd9d71f8bf645c1e829 Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Sat, 25 Feb 2023 18:20:05 -0500 Subject: [PATCH] Address review comments. --- src/inet/BasicPacketFilters.h | 14 +++++++++----- src/inet/tests/TestBasicPacketFilters.cpp | 2 +- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/src/inet/BasicPacketFilters.h b/src/inet/BasicPacketFilters.h index aabaf286581ec8..08f0b3fba1b5ec 100644 --- a/src/inet/BasicPacketFilters.h +++ b/src/inet/BasicPacketFilters.h @@ -42,7 +42,7 @@ class DropIfTooManyQueuedPacketsFilter : public chip::Inet::EndpointQueueFilter * * @param maxAllowedQueuedPackets - max number of pending-in-queue not yet processed predicate-matching packets */ - DropIfTooManyQueuedPacketsFilter(int maxAllowedQueuedPackets) : mMaxAllowedQueuedPackets(maxAllowedQueuedPackets) {} + DropIfTooManyQueuedPacketsFilter(size_t maxAllowedQueuedPackets) : mMaxAllowedQueuedPackets(maxAllowedQueuedPackets) {} /** * @brief Set the predicate to use for filtering @@ -68,7 +68,7 @@ class DropIfTooManyQueuedPacketsFilter : public chip::Inet::EndpointQueueFilter * * @param maxAllowedQueuedPackets - number of packets currently pending allowed. */ - void SetMaxQueuedPacketsLimit(int maxAllowedQueuedPackets) { mMaxAllowedQueuedPackets.store(maxAllowedQueuedPackets); } + void SetMaxQueuedPacketsLimit(size_t maxAllowedQueuedPackets) { mMaxAllowedQueuedPackets.store(maxAllowedQueuedPackets); } /** * @return the total number of packets dropped so far by the filter @@ -158,8 +158,12 @@ class DropIfTooManyQueuedPacketsFilter : public chip::Inet::EndpointQueueFilter return FilterOutcome::kAllowPacket; } + // If we ever go negative, we have mismatch ingress/egress filter via predicate and + // device may eventually starve. + VerifyOrDie(mNumQueuedPackets != 0); + --mNumQueuedPackets; - int numQueuedPackets = mNumQueuedPackets.load(); + size_t numQueuedPackets = mNumQueuedPackets.load(); if (numQueuedPackets == 0) { OnLastMatchDequeued(endpoint, pktInfo, pktPayload); @@ -176,8 +180,8 @@ class DropIfTooManyQueuedPacketsFilter : public chip::Inet::EndpointQueueFilter protected: PacketMatchPredicateFunc mPredicate = nullptr; void * mContext = nullptr; - std::atomic_int mNumQueuedPackets{ 0 }; - std::atomic_int mMaxAllowedQueuedPackets{ 0 }; + std::atomic_size_t mNumQueuedPackets{ 0 }; + std::atomic_size_t mMaxAllowedQueuedPackets{ 0 }; std::atomic_size_t mNumDroppedPackets{ 0u }; }; diff --git a/src/inet/tests/TestBasicPacketFilters.cpp b/src/inet/tests/TestBasicPacketFilters.cpp index c9df12d41043b2..ef07cab057eb89 100644 --- a/src/inet/tests/TestBasicPacketFilters.cpp +++ b/src/inet/tests/TestBasicPacketFilters.cpp @@ -38,7 +38,7 @@ using namespace chip::Inet; class DropIfTooManyQueuedPacketsHarness : public DropIfTooManyQueuedPacketsFilter { public: - DropIfTooManyQueuedPacketsHarness(int maxAllowedQueuedPackets) : DropIfTooManyQueuedPacketsFilter(maxAllowedQueuedPackets) {} + DropIfTooManyQueuedPacketsHarness(size_t maxAllowedQueuedPackets) : DropIfTooManyQueuedPacketsFilter(maxAllowedQueuedPackets) {} void OnDropped(const void * endpoint, const IPPacketInfo & pktInfo, const chip::System::PacketBufferHandle & pktPayload) override