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.

Sign up for GitHub

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

Jump to bottom

aya: Add bpf_map_delete_elem for XskMap #848

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

aya: Add bpf_map_delete_elem for XskMap #848

wants to merge 2 commits into from

Conversation

arctic-alpaca
Copy link
Contributor

@arctic-alpaca arctic-alpaca commented Dec 6, 2023 •
edited by tamird
Loading

Add clear_index() to be able to use bpf_map_delete_elem for XskMap. Test is added to the existing af_xdp integration test.

This change is Reviewable

@netlify Netlify
Copy link

netlify bot commented Dec 6, 2023 •
edited
Loading

✅ Deploy Preview for aya-rs-docs ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit fb0af68
🔍 Latest deploy log https://app.netlify.com/sites/aya-rs-docs/deploys/6734f948d8504500089f8abd
😎 Deploy Preview https://deploy-preview-848--aya-rs-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@mergify mergify bot added aya This is about aya (userspace) test A PR that improves test cases or CI labels Dec 6, 2023
@mergify Mergify
Copy link

mergify bot commented Dec 7, 2023

Hey @alessandrod, this pull request changes the Aya Public API and requires your review.

@mergify mergify bot requested a review from alessandrod December 7, 2023 14:46
@mergify mergify bot added the api/needs-review Makes an API change that needs review label Dec 7, 2023
dave-tucker
dave-tucker requested changes Dec 12, 2023
View reviewed changes
Copy link
Member

@dave-tucker dave-tucker left a comment •
edited
Loading

Choose a reason for hiding this comment

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

One small nit and one API related query for @alessandrod:

For eBPF maps that are array-like we expose get and set.
For eBPF maps that are hashmap-like we expose get, insert and remove.

Since BPF_MAP_TYPE_ARRAY doesn't support bpf_map_delete_elem we never implemented it, so there is no precedent set for naming.

Would you go with, delete, remove or clear?

aya/src/maps/xdp/xsk_map.rs Outdated Show resolved Hide resolved
aya/src/maps/xdp/xsk_map.rs
///
/// Returns [`MapError::OutOfBounds`] if `index` is out of bounds, [`MapError::SyscallError`]
/// if `bpf_map_update_elem` fails.
pub fn clear_index(&mut self, index: u32) -> Result<(), MapError> {
Copy link
Member

Choose a reason for hiding this comment

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

This should likely be either remove or delete. Hold please while we hear from @alessandrod.

@mergify Mergify
Copy link

mergify bot commented Feb 6, 2024

Hey @alessandrod, this pull request changes the Aya Public API and requires your review.

@arctic-alpaca arctic-alpaca force-pushed the xdp_delete branch 3 times, most recently from e09a4bb to 05d7bff Compare August 10, 2024 19:47
@arctic-alpaca
Copy link
Contributor Author

arctic-alpaca commented Dec 5, 2024 •
edited
Loading

@alessandrod Could you take a look at this please? Without the function, it's not possible (as far as I know) to clean up a single AF_XDP socket without detaching/unloading the whole program.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dave-tucker dave-tucker dave-tucker requested changes

@alessandrod alessandrod Awaiting requested review from alessandrod

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
api/needs-review Makes an API change that needs review aya This is about aya (userspace) test A PR that improves test cases or CI
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@arctic-alpaca @dave-tucker