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

Wrap calls from ebpf_drv.c in epoch calls #2189

Merged
merged 4 commits into from
Mar 22, 2023

Conversation

Alan-Jowett
Copy link
Member

@Alan-Jowett Alan-Jowett commented Mar 14, 2023

Description

Ensure that all calls from ebpf_drv.c are to API's that call epoch enter/exit as appropriate.
Replace dynamically sized thread entry table with fixed capacity thread entry table with block on full semantics.

Resolves: #2164
Resolves: #2165

Testing

CI/CD

Documentation

No.

@codecov
Copy link

codecov bot commented Mar 14, 2023

Codecov Report

Merging #2189 (b340eca) into main (caa0528) will decrease coverage by 1.69%.
The diff coverage is 87.06%.

@@            Coverage Diff             @@
##             main    #2189      +/-   ##
==========================================
- Coverage   83.59%   81.90%   -1.69%     
==========================================
  Files         151      143       -8     
  Lines       28217    26669    -1548     
==========================================
- Hits        23587    21843    -1744     
- Misses       4630     4826     +196     
Impacted Files Coverage Δ
libs/execution_context/ebpf_core.c 88.73% <50.00%> (-3.43%) ⬇️
libs/execution_context/ebpf_program.c 77.40% <50.00%> (-5.34%) ⬇️
libs/platform/user/ebpf_platform_user.cpp 80.77% <78.26%> (-2.65%) ⬇️
libs/platform/ebpf_epoch.c 89.41% <96.00%> (-2.11%) ⬇️
libs/execution_context/ebpf_link.c 90.64% <100.00%> (-1.07%) ⬇️
libs/platform/ebpf_state.c 83.82% <100.00%> (-7.36%) ⬇️
libs/platform/unit/platform_unit_test.cpp 97.70% <100.00%> (-1.31%) ⬇️
tests/end_to_end/end_to_end.cpp 97.64% <100.00%> (-0.06%) ⬇️

... and 59 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

dthaler
dthaler previously approved these changes Mar 14, 2023
saxena-anurag
saxena-anurag previously approved these changes Mar 14, 2023
@dthaler
Copy link
Collaborator

dthaler commented Mar 14, 2023

analysis failure:

         D:\a\ebpf-for-windows\ebpf-for-windows\libs\platform\ebpf_epoch.c(422): error C28167: The function 'ebpf_epoch_enter_no_failure' changes the IRQL and does not restore the IRQL before it exits. It should be annotated to reflect the change or the IRQL should be restored. IRQL was last set to 2 at line 424. [D:\a\ebpf-for-windows\ebpf-for-windows\libs\platform\kernel\platform_kernel.vcxproj]
         D:\a\ebpf-for-windows\ebpf-for-windows\libs\platform\ebpf_epoch.c(434): error C28167: The function 'ebpf_epoch_exit_no_failure' changes the IRQL and does not restore the IRQL before it exits. It should be annotated to reflect the change or the IRQL should be restored. IRQL was last set at line 437. [D:\a\ebpf-for-windows\ebpf-for-windows\libs\platform\kernel\platform_kernel.vcxproj]

@Alan-Jowett
Copy link
Member Author

Blocked on #2191

@dthaler dthaler added the blocked Blocked on another issue that must be done first label Mar 15, 2023
@Alan-Jowett Alan-Jowett dismissed stale reviews from saxena-anurag and dthaler via a445e46 March 16, 2023 16:34
@Alan-Jowett Alan-Jowett force-pushed the issue2165 branch 2 times, most recently from a445e46 to f7e8762 Compare March 16, 2023 18:09
@Alan-Jowett
Copy link
Member Author

No longer blocked on #2191

@Alan-Jowett Alan-Jowett removed the blocked Blocked on another issue that must be done first label Mar 16, 2023
libs/execution_context/ebpf_core.c Outdated Show resolved Hide resolved
libs/execution_context/ebpf_core.c Outdated Show resolved Hide resolved
libs/platform/ebpf_state.c Outdated Show resolved Hide resolved
saxena-anurag
saxena-anurag previously approved these changes Mar 18, 2023
dthaler
dthaler previously approved these changes Mar 18, 2023
libs/execution_context/ebpf_core.c Outdated Show resolved Hide resolved
libs/platform/ebpf_epoch.h Outdated Show resolved Hide resolved
@shankarseal
Copy link
Collaborator

Either add a more detailed description of the changes or at least update top level comments in the epoch module.

@Alan-Jowett
Copy link
Member Author

Switching to draft while I evaluate @shankarseal proposal.

@Alan-Jowett Alan-Jowett marked this pull request as draft March 21, 2023 01:03
auto-merge was automatically disabled March 21, 2023 01:03

Pull request was converted to draft

capacity table to avoid memory allocation failures.

Signed-off-by: Alan Jowett <alanjo@microsoft.com>
@Alan-Jowett Alan-Jowett marked this pull request as ready for review March 21, 2023 18:12
dv-msft
dv-msft previously approved these changes Mar 22, 2023
Signed-off-by: Alan Jowett <alanjo@microsoft.com>
libs/platform/ebpf_epoch.c Outdated Show resolved Hide resolved
libs/platform/ebpf_epoch.c Outdated Show resolved Hide resolved
Signed-off-by: Alan Jowett <alanjo@microsoft.com>
Signed-off-by: Alan Jowett <alanjo@microsoft.com>
@dthaler dthaler added this pull request to the merge queue Mar 22, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to no response for status checks Mar 22, 2023
@Alan-Jowett Alan-Jowett added this pull request to the merge queue Mar 22, 2023
@Alan-Jowett Alan-Jowett merged commit c55748d into microsoft:main Mar 22, 2023
@Alan-Jowett Alan-Jowett deleted the issue2165 branch March 22, 2023 22:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants