Skip to content

Commit 4ffedc7

Browse files
anrossianrossi
committed
Optimize reallocation strategy. Rename flags. Address nits
1 parent 37ae2d3 commit 4ffedc7

6 files changed

Lines changed: 71 additions & 126 deletions

File tree

‎src/core/configuration.c‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,7 @@ MsQuicConfigurationOpen(
128128
NULL,
129129
(CXPLAT_STORAGE_CHANGE_CALLBACK_HANDLER)QuicConfigurationSettingsChanged,
130130
Configuration,
131-
CXPLAT_STORAGE_OPEN_FLAG_READABLE,
131+
CXPLAT_STORAGE_OPEN_FLAG_READ,
132132
&Configuration->Storage);
133133
if (QUIC_FAILED(Status)) {
134134
QuicTraceLogWarning(
@@ -157,7 +157,7 @@ MsQuicConfigurationOpen(
157157
SpecificAppKey,
158158
(CXPLAT_STORAGE_CHANGE_CALLBACK_HANDLER)QuicConfigurationSettingsChanged,
159159
Configuration,
160-
CXPLAT_STORAGE_OPEN_FLAG_READABLE,
160+
CXPLAT_STORAGE_OPEN_FLAG_READ,
161161
&Configuration->AppSpecificStorage);
162162
if (QUIC_FAILED(Status)) {
163163
QuicTraceLogWarning(

‎src/core/library.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -419,7 +419,7 @@ MsQuicLibraryInitialize(
419419
NULL,
420420
MsQuicLibraryReadSettings,
421421
(void*)TRUE, // Non-null indicates registrations should be updated
422-
CXPLAT_STORAGE_OPEN_FLAG_READABLE,
422+
CXPLAT_STORAGE_OPEN_FLAG_READ,
423423
&MsQuicLib.Storage);
424424
if (QUIC_FAILED(Status)) {
425425
QuicTraceLogWarning(

‎src/core/unittest/SettingsTest.cpp‎

Lines changed: 1 addition & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -337,68 +337,6 @@ TEST(SettingsTest, StreamRecvWindowDefaultGetsOverridenByIndividualLimits)
337337
#define SETTINGS_SIZE_THRU_FIELD(SettingsType, Field) \
338338
(FIELD_OFFSET(SettingsType, Field) + sizeof(((SettingsType*)0)->Field))
339339

340-
TEST(SettingsTest, QuicSettingsSetDefault_SetsAllDefaultsWhenUnset)
341-
{
342-
QUIC_SETTINGS_INTERNAL Settings;
343-
CxPlatZeroMemory(&Settings, sizeof(Settings));
344-
345-
// Set all IsSet fields to 0 to simulate unset state
346-
Settings.IsSetFlags = 0;
347-
348-
QuicSettingsSetDefault(&Settings);
349-
350-
// Spot-check a few representative fields (add more as needed)
351-
ASSERT_EQ(Settings.SendBufferingEnabled, QUIC_DEFAULT_SEND_BUFFERING_ENABLE);
352-
ASSERT_EQ(Settings.PacingEnabled, QUIC_DEFAULT_SEND_PACING);
353-
ASSERT_EQ(Settings.MigrationEnabled, QUIC_DEFAULT_MIGRATION_ENABLED);
354-
ASSERT_EQ(Settings.DatagramReceiveEnabled, QUIC_DEFAULT_DATAGRAM_RECEIVE_ENABLED);
355-
ASSERT_EQ(Settings.MaxOperationsPerDrain, QUIC_MAX_OPERATIONS_PER_DRAIN);
356-
ASSERT_EQ(Settings.RetryMemoryLimit, QUIC_DEFAULT_RETRY_MEMORY_FRACTION);
357-
ASSERT_EQ(Settings.LoadBalancingMode, QUIC_DEFAULT_LOAD_BALANCING_MODE);
358-
ASSERT_EQ(Settings.FixedServerID, 0u);
359-
ASSERT_EQ(Settings.MaxWorkerQueueDelayUs, MS_TO_US(QUIC_MAX_WORKER_QUEUE_DELAY));
360-
ASSERT_EQ(Settings.MaxStatelessOperations, QUIC_MAX_STATELESS_OPERATIONS);
361-
ASSERT_EQ(Settings.InitialWindowPackets, QUIC_INITIAL_WINDOW_PACKETS);
362-
ASSERT_EQ(Settings.SendIdleTimeoutMs, QUIC_DEFAULT_SEND_IDLE_TIMEOUT_MS);
363-
ASSERT_EQ(Settings.InitialRttMs, QUIC_INITIAL_RTT);
364-
ASSERT_EQ(Settings.MaxAckDelayMs, QUIC_TP_MAX_ACK_DELAY_DEFAULT);
365-
ASSERT_EQ(Settings.DisconnectTimeoutMs, QUIC_DEFAULT_DISCONNECT_TIMEOUT);
366-
ASSERT_EQ(Settings.KeepAliveIntervalMs, QUIC_DEFAULT_KEEP_ALIVE_INTERVAL);
367-
ASSERT_EQ(Settings.IdleTimeoutMs, QUIC_DEFAULT_IDLE_TIMEOUT);
368-
ASSERT_EQ(Settings.HandshakeIdleTimeoutMs, QUIC_DEFAULT_HANDSHAKE_IDLE_TIMEOUT);
369-
ASSERT_EQ(Settings.PeerBidiStreamCount, 0u);
370-
ASSERT_EQ(Settings.PeerUnidiStreamCount, 0u);
371-
ASSERT_EQ(Settings.TlsClientMaxSendBuffer, QUIC_MAX_TLS_SERVER_SEND_BUFFER); // Note: last assignment in function
372-
ASSERT_EQ(Settings.StreamRecvWindowDefault, QUIC_DEFAULT_STREAM_FC_WINDOW_SIZE);
373-
ASSERT_EQ(Settings.StreamRecvWindowBidiLocalDefault, QUIC_DEFAULT_STREAM_FC_WINDOW_SIZE);
374-
ASSERT_EQ(Settings.StreamRecvWindowBidiRemoteDefault, QUIC_DEFAULT_STREAM_FC_WINDOW_SIZE);
375-
ASSERT_EQ(Settings.StreamRecvWindowUnidiDefault, QUIC_DEFAULT_STREAM_FC_WINDOW_SIZE);
376-
ASSERT_EQ(Settings.StreamRecvBufferDefault, QUIC_DEFAULT_STREAM_RECV_BUFFER_SIZE);
377-
ASSERT_EQ(Settings.ConnFlowControlWindow, QUIC_DEFAULT_CONN_FLOW_CONTROL_WINDOW);
378-
ASSERT_EQ(Settings.MaxBytesPerKey, QUIC_DEFAULT_MAX_BYTES_PER_KEY);
379-
ASSERT_EQ(Settings.ServerResumptionLevel, (uint8_t)QUIC_DEFAULT_SERVER_RESUMPTION_LEVEL);
380-
ASSERT_EQ(Settings.VersionNegotiationExtEnabled, QUIC_DEFAULT_VERSION_NEGOTIATION_EXT_ENABLED);
381-
ASSERT_EQ(Settings.MinimumMtu, QUIC_DPLPMTUD_DEFAULT_MIN_MTU);
382-
ASSERT_EQ(Settings.MaximumMtu, QUIC_DPLPMTUD_DEFAULT_MAX_MTU);
383-
ASSERT_EQ(Settings.MtuDiscoveryMissingProbeCount, QUIC_DPLPMTUD_MAX_PROBES);
384-
ASSERT_EQ(Settings.MtuDiscoverySearchCompleteTimeoutUs, QUIC_DPLPMTUD_RAISE_TIMER_TIMEOUT);
385-
ASSERT_EQ(Settings.MaxBindingStatelessOperations, QUIC_MAX_BINDING_STATELESS_OPERATIONS);
386-
ASSERT_EQ(Settings.StatelessOperationExpirationMs, QUIC_STATELESS_OPERATION_EXPIRATION_MS);
387-
ASSERT_EQ(Settings.CongestionControlAlgorithm, QUIC_CONGESTION_CONTROL_ALGORITHM_DEFAULT);
388-
ASSERT_EQ(Settings.DestCidUpdateIdleTimeoutMs, QUIC_DEFAULT_DEST_CID_UPDATE_IDLE_TIMEOUT_MS);
389-
ASSERT_EQ(Settings.GreaseQuicBitEnabled, QUIC_DEFAULT_GREASE_QUIC_BIT_ENABLED);
390-
ASSERT_EQ(Settings.EcnEnabled, QUIC_DEFAULT_ECN_ENABLED);
391-
ASSERT_EQ(Settings.HyStartEnabled, QUIC_DEFAULT_HYSTART_ENABLED);
392-
ASSERT_EQ(Settings.EncryptionOffloadAllowed, QUIC_DEFAULT_ENCRYPTION_OFFLOAD_ALLOWED);
393-
ASSERT_EQ(Settings.ReliableResetEnabled, QUIC_DEFAULT_RELIABLE_RESET_ENABLED);
394-
ASSERT_EQ(Settings.XdpEnabled, QUIC_DEFAULT_XDP_ENABLED);
395-
ASSERT_EQ(Settings.QTIPEnabled, QUIC_DEFAULT_QTIP_ENABLED);
396-
ASSERT_EQ(Settings.RioEnabled, QUIC_DEFAULT_RIO_ENABLED);
397-
ASSERT_EQ(Settings.OneWayDelayEnabled, QUIC_DEFAULT_ONE_WAY_DELAY_ENABLED);
398-
ASSERT_EQ(Settings.NetStatsEventEnabled, QUIC_DEFAULT_NET_STATS_EVENT_ENABLED);
399-
ASSERT_EQ(Settings.StreamMultiReceiveEnabled, QUIC_DEFAULT_STREAM_MULTI_RECEIVE_ENABLED);
400-
}
401-
402340
TEST(SettingsTest, QuicSettingsSetDefault_DoesNotOverwriteSetFields)
403341
{
404342
QUIC_SETTINGS_INTERNAL Settings;
@@ -461,7 +399,7 @@ class QuicStorageSettingScopeGuard {
461399
StorageName,
462400
nullptr,
463401
nullptr,
464-
CXPLAT_STORAGE_OPEN_FLAG_DELETEABLE | CXPLAT_STORAGE_OPEN_FLAG_WRITEABLE | CXPLAT_STORAGE_OPEN_FLAG_CREATE,
402+
CXPLAT_STORAGE_OPEN_FLAG_DELETE | CXPLAT_STORAGE_OPEN_FLAG_WRITE | CXPLAT_STORAGE_OPEN_FLAG_CREATE,
465403
&m_Storage));
466404
EXPECT_NE(m_Storage, nullptr);
467405
}

‎src/inc/quic_storage.h‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -35,10 +35,10 @@ void
3535
typedef CXPLAT_STORAGE_CHANGE_CALLBACK *CXPLAT_STORAGE_CHANGE_CALLBACK_HANDLER;
3636

3737
typedef enum CXPLAT_STORAGE_OPEN_FLAGS {
38-
CXPLAT_STORAGE_OPEN_FLAG_READABLE = 0x0,
39-
CXPLAT_STORAGE_OPEN_FLAG_WRITEABLE = 0x1,
40-
CXPLAT_STORAGE_OPEN_FLAG_DELETEABLE = 0x2,
41-
CXPLAT_STORAGE_OPEN_FLAG_CREATE = 0x4
38+
CXPLAT_STORAGE_OPEN_FLAG_READ = 0x0,
39+
CXPLAT_STORAGE_OPEN_FLAG_WRITE = 0x1,
40+
CXPLAT_STORAGE_OPEN_FLAG_DELETE = 0x2,
41+
CXPLAT_STORAGE_OPEN_FLAG_CREATE = 0x4
4242
} CXPLAT_STORAGE_OPEN_FLAGS;
4343

4444
DEFINE_ENUM_FLAG_OPERATORS(CXPLAT_STORAGE_OPEN_FLAGS);
@@ -113,7 +113,7 @@ CxPlatStorageDeleteValue(
113113
);
114114

115115
//
116-
// CLears all settings from a storage context.
116+
// Clears all settings from a storage context.
117117
//
118118
_IRQL_requires_max_(PASSIVE_LEVEL)
119119
QUIC_STATUS

‎src/platform/storage_winkernel.c‎

Lines changed: 50 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -344,11 +344,11 @@ CxPlatStorageOpen(
344344

345345
ACCESS_MASK DesiredAccess = KEY_READ | KEY_NOTIFY;
346346

347-
if (Flags & CXPLAT_STORAGE_OPEN_FLAG_WRITEABLE) {
347+
if (Flags & CXPLAT_STORAGE_OPEN_FLAG_WRITE) {
348348
DesiredAccess |= KEY_WRITE;
349349
}
350350

351-
if (Flags & CXPLAT_STORAGE_OPEN_FLAG_DELETEABLE) {
351+
if (Flags & CXPLAT_STORAGE_OPEN_FLAG_DELETE) {
352352
DesiredAccess |= DELETE;
353353
}
354354

@@ -647,74 +647,84 @@ CxPlatStorageClear(
647647
{
648648
QUIC_STATUS Status = QUIC_STATUS_SUCCESS;
649649
ULONG InfoLength = 0;
650-
ULONG AllocatedLength = 0;
650+
ULONG AllocatedLength = sizeof(KEY_VALUE_BASIC_INFORMATION) +
651+
sizeof(QUIC_SETTING_STATELESS_OPERATION_EXPIRATION);
651652
PKEY_VALUE_BASIC_INFORMATION Info = NULL;
652653

654+
Info = CXPLAT_ALLOC_PAGED(AllocatedLength, QUIC_POOL_PLATFORM_TMP_ALLOC);
655+
if (Info == NULL) {
656+
Status = QUIC_STATUS_OUT_OF_MEMORY;
657+
QuicTraceEvent(
658+
AllocFailure,
659+
"Allocation of '%s' failed. (%llu bytes)",
660+
"KEY_VALUE_BASIC_INFORMATION",
661+
sizeof(KEY_VALUE_BASIC_INFORMATION) +
662+
sizeof(QUIC_SETTING_STATELESS_OPERATION_EXPIRATION));
663+
goto Exit;
664+
}
665+
653666
//
654667
// Iterate through all values and delete them
655668
// We always use index 0 because deletion shifts the remaining values
656669
//
657670
while (TRUE) {
658-
//
659-
// First, query the required buffer size for the value name
660-
//
661671
Status = ZwEnumerateValueKey(
662672
Storage->RegKey,
663673
0, // Always use index 0 since we delete as we go
664674
KeyValueBasicInformation,
665-
NULL,
666-
0,
675+
Info,
676+
AllocatedLength,
667677
&InfoLength);
668678

669679
if (Status == STATUS_NO_MORE_ENTRIES) {
670680
Status = QUIC_STATUS_SUCCESS;
671681
break;
672-
} else if (Status != STATUS_BUFFER_OVERFLOW && Status != STATUS_BUFFER_TOO_SMALL) {
673-
if (QUIC_FAILED(Status)) {
674-
QuicTraceEvent(
675-
LibraryErrorStatus,
676-
"[ lib] ERROR, %u, %s.",
677-
Status,
678-
"ZwEnumerateValueKey (size query) failed");
679-
goto Exit;
680-
}
681-
}
682+
} else if (Status == STATUS_BUFFER_OVERFLOW || Status == STATUS_BUFFER_TOO_SMALL) {
683+
//
684+
// Reallocate buffer only if current buffer is too small
685+
//
686+
CXPLAT_DBG_ASSERT(InfoLength > AllocatedLength);
687+
CXPLAT_DBG_ASSERT(Info != NULL);
682688

683-
//
684-
// Allocate or reallocate buffer only if current buffer is too small
685-
//
686-
if (Info == NULL || InfoLength > AllocatedLength) {
687-
if (Info != NULL) {
688-
CXPLAT_FREE(Info, QUIC_POOL_PLATFORM_TMP_ALLOC);
689-
}
689+
CXPLAT_FREE(Info, QUIC_POOL_PLATFORM_TMP_ALLOC);
690690

691691
Info = CXPLAT_ALLOC_PAGED(InfoLength, QUIC_POOL_PLATFORM_TMP_ALLOC);
692692
if (Info == NULL) {
693693
Status = QUIC_STATUS_OUT_OF_MEMORY;
694694
QuicTraceEvent(
695695
AllocFailure,
696696
"Allocation of '%s' failed. (%llu bytes)",
697-
"KEY_VALUE_BASIC_INFORMATION",
697+
"KEY_VALUE_BASIC_INFORMATION (realloc)",
698698
InfoLength);
699699
goto Exit;
700700
}
701701
AllocatedLength = InfoLength;
702-
}
703702

704-
//
705-
// Get the value name
706-
//
707-
Status = ZwEnumerateValueKey(
708-
Storage->RegKey,
709-
0, // Always use index 0 since we delete as we go
710-
KeyValueBasicInformation,
711-
Info,
712-
AllocatedLength,
713-
&InfoLength);
703+
//
704+
// Get the value name
705+
//
706+
Status = ZwEnumerateValueKey(
707+
Storage->RegKey,
708+
0, // Always use index 0 since we delete as we go
709+
KeyValueBasicInformation,
710+
Info,
711+
AllocatedLength,
712+
&InfoLength);
713+
714+
if (Status == STATUS_NO_MORE_ENTRIES) {
715+
Status = QUIC_STATUS_SUCCESS;
716+
break;
717+
} else if (QUIC_FAILED(Status)) {
718+
QuicTraceEvent(
719+
LibraryErrorStatus,
720+
"[ lib] ERROR, %u, %s.",
721+
Status,
722+
"ZwEnumerateValueKey (realloc) failed");
723+
goto Exit;
724+
}
725+
726+
CXPLAT_DBG_ASSERT(InfoLength <= AllocatedLength);
714727

715-
if (Status == STATUS_NO_MORE_ENTRIES) {
716-
Status = QUIC_STATUS_SUCCESS;
717-
break;
718728
} else if (QUIC_FAILED(Status)) {
719729
QuicTraceEvent(
720730
LibraryErrorStatus,
@@ -724,8 +734,6 @@ CxPlatStorageClear(
724734
goto Exit;
725735
}
726736

727-
CXPLAT_DBG_ASSERT(InfoLength == AllocatedLength);
728-
729737
//
730738
// Create a UNICODE_STRING for the value name
731739
//

‎src/platform/storage_winuser.c‎

Lines changed: 12 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -154,27 +154,27 @@ CxPlatStorageOpen(
154154

155155
REGSAM DesiredAccess = KEY_READ | KEY_NOTIFY;
156156

157-
if (Flags & CXPLAT_STORAGE_OPEN_FLAG_WRITEABLE) {
157+
if (Flags & CXPLAT_STORAGE_OPEN_FLAG_WRITE) {
158158
DesiredAccess |= KEY_WRITE;
159159
}
160160

161-
if (Flags & CXPLAT_STORAGE_OPEN_FLAG_DELETEABLE) {
161+
if (Flags & CXPLAT_STORAGE_OPEN_FLAG_DELETE) {
162162
DesiredAccess |= DELETE;
163163
}
164164

165165
if (Flags & CXPLAT_STORAGE_OPEN_FLAG_CREATE) {
166166
Status =
167167
HRESULT_FROM_WIN32(
168-
RegCreateKeyExA(
169-
HKEY_LOCAL_MACHINE,
170-
FullKeyName,
171-
0,
172-
NULL,
173-
REG_OPTION_NON_VOLATILE,
174-
DesiredAccess,
175-
NULL,
176-
&Storage->RegKey,
177-
NULL));
168+
RegCreateKeyExA(
169+
HKEY_LOCAL_MACHINE,
170+
FullKeyName,
171+
0,
172+
NULL,
173+
REG_OPTION_NON_VOLATILE,
174+
DesiredAccess,
175+
NULL,
176+
&Storage->RegKey,
177+
NULL));
178178
if (QUIC_FAILED(Status)) {
179179
QuicTraceEvent(
180180
LibraryErrorStatus,
@@ -184,7 +184,6 @@ CxPlatStorageOpen(
184184
goto Exit;
185185
}
186186
} else {
187-
#pragma prefast(suppress:6001, "SAL can't track FullKeyName")
188187
Status =
189188
HRESULT_FROM_WIN32(
190189
RegOpenKeyExA(

0 commit comments

Comments
 (0)