Skip to content
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

Jump to bottom

bpf: improve trace events in SNAT path #28723

Merged
merged 2 commits into from Feb 6, 2024
Merged

bpf: improve trace events in SNAT path #28723

merged 2 commits into from Feb 6, 2024

Conversation

julianwiedmann
Copy link
Member

@julianwiedmann julianwiedmann commented Oct 23, 2023 •
edited 

Enhance trace events from the outbound SNAT path, to report the pre-SNAT IP address and the interface index of the egress interface.

@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Oct 23, 2023
@julianwiedmann
Copy link
Member Author

/test

@github-actions GitHub Actions
Copy link

This pull request has been automatically marked as stale because it
has not had recent activity. It will be closed if no further activity
occurs. Thank you for your contributions.

@github-actions github-actions bot added the stale The stale bot thinks this issue is old. Add "pinned" label to prevent this from becoming stale. label Dec 21, 2023
@github-actions GitHub Actions
Copy link

github-actions bot commented Jan 4, 2024

This pull request has not seen any activity since it was marked stale.
Closing.

@github-actions github-actions bot closed this Jan 4, 2024
@kaworu kaworu reopened this Jan 16, 2024
@kaworu kaworu self-requested a review January 16, 2024 12:50
@kaworu
Copy link
Member

kaworu commented Jan 16, 2024

@julianwiedmann should we add a note in tail_handle_snat_fwd_ipv6 to also populate saddr and use send_trace_notify6 when egress gateway IPv6 support is implemented? If yes, maybe here?

@julianwiedmann
Copy link
Member Author

@julianwiedmann should we add a note in tail_handle_snat_fwd_ipv6 to also populate saddr and use send_trace_notify6 when egress gateway IPv6 support is implemented? If yes, maybe here?

I'll need to refresh my memory, but pretty sure that we can wire up the IPv6 path even today :).

We'll populate the orig-saddr for all kinds of SNAT operations, not just EgressGW. So for instance plain IPv6 Masquerading in BPF, where we would report the pod's IP in the trace event.

@github-actions github-actions bot removed the stale The stale bot thinks this issue is old. Add "pinned" label to prevent this from becoming stale. label Jan 17, 2024
@julianwiedmann julianwiedmann added sig/datapath Impacts bpf/ or low-level forwarding details, including map management and monitor messages. area/monitor Impacts monitoring, access logging, flow logging, visibility of datapath traffic. kind/enhancement This would improve or streamline existing functionality. release-note/minor This PR changes functionality that users may find relevant to operating Cilium. labels Jan 23, 2024
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Jan 23, 2024
@julianwiedmann julianwiedmann added the feature/snat Relates to SNAT or Masquerading of traffic label Jan 23, 2024
@julianwiedmann julianwiedmann changed the title WIP bpf: nat: report pre-SNAT address in trace event bpf: improve trace events in SNAT path Jan 23, 2024
@julianwiedmann
Copy link
Member Author

/test

@julianwiedmann
Copy link
Member Author

@kaworu I rebased and added the IPv6 handling. Also making sure that the orig_addr is only set when we actually SNAT the packet.

Would you have a moment to double-check this still works as expected?

@julianwiedmann julianwiedmann marked this pull request as ready for review January 23, 2024 12:04
@julianwiedmann julianwiedmann requested a review from a team as a code owner January 23, 2024 12:04
@julianwiedmann
bpf: nat: report pre-SNAT address in trace event
7ef0b51 
When applying SNAT to a packet, also report the original source address in
the subsequent trace event. This helps to associate the internal and
external view of a connection.

We use the `orig_addr` field in the trace event, which was originally
introduced back with b3aa583 ("bpf: Report original source IP in TRACE_TO_LXC")

Signed-off-by: Julian Wiedmann <jwi@isovalent.com>
@julianwiedmann
bpf: nodeport: populate ifindex in NAT trace event
f40f274 
This helps to clarify the exact origin of a TO_NETWORK trace event.

Signed-off-by: Julian Wiedmann <jwi@isovalent.com>
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Needs backport from main to Backport pending to v1.15 in 1.15.1 Feb 11, 2024
@sayboras sayboras mentioned this pull request Feb 11, 2024
Closed
1 task
@sayboras sayboras added backport-pending/1.14 The backport for Cilium 1.14.x for this PR is in progress. and removed needs-backport/1.14 This PR / issue needs backporting to the v1.14 branch labels Feb 11, 2024
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Needs backport from main to Backport pending to v1.14 in 1.14.7 Feb 11, 2024
@sayboras
Copy link
Member

👋

Seems like we might need some extra commits from #26674 for 1.14 backport.

@sayboras sayboras added backport/author The backport will be carried out by the author of the PR. needs-backport/1.14 This PR / issue needs backporting to the v1.14 branch and removed backport-pending/1.14 The backport for Cilium 1.14.x for this PR is in progress. labels Feb 11, 2024
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Backport pending to v1.14 to Needs backport from main in 1.14.7 Feb 11, 2024
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Backport pending to v1.14 to Needs backport from main in 1.14.7 Feb 11, 2024
@github-actions github-actions bot added backport-done/1.15 The backport for Cilium 1.15.x for this PR is done. and removed backport-pending/1.15 The backport for Cilium 1.15.x for this PR is in progress. labels Feb 12, 2024
@maintainer-s-little-helper maintainer-s-little-helper bot removed this from Backport pending to v1.15 in 1.15.1 Feb 12, 2024
@michi-covalent michi-covalent added this to Needs backport from main in 1.14.8 Feb 13, 2024
@michi-covalent michi-covalent removed this from Needs backport from main in 1.14.7 Feb 13, 2024
@michi-covalent michi-covalent mentioned this pull request Feb 14, 2024
Merged
@julianwiedmann julianwiedmann mentioned this pull request Feb 19, 2024
Merged
@julianwiedmann julianwiedmann added backport-pending/1.14 The backport for Cilium 1.14.x for this PR is in progress. and removed needs-backport/1.14 This PR / issue needs backporting to the v1.14 branch labels Feb 19, 2024
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Backport pending to v1.14 in 1.14.7 Feb 19, 2024
@github-actions github-actions bot added backport-done/1.14 The backport for Cilium 1.14.x for this PR is done. and removed backport-pending/1.14 The backport for Cilium 1.14.x for this PR is in progress. labels Feb 22, 2024
@maintainer-s-little-helper maintainer-s-little-helper bot removed this from Backport pending to v1.14 in 1.14.7 Feb 22, 2024
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Backport done to v1.14 in 1.14.7 Feb 22, 2024
@julianwiedmann julianwiedmann mentioned this pull request Feb 29, 2024
Open
@julianwiedmann julianwiedmann mentioned this pull request Mar 11, 2024
Open
@thorn3r thorn3r mentioned this pull request Mar 13, 2024
Merged
@kaworu kaworu mentioned this pull request May 1, 2024
Open
@julianwiedmann julianwiedmann mentioned this pull request May 8, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jibi jibi jibi approved these changes

@kaworu kaworu Awaiting requested review from kaworu

Assignees
No one assigned
Labels
area/monitor Impacts monitoring, access logging, flow logging, visibility of datapath traffic. backport/author The backport will be carried out by the author of the PR. backport-done/1.14 The backport for Cilium 1.14.x for this PR is done. backport-done/1.15 The backport for Cilium 1.15.x for this PR is done. feature/snat Relates to SNAT or Masquerading of traffic kind/enhancement This would improve or streamline existing functionality. release-note/minor This PR changes functionality that users may find relevant to operating Cilium. sig/datapath Impacts bpf/ or low-level forwarding details, including map management and monitor messages.
Projects
No open projects
1.14.7
Backport done to v1.14
1.14.8
Needs backport from main
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@julianwiedmann @kaworu @sayboras @jibi