Add L2/L3 shim protocol parsing and presentation coverage#3
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
This PR expands PcapFlowLab’s ability to parse and present “shim”/encapsulation layers (VLAN/QinQ, IEEE 802.3 LLC/SNAP, PPPoE/PPP, MPLS Ethernet pseudowire, PBB, MACsec) while keeping decoding bounded and safe for malformed/truncated packets, and adds a large set of deterministic fixture-driven regression tests plus updated protocol-support documentation.
Changes:
- Extend core decode/import logic to support bounded inner parsing for IEEE 802.3 length frames, PPPoE Session, and MPLS Ethernet pseudowire continuation, and propagate “bounded end” semantics into payload extraction and packet-length derivation.
- Expand
PacketDetailsto represent new L2/L3 shim metadata (LLC/SNAP, PPPoE, PBB, MACsec, inner Ethernet, MPLS PW control word, VLAN TPID, truncation/preview fields). - Add comprehensive deterministic fixture tests and READMEs for VLAN, PPPoE, LLC/SNAP, MPLS pseudowire, PBB, and MACsec; update protocol support docs and test wiring.
Reviewed changes
Copilot reviewed 29 out of 120 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/VlanTests.cpp | Extends unit checks for VLAN TPID/truncation best-effort decoding. |
| tests/unit/VlanPcapFixtureTests.cpp | Adds deterministic VLAN parsing fixture regression tests and summary-layer assertions. |
| tests/unit/TestSupport.h | Extends test support API for recording failures and exposes failure storage helpers. |
| tests/unit/test_main.cpp | Changes test harness execution model (suite loop + failure recording). |
| tests/unit/PppoePcapFixtureTests.cpp | Adds PPPoE Session/Discovery/control fixture regression tests. |
| tests/unit/PbbPcapFixtureTests.cpp | Adds PBB / MAC-in-MAC fixture regression tests, including inner continuation and truncation cases. |
| tests/unit/PacketPayloadTests.cpp | Adds payload extraction coverage for LLC/SNAP ARP with declared-length bounding. |
| tests/unit/MplsPseudowirePcapFixtureTests.cpp | Adds MPLS Ethernet pseudowire fixture regression tests (control word, inner Ethernet/VLAN/LLC/SNAP). |
| tests/unit/MplsPcapFixtureTests.cpp | Adjusts MPLS fixture assertions for more flexible/robust summary validation and reason text. |
| tests/unit/MacsecPcapFixtureTests.cpp | Adds MACsec fixture regression tests (presentation-only, truncation, no-flow semantics). |
| tests/unit/LlcSnapPcapFixtureTests.cpp | Adds IEEE 802.3 LLC/SNAP fixture regression tests (known PID continuation + conservative fallbacks). |
| tests/data/parsing/vlan/README.md | Documents VLAN fixture set and local generator usage/assumptions. |
| tests/data/parsing/pppoe/README.md | Documents PPPoE/PPP fixture set and local generator usage/assumptions. |
| tests/data/parsing/pbb/README.md | Documents PBB fixture set and local generator usage/assumptions. |
| tests/data/parsing/mpls_pw/README.md | Documents MPLS pseudowire fixture set and local generator usage/assumptions. |
| tests/data/parsing/macsec/README.md | Documents MACsec fixture set and local generator usage/assumptions. |
| tests/data/parsing/llc_snap/README.md | Documents LLC/SNAP fixture set and local generator usage/assumptions. |
| src/core/services/PacketPayloadService.cpp | Applies bounded-packet semantics to payload extraction (IPv4/IPv6 and ARP). |
| src/core/services/CaptureImportProcessor.cpp | Adds IEEE 802.3 LLC/SNAP prefix handling, MPLS pseudowire continuation, PPPoE bounds, and richer unrecognized-reason classification. |
| src/core/domain/PacketDetails.h | Expands packet-detail model for new shim protocols and truncation/preview metadata. |
| src/core/domain/PacketDetails.cpp | Updates PacketDetails::empty() to include new protocol/detail flags. |
| src/core/decode/PacketDecoder.cpp | Applies bounded-packet semantics to flow decode; adds special handling for malformed UDP under bounded shims. |
| src/app/session/SelectedFlowPacketSemantics.cpp | Applies bounded-packet semantics when deriving transport payload lengths. |
| src/app/session/CaptureSession.cpp | Enables basic protocol-text generation for best-effort PBB/MACsec no-flow packets. |
| docs/protocols/protocol_support.md | Updates protocol-support matrix and parsing-fixture directory list for new shim coverage. |
| CMakeLists.txt | Adds new unit test sources to the build when BUILD_TESTING is enabled. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
68
to
+76
| void expect(bool condition, const char* expression, const char* file, int line) { | ||
| if (condition) { | ||
| return; | ||
| } | ||
|
|
||
| std::ostringstream builder; | ||
| std::ostringstream builder {}; | ||
| builder << file << ':' << line << " expectation failed: " << expression; | ||
| throw TestFailure(builder.str()); | ||
| record_failure_message(builder.str()); | ||
| } |
Comment on lines
308
to
+330
| if (payload->next_header == detail::kIpProtocolUdp) { | ||
| const auto udp_payload = detail::parse_udp_payload_bounds( | ||
| packet_bytes, | ||
| payload->payload_offset, | ||
| ipv6_offset + detail::kIpv6HeaderSize + ipv6_payload_length | ||
| ); | ||
| if (!udp_payload.has_value()) { | ||
| if (payload->payload_offset + detail::kUdpHeaderSize > packet_end || | ||
| bounded_bytes.size() < payload->payload_offset + detail::kUdpHeaderSize) { | ||
| return {}; | ||
| } | ||
|
|
||
| flow_key.src_port = detail::read_be16(packet_bytes, payload->payload_offset); | ||
| flow_key.dst_port = detail::read_be16(packet_bytes, payload->payload_offset + 2U); | ||
| flow_key.src_port = detail::read_be16(bounded_bytes, payload->payload_offset); | ||
| flow_key.dst_port = detail::read_be16(bounded_bytes, payload->payload_offset + 2U); | ||
| flow_key.protocol = ProtocolId::udp; | ||
|
|
||
| auto packet_ref = make_packet_ref(packet); | ||
| packet_ref.payload_length = static_cast<std::uint32_t>(udp_payload->payload_length); | ||
| if (const auto udp_payload = detail::parse_udp_payload_bounds( | ||
| bounded_bytes, | ||
| payload->payload_offset, | ||
| ipv6_offset + detail::kIpv6HeaderSize + ipv6_payload_length | ||
| ); | ||
| udp_payload.has_value()) { | ||
| packet_ref.payload_length = static_cast<std::uint32_t>(udp_payload->payload_length); | ||
| } else { | ||
| const auto payload_offset = payload->payload_offset + detail::kUdpHeaderSize; | ||
| packet_ref.payload_length = static_cast<std::uint32_t>( | ||
| packet_end > payload_offset ? (packet_end - payload_offset) : 0U); | ||
| } |
Comment on lines
+1
to
+6
| #include <algorithm> | ||
| #include <filesystem> | ||
| #include <initializer_list> | ||
| #include <string> | ||
| #include <vector> | ||
|
|
Owner
Author
|
Addressed the review comments:
|
Comment on lines
+42
to
+47
| PFL_EXPECT(details->ethernet.ether_type == 0x8100); | ||
| PFL_EXPECT(details->has_vlan); | ||
| PFL_EXPECT(details->vlan_tags.size() == 1); | ||
| PFL_EXPECT(details->vlan_tags[0].tpid == 0x8100); |
Comment on lines
+87
to
+92
| PFL_EXPECT(details->ethernet.ether_type == 0x88A8); | ||
| PFL_EXPECT(details->has_vlan); | ||
| PFL_EXPECT(details->vlan_tags.size() == 2); | ||
| PFL_EXPECT(details->vlan_tags[0].tpid == 0x88A8); |
Owner
Author
|
Addressed.
|
Comment on lines
+175
to
+184
| } else { | ||
| // Allow tuple extraction from malformed/truncated UDP only when a higher-level | ||
| // shim (currently PPPoE) has already bounded the visible packet bytes. | ||
| if (!network->bounded_packet_end.has_value()) { | ||
| return {}; | ||
| } | ||
| const auto payload_offset = transport_offset + detail::kUdpHeaderSize; | ||
| packet_ref.payload_length = static_cast<std::uint32_t>( | ||
| packet_end > payload_offset ? (packet_end - payload_offset) : 0U); | ||
| } |
Comment on lines
+326
to
+333
| } else { | ||
| if (!network->bounded_packet_end.has_value()) { | ||
| return {}; | ||
| } | ||
| const auto payload_offset = payload->payload_offset + detail::kUdpHeaderSize; | ||
| packet_ref.payload_length = static_cast<std::uint32_t>( | ||
| packet_end > payload_offset ? (packet_end - payload_offset) : 0U); | ||
| } |
Comment on lines
68
to
72
| void expect(bool condition, const char* expression, const char* file, int line) { | ||
| if (condition) { | ||
| return; | ||
| } | ||
|
|
Owner
Author
|
Addressed.
|
Comment on lines
+181
to
+182
| // Allow tuple extraction from malformed/truncated UDP only when a higher-level | ||
| // shim (currently PPPoE) has already bounded the visible packet bytes. |
Comment on lines
+77
to
+85
| struct PbbDetails { | ||
| bool present {false}; | ||
| bool itag_truncated {false}; | ||
| std::uint8_t available_bytes {0}; | ||
| std::uint8_t pcp {0}; | ||
| bool dei {false}; | ||
| bool uca {false}; | ||
| std::uint8_t reserved {0}; | ||
| std::uint32_t isid {0}; |
Owner
Author
|
Addressed.
|
Comment on lines
+60
to
+62
| const auto bounded_bytes = network->bounded_packet_end.has_value() | ||
| ? packet_bytes.first(std::min(*network->bounded_packet_end, packet_bytes.size())) | ||
| : packet_bytes; |
Comment on lines
68
to
+76
| void expect(bool condition, const char* expression, const char* file, int line) { | ||
| if (condition) { | ||
| return; | ||
| } | ||
|
|
||
| std::ostringstream builder; | ||
| std::ostringstream builder {}; | ||
| builder << file << ':' << line << " expectation failed: " << expression; | ||
| record_failure_message(builder.str()); | ||
| } |
| - Packets: 1 | ||
| - Layer chain: Ethernet / VLAN / partial IPv4 | ||
| - VLAN TPID: `0x8100` | ||
| - Expected behavior: capture truncation remains visible; parser stays safe after VLAN and may produce partial or unrecognized behavior depending on current implementation. Richer partial IPv4 presentation is future IPv4 parser work. |
Owner
Author
|
Addressed.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR extends PcapFlowLab’s L2/L3 shim protocol coverage and makes nested packet presentation more robust.
The branch adds fixture coverage, parser/presentation support, and regression tests for several encapsulation/shim formats:
The main goal is to improve analysis of captures where the useful inner flow is hidden behind one or more L2/L3 shim layers, while keeping malformed/truncated packets safe and inspectable.
Main changes
VLAN / QinQ
0x9100.PPPoE
Added deterministic PPPoE fixture generation and fixture tests.
Added PPPoE Session continuation into IPv4 / IPv6.
Added PPPoE Discovery and PPP control protocol presentation.
Added LCP / IPCP / IPv6CP basic presentation.
Added conservative handling for unknown PPP protocols.
Made PPPoE payload parsing respect declared payload bounds:
Shared partial IPv4 presentation
IEEE 802.3 LLC/SNAP
Added deterministic LLC/SNAP fixtures and fixture tests.
Added IEEE 802.3 length-field handling distinct from Ethernet II EtherType handling.
Added LLC and SNAP presentation:
Added continuation into IPv4 / IPv6 / ARP for known SNAP PIDs.
Allows known IPv4/IPv6/ARP SNAP PIDs even with non-zero OUI when the bounded payload validates.
Added conservative fallback for unknown SNAP PID and non-SNAP LLC.
Added robust handling for truncated LLC header, truncated SNAP header, and truncated inner IPv4.
Added bounded trailer presentation for IEEE 802.3 frames with captured bytes beyond declared length.
MPLS Ethernet pseudowire
Added deterministic MPLS pseudowire fixtures and regression tests.
Preserved existing direct MPLS-to-IPv4/IPv6 behavior.
Added Ethernet pseudowire continuation after MPLS BoS:
Added continuation into inner Ethernet, VLAN/QinQ, LLC/SNAP, IPv4/IPv6/ARP, TCP/UDP.
Added specific no-flow handling for:
Improved partial control-word presentation so missing fields are not zero-filled.
PBB / MAC-in-MAC
Added deterministic IEEE 802.1ah PBB fixtures and regression tests.
Added basic PBB I-TAG parsing for EtherType
0x88e7.Added inner customer Ethernet continuation:
Added outer B-TAG support before PBB I-TAG.
Added I-TAG metadata presentation:
Added partial truncated I-TAG presentation without fabricating missing I-SID fields.
Improved nested L2 Summary titles so inner Ethernet layers include Src/Dst when safely available.
Added PBB-specific no-flow Protocol tab details for conservative fallback cases.
MACsec / IEEE 802.1AE
Added deterministic MACsec fixtures and regression tests.
Added presentation-only MACsec support for EtherType
0x88e5.Added SecTAG metadata presentation:
Added optional SCI presentation:
Shows protected payload as opaque bounded data.
Shows 16-byte ICV as a separate layer when available.
Keeps all MACsec packets no-flow.
Does not decrypt, validate ICV, or recover inner flows.
Does not decode protected payload as IPv4/IPv6/ARP/TCP/UDP/QUIC.
Added E=0/C=0 secured-data polish:
Test coverage
This PR adds or updates deterministic fixture coverage for:
The fixture tests cover:
The unit test harness was also improved so soft assertions collect multiple fixture failures in one run instead of failing fast on the first mismatch.
Validation
Validated with the core fixture test suite locally.
Manual Wireshark-vs-PFL checks were performed for representative fixtures across:
The manual checks confirmed that PFL preserves useful nested structure while intentionally not copying every Wireshark presentation choice exactly.
Intentional non-goals
This PR does not implement:
The current UI can now show substantially richer packet details, and some packet summaries may exceed the comfortable visible area. A separate UI improvement branch is planned for packet-details layout and navigation.