Try using latest xdp #4652
Try using latest xdp #4652
Conversation
|
@mtfriesen |
ami-GS
force-pushed
the
dev/daiki/header_xdp
branch
from
ee5f896
to
4202c51
Compare
ami-GS
force-pushed
the
dev/daiki/header_xdp
branch
from
abc97b8
to
0f34406
Compare
ami-GS
force-pushed
the
dev/daiki/header_xdp
branch
from
0f34406
to
eda8668
Compare
|
Editing .gitmodules doesn't work for some reason. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #4652 +/- ##
==========================================
+ Coverage 85.80% 85.85% +0.04%
==========================================
Files 56 56
Lines 17361 17361
==========================================
+ Hits 14897 14905 +8
+ Misses 2464 2456 -8 ☔ View full report in Codecov by Sentry. 🚨 Try these New Features:
|
src/platform/CMakeLists.txt
Outdated
| XDP_API_VERSION=3 | ||
| XDP_INCLUDE_WINCOMMON |
There was a problem hiding this comment.
How about we define these in the code, before the header include?
There was a problem hiding this comment.
I also doubt MsQuic needs to define XDP_INCLUDE_WINCOMMON, though you can if you want.
There was a problem hiding this comment.
Removing XDP_INCLUDE_WINCOMMON causes build error
|
hmmmmm, local run works though
|
ami-GS
force-pushed
the
dev/daiki/header_xdp
branch
2 times, most recently
from
a2131ce
to
8827b09
Compare
ami-GS
force-pushed
the
dev/daiki/header_xdp
branch
from
8827b09
to
ad8092c
Compare
.gitmodules
Outdated
| @@ -15,4 +15,4 @@ | |||
| [submodule "submodules/xdp-for-windows"] | |||
| path = submodules/xdp-for-windows | |||
| url = https://github.com/microsoft/xdp-for-windows.git | |||
| branch = release/1.0 | |||
| branch = dev/daiki/xdp_api_v3 | |||
There was a problem hiding this comment.
needs to be a commit in main
There was a problem hiding this comment.
I think main branch cannot be built with msquic
microsoft/xdp-for-windows#741
There was a problem hiding this comment.
OK, please merge the XDP fix into XDP's main before rebasing this PR onto a main commit.
submodules/xdp-for-windows
Outdated
There was a problem hiding this comment.
needs to be a commit in main
| branch = release/1.0 | ||
| branch = 300d18c43f989ca22ad1be2f2836e2bbec6a2c4f |
There was a problem hiding this comment.
@mtfriesen what are your thoughts on a timeline for a new release branch (not necessarily an official release) from XDP?
There was a problem hiding this comment.
I'm not entirely sure I understand the distinction - XDP should have a release branch 1:1 with each major/minor version. Are you asking if we can do an unsupported/unsigned release for 1.x, and then either make it "official" later, or let 1.y become officially supported afterwards?
There was a problem hiding this comment.
I'm not asking for a one-off. I'm asking: when should we fork the next XDP release branch?
There was a problem hiding this comment.
Whenever needed by a dependent project. XDP keeps main in a releasable state at all times.
src/platform/datapath_raw_xdp_win.c
Outdated
| @@ -165,32 +165,37 @@ CxPlatGetRssQueueProcessors( | |||
| _Out_writes_to_(*Count, *Count) uint32_t* Queues | |||
| ) | |||
| { | |||
| UNREFERENCED_PARAMETER(Xdp); | |||
There was a problem hiding this comment.
Unless we expect to need this in the future, let's remove it from being passed in.
src/platform/datapath_raw_xdp_win.c
Outdated
| @@ -1065,7 +1056,11 @@ CxPlatDpRawInitialize( | |||
| Status = | |||
| CxPlatDpRawInterfaceInitialize( | |||
| Xdp, Interface, ClientRecvContextLength); | |||
| if (QUIC_FAILED(Status)) { | |||
| if (Status == QUIC_STATUS_NOT_FOUND && CxPlatListIsEmpty(&Xdp->Interfaces)) { | |||
There was a problem hiding this comment.
This isn't the same as HRESULT_FROM_WIN32(ERROR_FILE_NOT_FOUND). We could define a QUIC_STATUS_FILE_NOT_FOUND and use that.
Co-authored-by: Nick Banks <[email protected]>
Co-authored-by: Nick Banks <[email protected]>
|
|
|
No, not necessarily an XDP issue. |
|
Found interesting behavior. So I believe this is XDP related initialization/uninitialization issue? EDIT1: Oops, just |
|
main thread callstack is consistently same upto CxPlatTlsResetSchannel. |
Description
Try using latest header only XDP API
This should not be merged until
xdp-for-windowsrelease official version.Checking CI with new API
Testing
CI
Documentation
N/A