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

Load/Attach/Unload programs only once for connect_redirect tests. #2450

Merged
merged 34 commits into from
May 17, 2023
Merged

Load/Attach/Unload programs only once for connect_redirect tests. #2450

merged 34 commits into from
May 17, 2023

Conversation

gtrevi
Copy link
Collaborator

@gtrevi gtrevi commented May 9, 2023 •
edited
Loading

Closes #2415
Closes #2426

Description

This PR:

  • Optimizes the connect_redirect test execution by loading and attaching programs only once at startup, so all tests will reuse the same bpf_object.
  • As a cleanup, defines at an upper level a bpf_object_ptr unique pointer (in common_tests.h, as also drafted in Test Regression: connect redirect kernel tests failing  #2428), which is also used in end_to_end and stress_tests_um/km tests as well.

Testing

CI/CD

Documentation

n.a.

@gtrevi gtrevi mentioned this pull request May 9, 2023
Closed
@codecov
Copy link

codecov bot commented May 12, 2023 •
edited
Loading

Codecov Report

Merging #2450 (a0a1dad) into main (67ef157) will increase coverage by 0.01%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #2450      +/-   ##
==========================================
+ Coverage   84.00%   84.02%   +0.01%     
==========================================
  Files         154      155       +1     
  Lines       28830    28830              
==========================================
+ Hits        24220    24225       +5     
+ Misses       4610     4605       -5
Impacted Files Coverage Δ
tests/end_to_end/end_to_end.cpp 97.71% <ø> (-0.01%) ⬇️
tests/libs/common/common_tests.h 100.00% <100.00%> (ø)

... and 3 files with indirect coverage changes

@gtrevi gtrevi marked this pull request as ready for review May 12, 2023 22:14
@gtrevi gtrevi requested a review from poornagmsft as a code owner May 12, 2023 22:14
@gtrevi gtrevi requested a review from delaramamiri as a code owner May 12, 2023 22:14
@gtrevi gtrevi requested a review from dthaler May 12, 2023 23:51
@dthaler dthaler added bug Something isn't working tests labels May 15, 2023
dthaler
dthaler reviewed May 15, 2023
View reviewed changes
ebpf-for-windows.sln Outdated Show resolved Hide resolved
tests/libs/util/socket_helper.cpp Outdated Show resolved Hide resolved
tests/connect_redirect/connect_redirect_tests.cpp Outdated Show resolved Hide resolved
tests/connect_redirect/connect_redirect_tests.cpp Outdated Show resolved Hide resolved
tests/connect_redirect/connect_redirect_tests.cpp Outdated Show resolved Hide resolved
tests/connect_redirect/connect_redirect_tests.cpp Outdated Show resolved Hide resolved
tests/connect_redirect/connect_redirect_tests.cpp Outdated Show resolved Hide resolved
@gtrevi gtrevi requested a review from dthaler May 15, 2023 19:08
@gtrevi gtrevi enabled auto-merge May 16, 2023 18:17
dthaler
dthaler reviewed May 16, 2023
View reviewed changes
ebpf-for-windows.sln Outdated Show resolved Hide resolved
tests/libs/util/socket_helper.cpp Outdated Show resolved Hide resolved
@gtrevi gtrevi disabled auto-merge May 16, 2023 23:33
@dthaler dthaler enabled auto-merge May 17, 2023 17:03
@dthaler dthaler added this pull request to the merge queue May 17, 2023
Merged via the queue into microsoft:main with commit 5950a97 May 17, 2023
@gtrevi gtrevi deleted the gtrevi/optimize-connect_redirect branch May 17, 2023 18:43
This was referenced May 22, 2023
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dthaler dthaler dthaler approved these changes

@Alan-Jowett Alan-Jowett Alan-Jowett approved these changes

@poornagmsft poornagmsft Awaiting requested review from poornagmsft poornagmsft is a code owner

@saxena-anurag saxena-anurag Awaiting requested review from saxena-anurag saxena-anurag is a code owner

@shankarseal shankarseal Awaiting requested review from shankarseal shankarseal is a code owner

@dv-msft dv-msft Awaiting requested review from dv-msft dv-msft is a code owner

@delaramamiri delaramamiri Awaiting requested review from delaramamiri

Assignees
No one assigned
Labels
bug Something isn't working tests
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Load / unload only once for connect_redirect tests Test Regression: connect redirect kernel tests failing
3 participants
@gtrevi @dthaler @Alan-Jowett