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] When lo has IP override it when CTLB is disabled #8618

Merged

Conversation

tomastigera
Copy link
Contributor

@tomastigera tomastigera commented Mar 14, 2024

When CTLB is disabled, we route traffic for services via bpfnatin/out device. Since the final destination isn't resolved yet, Linux picks up an address set on loopback device (if there is any) as source. This may not be (likely is not) an address that can be used by the destination to return traffic. Therefore we need to replace it by the host's IP that is routable within the cluster.

We use the same mechanism as for replacing main host device IP with a tunnel IP when we need to reach a remote workload via an overlay.

If the IP set on loopback is the host IP (say du to dual ToR connectivity), that would work, because the override of the lo IP kicks in only if that IP differs.

Description

Related issues/PRs

Todos

  • Tests
  • Documentation
  • Release note

Release Note

ebpf: fixed source IP used by host when CTLB is disabled and loopback device has non-local IP set.

Reminder for the reviewer

Make sure that this PR has the correct labels and milestone set.

Every PR needs one docs-* label.

  • docs-pr-required: This change requires a change to the documentation that has not been completed yet.
  • docs-completed: This change has all necessary documentation completed.
  • docs-not-required: This change has no user-facing impact and requires no docs.

Every PR needs one release-note-* label.

  • release-note-required: This PR has user-facing changes. Most PRs should have this label.
  • release-note-not-required: This PR has no user-facing changes.

Other optional labels:

  • cherry-pick-candidate: This PR should be cherry-picked to an earlier release. For bug fixes only.
  • needs-operator-pr: This PR is related to install and requires a corresponding change to the operator.

@tomastigera tomastigera requested a review from a team as a code owner March 14, 2024 23:16
@marvin-tigera marvin-tigera added this to the Calico v3.28.0 milestone Mar 14, 2024
@marvin-tigera marvin-tigera added release-note-required Change has user-facing impact (no matter how small) docs-pr-required Change is not yet documented labels Mar 14, 2024
@tomastigera tomastigera added docs-not-required Docs not required for this change cherry-pick-candidate and removed docs-pr-required Change is not yet documented labels Mar 14, 2024
@tomastigera tomastigera force-pushed the tomas-bpf-ctlb-host-fix-src branch from 578282e to 6777ac7 Compare March 15, 2024 17:52
@tomastigera tomastigera force-pushed the tomas-bpf-ctlb-host-fix-src branch from 6777ac7 to ef265df Compare March 22, 2024 19:03
@tomastigera tomastigera force-pushed the tomas-bpf-ctlb-host-fix-src branch 2 times, most recently from e24efd0 to 2f97002 Compare April 3, 2024 17:46
@tomastigera tomastigera force-pushed the tomas-bpf-ctlb-host-fix-src branch 2 times, most recently from 6bd1392 to ad015b3 Compare April 12, 2024 05:01
Copy link
Member

@fasaxc fasaxc left a comment

Choose a reason for hiding this comment

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

LGTM

When CTLB is disabled, we route traffic for services via bpfnatin/out
device. Since the final destination isn't resolved yet, Linux picks up
an address set on loopback device (if there is any) as source. This may
not be (likely is not) an address that can be used by the destination to
return traffic. Therefore we need to replace it by the host's IP that is
routable within the cluster. So we set the host IP as a source on the
service routes.

We can only set the routes when there is a host IP, but that is also a
prerequisite for loading any bpf programs. When/if the host IP changes,
we mark all services as dirty so we reapply them with updated source.
@tomastigera tomastigera force-pushed the tomas-bpf-ctlb-host-fix-src branch from ad015b3 to f72b303 Compare April 12, 2024 16:58
@tomastigera
Copy link
Contributor Author

/merge-when-ready

@marvin-tigera
Copy link
Contributor

OK, I will merge the pull request when it's ready, leave the commits as is when I merge it, and leave the branch after I've merged it.

@marvin-tigera marvin-tigera merged commit 7a930d4 into projectcalico:master Apr 12, 2024
2 checks passed
tomastigera added a commit that referenced this pull request Apr 15, 2024
…-release-v3.28

[release-v3.28] Auto pick #8618: When lo has IP override it when CTLB is disabled
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cherry-pick-candidate docs-not-required Docs not required for this change merge-when-ready release-note-required Change has user-facing impact (no matter how small)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants