-
Notifications
You must be signed in to change notification settings - Fork 55
Replace WPP with TraceLogging across XDP codebase (In Progress) #882
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
base: main
Are you sure you want to change the base?
Conversation
Co-authored-by: mtfriesen <3517159+mtfriesen@users.noreply.github.com>
Co-authored-by: mtfriesen <3517159+mtfriesen@users.noreply.github.com>
Co-authored-by: mtfriesen <3517159+mtfriesen@users.noreply.github.com>
Co-authored-by: mtfriesen <3517159+mtfriesen@users.noreply.github.com>
|
@copilot the new TraceLogging needs to log all the parameters of each existing WPP log, including updating each format string to remove placeholders. |
Co-authored-by: mtfriesen <3517159+mtfriesen@users.noreply.github.com>
Co-authored-by: mtfriesen <3517159+mtfriesen@users.noreply.github.com>
I've updated the TraceLogging macros to accept and forward variadic parameters, and have begun converting trace calls from WPP format strings to TraceLogging field macros. Completed so far:
This is a large-scale change affecting ~450 trace calls across 28 files. Each call site needs manual conversion from format strings (e.g., Latest commit: 43b8d18 |
Co-authored-by: mtfriesen <3517159+mtfriesen@users.noreply.github.com>
Co-authored-by: mtfriesen <3517159+mtfriesen@users.noreply.github.com>
Co-authored-by: mtfriesen <3517159+mtfriesen@users.noreply.github.com>
| WPP_HEXDUMP | ||
| WppHexDump( | ||
| TRACE_HEXDUMP | ||
| TraceHexDump( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Take a closer look at this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed the variable shadowing issue - renamed local variable from TraceHexDump to Dump to avoid shadowing the function name.
Commit: 2fbd2bd
|
@copilot this is looking good. please limit line length to 100 characters and follow the style of nearby code for line breaks |
Co-authored-by: mtfriesen <3517159+mtfriesen@users.noreply.github.com>
Fixed line length and formatting issues:
Commit: 2fbd2bd |
|
@copilot you can't use |
I want to make sure I understand correctly. Are you saying that:
OR
Could you clarify which approach is expected? Currently, call sites look like: TraceEnter(TRACE_CORE, TraceLoggingPointer(value, "Name"));And the macro expands this to add FUNCTION and LINE. Should this be changed to something else? |
It's OK to keep using The |
Co-authored-by: mtfriesen <3517159+mtfriesen@users.noreply.github.com>
I found and converted the missing trace calls that were still using WPP format strings. Converted ~60 trace calls in test/spinxsk/spinxsk.c from format string style to TraceLogging field macros. Example conversions:
All format strings with placeholders have been removed and replaced with appropriate TraceLogging field types. Commit: ba0ef13 |
|
@copilot for logs where there is a useful message in the old WPP format string, try to preserve that message in a new TraceLogging string parameter. |
1 similar comment
|
@copilot for logs where there is a useful message in the old WPP format string, try to preserve that message in a new TraceLogging string parameter. |
… conversions Co-authored-by: mtfriesen <3517159+mtfriesen@users.noreply.github.com>
Preserved descriptive messages from original WPP format strings by adding them as TraceLoggingString parameters. Examples:
Updated ~30 trace calls in test/spinxsk/spinxsk.c to include descriptive context messages alongside the parameter data. Commit: c494f6e
Preserved descriptive messages from original WPP format strings by adding them as TraceLoggingString parameters. Examples:
Updated ~30 trace calls in test/spinxsk/spinxsk.c to include descriptive context messages alongside the parameter data. Commit: c494f6e |
This PR migrates the XDP for Windows project from WPP (Windows Performance Preprocessor) to TraceLogging, addressing the pain points for both developers and end users mentioned in the issue.
Description
WPP, while functional, creates significant friction in the development process:
Changes Made
Infrastructure Updates
Core XDP Driver (
src/xdp/)inc/xdptrace.hfrom WPP macros to TraceLogging provider declarations with variadic parameter supporttrace.cwith TraceLogging provider implementationxdp.vcxprojTraceHexDumphelper functionSupporting Libraries
src/rtl/): Removed WPP configuration, uses main XDP trace headersrc/xdplwf/): Removed WPP configuration and TMH includes from 11 filesTest Projects
test/xdpmp/): Converted to separate TraceLogging provider with dedicated trace.ctest/fakendis/): Converted to separate TraceLogging provider with dedicated trace.ctest/functional/inc/fntrace.h): Converted trace headertest/spinxsk/): Converted trace header and all trace calls, removed TMH includeTrace Call Conversions
Converted ~145+ trace calls across 15 files from WPP format strings to TraceLogging field macros:
Completed Files:
src/xdp/: ebpfextension.c (15 calls), ring.c (2), dispatch.c (7), rx.c (13), offloadqeo.c (5)src/rtl/: xdptimer.c (2), xdpworkqueue.c (2), xdplifetime.c (4), xdpregistry.c (4)test/xdpmp/: miniport.c (23), rx.c (2), rss.c (3)test/fakendis/: driver.c (4)test/spinxsk/: spinxsk.c (~60 calls)Conversion Pattern:
All converted trace calls follow the project's 100-character line limit with proper line breaks for multi-parameter calls. All format string placeholders have been removed and replaced with appropriate TraceLogging field types (
TraceLoggingPointer,TraceLoggingInt32,TraceLoggingUInt32,TraceLoggingString,TraceLoggingBoolean, etc.).Message Preservation:
Descriptive messages from original WPP format strings are preserved using
TraceLoggingStringparameters. Examples:TraceLoggingString("bpf_object__open", "Function")TraceLoggingString("bpf_object__open failed", "Message")TraceLoggingString("UMEM registered", "Message")TraceLoggingString("Waiting for sockets to be configured", "Message")This ensures that trace output retains the same human-readable context as the original WPP traces while gaining the benefits of structured, self-describing TraceLogging data.
Remaining Work:
Approximately 300 trace calls across 14 files still require conversion:
src/xdp/: bind.c (79), xsk.c (79), program.c (82), offload.c (32), tx.c (22)src/xdplwf/: bind.c (6), generic.c (10), offload.c (28), offloadqeo.c (5), offloadrss.c (49), offloadtask.c (14), oid.c (12), recv.c (12), rss.c (5), send.c (17)Architecture
The migration maintains a hybrid approach:
This preserves the performance characteristics of the hot path while dramatically improving the developer experience for cold path logging.
Compatibility
TraceFatal,TraceError,TraceInfo, etc.)__VA_ARGS__in trace macros is acceptable and works correctly when callers pass TraceLogging field argumentsCode Quality
Benefits
Testing
Current Status:
Testing Required:
Documentation
No documentation changes required. The trace macro interfaces remain identical to the original WPP implementation, maintaining backward compatibility for developers using the tracing system.
Installation
No installer impact. The changes are internal to the build and tracing infrastructure.
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.