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: fix that no IP set filter was set at start-of-day. #8101

Merged
merged 3 commits into from Oct 20, 2023

Conversation

fasaxc
Copy link
Member

@fasaxc fasaxc commented Oct 10, 2023

Description

In BPF mode, we don't want to program linux IP sets unless we're using "untracked" policy, which is partially implemented in iptables. The filter to prevent IP set programming was only set after a policy churn.

  • Defer calculation of the filter until CompleteDeferredWork; this avoids doing O(n^2) work if there's a lot of policy churn.
  • Fix that transition from untracked to normal policy wasn't handled correctly. The chains and IP sets should be removed in that case.
  • Make sure the "dirty" flag is set at start of day so that the programming triggers before the first dataplane apply().

Related issues/PRs

CORE-9898

Todos

  • Tests
  • Documentation
  • Release note

Release Note

BPF mode: fix that netlink IP sets were programmed even in BPF mode until the first policy/endpoint deletion event.

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.

@marvin-tigera marvin-tigera added this to the Calico v3.27.0 milestone Oct 10, 2023
@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 Oct 10, 2023
@fasaxc fasaxc added docs-not-required Docs not required for this change and removed docs-pr-required Change is not yet documented labels Oct 10, 2023
@fasaxc fasaxc force-pushed the fix-ipset-filter branch 3 times, most recently from 95e9b23 to c449c71 Compare October 11, 2023 15:10
@fasaxc fasaxc marked this pull request as ready for review October 11, 2023 15:36
@fasaxc fasaxc requested a review from a team as a code owner October 11, 2023 15:36
@fasaxc
Copy link
Member Author

fasaxc commented Oct 12, 2023

Added a UT with the markups.

@fasaxc
Copy link
Member Author

fasaxc commented Oct 12, 2023

/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
Copy link
Contributor

Removing "merge-when-ready" label due to new commits

@fasaxc fasaxc force-pushed the fix-ipset-filter branch 2 times, most recently from c23261a to 7c977ea Compare October 13, 2023 12:59
In BPF mode, we don't want to program linux IP sets
unless we're using "untracked" policy, which is partially
implemented in iptables.  The filter to prevent IP set
programming was only set after a policy churn.

- Defer calculation of the filter until CompleteDeferredWork.
- Make sure the "dirty" flag is set at start of day so that
  the programming triggers before the first dataplane apply().
Includes cleanup of IP set reading functions.
@fasaxc
Copy link
Member Author

fasaxc commented Oct 16, 2023

/merge-when-ready

@fasaxc fasaxc merged commit 1e5c503 into projectcalico:master Oct 20, 2023
2 checks passed
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 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.

None yet

3 participants