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

features: make HaveProgramHelper more accurate #1641

Merged
merged 1 commit into from
Jan 10, 2025

Conversation

lmb
Copy link
Collaborator

@lmb lmb commented Jan 8, 2025

Parse the verifier output to figure out whether a helper call is really not supported. This makes the result more reliable and allows us to get rid of asm.BuiltinFunc.Max().

@github-actions github-actions bot added the breaking-change Changes exported API label Jan 8, 2025
@lmb lmb force-pushed the features-helper-probe branch 2 times, most recently from 1afe479 to ee5d9a6 Compare January 10, 2025 08:18
@lmb lmb marked this pull request as ready for review January 10, 2025 09:23
@lmb lmb requested review from rgo3 and a team as code owners January 10, 2025 09:23
@lmb lmb force-pushed the features-helper-probe branch from ee5d9a6 to 710ebd1 Compare January 10, 2025 11:27
Copy link
Contributor

@rgo3 rgo3 left a comment

Choose a reason for hiding this comment

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

Cool, I think we thought about doing this since the beginning but never deemed it necessary. Did you find an instance where just checking EINVAL results in a false-negative?

Parse the verifier output to figure out whether a helper call is
really not supported. This makes the result more reliable and
allows us to get rid of asm.BuiltinFunc.Max().

Signed-off-by: Lorenz Bauer <[email protected]>
@lmb lmb force-pushed the features-helper-probe branch from 710ebd1 to 25d61ba Compare January 10, 2025 11:46
@lmb
Copy link
Collaborator Author

lmb commented Jan 10, 2025

Did you find an instance where just checking EINVAL results in a false-negative?

No, the motivation is to get rid of Max() for the Windows work.

@lmb lmb merged commit f3b4cde into cilium:main Jan 10, 2025
16 of 17 checks passed
@lmb lmb deleted the features-helper-probe branch January 10, 2025 13:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking-change Changes exported API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants