Bug 1852924 - When pacer returns a small delay, pretend to wait and get the next packet to send, r=mt

marco-c · marco-c · commit 2001ab043a62 · 2023-11-08T16:00:08.000Z
Differential Revision: https://phabricator.services.mozilla.com/D191325 UltraBlame original commit: f5d7c3a00f2bdc43070a0a547ddcd9027f3a2d02
diff --git a/dom/network/UDPSocketParent.cpp b/dom/network/UDPSocketParent.cpp
@@ -360,7 +360,8 @@ void UDPSocketParent::Send(const nsTArray<uint8_t>& aData,
     }
     case UDPSocketAddr::TNetAddr: {
       const NetAddr& addr(aAddr.get_NetAddr());
-      rv = mSocket->SendWithAddress(&addr, aData, &count);
+      rv = mSocket->SendWithAddress(&addr, aData.Elements(), aData.Length(),
+                                    &count);
       break;
     }
     default:
diff --git a/modules/libpref/init/StaticPrefList.yaml b/modules/libpref/init/StaticPrefList.yaml
@@ -12429,6 +12429,14 @@
   mirror: always
   rust: true
 
+# It represents the maximum duration that we used to accumulate
+# callback timeouts before we set a timer and break out of the loop.
+- name: network.http.http3.max_accumlated_time_ms
+  type: RelaxedAtomicUint32
+  value: 1
+  mirror: always
+  rust: true
+
 # When true, a http request will be upgraded to https when HTTPS RR is
 # available.
 - name: network.dns.upgrade_with_https_rr
diff --git a/netwerk/base/nsIUDPSocket.idl b/netwerk/base/nsIUDPSocket.idl
@@ -194,7 +194,8 @@ interface nsIUDPSocket : nsISupports
      * @return number of bytes written. (0 or length of data)
      */
     [noscript] unsigned long sendWithAddress([const] in NetAddrPtr addr,
-                                             in Array<uint8_t> data);
+                                             [array, size_is(length), const] in uint8_t data,
+                                             in unsigned long length);
 
     /**
      * sendBinaryStream
diff --git a/netwerk/base/nsUDPSocket.cpp b/netwerk/base/nsUDPSocket.cpp
@@ -1004,7 +1004,8 @@ PendingSend::OnLookupComplete(nsICancelable* request, nsIDNSRecord* aRecord,
   NetAddr addr;
   if (NS_SUCCEEDED(rec->GetNextAddr(mPort, &addr))) {
     uint32_t count;
-    nsresult rv = mSocket->SendWithAddress(&addr, mData, &count);
+    nsresult rv = mSocket->SendWithAddress(&addr, mData.Elements(),
+                                           mData.Length(), &count);
     NS_ENSURE_SUCCESS(rv, rv);
   }
 
@@ -1069,7 +1070,7 @@ class SendRequestRunnable : public Runnable {
 NS_IMETHODIMP
 SendRequestRunnable::Run() {
   uint32_t count;
-  mSocket->SendWithAddress(&mAddr, mData, &count);
+  mSocket->SendWithAddress(&mAddr, mData.Elements(), mData.Length(), &count);
   return NS_OK;
 }
 
@@ -1137,13 +1138,12 @@ nsUDPSocket::SendWithAddr(nsINetAddr* aAddr, const nsTArray<uint8_t>& aData,
 
   NetAddr netAddr;
   aAddr->GetNetAddr(&netAddr);
-  return SendWithAddress(&netAddr, aData, _retval);
+  return SendWithAddress(&netAddr, aData.Elements(), aData.Length(), _retval);
 }
 
 NS_IMETHODIMP
-nsUDPSocket::SendWithAddress(const NetAddr* aAddr,
-                             const nsTArray<uint8_t>& aData,
-                             uint32_t* _retval) {
+nsUDPSocket::SendWithAddress(const NetAddr* aAddr, const uint8_t* aData,
+                             uint32_t aLength, uint32_t* _retval) {
   NS_ENSURE_ARG(aAddr);
   NS_ENSURE_ARG_POINTER(_retval);
 
@@ -1167,8 +1167,7 @@ nsUDPSocket::SendWithAddress(const NetAddr* aAddr,
       return NS_ERROR_FAILURE;
     }
     int32_t count =
-        PR_SendTo(mFD, aData.Elements(), sizeof(uint8_t) * aData.Length(), 0,
-                  &prAddr, PR_INTERVAL_NO_WAIT);
+        PR_SendTo(mFD, aData, aLength, 0, &prAddr, PR_INTERVAL_NO_WAIT);
     if (count < 0) {
       PRErrorCode code = PR_GetError();
       return ErrorAccordingToNSPR(code);
@@ -1177,15 +1176,15 @@ nsUDPSocket::SendWithAddress(const NetAddr* aAddr,
     *_retval = count;
   } else {
     FallibleTArray<uint8_t> fallibleArray;
-    if (!fallibleArray.InsertElementsAt(0, aData, fallible)) {
+    if (!fallibleArray.AppendElements(aData, aLength, fallible)) {
       return NS_ERROR_OUT_OF_MEMORY;
     }
 
     nsresult rv = mSts->Dispatch(
         new SendRequestRunnable(this, *aAddr, std::move(fallibleArray)),
         NS_DISPATCH_NORMAL);
     NS_ENSURE_SUCCESS(rv, rv);
-    *_retval = aData.Length();
+    *_retval = aLength;
   }
   return NS_OK;
 }
diff --git a/netwerk/protocol/http/Http3Session.cpp b/netwerk/protocol/http/Http3Session.cpp
@@ -71,10 +71,18 @@ Http3Session::Http3Session() {
   mThroughCaptivePortal = gHttpHandler->GetThroughCaptivePortal();
 }
 
-static nsresult StringAndPortToNetAddr(nsACString& remoteAddrStr,
-                                       uint16_t remotePort, NetAddr* netAddr) {
-  if (NS_FAILED(netAddr->InitFromString(remoteAddrStr, remotePort))) {
-    return NS_ERROR_FAILURE;
+static nsresult RawBytesToNetAddr(uint16_t aFamily, const uint8_t* aRemoteAddr,
+                                  uint16_t remotePort, NetAddr* netAddr) {
+  if (aFamily == AF_INET) {
+    netAddr->inet.family = AF_INET;
+    netAddr->inet.port = htons(remotePort);
+    memcpy(&netAddr->inet.ip, aRemoteAddr, 4);
+  } else if (aFamily == AF_INET6) {
+    netAddr->inet6.family = AF_INET6;
+    netAddr->inet6.port = htons(remotePort);
+    memcpy(&netAddr->inet6.ip.u8, aRemoteAddr, 16);
+  } else {
+    return NS_ERROR_UNEXPECTED;
   }
 
   return NS_OK;
@@ -133,6 +141,7 @@ nsresult Http3Session::Init(const nsHttpConnectionInfo* aConnInfo,
       StaticPrefs::network_http_http3_max_stream_data(),
       StaticPrefs::network_http_http3_version_negotiation_enabled(),
       mConnInfo->GetWebTransport(), gHttpHandler->Http3QlogDir(), datagramSize,
+      StaticPrefs::network_http_http3_max_accumlated_time_ms(),
       getter_AddRefs(mHttp3Connection));
   if (NS_FAILED(rv)) {
     return rv;
@@ -882,46 +891,46 @@ nsresult Http3Session::ProcessOutput(nsIUDPSocket* socket) {
   LOG(("Http3Session::ProcessOutput reader=%p, [this=%p]", mUdpConn.get(),
        this));
 
-  
-  
-  while (true) {
-    nsTArray<uint8_t> packetToSend;
-    nsAutoCString remoteAddrStr;
-    uint16_t port = 0;
-    uint64_t timeout = 0;
-    if (!mHttp3Connection->ProcessOutput(&remoteAddrStr, &port, packetToSend,
-                                         &timeout)) {
-      SetupTimer(timeout);
-      break;
-    }
-    MOZ_ASSERT(packetToSend.Length());
-    LOG(
-        ("Http3Session::ProcessOutput sending packet with %u bytes to %s "
-         "port=%d [this=%p].",
-         (uint32_t)packetToSend.Length(),
-         PromiseFlatCString(remoteAddrStr).get(), port, this));
-
-    uint32_t written = 0;
-    NetAddr addr;
-    if (NS_FAILED(StringAndPortToNetAddr(remoteAddrStr, port, &addr))) {
-      continue;
-    }
-    nsresult rv = socket->SendWithAddress(&addr, packetToSend, &written);
+  mSocket = socket;
+  nsresult rv = mHttp3Connection->ProcessOutputAndSend(
+      this,
+      [](void* aContext, uint16_t aFamily, const uint8_t* aAddr, uint16_t aPort,
+         const uint8_t* aData, uint32_t aLength) {
+        Http3Session* self = (Http3Session*)aContext;
+
+        uint32_t written = 0;
+        NetAddr addr;
+        if (NS_FAILED(RawBytesToNetAddr(aFamily, aAddr, aPort, &addr))) {
+          return NS_OK;
+        }
 
-    LOG(("Http3Session::ProcessOutput sending packet rv=%d osError=%d",
-         static_cast<int32_t>(rv), NS_FAILED(rv) ? PR_GetOSError() : 0));
-    if (NS_FAILED(rv) && (rv != NS_BASE_STREAM_WOULD_BLOCK)) {
-      mSocketError = rv;
-      
-      
-      
-      return rv;
-    }
-    mTotalBytesWritten += packetToSend.Length();
-    mLastWriteTime = PR_IntervalNow();
-  }
+        LOG3(
+            ("Http3Session::ProcessOutput sending packet with %u bytes to %s "
+             "port=%d [this=%p].",
+             aLength, addr.ToString().get(), aPort, self));
 
-  return NS_OK;
+        nsresult rv =
+            self->mSocket->SendWithAddress(&addr, aData, aLength, &written);
+
+        LOG(("Http3Session::ProcessOutput sending packet rv=%d osError=%d",
+             static_cast<int32_t>(rv), NS_FAILED(rv) ? PR_GetOSError() : 0));
+        if (NS_FAILED(rv) && (rv != NS_BASE_STREAM_WOULD_BLOCK)) {
+          self->mSocketError = rv;
+          
+          
+          
+          return rv;
+        }
+        self->mTotalBytesWritten += aLength;
+        self->mLastWriteTime = PR_IntervalNow();
+        return NS_OK;
+      },
+      [](void* aContext, uint64_t timeout) {
+        Http3Session* self = (Http3Session*)aContext;
+        self->SetupTimer(timeout);
+      });
+  mSocket = nullptr;
+  return rv;
 }
 
 
@@ -955,7 +964,8 @@ void Http3Session::SetupTimer(uint64_t aTimeout) {
     return;
   }
 
-  LOG(("Http3Session::SetupTimer to %" PRIu64 "ms [this=%p].", aTimeout, this));
+  LOG3(
+      ("Http3Session::SetupTimer to %" PRIu64 "ms [this=%p].", aTimeout, this));
 
   
   mTimerShouldTrigger =
diff --git a/netwerk/protocol/http/Http3Session.h b/netwerk/protocol/http/Http3Session.h
@@ -375,6 +375,10 @@ class Http3Session final : public nsAHttpTransaction, public nsAHttpConnection {
   bool mHasWebTransportSession = false;
   
   bool mDontExclude = false;
+  
+  
+  
+  nsIUDPSocket* mSocket;
 };
 
 NS_DEFINE_STATIC_IID_ACCESSOR(Http3Session, NS_HTTP3SESSION_IID);
diff --git a/netwerk/socket/neqo_glue/NeqoHttp3Conn.h b/netwerk/socket/neqo_glue/NeqoHttp3Conn.h
@@ -19,12 +19,12 @@ class NeqoHttp3Conn final {
                        uint64_t aMaxData, uint64_t aMaxStreamData,
                        bool aVersionNegotiation, bool aWebTransport,
                        const nsACString& aQlogDir, uint32_t aDatagramSize,
-                       NeqoHttp3Conn** aConn) {
-    return neqo_http3conn_new(&aOrigin, &aAlpn, &aLocalAddr, &aRemoteAddr,
-                              aMaxTableSize, aMaxBlockedStreams, aMaxData,
-                              aMaxStreamData, aVersionNegotiation,
-                              aWebTransport, &aQlogDir, aDatagramSize,
-                              (const mozilla::net::NeqoHttp3Conn**)aConn);
+                       uint32_t aMaxAccumulatedTime, NeqoHttp3Conn** aConn) {
+    return neqo_http3conn_new(
+        &aOrigin, &aAlpn, &aLocalAddr, &aRemoteAddr, aMaxTableSize,
+        aMaxBlockedStreams, aMaxData, aMaxStreamData, aVersionNegotiation,
+        aWebTransport, &aQlogDir, aDatagramSize, aMaxAccumulatedTime,
+        (const mozilla::net::NeqoHttp3Conn**)aConn);
   }
 
   void Close(uint64_t aError) { neqo_http3conn_close(this, aError); }
@@ -46,11 +46,10 @@ class NeqoHttp3Conn final {
     return neqo_http3conn_process_input(this, &aRemoteAddr, &aPacket);
   }
 
-  bool ProcessOutput(nsACString* aRemoteAddr, uint16_t* aPort,
-                     nsTArray<uint8_t>& aData, uint64_t* aTimeout) {
-    aData.TruncateLength(0);
-    return neqo_http3conn_process_output(this, aRemoteAddr, aPort, &aData,
-                                         aTimeout);
+  nsresult ProcessOutputAndSend(void* aContext, SendFunc aSendFunc,
+                                SetTimerFunc aSetTimerFunc) {
+    return neqo_http3conn_process_output_and_send(this, aContext, aSendFunc,
+                                                  aSetTimerFunc);
   }
 
   nsresult GetEvent(Http3Event* aEvent, nsTArray<uint8_t>& aData) {
diff --git a/netwerk/socket/neqo_glue/src/lib.rs b/netwerk/socket/neqo_glue/src/lib.rs
diff --git a/netwerk/test/gtest/TestUDPSocket.cpp b/netwerk/test/gtest/TestUDPSocket.cpp

Original file line number	Diff line number	Diff line change
`@@ -360,7 +360,8 @@ void UDPSocketParent::Send(const nsTArray<uint8_t>& aData,`
`360`	`360`	`}`
`361`	`361`	`case UDPSocketAddr::TNetAddr: {`
`362`	`362`	`const NetAddr& addr(aAddr.get_NetAddr());`
`363`		`- rv = mSocket->SendWithAddress(&addr, aData, &count);`
	`363`	`+ rv = mSocket->SendWithAddress(&addr, aData.Elements(), aData.Length(),`
	`364`	`+ &count);`
`364`	`365`	`break;`
`365`	`366`	`}`
`366`	`367`	`default:`