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

aya: Remove iter_key from LPM Trie API #526

Merged
merged 1 commit into from
Jul 1, 2023
Merged

Conversation

dave-tucker
Copy link
Member

@dave-tucker dave-tucker commented Feb 14, 2023

Based on the discussion in Discord1 we've decided to drop the
iter_keys() API for LPM Trie. According to the kernel self-tests and
experimentation done in Aya, providing a key into bpf_map_get_next_id
will either:

  • If key is an EXACT match, proceed iterating through all keys in the
    trie from this point
  • If key is NOT an EXACT match, proceed iterating through all keys in
    the trie starting at the leftmost entry.

An API in Aya could be crafted that gets the LPM match + less specific
matches for a prefix using these semantics BUT it would only apply to
userspace. Therefore we've opted out of fixing this.

@netlify
Copy link

netlify bot commented Feb 14, 2023

Deploy Preview for aya-rs-docs ready!

Name Link
🔨 Latest commit 00c480d
🔍 Latest deploy log https://app.netlify.com/sites/aya-rs-docs/deploys/649f2b44eb8f9a0008c9f01c
😎 Deploy Preview https://deploy-preview-526--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.

Copy link
Member

@vadorovsky vadorovsky left a comment

Choose a reason for hiding this comment

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

In general looks good, just one question.

aya/src/maps/lpm_trie.rs Show resolved Hide resolved
@dave-tucker dave-tucker added the api/needs-review Makes an API change that needs review label Feb 23, 2023
@dave-tucker
Copy link
Member Author

dave-tucker commented Feb 23, 2023

@alessandrod are you ok with the API https://github.com/aya-rs/aya/pull/526/files#diff-f6d6661573434c62eecc651f4986dfa4e5235ad59ee99bcb4478a370a7fb8621R124 here that adds create from userspace for this map type?

The use cases are:

  1. Tests 🎉
  2. You could create a map, seed with some data, pin the map to /sys/fs/bpf/my_app/FOO and then load your program with BpfLoader::new().map_pin_path("/sys/fs/bpf/my_app").load() which would re-use the pinned map vs. creating a new one.

@dave-tucker dave-tucker added fix A PR that is a small change or fixes a bug aya-bpf This is about aya-bpf (kernel) aya This is about aya (userspace) feature A PR that implements a new feature or enhancement and removed fix A PR that is a small change or fixes a bug aya-bpf This is about aya-bpf (kernel) labels Feb 23, 2023
@dave-tucker dave-tucker changed the title aya: Streamline the LPM Trie API aya: Remove iter_key from LPM Trie API. Add create() API. Feb 23, 2023
@dave-tucker dave-tucker added this to the 0.12 milestone Feb 23, 2023
@vadorovsky
Copy link
Member

@dave-tucker Ping, would be nice to have Alessandro's comments addressed :)

Based on the discussion in Discord we've decided to drop the
iter_key() API for LPM Trie. According to the kernel self-tests and
experimentation done in Aya, providing a key into bpf_map_get_next_id
will either:

- If key is an EXACT match, proceed iterating through all keys in the
trie from this point
- If key is NOT an EXACT match, proceed iterating through all keys in
the trie starting at the leftmost entry.

An API in Aya could be crafted that gets the LPM match + less specific
matches for a prefix using these semantics BUT it would only apply to
userspace. Therefore we've opted out of fixing this.

Signed-off-by: Dave Tucker <dave@dtucker.co.uk>
@dave-tucker dave-tucker changed the title aya: Remove iter_key from LPM Trie API. Add create() API. aya: Remove iter_key from LPM Trie API Jun 30, 2023
@dave-tucker dave-tucker removed the api/needs-review Makes an API change that needs review label Jun 30, 2023
@alessandrod alessandrod merged commit 76d35d1 into aya-rs:main Jul 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
aya This is about aya (userspace) feature A PR that implements a new feature or enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants