Updating Parser to work with GTP-U

[trvon]

In one of my research experiments, recirculated GTP-U packets triggered the PacketTooShort error. I assumed this was because the parser’s bounds were still compared against Packet::get_ingress_length(), even after the INT shim/BeamSec insertion increased the buffer size. During the second-pass parsing, this stale length caused the deparser to emit only the original payload, resulting in the omission of telemetry headers.

To address this issue, I updated all parser bounds checks (VL extracts, fixed extracts, shift/advance operations) to rely on Packet::get_data_size(), which provides the current length of the live buffer. This change ensures that the parser logic accurately reflects the real payload and only raises an error when the buffer itself is insufficient in size. I also added unit tests that simulate ingress_length < actual buffer.

[Copilot]

Pull Request Overview

Updates the parser to use the live buffer size instead of the original ingress length, ensuring parsing operations account for added headers (e.g., GTP-U/INT) during recirculation.

• Replace get_ingress_length() with get_data_size() in VL extracts and shift/advance operations.
• Add unit tests to validate parsing when data_size > ingress_length for shift, advance, and VL extract.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.

File	Description
src/bm_sim/parser.cpp	Switch bounds checks from ingress_length to data_size across extract_VL and shift/advance operations.
tests/test_parser.cpp	Add tests ensuring shift, advance, and VL extract succeed when the live buffer is larger than ingress_length.

---

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

[Copilot]

This subtraction can underflow if *bytes_parsed exceeds pkt->get_data_size(), yielding a large size_t and bypassing the bounds check. Consider guarding against underflow explicitly, e.g., first check if (*bytes_parsed > pkt->get_data_size()) or use a two-part condition: (*bytes_parsed > pkt->get_data_size()) || (pkt->get_data_size() - *bytes_parsed < bytes_to_extract).

Suggested change

	if (pkt->get_data_size() - *bytes_parsed < bytes_to_extract)
	if (*bytes_parsed > pkt->get_data_size() ||
	pkt->get_data_size() - *bytes_parsed < bytes_to_extract)

[Copilot]

Potential unsigned underflow in the subtraction if *bytes_parsed > pkt->get_data_size(). Use a defensive check to prevent underflow, e.g., (*bytes_parsed > pkt->get_data_size()) || (pkt->get_data_size() - *bytes_parsed < shift_bytes).

Suggested change

	if (pkt->get_data_size() - *bytes_parsed < shift_bytes)
	if ((*bytes_parsed > pkt->get_data_size()) ||
	(pkt->get_data_size() - *bytes_parsed < shift_bytes))

[Copilot]

Potential unsigned underflow on size_t subtraction if *bytes_parsed exceeds pkt->get_data_size(). Prefer a two-part guard: (*bytes_parsed > pkt->get_data_size()) || (pkt->get_data_size() - *bytes_parsed < shift_bytes_uint).

[Copilot]

Same unsigned underflow risk as elsewhere; subtracting before the comparison can wrap if *bytes_parsed > pkt->get_data_size(). Add an explicit pre-check for (*bytes_parsed > pkt->get_data_size()) or use a two-part condition that avoids underflow.

[Copilot]

Unsigned underflow risk on the subtraction; guard with an explicit check for (*bytes_parsed > pkt->get_data_size()) before subtracting, or combine as: (*bytes_parsed > pkt->get_data_size()) || (pkt->get_data_size() - *bytes_parsed < shift_bytes_uint).

Suggested change

	if (pkt->get_data_size() - *bytes_parsed < shift_bytes_uint)
	if ((*bytes_parsed > pkt->get_data_size()) ||
	(pkt->get_data_size() - *bytes_parsed < shift_bytes_uint))

[antoninbas]

It's not clear to me what the issue is based on this PR description.
As far as I can tell, the ingress_length is set correctly (based on what was produced by the deparser) for recirculated packets:

behavioral-model/targets/simple_switch/simple_switch.cpp

Line 762 in 68f4a97

packet_copy->set_ingress_length(packet_size);

It would be helpful to clarify this, as well as provide a P4 program to reproduce the issue. Adding @jafingerhut in case he has some time to help reproduce the issue / review the patch.

[trvon]

I do not have a clean way to share my p4 code, but I will reply with artifacts after I can reproduce the issue with a smaller p4 program and ptf test.