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

Add AttachIter to wrap bpf_program__attach_iter #254

Merged
merged 2 commits into from
Mar 19, 2023

Conversation

mozillazg
Copy link
Contributor

#91

@mozillazg mozillazg force-pushed the iter branch 3 times, most recently from c6bdb44 to 90a8515 Compare October 4, 2022 14:41
@mozillazg
Copy link
Contributor Author

Error: ./libbpfgo.go:133:27: error: variable has incomplete type 'union bpf_iter_link_info'
union bpf_iter_link_info linfo;

@geyslan @grantseltzer I have no idea how to fix this error. Could you help me? Thanks!

BTW, The selftests can be run successfully on my vagrant host.

@grantseltzer
Copy link
Contributor

I'm taking a look now! @mozillazg

libbpfgo.go Outdated Show resolved Hide resolved
libbpfgo.go Outdated Show resolved Hide resolved
libbpfgo.go Outdated Show resolved Hide resolved
@geyslan
Copy link
Member

geyslan commented Oct 4, 2022

@mozillazg thank you for your contribution 🙇🏼‍♂️. I reviewed the C part so far.

Regarding the check error, we'll take a look soon, but if you are able to do some tests, try seeing if the header that declares bpf_iter_link_info is being included correctly (uapi/linux/bpf.h): https://github.com/libbpf/libbpf/blob/1714037104da56308ddb539ae0a362a9936121ff/include/uapi/linux/bpf.h#L98-L113

Error: ./libbpfgo.go:132:27: error: variable has incomplete type 'union bpf_iter_link_info'
        union bpf_iter_link_info linfo;
                                 ^
./output/bpf/bpf.h:303:7: note: forward declaration of 'union bpf_iter_link_info'
union bpf_iter_link_info; /* defined in up-to-date linux/bpf.h */
      ^
1 error generated.

@geyslan
Copy link
Member

geyslan commented Oct 5, 2022

BTW, The selftests can be run successfully on my vagrant host.

@mozillazg for your vagrant host do you mean the one provided by libbpfgo?

-- EDIT

We've just updated the main branch bumping versions. Could you please rebase this PR on top of it? Let's keep an eye in the check results again. Thanks.

@mozillazg
Copy link
Contributor Author

We've just updated the main branch bumping versions. Could you please rebase this PR on top of it? Let's keep an eye in the check results again. Thanks.

@geyslan

After merging the main branch all checks have passed.

@grantseltzer grantseltzer added this to the v0.4.2-libbpf-1.0.0 milestone Oct 10, 2022
@grantseltzer
Copy link
Contributor

My apologies for the delay in getting to review this properly, will have good feedback for you by end of week!

libbpfgo.go Show resolved Hide resolved
@rafaeldtinoco rafaeldtinoco modified the milestones: v0.4.2-libbpf-1.0.0, next-feature-milestone Dec 12, 2022
@mozillazg mozillazg requested review from geyslan and removed request for grantseltzer February 5, 2023 13:11
Copy link
Member

@geyslan geyslan left a comment

Choose a reason for hiding this comment

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

Overall it LGTM, just some handling over version control and API is required.

selftest/iter/main.bpf.c Outdated Show resolved Hide resolved
selftest/iter/main.go Outdated Show resolved Hide resolved
libbpfgo.go Show resolved Hide resolved
libbpfgo.h Outdated Show resolved Hide resolved
libbpfgo.h Show resolved Hide resolved
@mozillazg mozillazg force-pushed the iter branch 2 times, most recently from b33d2e0 to 871b2b4 Compare February 8, 2023 13:26
@mozillazg mozillazg requested a review from geyslan February 8, 2023 13:29
@rafaeldtinoco
Copy link
Contributor

rafaeldtinoco commented Feb 20, 2023

@mozillazg I'm sorry this was left unattended. Is this still valid ? I'm converting to DRAFT just so you can rebase (and squash so many commits into meaningful ones) so we can review/merge it. Thank you!

Post Edit. I saw that @geyslan is giving attention and asked changes recently. So, either way, feel free to remove draft once this is ready.

@mozillazg mozillazg marked this pull request as ready for review March 4, 2023 05:17
@mozillazg
Copy link
Contributor Author

Ready for review.

selftest/common/vmlinux.h Outdated Show resolved Hide resolved
selftest/iter/go.mod Show resolved Hide resolved
selftest/cgroup-legacy/main.bpf.c Outdated Show resolved Hide resolved
@mozillazg mozillazg requested a review from geyslan March 7, 2023 00:51
Copy link
Member

@geyslan geyslan left a comment

Choose a reason for hiding this comment

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

LGTM

@geyslan geyslan merged commit f17cb5b into aquasecurity:main Mar 19, 2023
@geyslan
Copy link
Member

geyslan commented Mar 19, 2023

@mozillazg Thank you so much for always contributing.

mozillazg added a commit to mozillazg/libbpfgo that referenced this pull request Mar 24, 2023
@mozillazg mozillazg mentioned this pull request Mar 24, 2023
geyslan pushed a commit that referenced this pull request Mar 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants