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

[v1.13] loader: explicitly insert endpoint policy programs into policy maps #29309

Merged
merged 1 commit into from
Nov 22, 2023

Conversation

ti-mo
Copy link
Contributor

@ti-mo ti-mo commented Nov 21, 2023

Manual backport of #29307.

Avoid missed tail calls due to inserting policy programs too early during endpoint regeneration

@ti-mo ti-mo added release-note/bug This PR fixes an issue in a previous release of Cilium. kind/backports This PR provides functionality previously merged into master. backport/1.13 This PR represents a backport for Cilium 1.13.x of a PR that was merged to main. labels Nov 21, 2023
@ti-mo ti-mo changed the title loader: explicitly insert endpoint policy programs into policy maps [v1.13] loader: explicitly insert endpoint policy programs into policy maps Nov 21, 2023
@ti-mo
Copy link
Contributor Author

ti-mo commented Nov 21, 2023

/test-backport-1.13

@ti-mo ti-mo force-pushed the tb/v1.13-policy-progs branch 2 times, most recently from 18f47f0 to c39687a Compare November 21, 2023 14:10
@ti-mo
Copy link
Contributor Author

ti-mo commented Nov 21, 2023

/test-backport-1.13

@ti-mo
Copy link
Contributor Author

ti-mo commented Nov 21, 2023

/test-backport-1.13

[ upstream commit 4098439 ]

BPF ELF loaders generally populate maps in a non-deterministic order when it
comes to the order of the maps themselves. During some loads, cilium_calls
may be populated first, sometimes cilium_(egress)call_policy gets populated
first. The latter case is a problem since existing endpoints will constantly
get their cilium_call_policy entry invoked for incoming packets. If its tail
call map(s) are not yet populated, that will result in missed tail calls
leading to packet drops.

This commit zeroes the call maps' MapSpec.Contents and manually resolves
[]MapKV to the referenced policy progs, both for ingress and egress (l7)
handlers. Policy programs are explicitly inserted into the respective call
maps after the entrypoint(s) have been attached.

This calls for major refactoring -- inserting the policy program is equal to
attaching an entrypoint to a BPF hook and should be done explicitly by an
endpoint manager that has access to the Endpoint object. This would make
extracting the endpoint ID from the ELF section name redundant.

Signed-off-by: Timo Beckers <timo@isovalent.com>
@ti-mo
Copy link
Contributor Author

ti-mo commented Nov 21, 2023

/test-backport-1.13

1 similar comment
@ti-mo
Copy link
Contributor Author

ti-mo commented Nov 21, 2023

/test-backport-1.13

@ti-mo ti-mo marked this pull request as ready for review November 22, 2023 10:58
@ti-mo ti-mo requested a review from a team as a code owner November 22, 2023 10:58
@maintainer-s-little-helper maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Nov 22, 2023
@lmb lmb merged commit baae7aa into cilium:v1.13 Nov 22, 2023
60 of 61 checks passed
@ti-mo ti-mo deleted the tb/v1.13-policy-progs branch November 22, 2023 12:01
brb added a commit that referenced this pull request Nov 22, 2023
The issue got fixed by #29309.

Signed-off-by: Martynas Pumputis <m@lambda.lt>
pchaigno pushed a commit that referenced this pull request Nov 22, 2023
The issue got fixed by #29309.

Signed-off-by: Martynas Pumputis <m@lambda.lt>
qmonnet pushed a commit that referenced this pull request Nov 28, 2023
[ upstream commit 9ecca98 ]

The issue got fixed by #29309.

Signed-off-by: Martynas Pumputis <m@lambda.lt>
Signed-off-by: Quentin Monnet <quentin@isovalent.com>
joamaki pushed a commit that referenced this pull request Nov 29, 2023
[ upstream commit 9ecca98 ]

The issue got fixed by #29309.

Signed-off-by: Martynas Pumputis <m@lambda.lt>
Signed-off-by: Jussi Maki <jussi@isovalent.com>
joamaki pushed a commit that referenced this pull request Nov 30, 2023
[ upstream commit 9ecca98 ]

The issue got fixed by #29309.

Signed-off-by: Martynas Pumputis <m@lambda.lt>
Signed-off-by: Jussi Maki <jussi@isovalent.com>
joamaki pushed a commit that referenced this pull request Nov 30, 2023
[ upstream commit 9ecca98 ]

The issue got fixed by #29309.

Signed-off-by: Martynas Pumputis <m@lambda.lt>
Signed-off-by: Jussi Maki <jussi@isovalent.com>
aanm pushed a commit that referenced this pull request Dec 1, 2023
[ upstream commit 9ecca98 ]

The issue got fixed by #29309.

Signed-off-by: Martynas Pumputis <m@lambda.lt>
Signed-off-by: Jussi Maki <jussi@isovalent.com>
qmonnet pushed a commit that referenced this pull request Dec 5, 2023
[ upstream commit 9ecca98 ]

The issue got fixed by #29309.

Signed-off-by: Martynas Pumputis <m@lambda.lt>
Signed-off-by: Quentin Monnet <quentin@isovalent.com>
pjablonski123 pushed a commit to pjablonski123/cilium that referenced this pull request Dec 15, 2023
The issue got fixed by cilium#29309.

Signed-off-by: Martynas Pumputis <m@lambda.lt>
qmonnet pushed a commit that referenced this pull request Dec 18, 2023
[ upstream commit 9ecca98 ]

The issue got fixed by #29309.

Signed-off-by: Martynas Pumputis <m@lambda.lt>
Signed-off-by: Quentin Monnet <quentin@isovalent.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport/1.13 This PR represents a backport for Cilium 1.13.x of a PR that was merged to main. kind/backports This PR provides functionality previously merged into master. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/bug This PR fixes an issue in a previous release of Cilium.
Projects
No open projects
Status: Released
Development

Successfully merging this pull request may close these issues.

None yet

2 participants