Skip to content

Commit 12cdbd5

Browse files
MerinoSheepLiam Merinoseladb
authored
Fix #2081: Apply zstd compression level reliably across the documente… (#2113)
* Fix #2081: Apply zstd compression level reliably across the documented range * Fix flaky zstd compression-level test The test compared output sizes for levels 1 vs 10 on a 20KB pcap; on inputs that small, zstd produces identical output for both levels, so the GREATER_THAN assertion always failed. Switch to a larger input (EXAMPLE2_PCAPNG_PATH, ~26KB) and widen the gap to levels 1 vs 22 (zstd's documented max), which produces a clear size delta. Also brace the single-statement while body per review nit. * Skip zstd compression-level test when zstd is not configured --------- Co-authored-by: Liam Merino <merino@uber.com> Co-authored-by: seladb <pcapplusplus@gmail.com>
1 parent 3bad1dd commit 12cdbd5

5 files changed

Lines changed: 45 additions & 2 deletions

File tree

3rdParty/LightPcapNg/LightPcapNg/src/light_zstd_compression.c

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -57,8 +57,13 @@ _compression_t * get_zstd_compression_context(int compression_level)
5757
context->buffer_out_max_size = max(ZSTD_CStreamOutSize(), COMPRESSION_BUFFER_IN_MAX_SIZE);
5858
context->buffer_in = malloc(context->buffer_in_max_size);
5959
context->buffer_out = malloc(context->buffer_out_max_size);
60-
context->compression_level = compression_level * 2; //Input is scale 0-10 but zstd goes 0 - 20!
61-
assert(!ZSTD_isError(ZSTD_CCtx_setParameter(context->cctx, ZSTD_c_compressionLevel, compression_level)));
60+
// Map 0-10 input scale onto zstd's 1-22 range; preserve 0 as the "default
61+
// compression" sentinel. setParameter must run outside assert() so its
62+
// side effect is not stripped under NDEBUG.
63+
context->compression_level = compression_level > 0 ? (compression_level * 2) + 1 : 0;
64+
size_t const set_param_result = ZSTD_CCtx_setParameter(context->cctx, ZSTD_c_compressionLevel, context->compression_level);
65+
assert(!ZSTD_isError(set_param_result));
66+
(void)set_param_result;
6267

6368
return context;
6469
}

Tests/Pcap++Test/Common/PcapFileNamesDef.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
#define EXAMPLE_PCAPNG_WRITE_PATH "PcapExamples/many_interfaces_copy.pcapng"
2121
#define EXAMPLE2_PCAPNG_WRITE_PATH "PcapExamples/pcapng-example-write.pcapng"
2222
#define EXAMPLE_PCAPNG_ZSTD_WRITE_PATH "PcapExamples/many_interfaces_copy.pcapng.zstd"
23+
#define EXAMPLE_PCAPNG_ZSTD_LEVELS_WRITE_PATH "PcapExamples/many_interfaces_levels.pcapng.zstd"
2324
#define EXAMPLE2_PCAPNG_ZSTD_WRITE_PATH "PcapExamples/pcapng-example-write.pcapng.zstd"
2425
#define EXAMPLE2_PCAPNG_ZST_WRITE_PATH "PcapExamples/pcapng-example-write.pcapng.zst"
2526
#define EXAMPLE_PCAPNG_INTERFACES_PATH "PcapExamples/too_many_interfaces.pcapng"

Tests/Pcap++Test/TestDefinition.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ PTF_TEST_CASE(TestPcapFileReadAdv);
3030
PTF_TEST_CASE(TestPcapFileWriteAdv);
3131
PTF_TEST_CASE(TestPcapFileAppend);
3232
PTF_TEST_CASE(TestPcapNgFileReadWrite);
33+
PTF_TEST_CASE(TestPcapNgZstdCompressionLevels);
3334
PTF_TEST_CASE(TestPcapNgFileReadWriteAdv);
3435
PTF_TEST_CASE(TestPcapNgFileTooManyInterfaces);
3536
PTF_TEST_CASE(TestPcapFileReadLinkTypeIPv6);

Tests/Pcap++Test/Tests/FileTests.cpp

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1314,6 +1314,41 @@ PTF_TEST_CASE(TestPcapNgFileReadWrite)
13141314

13151315
} // TestPcapNgFileReadWrite
13161316

1317+
PTF_TEST_CASE(TestPcapNgZstdCompressionLevels)
1318+
{
1319+
#ifdef USE_Z_STD
1320+
// If the compression level is silently ignored, low and high levels
1321+
// produce identical output sizes. PcapPlusPlus exposes levels 0-10,
1322+
// which the zstd wrapper maps to zstd's native 1-22 range. Test three
1323+
// points to verify the level scales monotonically, not just that the
1324+
// endpoints differ.
1325+
std::array<int, 3> sizes = { 0, 0, 0 };
1326+
const std::array<int, 3> levels = { 1, 5, 10 };
1327+
for (int i = 0; i < 3; i++)
1328+
{
1329+
pcpp::PcapNgFileReaderDevice readerDev(EXAMPLE2_PCAPNG_PATH);
1330+
PTF_ASSERT_TRUE(readerDev.open());
1331+
1332+
pcpp::PcapNgFileWriterDevice writerDev(EXAMPLE_PCAPNG_ZSTD_LEVELS_WRITE_PATH, levels[i]);
1333+
PTF_ASSERT_TRUE(writerDev.open());
1334+
1335+
pcpp::RawPacket rawPacket;
1336+
while (readerDev.getNextPacket(rawPacket))
1337+
{
1338+
PTF_ASSERT_TRUE(writerDev.writePacket(rawPacket));
1339+
}
1340+
readerDev.close();
1341+
writerDev.close();
1342+
1343+
sizes[i] = getFileLength(EXAMPLE_PCAPNG_ZSTD_LEVELS_WRITE_PATH);
1344+
}
1345+
PTF_ASSERT_GREATER_THAN(sizes[0], sizes[1]);
1346+
PTF_ASSERT_GREATER_THAN(sizes[1], sizes[2]);
1347+
#else
1348+
PTF_SKIP_TEST("Zstd is not configured");
1349+
#endif
1350+
} // TestPcapNgZstdCompressionLevels
1351+
13171352
PTF_TEST_CASE(TestPcapNgFileReadWriteAdv)
13181353
{
13191354
pcpp::PcapNgFileReaderDevice readerDev(EXAMPLE2_PCAPNG_PATH);

Tests/Pcap++Test/main.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -230,6 +230,7 @@ int main(int argc, char* argv[])
230230
PTF_RUN_TEST(TestPcapFileReadAdv, "no_network;pcap");
231231
PTF_RUN_TEST(TestPcapFileWriteAdv, "no_network;pcap");
232232
PTF_RUN_TEST(TestPcapNgFileReadWrite, "no_network;pcap;pcapng");
233+
PTF_RUN_TEST(TestPcapNgZstdCompressionLevels, "no_network;pcap;pcapng");
233234
PTF_RUN_TEST(TestPcapNgFileReadWriteAdv, "no_network;pcap;pcapng");
234235
PTF_RUN_TEST(TestPcapNgFileTooManyInterfaces, "no_network;pcap;pcapng");
235236
PTF_RUN_TEST(TestPcapNgFilePrecision, "no_network;pcapng");

0 commit comments

Comments
 (0)