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

_ebpf_link_instance_invoke_batch_begin/_ebpf_link_instance_invoke_batch_end use expensive EX_RUNDOWN_REF #3483

Closed
Alan-Jowett opened this issue Apr 23, 2024 · 2 comments · Fixed by #3534
Assignees
Labels
optimization Affects perf but not correctness or applicability P1 triaged Discussed in a triage meeting
Milestone

Comments

@Alan-Jowett
Copy link
Member

_ebpf_link_instance_invoke_batch_begin and _ebpf_link_instance_invoke_batch_end use the expensive EX_RUNDOWN_REF construct.

Given that this block of code is already protected by epoch_enter/epoch_exit, the code should rely on epoch protection to detect when the last program using a program information has exited.

Proposed changes:

  1. In _ebpf_program_type_specific_program_information_detach_provider set program->extension_program_data to NULL, then call ebpf_epoch_synchronize.
  2. In ebpf_program_invoke check if extension_program_data is NULL and fail the invoke.
  3. Remove rundown protection from batch_begin/batch_end.

This will achieve two things:

  1. Speed up batch begin/end.
  2. Allow batch across different programs.
@dthaler dthaler added the optimization Affects perf but not correctness or applicability label Apr 29, 2024
@dahavey dahavey added the triaged Discussed in a triage meeting label Apr 29, 2024
@dahavey dahavey added this to the 2405 milestone Apr 29, 2024
@shankarseal shankarseal added the P1 label May 8, 2024
@Alan-Jowett
Copy link
Member Author

Upon review, this may actually be entirely redundant.

  1. When the ref-count on the program reaches zero, it calls _ebpf_program_zero_ref_count
  2. _ebpf_program_zero_ref_count calls _ebpf_program_detach_links
  3. _ebpf_program_detach_links calls NmrDeregisterClient on the hook provider.

As long as the hook provider is well formed and stops invoking the BPF program before it finishes detaching, the program can't be unloaded while a call is being dispatched.

@shankarseal
Copy link
Collaborator

@Alan-Jowett
Re-opening the issue for phase 2 of the changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
optimization Affects perf but not correctness or applicability P1 triaged Discussed in a triage meeting
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants