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.

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

bpf_metadata: extract restoreLocalAddress #1133

Conversation

mhofstetter
Copy link
Member

Currently, extractSocketMetadata also restores the local address for the use by the OriginalDestCluster.

To keep the function free of side-effects, the logic that restores the local address gets extracted into a function within the SocketMetadata that gets called from the BPF metadata listener after extracting the BPF metadata from the socket.

Currently, `extractSocketMetadata` also restores the local address
for the use by the `OriginalDestCluster`.

To keep the function free of side-effects, the logic that restores the
local address gets extracted into a function within the `SocketMetadata`
that gets called from the BPF metadata listener after extracting the
BPF metadata from the socket.

Signed-off-by: Marco Hofstetter <[email protected]>
According to the Envoy sourcecode, restoration of the local
address should only be performed if it changes from the
current local address.

```
... This should only be called when restoring the original destination address
of a connection redirected by iptables REDIRECT. The caller is responsible for
making sure the new address is actually different.
```

Therefore, this commit introduces a check before restoring the local address.

Signed-off-by: Marco Hofstetter <[email protected]>
@mhofstetter mhofstetter force-pushed the pr/mhofstetter/bpfmetadata-extract-restorelocaladdress branch from 72f07fc to 0e8a156 Compare January 22, 2025 08:39
@mhofstetter mhofstetter marked this pull request as ready for review January 22, 2025 08:43
@mhofstetter mhofstetter requested a review from a team as a code owner January 22, 2025 08:43
Copy link
Member

@sayboras sayboras left a comment

Choose a reason for hiding this comment

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

Nice, also the test 👍

@jrajahalme jrajahalme added this pull request to the merge queue Jan 22, 2025
Merged via the queue into cilium:main with commit 7717128 Jan 22, 2025
5 checks passed
@mhofstetter mhofstetter deleted the pr/mhofstetter/bpfmetadata-extract-restorelocaladdress branch January 22, 2025 09:00
@mhofstetter mhofstetter mentioned this pull request Jan 28, 2025
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants