fix(xdp): propagate XDP rule plumbing failures to caller

Fixes #5935

CxPlatDpRawPlumbRulesOnSocket and CxPlatDpRawInterfaceAddRules now return
QUIC_STATUS to propagate XDP rule plumbing failures (XdpCreateProgram,
allocation, rule overflow) to the caller.

On install failure, CxPlatDpRawPlumbRulesOnSocket stops at the first failed
interface and returns the error. The caller (RawSocketCreateUdp) performs
best-effort cleanup by re-invoking PlumbRulesOnSocket with IsCreated=FALSE
before returning failure.

RawSocketCreateUdp checks the return value and rolls back the socket
registration on failure. RawSocketDelete deletion is best-effort: logs a
warning on failure.

Co-authored-by: Jack He <jackhe@microsoft.com>

Author: MarkedMuichiro

src/generated/linux/datapath_raw.c.clog.h

@@ -41,6 +41,26 @@ tracepoint(CLOG_DATAPATH_RAW_C, AllocFailure , arg2, arg3);\
 
 
 
+/*----------------------------------------------------------
+// Decoder Ring for LibraryErrorStatus
+// [ lib] ERROR, %u, %s.
+// QuicTraceEvent(
+            LibraryErrorStatus,
+            "[ lib] ERROR, %u, %s.",
+            PlumbStatus,
+            "CxPlatDpRawPlumbRulesOnSocket (delete)");
+// arg2 = arg2 = PlumbStatus = arg2
+// arg3 = arg3 = "CxPlatDpRawPlumbRulesOnSocket (delete)" = arg3
+----------------------------------------------------------*/
+#ifndef _clog_4_ARGS_TRACE_LibraryErrorStatus
+#define _clog_4_ARGS_TRACE_LibraryErrorStatus(uniqueId, encoded_arg_string, arg2, arg3)\
+tracepoint(CLOG_DATAPATH_RAW_C, LibraryErrorStatus , arg2, arg3);\
+
+#endif
+
+
+
+
 /*----------------------------------------------------------
 // Decoder Ring for DatapathRecv
 // [data][%p] Recv %u bytes (segment=%hu) Src=%!ADDR! Dst=%!ADDR!

src/generated/linux/datapath_raw.c.clog.h.lttng.h

@@ -24,6 +24,29 @@ TRACEPOINT_EVENT(CLOG_DATAPATH_RAW_C, AllocFailure,
 
 
 
+/*----------------------------------------------------------
+// Decoder Ring for LibraryErrorStatus
+// [ lib] ERROR, %u, %s.
+// QuicTraceEvent(
+            LibraryErrorStatus,
+            "[ lib] ERROR, %u, %s.",
+            PlumbStatus,
+            "CxPlatDpRawPlumbRulesOnSocket (delete)");
+// arg2 = arg2 = PlumbStatus = arg2
+// arg3 = arg3 = "CxPlatDpRawPlumbRulesOnSocket (delete)" = arg3
+----------------------------------------------------------*/
+TRACEPOINT_EVENT(CLOG_DATAPATH_RAW_C, LibraryErrorStatus,
+    TP_ARGS(
+        unsigned int, arg2,
+        const char *, arg3), 
+    TP_FIELDS(
+        ctf_integer(unsigned int, arg2, arg2)
+        ctf_string(arg3, arg3)
+    )
+)
+
+
+
 /*----------------------------------------------------------
 // Decoder Ring for DatapathRecv
 // [data][%p] Recv %u bytes (segment=%hu) Src=%!ADDR! Dst=%!ADDR!

src/platform/datapath_raw.c

@@ -174,7 +174,19 @@ RawSocketDelete(
     _In_ CXPLAT_SOCKET_RAW* Socket
     )
 {
-    CxPlatDpRawPlumbRulesOnSocket(Socket, FALSE);
+    //
+    // Rule removal on socket deletion is best-effort. A failure here means
+    // some XDP rules may linger until the interface is reinitialized, but
+    // there is nothing useful we can do at this point.
+    //
+    QUIC_STATUS PlumbStatus = CxPlatDpRawPlumbRulesOnSocket(Socket, FALSE);
+    if (QUIC_FAILED(PlumbStatus)) {
+        QuicTraceEvent(
+            LibraryErrorStatus,
+            "[ lib] ERROR, %u, %s.",
+            PlumbStatus,
+            "CxPlatDpRawPlumbRulesOnSocket (delete)");
+    }
     CxPlatRemoveSocket(&Socket->RawDatapath->SocketPool, Socket);
     CxPlatRundownReleaseAndWait(&Socket->RawRundown);
     if (Socket->PausedTcpSend) {

src/platform/datapath_raw.h

@@ -148,7 +148,7 @@ CxPlatDpRawUpdatePollingIdleTimeout(
 // that it should update any filtering rules as necessary.
 //
 _IRQL_requires_max_(PASSIVE_LEVEL)
-void
+QUIC_STATUS
 CxPlatDpRawPlumbRulesOnSocket(
     _In_ CXPLAT_SOCKET_RAW* Socket,
     _In_ BOOLEAN IsCreated

src/platform/datapath_raw_win.c

@@ -170,7 +170,28 @@ RawSocketCreateUdp(
         goto Error;
     }
 
-    CxPlatDpRawPlumbRulesOnSocket(Socket, TRUE);
+    Status = CxPlatDpRawPlumbRulesOnSocket(Socket, TRUE);
+    if (QUIC_FAILED(Status)) {
+        //
+        // CxPlatDpRawPlumbRulesOnSocket(TRUE) stops at the first interface
+        // where rule installation fails, so some interfaces may already have
+        // partial state (rules installed or port bits set) that must be rolled
+        // back. Reuse the IsCreated=FALSE path for best-effort cleanup; it is
+        // safe to call against interfaces where nothing was installed (no-op
+        // for those). Cleanup failures are logged but do not change the
+        // returned error.
+        //
+        QUIC_STATUS CleanupStatus = CxPlatDpRawPlumbRulesOnSocket(Socket, FALSE);
+        if (QUIC_FAILED(CleanupStatus)) {
+            QuicTraceEvent(
+                LibraryErrorStatus,
+                "[ lib] ERROR, %u, %s.",
+                CleanupStatus,
+                "CxPlatDpRawPlumbRulesOnSocket cleanup");
+        }
+        CxPlatRemoveSocket(&Raw->SocketPool, Socket);
+        goto Error;
+    }
 
 Error:
 

src/platform/datapath_raw_xdp_win.c

@@ -853,7 +853,7 @@ CxPlatDpRawInterfaceInitialize(
 
 _IRQL_requires_max_(PASSIVE_LEVEL)
 _Requires_lock_held_(Interface->RuleLock)
-void
+QUIC_STATUS
 CxPlatDpRawInterfaceUpdateRules(
     _In_ XDP_INTERFACE* Interface
     )
@@ -864,6 +864,8 @@ CxPlatDpRawInterfaceUpdateRules(
         .SubLayer = XDP_HOOK_INSPECT,
     };
 
+    QUIC_STATUS ReturnStatus = QUIC_STATUS_SUCCESS;
+
     for (uint32_t i = 0; i < Interface->QueueCount; i++) {
 
         CXPLAT_QUEUE* Queue = &Interface->Queues[i];
@@ -882,16 +884,14 @@ CxPlatDpRawInterfaceUpdateRules(
                 Interface->RuleCount,
                 &NewRxProgram);
         if (QUIC_FAILED(Status)) {
-            //
-            // TODO - Figure out how to better handle failure and revert changes.
-            // This will likely require working with XDP to get an improved API;
-            // possibly to update all queues at once.
-            //
             QuicTraceEvent(
                 LibraryErrorStatus,
                 "[ lib] ERROR, %u, %s.",
                 Status,
                 "XdpCreateProgram");
+            if (QUIC_SUCCEEDED(ReturnStatus)) {
+                ReturnStatus = Status;
+            }
             continue;
         }
 
@@ -901,10 +901,12 @@ CxPlatDpRawInterfaceUpdateRules(
 
         Queue->RxProgram = NewRxProgram;
     }
+
+    return ReturnStatus;
 }
 
 _IRQL_requires_max_(PASSIVE_LEVEL)
-void
+QUIC_STATUS
 CxPlatDpRawInterfaceAddRules(
     _In_ XDP_INTERFACE* Interface,
     _In_reads_(Count) const XDP_RULE* Rules,
@@ -914,6 +916,8 @@ CxPlatDpRawInterfaceAddRules(
 #pragma warning(push)
 #pragma warning(disable:6386) // Buffer overrun while writing to 'NewRules' - FALSE POSITIVE
 
+    QUIC_STATUS Status;
+
     CxPlatLockAcquire(&Interface->RuleLock);
     // TODO - Don't always allocate a new array?
 
@@ -923,7 +927,7 @@ CxPlatDpRawInterfaceAddRules(
             "[ lib] ERROR, %s.",
             "No more room for rules");
         CxPlatLockRelease(&Interface->RuleLock);
-        return;
+        return QUIC_STATUS_BUFFER_TOO_SMALL;
     }
 
     const size_t OldSize = sizeof(XDP_RULE) * (size_t)Interface->RuleCount;
@@ -937,7 +941,7 @@ CxPlatDpRawInterfaceAddRules(
             "XDP_RULE",
             NewSize);
         CxPlatLockRelease(&Interface->RuleLock);
-        return;
+        return QUIC_STATUS_OUT_OF_MEMORY;
     }
 
     if (Interface->RuleCount > 0) {
@@ -952,11 +956,13 @@ CxPlatDpRawInterfaceAddRules(
     }
     Interface->Rules = NewRules;
 
-    CxPlatDpRawInterfaceUpdateRules(Interface);
+    Status = CxPlatDpRawInterfaceUpdateRules(Interface);
 
     CxPlatLockRelease(&Interface->RuleLock);
 
 #pragma warning(pop)
+
+    return Status;
 }
 
 _IRQL_requires_max_(PASSIVE_LEVEL)
@@ -1423,12 +1429,13 @@ CxPlatDpRawClearPortBit(
 }
 
 _IRQL_requires_max_(PASSIVE_LEVEL)
-void
+QUIC_STATUS
 CxPlatDpRawPlumbRulesOnSocket(
     _In_ CXPLAT_SOCKET_RAW* Socket,
     _In_ BOOLEAN IsCreated
     )
 {
+    QUIC_STATUS Status = QUIC_STATUS_SUCCESS;
     XDP_DATAPATH* Xdp = (XDP_DATAPATH*)Socket->RawDatapath;
     if (Socket->Wildcard) {
         XDP_RULE Rules[5] = {0};
@@ -1504,7 +1511,17 @@ CxPlatDpRawPlumbRulesOnSocket(
         for (Entry = Xdp->Interfaces.Flink; Entry != &Xdp->Interfaces; Entry = Entry->Flink) {
             XDP_INTERFACE* Interface = CONTAINING_RECORD(Entry, XDP_INTERFACE, Link);
             if (IsCreated) {
-                CxPlatDpRawInterfaceAddRules(Interface, Rules, RulesSize);
+                QUIC_STATUS AddStatus = CxPlatDpRawInterfaceAddRules(Interface, Rules, RulesSize);
+                if (QUIC_FAILED(AddStatus)) {
+                    //
+                    // Stop on first failure and propagate. The caller is
+                    // responsible for invoking this function again with
+                    // IsCreated=FALSE to roll back any rules already
+                    // installed on previous interfaces.
+                    //
+                    Status = AddStatus;
+                    break;
+                }
             } else {
                 CxPlatDpRawInterfaceRemoveRules(Interface, Rules, RulesSize);
             }
@@ -1566,14 +1583,37 @@ CxPlatDpRawPlumbRulesOnSocket(
                             "Allocation of '%s' failed. (%llu bytes)",
                             "PortSet",
                             XDP_PORT_SET_BUFFER_SIZE);
-                        return;
+                        Status = QUIC_STATUS_OUT_OF_MEMORY;
+                        break;
                     }
                     CxPlatDpRawSetPortBit(
                         (uint8_t*)NewRule.Pattern.IpPortSet.PortSet.PortSet,
                         Socket->LocalAddress.Ipv4.sin_port);
                     memcpy(
                         &NewRule.Pattern.IpPortSet.Address, IpAddress, IpAddressSize);
-                    CxPlatDpRawInterfaceAddRules(Interface, &NewRule, 1);
+                    QUIC_STATUS AddStatus = CxPlatDpRawInterfaceAddRules(Interface, &NewRule, 1);
+                    if (QUIC_FAILED(AddStatus)) {
+                        Status = AddStatus;
+                        if (AddStatus == QUIC_STATUS_OUT_OF_MEMORY ||
+                            AddStatus == QUIC_STATUS_BUFFER_TOO_SMALL) {
+                            //
+                            // The rule was not copied into Interface->Rules — we still own
+                            // the PortSet buffer and must free it. On XdpCreateProgram failure,
+                            // the rule was already added to Interface->Rules which now owns
+                            // the PortSet buffer; do not free it in that case.
+                            //
+                            CxPlatFree(
+                                (uint8_t*)NewRule.Pattern.IpPortSet.PortSet.PortSet,
+                                PORT_SET_TAG);
+                        }
+                        //
+                        // Stop on first failure and propagate. The caller is
+                        // responsible for invoking this function again with
+                        // IsCreated=FALSE to roll back any port bits already
+                        // set on previous interfaces.
+                        //
+                        break;
+                    }
                 }
             } else {
                 //
@@ -1588,6 +1628,8 @@ CxPlatDpRawPlumbRulesOnSocket(
             }
         }
     }
+
+    return Status;
 }
 
 _IRQL_requires_max_(DISPATCH_LEVEL)

src/platform/datapath_xplat.c

@@ -162,17 +162,18 @@ CxPlatSocketCreateUdp(
                 QuicTraceLogVerbose(
                     RawSockCreateFail,
                     "[sock] Failed to create raw socket, status:%d", Status);
-                BOOLEAN IsWildcardAddr = Config->LocalAddress == NULL || QuicAddrIsWildCard(Config->LocalAddress);
-                if (IsWildcardAddr && RequiresQtip) {
+                BOOLEAN IsServerSocket = !(*NewSocket)->HasFixedRemoteAddress;
+                if (IsServerSocket && RequiresQtip) {
                     //
                     // This retry loop is purely for QTIP listener sockets that try to reserve both a UDP/TCP port,
                     // which may run into a port collision for TCP if the UDP ephemeral port collides with something
                     // in the TCP pool. So just try it again.
                     //
                     CxPlatSocketDelete(*NewSocket);
+                    *NewSocket = NULL;
                     continue;
                 }
-                if ((!RequiresQtip && !CibirRequested) || ((*NewSocket)->HasFixedRemoteAddress && CibirRequested && !RequiresQtip)) {
+                if ((!RequiresQtip && !CibirRequested) || (!IsServerSocket && CibirRequested && !RequiresQtip)) {
                     //
                     // Allow fallback to OS UDP sockets in these 2 cases only:
                     //  - XDP with no QTIP and no CIBIR.
@@ -187,6 +188,7 @@ CxPlatSocketCreateUdp(
                     Status = QUIC_STATUS_SUCCESS;
                 } else {
                     CxPlatSocketDelete(*NewSocket);
+                    *NewSocket = NULL;
                 }
                 goto Error;
             }
@@ -195,6 +197,7 @@ CxPlatSocketCreateUdp(
                 ErrNoXdpForQtip,
                 "[sock] Error: app requested QTIP but XDP not enabled/available/initialized.");
             CxPlatSocketDelete(*NewSocket);
+            *NewSocket = NULL;
             Status = QUIC_STATUS_INVALID_STATE;
             goto Error;
         } else if (CibirRequested) {

src/platform/unittest/DataPathTest.cpp

@@ -1357,4 +1357,55 @@ TEST_P(DataPathTest, TcpDataServer)
     ASSERT_TRUE(CxPlatEventWaitWithTimeout(ClientContext.ReceiveEvent, 500));
 }
 
+//
+// CxPlatSetAllocFailDenominator is only available in DEBUG builds, so
+// this test must be DEBUG-only.
+//
+#ifdef DEBUG
+TEST_F(DataPathTest, XdpRuleAddOomCleanup)
+{
+    //
+    // Verify the XDP rule plumbing failure path: when CxPlatDpRawInterfaceAddRules
+    // fails (e.g. due to allocation failure), CxPlatDpRawPlumbRulesOnSocket must
+    // propagate the error and RawSocketCreateUdp must invoke
+    // CxPlatDpRawPlumbRulesOnSocket(FALSE) for best-effort cleanup of any
+    // partially-installed state, then return failure to the caller without
+    // leaking resources or crashing. Only meaningful when running with the
+    // XDP/DuoNic datapath.
+    //
+    if (!UseDuoNic) {
+        GTEST_SKIP();
+    }
+
+    QUIC_GLOBAL_EXECUTION_CONFIG Config = { QUIC_GLOBAL_EXECUTION_CONFIG_FLAG_NONE, 0, 1, {0} };
+    CxPlatDataPath Datapath(&EmptyUdpCallbacks, nullptr, 0, &Config);
+    VERIFY_QUIC_SUCCESS(Datapath.GetInitStatus());
+    ASSERT_TRUE(Datapath.IsSupported(CXPLAT_DATAPATH_FEATURE_RAW, CXPLAT_SOCKET_FLAG_XDP));
+
+    //
+    // Create a client (connected) QTIP socket — providing a RemoteAddress
+    // makes RawSocketCreateUdp take the connected-socket path which reaches
+    // CxPlatDpRawPlumbRulesOnSocket. A non-wildcard local + no remote would
+    // be rejected with QUIC_STATUS_INVALID_STATE before rule plumbing runs.
+    // QTIP also disables the wildcard retry loop and the OS-socket fallback
+    // in CxPlatSocketCreateUdp so the rule-plumbing failure propagates
+    // deterministically.
+    //
+    QuicAddr RemoteAddr = GetNewLocalIPv4();
+
+    //
+    // Fail every 2nd allocation so the socket struct (alloc #1) succeeds and
+    // the XDP rule array allocation inside CxPlatDpRawInterfaceAddRules
+    // (alloc #2) is the one that fails, exercising both the failure-propagation
+    // path and the RawSocketCreateUdp-driven cleanup path.
+    //
+    CxPlatSetAllocFailDenominator(-2);
+    {
+        CxPlatSocket Socket(Datapath, nullptr, &RemoteAddr.SockAddr, nullptr, CXPLAT_SOCKET_FLAG_XDP | CXPLAT_SOCKET_FLAG_QTIP);
+        ASSERT_TRUE(QUIC_FAILED(Socket.GetInitStatus()));
+    }
+    CxPlatSetAllocFailDenominator(0);
+}
+#endif // DEBUG
+
 INSTANTIATE_TEST_SUITE_P(DataPathTest, DataPathTest, ::testing::Values(4, 6), testing::PrintToStringParamName());