Make netebpfext unit test resilient to fault injection. - #5547
Make netebpfext unit test resilient to fault injection.#5547Shankar Seal (shankarseal) wants to merge 2 commits into
Conversation
This comment was marked as resolved.
This comment was marked as resolved.
0be3f74 to
a8e58db
Compare
This comment was marked as resolved.
This comment was marked as resolved.
1 similar comment
This comment was marked as resolved.
This comment was marked as resolved.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: ea587460-e416-414f-9687-d52ae0a34d8f
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: ea587460-e416-414f-9687-d52ae0a34d8f
dba2b3f to
60bebfe
Compare
This comment was marked as resolved.
This comment was marked as resolved.
| <PlatformToolset>$(EbpfPlatformToolset)</PlatformToolset> | ||
| <CharacterSet>Unicode</CharacterSet> | ||
| </PropertyGroup> | ||
| <PropertyGroup Condition="'$(Configuration)'=='NativeOnlyDebug'" Label="Configuration"> |
There was a problem hiding this comment.
we dont really need nativeonlydebug flavor here. we just need to configure the solution to build "debug" flavor of this project when "nativeonlydebug" is being built (similarly for nativeonlyrelease)
| TEST_CASE("query program info", "[netebpfext]") | ||
| { | ||
| netebpf_ext_helper_t helper; | ||
| helper.require_initialized(); |
There was a problem hiding this comment.
nit: since we are already adding this to every test case, we can probably skip suspend / resume entirely and just add a check in require_initialized that either it should be initialized or fault injection should be enabled.
There was a problem hiding this comment.
🟡 Changes recommended
The new nmr_client_handle registration wrapper treats NULL handles as “registered” and may call NmrDeregisterClient(NULL) on registration failure, which can reintroduce crashes under fault injection.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR hardens the netebpfext unit tests against usersim fault injection during NMR/WFP initialization, preventing null-provider crashes and improving CI diagnostics by ensuring fault-injection logs are collected.
Changes:
- Add a fault-injection policy to the
netebpf_ext_helperand introducerequire_initialized()so tests fail fast if required NMR providers didn’t bind. - Add a new unit test that validates all expected NMR bindings while fault injection is enabled.
- Enable building
netebpfext_unitunder theNativeOnlyDebugconfiguration and add a CI step to copy fault-injection logs intoTestLogsfor upload.
File summaries
| File | Description |
|---|---|
| tests/netebpfext_unit/netebpfext_unit.vcxproj | Adds NativeOnlyDebug configuration so the unit test participates in native-only builds. |
| tests/netebpfext_unit/netebpfext_unit.cpp | Adds an NMR-binding validation test and enforces initialization checks across existing tests. |
| tests/netebpfext_unit/netebpf_ext_helper.h | Adds fault-injection policy controls and helper state to validate initialization/bindings. |
| tests/netebpfext_unit/netebpf_ext_helper.cpp | Suspends/resumes fault injection around helper init/teardown and tracks successful hook-provider bindings. |
| ebpf-for-windows.sln | Wires the netebpfext_unit project into NativeOnlyDebug solution configuration. |
| .github/workflows/reusable-test.yml | Copies fault-injection logs into TestLogs so they get uploaded with test artifacts. |
Review details
- Files reviewed: 7/7 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| bool | ||
| is_registered() const | ||
| { | ||
| return nmr_client_handle != INVALID_HANDLE_VALUE; | ||
| } |
Co-authored-by: Copilot App 223556219+Copilot@users.noreply.github.com
Description
Fixes #5460 .
Issues
FwpmFilterAdd0()in usersim.dll to fail during the netebpfext test case initialization, prior to NMR hook provider could bind, leaving the corresponding provider unavailable. Subsequent tests assumed initialization had succeeded, accessed the missing provider data, and crashed on a null pointer.fault.logfile was not getting uploadedChanges
The PR includes the following changes.
netebpfexttest helper initializes and tears down.NativeOnlyDebugconfiguration tonetebpfext_unit.vcxproj.Testing
Do any existing tests cover this change? Are new tests needed?
If new tests were added:
Documentation
Is there any documentation impact for this change?
Installation
Is there any installer impact for this change?