bpf:trace: adjust TRACE_PAYLOAD_LEN for overlay packets#38984
Merged
Conversation
smagnani96
added
kind/enhancement
smagnani96
force-pushed
the
pr/smagnani96/adjust-trace-payload
branch
from
516cdd4 to
6a583b3
Compare
Member
You might want to consider one level of VLAN header in this calculation, to be on the safe side. |
smagnani96
changed the base branch from
main
to
pr/smagnani96/tracing-struct-refactoring
smagnani96
force-pushed
the
pr/smagnani96/adjust-trace-payload
branch
2 times, most recently
from
c2de6db to
198645b
Compare
smagnani96
changed the base branch from
pr/smagnani96/tracing-struct-refactoring
to
pr/smagnani96/monitor-decode-overlay
smagnani96
force-pushed
the
pr/smagnani96/adjust-trace-payload
branch
from
198645b to
41c36ed
Compare
smagnani96
had a problem deploying
to
release-base-images
GitHub Actions
Failure
smagnani96
force-pushed
the
pr/smagnani96/monitor-decode-overlay
branch
from
7e24d18 to
1a3e906
Compare
smagnani96
changed the title
smagnani96
force-pushed
the
pr/smagnani96/monitor-decode-overlay
branch
from
1a3e906 to
2a0de51
Compare
smagnani96
force-pushed
the
pr/smagnani96/adjust-trace-payload
branch
2 times, most recently
from
ea3a8a6 to
1f3ddaa
Compare
smagnani96
force-pushed
the
pr/smagnani96/monitor-decode-overlay
branch
from
2a0de51 to
58cbe6a
Compare
smagnani96
force-pushed
the
pr/smagnani96/adjust-trace-payload
branch
from
1f3ddaa to
7d74669
Compare
smagnani96
force-pushed
the
pr/smagnani96/adjust-trace-payload
branch
from
7d74669 to
10f4b10
Compare
smagnani96
changed the base branch from
pr/smagnani96/monitor-decode-overlay
to
pr/smagnani96/bpf-overlay-classifiers
smagnani96
force-pushed
the
pr/smagnani96/adjust-trace-payload
branch
from
10f4b10 to
12b48a2
Compare
smagnani96
force-pushed
the
pr/smagnani96/adjust-trace-payload
branch
from
12b48a2 to
e3a60f0
Compare
smagnani96
added
the
dont-merge/blocked
smagnani96
force-pushed
the
pr/smagnani96/adjust-trace-payload
branch
from
e3a60f0 to
bb12e4e
Compare
brb
approved these changes
Jun 16, 2025
kaworu
approved these changes
Jun 17, 2025
kaworu
added
dont-merge/needs-rebase
With this commit, we move TRACE_PAYLOAD_LEN from being a pure macro to being a config variable. This is purely to adapt to new BPF config and subsequent changes to introduce a new related config variable for tracing overlay packets. Signed-off-by: Simone Magnani <[email protected]>
This commit introduces a new node config variable trace_payload_len_overlay.
This variable will be enabled in subsequent commit, so that we can push
more bytes during trace notification for overlay traffic. In fact, the
old TRACE_PAYLOAD_LEN (128B) might not be enough for Hubble to decode
overlay packets such as:
- TCPv6 with options over VXLANv4 (>=134B) -- decode error
- TCPv6 with SRv6 segments (>=136B) -- decode error
- {ICMP,UDP,TCP}v6 over (future) VXLANv6 (>=132B) -- decode error
Given that now the datapath can distinguish between native and overlay
traffic, let's make use of it.
Signed-off-by: Simone Magnani <[email protected]>
With this patch we simply refactors the checks to enable Overlay classifiers during classification into specific helper functions for code clarity and reusability (see subsequent patch). Signed-off-by: Simone Magnani <[email protected]>
smagnani96
force-pushed
the
pr/smagnani96/adjust-trace-payload
branch
from
bb12e4e to
7713995
Compare
smagnani96
removed
the
dont-merge/needs-rebase
This commits adds an helper function `compute_capture_length` to compute the length of the capture associated to the trace/drop notification event. While doing this, we now postpone the computation of such value in trace/drop notification to save computational time and verifier complexity, given the value would not be used in case the notification should not be emitted. With the next commit, we further extend this helper to account for Overlay packets that require additional bytes. Signed-off-by: Simone Magnani <[email protected]>
smagnani96
force-pushed
the
pr/smagnani96/adjust-trace-payload
branch
from
7713995 to
34ba9bc
Compare
This commit make use of the previously introduced trace_payload_len_overlay. Whenever the dataplane is able to classify a packet as OVERLAY, let's push additional bytes in the notification event to allow Hubble decoding the packet. An example of failure before this patch is the `TCP data offset greater than packet length` error. This error can happen with an TCPv6 SYN (options MSS) over VXLAN: we're pushing 128B, but 144B would be needed for correctly decoding the TCP options. The utility function `ctx_capture_length` returns the correct capture length for the notification event. It makes use of classifiers flags and observation point. When provided with the monitor value (trace notifications, as in drop notifications we use 0), the helper accounts for it: * if `monitor` is 0, then the utility selects the default value based on the provided flags, either trace_payload_len or trace_payload_len_overlay. * if monitor is the default trace_payload_len used in the codebase, then the helper still checks the flags and transparently adjusts to trace_payload_len_overlay with overlay traffic. Normal usages should still prefer trace_payload_len in the codebase: the dapath will transparently adjust it to trace_payload_len_overlay during the trace notification event if/when needed. Signed-off-by: Simone Magnani <[email protected]>
smagnani96
force-pushed
the
pr/smagnani96/adjust-trace-payload
branch
from
34ba9bc to
eafc457
Compare
Contributor
Author
No functional changes wrt previous reviews. |
Contributor
Author
|
/test |
maintainer-s-little-helper
bot
added
ready-to-merge
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
7 participants
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.
Blocked by #39650.Good to have (non-blocking) for #37634.As described in commit messages, when tracing overlay packets the default TRACE_PAYLOAD_LEN might not be enough. An example would be an Overlay IPv4 packet carrying an IPv6 TCP packet with Options (MSS, etc., such as a SYN packet). In that case, Hubble would fail decoding such packet due to missing bytes. Same problems would arise in case of IPv6 underlay.
For this reason:
TRACE_PAYLOAD_LENtoCONFIG(trace_payload_len)for coherency with the new datapath config, and use it to trace native packetsCONFIG(trace_payload_len_overlay)and use it for tracing overlay packetsFixes: #39030.