Skip to content

Commit

Permalink
Connect UDP sockets. (#4270)
Browse files Browse the repository at this point in the history
Allow Cobalt to connect UDP sockets. This saves kernel CPU time. In the
download
benchmark page, this results in ~7% higher achieved bandwidth and ~5%
less total
CPU usage.

Since this will also detect unreachable IP networks sooner, it may also
help with connection latency in some networks.

b/373726636
b/205134049
  • Loading branch information
jellefoks authored Oct 16, 2024
1 parent 1352872 commit d955af2
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 24 deletions.
53 changes: 35 additions & 18 deletions net/socket/udp_socket_starboard.cc
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ int UDPSocketStarboard::GetLocalAddress(IPEndPoint* address) const {
int UDPSocketStarboard::Read(IOBuffer* buf,
int buf_len,
CompletionOnceCallback callback) {
return RecvFrom(buf, buf_len, NULL, std::move(callback));
return RecvFrom(buf, buf_len, nullptr, std::move(callback));
}

int UDPSocketStarboard::RecvFrom(IOBuffer* buf,
Expand Down Expand Up @@ -206,9 +206,7 @@ int UDPSocketStarboard::Write(IOBuffer* buf,
int buf_len,
CompletionOnceCallback callback,
const NetworkTrafficAnnotationTag&) {
DCHECK(remote_address_);
return SendToOrWrite(buf, buf_len, remote_address_.get(),
std::move(callback));
return SendToOrWrite(buf, buf_len, nullptr, std::move(callback));
}

int UDPSocketStarboard::SendTo(IOBuffer* buf,
Expand Down Expand Up @@ -271,17 +269,29 @@ int UDPSocketStarboard::InternalConnect(const IPEndPoint& address) {
DCHECK(!remote_address_.get());

int rv = 0;
// Cobalt does random bind despite bind_type_ because we do not connect
// UDP sockets but Chromium does. And if a socket does recvfrom() without
// any sendto() before, it needs to be bound to have a local port.
rv = RandomBind(address.GetFamily() == ADDRESS_FAMILY_IPV4 ?
IPAddress::IPv4AllZeros() : IPAddress::IPv6AllZeros());
if (bind_type_ == DatagramSocket::RANDOM_BIND) {
// Construct IPAddress of appropriate size (IPv4 or IPv6) of 0s,
// representing INADDR_ANY or in6addr_any.
rv = RandomBind(address.GetFamily() == ADDRESS_FAMILY_IPV4
? IPAddress::IPv4AllZeros()
: IPAddress::IPv6AllZeros());
}

if (rv != OK)
return rv;

SbSocketAddress storage;
if (!address.ToSbSocketAddress(&storage)) {
return ERR_ADDRESS_INVALID;
}

remote_address_.reset(new IPEndPoint(address));

SbSocketError result = SbSocketConnect(socket_, &storage);
if (result != kSbSocketOk) {
return MapLastSocketError(socket_);
}

return OK;
}

Expand Down Expand Up @@ -459,16 +469,16 @@ int UDPSocketStarboard::InternalRecvFrom(IOBuffer* buf,
int buf_len,
IPEndPoint* address) {
SbSocketAddress sb_address;
int bytes_transferred =
SbSocketReceiveFrom(socket_, buf->data(), buf_len, &sb_address);
int bytes_transferred = SbSocketReceiveFrom(socket_, buf->data(), buf_len,
address ? &sb_address : nullptr);
int result;
if (bytes_transferred >= 0) {
result = bytes_transferred;
// Passing in NULL address is allowed. This is only to align with other
// platform's implementation.
if (address && !address->FromSbSocketAddress(&sb_address)) {
result = ERR_ADDRESS_INVALID;
} else if (bytes_transferred == buf_len) {
} else if (bytes_transferred >= buf_len) {
result = ERR_MSG_TOO_BIG;
}
} else {
Expand All @@ -477,8 +487,9 @@ int UDPSocketStarboard::InternalRecvFrom(IOBuffer* buf,

if (result != ERR_IO_PENDING) {
IPEndPoint log_address;
if (result < 0 || !log_address.FromSbSocketAddress(&sb_address)) {
LogRead(result, buf->data(), NULL);
if (result < 0 || !address ||
!log_address.FromSbSocketAddress(&sb_address)) {
LogRead(result, buf->data(), nullptr);
} else {
LogRead(result, buf->data(), &log_address);
}
Expand All @@ -491,19 +502,25 @@ int UDPSocketStarboard::InternalSendTo(IOBuffer* buf,
int buf_len,
const IPEndPoint* address) {
SbSocketAddress sb_address;
if (!address || !address->ToSbSocketAddress(&sb_address)) {
if (address && !address->ToSbSocketAddress(&sb_address)) {
int result = ERR_FAILED;
LogWrite(result, NULL, NULL);
return result;
}

int result = SbSocketSendTo(socket_, buf->data(), buf_len, &sb_address);
int result = SbSocketSendTo(socket_, buf->data(), buf_len,
address ? &sb_address : nullptr);

if (result < 0)
result = MapLastSocketError(socket_);

if (result != ERR_IO_PENDING)
LogWrite(result, buf->data(), address);
if (result != ERR_IO_PENDING) {
if (!address) {
LogWrite(result, buf->data(), nullptr);
} else {
LogWrite(result, buf->data(), address);
}
}

return result;
}
Expand Down
6 changes: 0 additions & 6 deletions starboard/shared/posix/socket_send_to.cc
Original file line number Diff line number Diff line change
Expand Up @@ -57,12 +57,6 @@ int SbSocketSendTo(SbSocket socket,
socket->error = sbposix::TranslateSocketErrno(errno);
return -1;
} else if (socket->protocol == kSbSocketProtocolUdp) {
if (!destination) {
SB_LOG(FATAL) << "No destination passed to UDP send.";
socket->error = kSbSocketErrorFailed;
return -1;
}

sbposix::SockAddr sock_addr;
const sockaddr* sockaddr = NULL;
socklen_t sockaddr_length = 0;
Expand Down

0 comments on commit d955af2

Please sign in to comment.