microsoft / microsoft/ebpf-for-windows
netebpfext: consolidate duplicated code across hooks
- Dominant language
- C
- Stars
- 3.6k
- Forks
- 311
- Avg merge
- 6d 10h
- Merged PRs (30d)
- 21
Description
### Describe the feature you'd like supported
**Origin**: Surfaced in https://github.com/microsoft/ebpf-for-windows/pull/5272#discussion_r3301913869 (shankarseal). Deferred to a separate issue so PR #5272 stays scoped to the bind hook implementation.
**Problem**
netebpfext has accumulated a lot of near-duplicate code across the various WFP-backed hook implementations. The same scaffolding is repeated for each hook with only small per-hook variations:
1. Classify functions in net_ebpf_ext_sock_addr.c (single file, ~127 KB):
- net_ebpf_extension_sock_addr_authorize_connection_classify
- net_ebpf_extension_sock_addr_authorize_recv_accept_classify
- net_ebpf_extension_sock_addr_redirect_connection_classify
- net_ebpf_extension_sock_addr_bind_classify
Each does the same type of work: validate the WFP filter context, locate attached eBPF clients, marshal a bpf_sock_addr_t from WFP metadata, dispatch to attached programs, map verdicts back to WFP actions, and emit traces/stats. Differences are mostly the WFP layer fields read and the verdict→action mapping.
2. Per-hook field-table + copy-function pairs in the same file:
- wfp_connection_fields[] + _copy_wfp_connection_fields
- wfp_bind_fields[] + _copy_wfp_bind_fields
- wfp_listen_fields[] + _copy_wfp_listen_fields (added in #5268)
Same pattern repeats across files: wfp_flow_established_fields[] in net_ebpf_ext_sock_ops.c, the legacy bind fields in net_ebpf_ext_bind.c. Each is a hand-written list of FWPS_FIELD__* indices plus a function that pulls those indices out of FWPS_INCOMING_VALUES and writes them into the per-hook context struct.
3. Per-hook NMR provider/filter-parameters arrays with identical structure but one entry per attach type, several of which only differ in the WFP layer GUID and callout GUID.
There is also duplication in the test code that could be factored out:
- `(user_ip6[0] ^ user_ip6[3]) ^ (user_port << 16)` IPv6 connection-ID hash appears in four places:
- `tests/sample/cgroup_sock_addr_helpers.c:106` - `test_sock_addr_helpers_v6` (BPF, connect_authorization6)
- `tests/sample/cgroup_sock_addr_helpers.c:251` - `test_bind_helpers_v6` (BPF, bind6)
- `tests/socket/socket_tests.cpp:1055` - `helper_functions_validation_test` (host)
- `tests/socket/socket_tests.cpp:941` - `bind_helper_functions_validation_test` (host)
- This would be better to have in a single shared function to avoid the logic duplication, document it, and avoid needing to keep them in sync.
### Proposed solution
Introduce a generic, table-driven classify path:
1. One classify function which takes by a hook descriptor struct that captures the per-hook bits.
2. One field-extract helper that takes a wfp_ale_layer_fields_t and an output context, replacing the per-hook _copy_wfp_*_fields functions.
3. Collapse the per-hook filter-parameters arrays by deriving them from a hook descriptor table (attach type → layer GUID → callout GUID → classify entry point).
Also add a v4/v6 connection hash helper function to share across all tests.
After the refactor, adding a new attach type should be ~one entry in a descriptor table plus any genuinely novel per-hook logic, rather than copying ~60 lines of classify boilerplate and a 30-line field-copy helper.
### Additional context
_No response_
Contributor guide
Assessment
This issue has not been assessed yet.