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: add improved helper for program-internal tail-call #30001

Merged
merged 1 commit into from
Dec 20, 2023

Conversation

julianwiedmann
Copy link
Member

The naming of ep_tail_call() is very misleading - it doesn't call to an endpoint's policy tail-call (in POLICY_CALL_MAP), and it's also used by programs that are not associated with an endpoint (eg. bpf_xdp and bpf_overlay). Instead it's used for a tail-call in the program's internal tail-call map.

So start off with introducing a new helper with improved naming. Then let this helper return DROP_MISSED_TAIL_CALL (instead of every caller open-coding the same value), and also setting the index of the missed tail-call in ext_err where available.

Finally steal a bit of compiler magic from the kernel, and enforce that callers check for returned errors. We've had too many cases in the past where we forgot to return the DROP_MISSED_TAIL_CALL.

Then convert a few initial callers to demonstrate the usage.

@julianwiedmann julianwiedmann added sig/datapath Impacts bpf/ or low-level forwarding details, including map management and monitor messages. release-note/misc This PR makes changes that have no direct user impact. labels Dec 20, 2023
The naming of ep_tail_call() is very misleading - it *doesn't* call to
an endpoint's policy tail-call (in POLICY_CALL_MAP), and it's also used
by programs that are *not* associated with an endpoint (eg. bpf_xdp and
bpf_overlay). Instead it's used for a tail-call in the program's internal
tail-call map.

So start off with introducing a new helper with improved naming. Then let
this helper return DROP_MISSED_TAIL_CALL (instead of every caller
open-coding the same value), and also setting the index of the missed
tail-call in `ext_err` where available.

Finally steal a bit of compiler magic from the kernel, and enforce that
callers check for returned errors. We've had too many cases in the past
where we forgot to return the DROP_MISSED_TAIL_CALL.

Then convert a few initial callers to demonstrate the usage.

Signed-off-by: Julian Wiedmann <jwi@isovalent.com>
Copy link
Member

@borkmann borkmann left a comment

Choose a reason for hiding this comment

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

looks good to me, nice improvement

Copy link
Member

@qmonnet qmonnet left a comment

Choose a reason for hiding this comment

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

I agree with Daniel, this looks nice!

@julianwiedmann
Copy link
Member Author

/test

@julianwiedmann julianwiedmann marked this pull request as ready for review December 20, 2023 11:27
@julianwiedmann julianwiedmann requested a review from a team as a code owner December 20, 2023 11:27
@julianwiedmann
Copy link
Member Author

Will follow-up with converting the other callers, and applying the same pattern to the policy map tail-calls.

Copy link
Contributor

@ldelossa ldelossa left a comment

Choose a reason for hiding this comment

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

Much better 👍

@julianwiedmann julianwiedmann added this pull request to the merge queue Dec 20, 2023
Merged via the queue into cilium:main with commit 4135f5b Dec 20, 2023
62 checks passed
@julianwiedmann julianwiedmann deleted the 1.16-bpf-tail-calls branch December 20, 2023 16:10
@julianwiedmann
Copy link
Member Author

Will follow-up with converting the other callers, and applying the same pattern to the policy map tail-calls.

The story continues in #30288.

@julianwiedmann julianwiedmann added the backport-pending/1.15 The backport for Cilium 1.15.x for this PR is in progress. label May 9, 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 May 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-done/1.15 The backport for Cilium 1.15.x for this PR is done. release-note/misc This PR makes changes that have no direct user impact. sig/datapath Impacts bpf/ or low-level forwarding details, including map management and monitor messages.
Projects
No open projects
Status: Released
Development

Successfully merging this pull request may close these issues.

None yet

4 participants