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.

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

Add support for both Qtip and UDP traffic for Server Listeners. #4803

Draft
wants to merge 28 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
63a4f6f
first iteration
ProjectsByJackHe Feb 7, 2025
4312eea
test code sanity check
ProjectsByJackHe Feb 7, 2025
f865630
full revert
ProjectsByJackHe Feb 7, 2025
19354ee
test this out
ProjectsByJackHe Feb 10, 2025
a2511c3
update control.cpp as well
ProjectsByJackHe Feb 10, 2025
2db51dd
update all instances of connAndPing
ProjectsByJackHe Feb 10, 2025
0a1ec39
sanity check
ProjectsByJackHe Feb 10, 2025
07aedfd
do not define multiple of these registrations
ProjectsByJackHe Feb 10, 2025
ae85b9f
turn QTIP off right after Listener.Start
ProjectsByJackHe Feb 11, 2025
cf38283
don't pass nullptr as size
ProjectsByJackHe Feb 11, 2025
1c9fa0b
revert back to old code - useqtip test SHOULD fail
ProjectsByJackHe Feb 11, 2025
93ff078
move the server config instantiation after we set qtip to be off
ProjectsByJackHe Feb 11, 2025
ba3c146
add sanity checks
ProjectsByJackHe Feb 11, 2025
c7b920a
fix sanity check
ProjectsByJackHe Feb 11, 2025
ebd9fd9
revise sanity check
ProjectsByJackHe Feb 11, 2025
5b2de37
better sanity check
ProjectsByJackHe Feb 12, 2025
b590242
set socket->usetcp to always false and see what happens when we enabl…
ProjectsByJackHe Feb 13, 2025
5486743
resolve warning for winkernel
ProjectsByJackHe Feb 13, 2025
1495588
add a couple of setparams
ProjectsByJackHe Feb 13, 2025
cae946f
add a few getparams
ProjectsByJackHe Feb 13, 2025
6629817
remember to generate cs
ProjectsByJackHe Feb 14, 2025
1adaad5
resolve merge conflicts
ProjectsByJackHe Feb 14, 2025
455b833
adjust hex
ProjectsByJackHe Feb 14, 2025
1c0c411
proper parameter definitions
ProjectsByJackHe Feb 14, 2025
0f5a6c6
delete include
ProjectsByJackHe Feb 14, 2025
a68f0f5
don't hardcode 0
ProjectsByJackHe Feb 14, 2025
64a9dff
add back removed include
ProjectsByJackHe Feb 14, 2025
38ba016
try and see if this works
ProjectsByJackHe Feb 15, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 29 additions & 0 deletions src/core/connection.c
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,8 @@
#if DEBUG
Connection->RefTypeCount[QUIC_CONN_REF_HANDLE_OWNER] = 1;
#endif
Connection->State.UseQTIP = 0;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unnecessary as we always zero-init the entire connection.

Connection->State.AppDidSetQTIP = 0;
Connection->PartitionID = PartitionId;
Connection->State.Allocated = TRUE;
Connection->State.ShareBinding = IsServer;
Expand Down Expand Up @@ -6753,6 +6755,15 @@
Status = QUIC_STATUS_SUCCESS;
break;
#endif
case QUIC_PARAM_CONN_QTIP:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These case statements should be kept ordered by value.

if (BufferLength != sizeof(BOOLEAN) || Buffer == NULL) {
Status = QUIC_STATUS_INVALID_PARAMETER;
break;

Check warning on line 6761 in src/core/connection.c

View check run for this annotation

Codecov / codecov/patch

src/core/connection.c#L6759-L6761

Added lines #L6759 - L6761 were not covered by tests
}
Connection->State.UseQTIP = *(BOOLEAN*)Buffer;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You shouldn't let them change after the connection has been started.

Connection->State.AppDidSetQTIP = TRUE;
Status = QUIC_STATUS_SUCCESS;
break;

Check warning on line 6766 in src/core/connection.c

View check run for this annotation

Codecov / codecov/patch

src/core/connection.c#L6763-L6766

Added lines #L6763 - L6766 were not covered by tests

default:
Status = QUIC_STATUS_INVALID_PARAMETER;
Expand Down Expand Up @@ -7284,6 +7295,24 @@
Status = QUIC_STATUS_SUCCESS;
break;

case QUIC_PARAM_CONN_QTIP:
if (*BufferLength < sizeof(BOOLEAN)) {
*BufferLength = sizeof(BOOLEAN);
Status = QUIC_STATUS_BUFFER_TOO_SMALL;
break;

Check warning on line 7302 in src/core/connection.c

View check run for this annotation

Codecov / codecov/patch

src/core/connection.c#L7299-L7302

Added lines #L7299 - L7302 were not covered by tests
}

if (Buffer == NULL) {
Status = QUIC_STATUS_INVALID_PARAMETER;
break;

Check warning on line 7307 in src/core/connection.c

View check run for this annotation

Codecov / codecov/patch

src/core/connection.c#L7305-L7307

Added lines #L7305 - L7307 were not covered by tests
}

*BufferLength = sizeof(BOOLEAN);
*(BOOLEAN*)Buffer = Connection->State.UseQTIP;

Check warning on line 7311 in src/core/connection.c

View check run for this annotation

Codecov / codecov/patch

src/core/connection.c#L7310-L7311

Added lines #L7310 - L7311 were not covered by tests

Status = QUIC_STATUS_SUCCESS;
break;

Check warning on line 7314 in src/core/connection.c

View check run for this annotation

Codecov / codecov/patch

src/core/connection.c#L7313-L7314

Added lines #L7313 - L7314 were not covered by tests

default:
Status = QUIC_STATUS_INVALID_PARAMETER;
break;
Expand Down
10 changes: 10 additions & 0 deletions src/core/connection.h
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,16 @@ typedef union QUIC_CONNECTION_STATE {
//
BOOLEAN DisableVneTp : 1;
#endif

//
// Whether to use QTIP on sends for this connection.
//
BOOLEAN UseQTIP : 1;

//
// Whether or not the app explicitly set the UseQTIP flag.
//
BOOLEAN AppDidSetQTIP : 1;
};
} QUIC_CONNECTION_STATE;

Expand Down
30 changes: 30 additions & 0 deletions src/core/listener.c
Original file line number Diff line number Diff line change
Expand Up @@ -769,6 +769,17 @@
return QUIC_STATUS_SUCCESS;
}

if (Param == QUIC_PARAM_LISTENER_QTIP) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably convert this function back to using a switch statement now that there is more than one.

if (BufferLength > sizeof(uint8_t)) {
return QUIC_STATUS_INVALID_PARAMETER;

Check warning on line 774 in src/core/listener.c

View check run for this annotation

Codecov / codecov/patch

src/core/listener.c#L773-L774

Added lines #L773 - L774 were not covered by tests
}
if (Buffer == NULL) {
return QUIC_STATUS_INVALID_PARAMETER;

Check warning on line 777 in src/core/listener.c

View check run for this annotation

Codecov / codecov/patch

src/core/listener.c#L776-L777

Added lines #L776 - L777 were not covered by tests
}
Listener->UseQTIP = *(uint8_t*)Buffer;
return QUIC_STATUS_SUCCESS;

Check warning on line 780 in src/core/listener.c

View check run for this annotation

Codecov / codecov/patch

src/core/listener.c#L779-L780

Added lines #L779 - L780 were not covered by tests
}

return QUIC_STATUS_INVALID_PARAMETER;
}

Expand Down Expand Up @@ -855,6 +866,25 @@
Status = QUIC_STATUS_SUCCESS;
break;

case QUIC_PARAM_LISTENER_QTIP:

if (*BufferLength < sizeof(uint8_t)) {
*BufferLength = sizeof(uint8_t);
Status = QUIC_STATUS_BUFFER_TOO_SMALL;
break;

Check warning on line 874 in src/core/listener.c

View check run for this annotation

Codecov / codecov/patch

src/core/listener.c#L871-L874

Added lines #L871 - L874 were not covered by tests
}

if (Buffer == NULL) {
Status = QUIC_STATUS_INVALID_PARAMETER;
break;

Check warning on line 879 in src/core/listener.c

View check run for this annotation

Codecov / codecov/patch

src/core/listener.c#L877-L879

Added lines #L877 - L879 were not covered by tests
}

*BufferLength = sizeof(uint8_t);
*(uint8_t*)Buffer = Listener->UseQTIP;

Check warning on line 883 in src/core/listener.c

View check run for this annotation

Codecov / codecov/patch

src/core/listener.c#L882-L883

Added lines #L882 - L883 were not covered by tests

Status = QUIC_STATUS_SUCCESS;
break;

Check warning on line 886 in src/core/listener.c

View check run for this annotation

Codecov / codecov/patch

src/core/listener.c#L885-L886

Added lines #L885 - L886 were not covered by tests

default:
Status = QUIC_STATUS_INVALID_PARAMETER;
break;
Expand Down
5 changes: 5 additions & 0 deletions src/core/listener.h
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,11 @@ typedef struct QUIC_LISTENER {
//
uint8_t CibirId[2 + QUIC_MAX_CIBIR_LENGTH];

//
// Whether or not this listener accepts QTIP traffic (BOOLEAN)
//
uint8_t UseQTIP : 1;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make this a boolean and put it up with the others for now.


} QUIC_LISTENER;

#ifdef QUIC_SILO
Expand Down
3 changes: 3 additions & 0 deletions src/core/send.c
Original file line number Diff line number Diff line change
Expand Up @@ -1190,6 +1190,9 @@ QuicSendFlush(
QUIC_CONNECTION* Connection = QuicSendGetConnection(Send);
QUIC_PATH* Path = &Connection->Paths[0];

Path->Route.UseQTIP = Connection->State.UseQTIP;
Path->Route.AppDidSetQTIP = Connection->State.AppDidSetQTIP;

CXPLAT_DBG_ASSERT(!Connection->State.HandleClosed);

if (!CxPlatIsRouteReady(Connection, Path)) {
Expand Down
6 changes: 6 additions & 0 deletions src/cs/lib/msquic_generated.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3372,6 +3372,9 @@ internal static unsafe partial class MsQuic
[NativeTypeName("#define QUIC_PARAM_LISTENER_CIBIR_ID 0x04000002")]
internal const uint QUIC_PARAM_LISTENER_CIBIR_ID = 0x04000002;

[NativeTypeName("#define QUIC_PARAM_LISTENER_QTIP 0x04000004")]
internal const uint QUIC_PARAM_LISTENER_QTIP = 0x04000004;

[NativeTypeName("#define QUIC_PARAM_CONN_QUIC_VERSION 0x05000000")]
internal const uint QUIC_PARAM_CONN_QUIC_VERSION = 0x05000000;

Expand Down Expand Up @@ -3450,6 +3453,9 @@ internal static unsafe partial class MsQuic
[NativeTypeName("#define QUIC_PARAM_CONN_SEND_DSCP 0x05000019")]
internal const uint QUIC_PARAM_CONN_SEND_DSCP = 0x05000019;

[NativeTypeName("#define QUIC_PARAM_CONN_QTIP 0x0500001A")]
internal const uint QUIC_PARAM_CONN_QTIP = 0x0500001A;

[NativeTypeName("#define QUIC_PARAM_TLS_HANDSHAKE_INFO 0x06000000")]
internal const uint QUIC_PARAM_TLS_HANDSHAKE_INFO = 0x06000000;

Expand Down
5 changes: 5 additions & 0 deletions src/inc/msquic.h
Original file line number Diff line number Diff line change
Expand Up @@ -889,6 +889,7 @@ typedef struct QUIC_SCHANNEL_CREDENTIAL_ATTRIBUTE_W {
#define QUIC_PARAM_LISTENER_STATS 0x04000001 // QUIC_LISTENER_STATISTICS
#ifdef QUIC_API_ENABLE_PREVIEW_FEATURES
#define QUIC_PARAM_LISTENER_CIBIR_ID 0x04000002 // uint8_t[] {offset, id[]}
#define QUIC_PARAM_LISTENER_QTIP 0x04000004 // uint8_t (BOOLEAN)
#endif

//
Expand Down Expand Up @@ -925,6 +926,10 @@ typedef struct QUIC_SCHANNEL_CREDENTIAL_ATTRIBUTE_W {
#define QUIC_PARAM_CONN_ORIG_DEST_CID 0x05000018 // uint8_t[]
#define QUIC_PARAM_CONN_SEND_DSCP 0x05000019 // uint8_t

#ifdef QUIC_API_ENABLE_PREVIEW_FEATURES
#define QUIC_PARAM_CONN_QTIP 0x0500001A // uint8_t (BOOLEAN)
#endif

//
// Parameters for TLS.
//
Expand Down
2 changes: 2 additions & 0 deletions src/inc/quic_datapath.h
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,8 @@ typedef struct CXPLAT_ROUTE {
CXPLAT_ROUTE_STATE State;
CXPLAT_RAW_TCP_STATE TcpState;

uint8_t UseQTIP : 1; // TRUE if the route is using QTIP
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Put this after DatapathType

uint8_t AppDidSetQTIP : 1; // TRUE if the app explicitly set the UseQTIP flag
} CXPLAT_ROUTE;

//
Expand Down
7 changes: 6 additions & 1 deletion src/platform/datapath_raw.c
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,12 @@ CxPlatDpRawRxEthernet(

if (Socket) {
if (PacketChain->Reserved == L4_TYPE_UDP || PacketChain->Reserved == L4_TYPE_TCP) {
uint8_t SocketType = Socket->UseTcp ? L4_TYPE_TCP : L4_TYPE_UDP;
//
// If we have UseTcp enabled, we should still support UDP type of packets.
//
// uint8_t SocketType = (Socket->UseTcp && PacketChain->Reserved == L4_TYPE_TCP) ? L4_TYPE_TCP : L4_TYPE_UDP;

uint8_t SocketType = (Socket->UseTcp) ? L4_TYPE_TCP : L4_TYPE_UDP; // This is the old code

//
// Found a match. Chain and deliver contiguous packets with the same 4-tuple.
Expand Down
Loading
Loading