Skip to content

Fix #2081: Apply zstd compression level reliably across the documente… #2113

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
seladb merged 4 commits into from
Apr 30, 2026
+45 −2
Merged

Fix #2081: Apply zstd compression level reliably across the documente… #2113

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a957091
Fix #2081: Apply zstd compression level reliably across the documente…
Apr 24, 2026
322eeca
Fix flaky zstd compression-level test
Apr 26, 2026
6543a42
Skip zstd compression-level test when zstd is not configured
Apr 29, 2026
48dc05a
Merge branch 'dev' into fix/issue-2081-zstd-compression-level
seladb Apr 30, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions 3rdParty/LightPcapNg/LightPcapNg/src/light_zstd_compression.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,13 @@ _compression_t * get_zstd_compression_context(int compression_level)
context->buffer_out_max_size = max(ZSTD_CStreamOutSize(), COMPRESSION_BUFFER_IN_MAX_SIZE);
context->buffer_in = malloc(context->buffer_in_max_size);
context->buffer_out = malloc(context->buffer_out_max_size);
context->compression_level = compression_level * 2; //Input is scale 0-10 but zstd goes 0 - 20!
assert(!ZSTD_isError(ZSTD_CCtx_setParameter(context->cctx, ZSTD_c_compressionLevel, compression_level)));
// Map 0-10 input scale onto zstd's 1-22 range; preserve 0 as the "default
// compression" sentinel. setParameter must run outside assert() so its
// side effect is not stripped under NDEBUG.
context->compression_level = compression_level > 0 ? (compression_level * 2) + 1 : 0;
size_t const set_param_result = ZSTD_CCtx_setParameter(context->cctx, ZSTD_c_compressionLevel, context->compression_level);
assert(!ZSTD_isError(set_param_result));
(void)set_param_result;

return context;
}
Expand Down
1 change: 1 addition & 0 deletions Tests/Pcap++Test/Common/PcapFileNamesDef.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#define EXAMPLE_PCAPNG_WRITE_PATH "PcapExamples/many_interfaces_copy.pcapng"
#define EXAMPLE2_PCAPNG_WRITE_PATH "PcapExamples/pcapng-example-write.pcapng"
#define EXAMPLE_PCAPNG_ZSTD_WRITE_PATH "PcapExamples/many_interfaces_copy.pcapng.zstd"
#define EXAMPLE_PCAPNG_ZSTD_LEVELS_WRITE_PATH "PcapExamples/many_interfaces_levels.pcapng.zstd"
#define EXAMPLE2_PCAPNG_ZSTD_WRITE_PATH "PcapExamples/pcapng-example-write.pcapng.zstd"
#define EXAMPLE2_PCAPNG_ZST_WRITE_PATH "PcapExamples/pcapng-example-write.pcapng.zst"
#define EXAMPLE_PCAPNG_INTERFACES_PATH "PcapExamples/too_many_interfaces.pcapng"
Expand Down
1 change: 1 addition & 0 deletions Tests/Pcap++Test/TestDefinition.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ PTF_TEST_CASE(TestPcapFileReadAdv);
PTF_TEST_CASE(TestPcapFileWriteAdv);
PTF_TEST_CASE(TestPcapFileAppend);
PTF_TEST_CASE(TestPcapNgFileReadWrite);
PTF_TEST_CASE(TestPcapNgZstdCompressionLevels);
PTF_TEST_CASE(TestPcapNgFileReadWriteAdv);
PTF_TEST_CASE(TestPcapNgFileTooManyInterfaces);
PTF_TEST_CASE(TestPcapFileReadLinkTypeIPv6);
Expand Down
35 changes: 35 additions & 0 deletions Tests/Pcap++Test/Tests/FileTests.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1314,6 +1314,41 @@ PTF_TEST_CASE(TestPcapNgFileReadWrite)

} // TestPcapNgFileReadWrite

PTF_TEST_CASE(TestPcapNgZstdCompressionLevels)
{
#ifdef USE_Z_STD
// If the compression level is silently ignored, low and high levels
// produce identical output sizes. PcapPlusPlus exposes levels 0-10,
// which the zstd wrapper maps to zstd's native 1-22 range. Test three
// points to verify the level scales monotonically, not just that the
// endpoints differ.
std::array<int, 3> sizes = { 0, 0, 0 };
const std::array<int, 3> levels = { 1, 5, 10 };
for (int i = 0; i < 3; i++)
{
pcpp::PcapNgFileReaderDevice readerDev(EXAMPLE2_PCAPNG_PATH);
PTF_ASSERT_TRUE(readerDev.open());

pcpp::PcapNgFileWriterDevice writerDev(EXAMPLE_PCAPNG_ZSTD_LEVELS_WRITE_PATH, levels[i]);
PTF_ASSERT_TRUE(writerDev.open());

pcpp::RawPacket rawPacket;
while (readerDev.getNextPacket(rawPacket))
{
PTF_ASSERT_TRUE(writerDev.writePacket(rawPacket));
Comment thread
Dimi1010 marked this conversation as resolved.
}
readerDev.close();
writerDev.close();

sizes[i] = getFileLength(EXAMPLE_PCAPNG_ZSTD_LEVELS_WRITE_PATH);
}
PTF_ASSERT_GREATER_THAN(sizes[0], sizes[1]);
PTF_ASSERT_GREATER_THAN(sizes[1], sizes[2]);
Comment on lines +1345 to +1346

@seladb seladb Apr 27, 2026

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CI is failing for almost all platforms, maybe this is not always true? 🤔
I asked Claude and it suggests checking the window size from the header. Compression levels 1, 5, and 10 should result with different windows sizes

@MerinoSheep MerinoSheep Apr 27, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I took a look at .github/workflows/build_and_test.yml and it seems the only one that passes is the ubuntu container that has zstd configured on. From what I've seen if zstd is not available at run time the writer silently writes uncompressed files with the zstd extension.

@Dimi1010 Dimi1010 Apr 27, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That appears to be because the compression context in LightPcapNG is NULL if its compiled without zst. It effectively performs a silent fallback to uncompressed, since NULL is also used if no compression is requested.

I suppose we could catch that in the constructor or open call of the PcapNg writer.

PR #1962 does add some methods to detect if zst is detected at compile time, tho that one is still open.

@seladb seladb Apr 28, 2026

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe the test can use the USE_Z_STD macro that is used in LighPcapNg, otherwise it can skip the test:

PTF_TEST_CASE(TestPcapNgZstdCompressionLevels)
{
#ifdef USE_Z_STD
	// The test...
#else
	PTF_SKIP_TEST("Zstd is not configured");
#endif
}

@MerinoSheep MerinoSheep Apr 29, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Passing now 😄

#else
PTF_SKIP_TEST("Zstd is not configured");
#endif
} // TestPcapNgZstdCompressionLevels

PTF_TEST_CASE(TestPcapNgFileReadWriteAdv)
{
pcpp::PcapNgFileReaderDevice readerDev(EXAMPLE2_PCAPNG_PATH);
Expand Down
1 change: 1 addition & 0 deletions Tests/Pcap++Test/main.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,7 @@ int main(int argc, char* argv[])
PTF_RUN_TEST(TestPcapFileReadAdv, "no_network;pcap");
PTF_RUN_TEST(TestPcapFileWriteAdv, "no_network;pcap");
PTF_RUN_TEST(TestPcapNgFileReadWrite, "no_network;pcap;pcapng");
PTF_RUN_TEST(TestPcapNgZstdCompressionLevels, "no_network;pcap;pcapng");
PTF_RUN_TEST(TestPcapNgFileReadWriteAdv, "no_network;pcap;pcapng");
PTF_RUN_TEST(TestPcapNgFileTooManyInterfaces, "no_network;pcap;pcapng");
PTF_RUN_TEST(TestPcapNgFilePrecision, "no_network;pcapng");
Expand Down
Loading