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

Allow prefix match on program / object pin paths #4161

Open
lmb opened this issue Jan 27, 2025 · 1 comment
Open

Allow prefix match on program / object pin paths #4161

lmb opened this issue Jan 27, 2025 · 1 comment
Assignees
Labels
enhancement New feature or request optimization Affects perf but not correctness or applicability P3 triaged Discussed in a triage meeting
Milestone

Comments

@lmb
Copy link
Collaborator

lmb commented Jan 27, 2025

Describe the feature you'd like supported

ebpf_get_next_pinned_program_path currently returns the next path after start_path:

_Must_inspect_result_ ebpf_result_t
ebpf_get_next_pinned_program_path(
_In_z_ const char* start_path, _Out_writes_z_(EBPF_MAX_PIN_PATH_LENGTH) char* next_path) EBPF_NO_EXCEPT;

One interpretation of the documentation is that "next" refers to the lexicographically following / greater string:

start_path: my/pin/path/ -> next_path:my/pin/path/map

In fact, "next" refers to the order of entries in the internal pinning map, which is unspecified / implementation detail. Passing an arbitrary path as "start_path" is therefore not well specified.

This is a problem when trying to clean up all pins that share a prefix, for example to clean up after a unit test (this is what I'm trying to achieve). The only way to achieve this right now is to iterate ALL pins, which can get slow.

Another problem is that defining "next" like this means that iteration is not well defined in the presence of concurrent deletions:

char pinpath[EBPF_MAX_PIN_PATH_LENGTH] = "";
    while (ebpf_get_next_pinned_program_path(pinpath, pinpath) == EBPF_SUCCESS) {
        ebpf_api_unpin_object(pinpath, strlen(pinpath));
    }

Here pinpath is deleted, and hence the kernel side code can't find the correct entry to resume from.

Proposed solution

Change the function so that "next" has the meaning of "following in lexicographical order". This might require making the pinning table an ordered datatype.

Additional context

No response

@lmb lmb added the enhancement New feature or request label Jan 27, 2025
@shankarseal shankarseal added P3 optimization Affects perf but not correctness or applicability triaged Discussed in a triage meeting labels Jan 27, 2025
@shankarseal shankarseal added this to the 2503 milestone Jan 27, 2025
@lmb
Copy link
Collaborator Author

lmb commented Jan 27, 2025

  • @Alan-Jowett raised the question whether /my/pin/path should return items that follow lexicographically but do not share the prefix. I think it should be prefix match + lexicographical next.
  • @Alan-Jowett also asked whether we should treat directory separator specially. I think this might make sense, for example we could normalise both \ and / to a single separator. This would ease interoperability between applications using different directory separators.

lmb added a commit to isovalent/ebpf-for-windows that referenced this issue Jan 28, 2025
It's currently not possible to find pinned maps and links. Add a
function ebpf_get_next_pinned_object_path which allows doing just
that.

The paths are returned in lexicographical order, which allows user
space to performa a prefix match on the paths. Behind the scenes
this is implemented using a linear scan of the hash table, which
has quadratic running time. The same is already the case for finding
the next object ID, so going with the more desirable semantics
makes sense.

The existing ebpf_get_next_pinned_program_path() is deprecated because
it is less flexible than the new function.

Updates microsoft#4161
lmb added a commit to isovalent/ebpf-for-windows that referenced this issue Jan 28, 2025
It's currently not possible to find pinned maps and links. Add a
function ebpf_get_next_pinned_object_path which allows doing just
that.

The paths are returned in lexicographical order, which allows user
space to performa a prefix match on the paths. Behind the scenes
this is implemented using a linear scan of the hash table, which
has quadratic running time. The same is already the case for finding
the next object ID, so going with the more desirable semantics
makes sense.

The existing ebpf_get_next_pinned_program_path() is deprecated because
it is less flexible than the new function.

Updates microsoft#4161
lmb added a commit to isovalent/ebpf-for-windows that referenced this issue Jan 28, 2025
It's currently not possible to find pinned maps and links. Add a
function ebpf_get_next_pinned_object_path which allows doing just
that.

The paths are returned in lexicographical order, which allows user
space to performa a prefix match on the paths. Behind the scenes
this is implemented using a linear scan of the hash table, which
has quadratic running time. The same is already the case for finding
the next object ID, so going with the more desirable semantics
makes sense.

The existing ebpf_get_next_pinned_program_path() is deprecated because
it is less flexible than the new function.

Updates microsoft#4161
lmb added a commit to isovalent/ebpf-for-windows that referenced this issue Jan 28, 2025
It's currently not possible to find pinned maps and links. Add a
function ebpf_get_next_pinned_object_path which allows doing just
that.

The paths are returned in lexicographical order, which allows user
space to performa a prefix match on the paths. Behind the scenes
this is implemented using a linear scan of the hash table, which
has quadratic running time. The same is already the case for finding
the next object ID, so going with the more desirable semantics
makes sense.

The existing ebpf_get_next_pinned_program_path() is deprecated because
it is less flexible than the new function.

Updates microsoft#4161
@shankarseal shankarseal modified the milestones: 2503, Backlog, 2501, 2502, 2504, 2505 Jan 29, 2025
lmb added a commit to isovalent/ebpf-for-windows that referenced this issue Feb 3, 2025
It's currently not possible to find pinned maps and links. Add a
function ebpf_get_next_pinned_object_path which allows doing just
that.

The paths are returned in lexicographical order, which allows user
space to performa a prefix match on the paths. Behind the scenes
this is implemented using a linear scan of the hash table, which
has quadratic running time. The same is already the case for finding
the next object ID, so going with the more desirable semantics
makes sense.

The existing ebpf_get_next_pinned_program_path() is deprecated because
it is less flexible than the new function.

Updates microsoft#4161
lmb added a commit to isovalent/ebpf-for-windows that referenced this issue Feb 4, 2025
It's currently not possible to find pinned maps and links. Add a
function ebpf_get_next_pinned_object_path which allows doing just
that.

The paths are returned in lexicographical order, which allows user
space to performa a prefix match on the paths. Behind the scenes
this is implemented using a linear scan of the hash table, which
has quadratic running time. The same is already the case for finding
the next object ID, so going with the more desirable semantics
makes sense.

The existing ebpf_get_next_pinned_program_path() is deprecated because
it is less flexible than the new function.

Updates microsoft#4161
lmb added a commit to isovalent/ebpf-for-windows that referenced this issue Feb 4, 2025
It's currently not possible to find pinned maps and links. Add a
function ebpf_get_next_pinned_object_path which allows doing just
that.

The paths are returned in lexicographical order, which allows user
space to performa a prefix match on the paths. Behind the scenes
this is implemented using a linear scan of the hash table, which
has quadratic running time. The same is already the case for finding
the next object ID, so going with the more desirable semantics
makes sense.

The existing ebpf_get_next_pinned_program_path() is deprecated because
it is less flexible than the new function.

Updates microsoft#4161
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request optimization Affects perf but not correctness or applicability P3 triaged Discussed in a triage meeting
Projects
None yet
Development

No branches or pull requests

3 participants