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

fix dataplane loader #312

Closed
wants to merge 6 commits into from
Closed

fix dataplane loader #312

wants to merge 6 commits into from

Conversation

jokestax
Copy link
Contributor

@jokestax jokestax commented Dec 2, 2024

Description

The error was error cannot find value 'bpf-program' in this scope,because the variable bpf_program is only available when debug_assertions is enabled, so fixed it by using correct variable whether pogram is in release build or debug and also the include_bytes_aligned! macro was unable to load the file at runtime. Replaced with fs::read() to load the file

Testing

To test this change:

  1. Clone the repository.
  2. Run make build.image.dataplane.
  3. Ensure the build completes successfully without any errors.

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: jokestax
Once this PR has been reviewed and has the lgtm label, please assign astoycos for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Dec 2, 2024
@k8s-ci-robot k8s-ci-robot requested a review from astoycos December 2, 2024 09:50
@k8s-ci-robot k8s-ci-robot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Dec 2, 2024
@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 2, 2024
@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 2, 2024
Copy link
Member

@shaneutt shaneutt left a comment

Choose a reason for hiding this comment

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

Thank you so much for your contribution @jokestax we appreciate you joining us here 👋

I have a couple questions about the patch, let me know your thoughts?

dataplane/loader/src/main.rs Show resolved Hide resolved
dataplane/loader/src/main.rs Show resolved Hide resolved
@shaneutt
Copy link
Member

shaneutt commented Dec 2, 2024

@jokestax If I'm understanding correctly, the problem is resolved. Would it be reasonable to consider this closed or was there more to do here? Sorry for the bad timing with 427839a!

@jokestax
Copy link
Contributor Author

jokestax commented Dec 2, 2024

@shaneutt ,yes, the issue is resolved. we can go ahead and close it. Thank you again for the quick response and fix 🙌

@shaneutt
Copy link
Member

shaneutt commented Dec 2, 2024

No thank you, sorry about the overlap! If there's other areas here you're interested in contributing to, please do feel free to reach out on #blixt on https://kubernetes.slack.com and chat with us!

@shaneutt shaneutt closed this Dec 2, 2024
@jokestax
Copy link
Contributor Author

jokestax commented Dec 2, 2024

Thank you! I'm already part of the community 😊 and very excited to contribute to the project. I'll definitely take up some tasks. I was playing around with it and was thinking of taking on this task: #25. Is that okay?

@shaneutt
Copy link
Member

shaneutt commented Dec 2, 2024

Oh, yep! I see you joined the Slack already cool. Yes feel free to take a stab at that one, it's a bit old: if it seems kinda crunchy or if you need support please reach out in Slack. 🥂

@jokestax
Copy link
Contributor Author

jokestax commented Dec 2, 2024

Thanks a lot! I'll get started and reach out in Slack!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants