Skip to content

Commit

Permalink
Bug 1852924 - When pacer returns a small delay, pretend to wait and g…
Browse files Browse the repository at this point in the history
…et the next packet to send, r=mt

Differential Revision: https://phabricator.services.mozilla.com/D191325
  • Loading branch information
KershawChang committed Nov 6, 2023
1 parent c377b04 commit 91bbb88
Show file tree
Hide file tree
Showing 9 changed files with 178 additions and 104 deletions.
3 changes: 2 additions & 1 deletion dom/network/UDPSocketParent.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
8 changes: 8 additions & 0 deletions modules/libpref/init/StaticPrefList.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 2 additions & 1 deletion netwerk/base/nsIUDPSocket.idl
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
19 changes: 9 additions & 10 deletions netwerk/base/nsUDPSocket.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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);

Expand All @@ -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);
Expand All @@ -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;
}
Expand Down
98 changes: 54 additions & 44 deletions netwerk/protocol/http/Http3Session.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -882,46 +891,46 @@ nsresult Http3Session::ProcessOutput(nsIUDPSocket* socket) {
LOG(("Http3Session::ProcessOutput reader=%p, [this=%p]", mUdpConn.get(),
this));

// Check if we have a packet that could not have been sent in a previous
// iteration or maybe get new packets to send.
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);

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;
// If there was an error that is not NS_BASE_STREAM_WOULD_BLOCK
// return from here. We do not need to set a timer, because we
// will close the connection.
return rv;
}
mTotalBytesWritten += packetToSend.Length();
mLastWriteTime = PR_IntervalNow();
}
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;
}

return NS_OK;
LOG3(
("Http3Session::ProcessOutput sending packet with %u bytes to %s "
"port=%d [this=%p].",
aLength, addr.ToString().get(), aPort, self));

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;
// If there was an error that is not NS_BASE_STREAM_WOULD_BLOCK
// return from here. We do not need to set a timer, because we
// will close the connection.
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;
}

// This is only called when timer expires.
Expand Down Expand Up @@ -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));

// Remember the time when the timer should trigger.
mTimerShouldTrigger =
Expand Down
4 changes: 4 additions & 0 deletions netwerk/protocol/http/Http3Session.h
Original file line number Diff line number Diff line change
Expand Up @@ -375,6 +375,10 @@ class Http3Session final : public nsAHttpTransaction, public nsAHttpConnection {
bool mHasWebTransportSession = false;
// When true, we don't add this connection info into the Http/3 excluded list.
bool mDontExclude = false;
// The lifetime of the UDP socket is managed by the HttpConnectionUDP. This
// is only used in Http3Session::ProcessOutput. Using raw pointer here to
// improve performance.
nsIUDPSocket* mSocket;
};

NS_DEFINE_STATIC_IID_ACCESSOR(Http3Session, NS_HTTP3SESSION_IID);
Expand Down
21 changes: 10 additions & 11 deletions netwerk/socket/neqo_glue/NeqoHttp3Conn.h
Original file line number Diff line number Diff line change
Expand Up @@ -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); }
Expand All @@ -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) {
Expand Down
Loading

0 comments on commit 91bbb88

Please sign in to comment.