Skip to content

Commit

Permalink
[p2p] Remove traffic annotation from send IPC
Browse files Browse the repository at this point in the history
`NetworkTrafficAnnotationTag` is constant for whole p2p session.
It is not necessary that `Send()` IPC carries the constant tag for every
packet.
Move it as a constant property, to reduce p2p IPC payload.

Bug: 1376527
Change-Id: I9842a22f226c520b9bdc23417bb1244b8dc98d1c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4049888
Reviewed-by: Crisrael Lucero <crisrael@google.com>
Reviewed-by: Joe Mason <joenotcharles@google.com>
Commit-Queue: Jianhui J Dai <jianhui.j.dai@intel.com>
Reviewed-by: Florent Castelli <orphis@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1076597}
  • Loading branch information
daijh authored and Chromium LUCI CQ committed Nov 29, 2022
1 parent b2c7c0f commit be6b093
Show file tree
Hide file tree
Showing 14 changed files with 180 additions and 278 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -47,15 +47,17 @@ class MockWebRtcDependencies : public network::mojom::P2PSocketManager,
network::mojom::P2PSocketManager::GetHostAddressWithFamilyCallback
callback),
(override));
MOCK_METHOD(void,
CreateSocket,
(network::P2PSocketType type,
const net::IPEndPoint& local_address,
const network::P2PPortRange& port_range,
const network::P2PHostAndIPEndPoint& remote_address,
mojo::PendingRemote<network::mojom::P2PSocketClient> client,
mojo::PendingReceiver<network::mojom::P2PSocket> receiver),
(override));
MOCK_METHOD(
void,
CreateSocket,
(network::P2PSocketType type,
const net::IPEndPoint& local_address,
const network::P2PPortRange& port_range,
const network::P2PHostAndIPEndPoint& remote_address,
const net::MutableNetworkTrafficAnnotationTag& traffic_annotation,
mojo::PendingRemote<network::mojom::P2PSocketClient> client,
mojo::PendingReceiver<network::mojom::P2PSocket> receiver),
(override));

// sharing::mojom::MdnsResponderFactory overrides:
MOCK_METHOD(
Expand Down
7 changes: 4 additions & 3 deletions chrome/services/sharing/webrtc/p2p_socket_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,9 @@ void P2PSocketClient::Init(network::P2PSocketType type,
state_ = STATE_OPENING;
socket_manager_->CreateSocket(
type, local_address, network::P2PPortRange(min_port, max_port),
remote_address, receiver_.BindNewPipeAndPassRemote(),
remote_address,
net::MutableNetworkTrafficAnnotationTag(traffic_annotation_),
receiver_.BindNewPipeAndPassRemote(),
socket_.BindNewPipeAndPassReceiver());
receiver_.set_disconnect_handler(base::BindOnce(
&P2PSocketClient::OnConnectionError, base::Unretained(this)));
Expand All @@ -81,8 +83,7 @@ void P2PSocketClient::SendWithPacketId(const net::IPEndPoint& address,
base::span<const uint8_t> data,
const rtc::PacketOptions& options,
uint64_t packet_id) {
socket_->Send(data, network::P2PPacketInfo(address, options, packet_id),
net::MutableNetworkTrafficAnnotationTag(traffic_annotation_));
socket_->Send(data, network::P2PPacketInfo(address, options, packet_id));
}

void P2PSocketClient::SetOption(network::P2PSocketOption option, int value) {
Expand Down
18 changes: 10 additions & 8 deletions services/network/p2p/socket.cc
Original file line number Diff line number Diff line change
Expand Up @@ -141,26 +141,28 @@ std::unique_ptr<P2PSocket> P2PSocket::Create(
mojo::PendingRemote<mojom::P2PSocketClient> client,
mojo::PendingReceiver<mojom::P2PSocket> socket,
P2PSocketType type,
const net::NetworkTrafficAnnotationTag& traffic_annotation,
net::NetLog* net_log,
ProxyResolvingClientSocketFactory* proxy_resolving_socket_factory,
P2PMessageThrottler* throttler) {
switch (type) {
case P2P_SOCKET_UDP:
return std::make_unique<P2PSocketUdp>(
delegate, std::move(client), std::move(socket), throttler, net_log);
return std::make_unique<P2PSocketUdp>(delegate, std::move(client),
std::move(socket), throttler,
traffic_annotation, net_log);
case P2P_SOCKET_TCP_CLIENT:
case P2P_SOCKET_SSLTCP_CLIENT:
case P2P_SOCKET_TLS_CLIENT:
return std::make_unique<P2PSocketTcp>(delegate, std::move(client),
std::move(socket), type,
proxy_resolving_socket_factory);
return std::make_unique<P2PSocketTcp>(
delegate, std::move(client), std::move(socket), type,
traffic_annotation, proxy_resolving_socket_factory);

case P2P_SOCKET_STUN_TCP_CLIENT:
case P2P_SOCKET_STUN_SSLTCP_CLIENT:
case P2P_SOCKET_STUN_TLS_CLIENT:
return std::make_unique<P2PSocketStunTcp>(delegate, std::move(client),
std::move(socket), type,
proxy_resolving_socket_factory);
return std::make_unique<P2PSocketStunTcp>(
delegate, std::move(client), std::move(socket), type,
traffic_annotation, proxy_resolving_socket_factory);
}

NOTREACHED();
Expand Down
1 change: 1 addition & 0 deletions services/network/p2p/socket.h
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) P2PSocket : public mojom::P2PSocket {
mojo::PendingRemote<mojom::P2PSocketClient> client,
mojo::PendingReceiver<mojom::P2PSocket> socket,
P2PSocketType type,
const net::NetworkTrafficAnnotationTag& traffic_annotation,
net::NetLog* net_log,
ProxyResolvingClientSocketFactory* proxy_resolving_socket_factory,
P2PMessageThrottler* throttler);
Expand Down
2 changes: 2 additions & 0 deletions services/network/p2p/socket_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -347,6 +347,7 @@ void P2PSocketManager::CreateSocket(
const net::IPEndPoint& local_address,
const P2PPortRange& port_range,
const P2PHostAndIPEndPoint& remote_address,
const net::MutableNetworkTrafficAnnotationTag& traffic_annotation,
mojo::PendingRemote<mojom::P2PSocketClient> client,
mojo::PendingReceiver<mojom::P2PSocket> receiver) {
if (port_range.min_port > port_range.max_port ||
Expand All @@ -366,6 +367,7 @@ void P2PSocketManager::CreateSocket(
}
std::unique_ptr<P2PSocket> socket =
P2PSocket::Create(this, std::move(client), std::move(receiver), type,
net::NetworkTrafficAnnotationTag(traffic_annotation),
url_request_context_->net_log(),
proxy_resolving_socket_factory_.get(), &throttler_);

Expand Down
14 changes: 8 additions & 6 deletions services/network/p2p/socket_manager.h
Original file line number Diff line number Diff line change
Expand Up @@ -109,12 +109,14 @@ class P2PSocketManager
int address_family,
bool enable_mdns,
mojom::P2PSocketManager::GetHostAddressCallback callback) override;
void CreateSocket(P2PSocketType type,
const net::IPEndPoint& local_address,
const P2PPortRange& port_range,
const P2PHostAndIPEndPoint& remote_address,
mojo::PendingRemote<mojom::P2PSocketClient> client,
mojo::PendingReceiver<mojom::P2PSocket> receiver) override;
void CreateSocket(
P2PSocketType type,
const net::IPEndPoint& local_address,
const P2PPortRange& port_range,
const P2PHostAndIPEndPoint& remote_address,
const net::MutableNetworkTrafficAnnotationTag& traffic_annotation,
mojo::PendingRemote<mojom::P2PSocketClient> client,
mojo::PendingReceiver<mojom::P2PSocket> receiver) override;

// mojom::P2PTrustedSocketManager overrides:
void StartRtpDump(bool incoming, bool outgoing) override;
Expand Down
46 changes: 20 additions & 26 deletions services/network/p2p/socket_tcp.cc
Original file line number Diff line number Diff line change
Expand Up @@ -50,11 +50,8 @@ bool IsPseudoTlsClientSocket(P2PSocketType type) {
P2PSocketTcp::SendBuffer::SendBuffer() : rtc_packet_id(-1) {}
P2PSocketTcp::SendBuffer::SendBuffer(
int32_t rtc_packet_id,
scoped_refptr<net::DrainableIOBuffer> buffer,
const net::NetworkTrafficAnnotationTag traffic_annotation)
: rtc_packet_id(rtc_packet_id),
buffer(buffer),
traffic_annotation(traffic_annotation) {}
scoped_refptr<net::DrainableIOBuffer> buffer)
: rtc_packet_id(rtc_packet_id), buffer(buffer) {}
P2PSocketTcp::SendBuffer::SendBuffer(const SendBuffer& rhs) = default;
P2PSocketTcp::SendBuffer::~SendBuffer() = default;

Expand All @@ -63,9 +60,11 @@ P2PSocketTcpBase::P2PSocketTcpBase(
mojo::PendingRemote<mojom::P2PSocketClient> client,
mojo::PendingReceiver<mojom::P2PSocket> socket,
P2PSocketType type,
const net::NetworkTrafficAnnotationTag& traffic_annotation,
ProxyResolvingClientSocketFactory* proxy_resolving_socket_factory)
: P2PSocket(delegate, std::move(client), std::move(socket), P2PSocket::TCP),
type_(type),
traffic_annotation_(traffic_annotation),
proxy_resolving_socket_factory_(proxy_resolving_socket_factory) {}

P2PSocketTcpBase::~P2PSocketTcpBase() = default;
Expand Down Expand Up @@ -267,7 +266,7 @@ void P2PSocketTcpBase::DoWrite() {
int result = socket_->Write(
write_buffer_.buffer.get(), write_buffer_.buffer->BytesRemaining(),
base::BindOnce(&P2PSocketTcp::OnWritten, base::Unretained(this)),
net::NetworkTrafficAnnotationTag(write_buffer_.traffic_annotation));
traffic_annotation_);

if (result == net::ERR_IO_PENDING) {
write_pending_ = true;
Expand Down Expand Up @@ -350,10 +349,8 @@ bool P2PSocketTcpBase::HandleReadResult(int result) {
return true;
}

void P2PSocketTcpBase::Send(
base::span<const uint8_t> data,
const P2PPacketInfo& packet_info,
const net::MutableNetworkTrafficAnnotationTag& traffic_annotation) {
void P2PSocketTcpBase::Send(base::span<const uint8_t> data,
const P2PPacketInfo& packet_info) {
// Renderer should use this socket only to send data to |remote_address_|.
if (data.size() > kMaximumPacketSize ||
!(packet_info.destination == remote_address_.ip_address)) {
Expand All @@ -374,8 +371,7 @@ void P2PSocketTcpBase::Send(
}
}

DoSend(packet_info.destination, data, packet_info.packet_options,
net::NetworkTrafficAnnotationTag(traffic_annotation));
DoSend(packet_info.destination, data, packet_info.packet_options);
}

void P2PSocketTcpBase::SetOption(P2PSocketOption option, int32_t value) {
Expand All @@ -399,11 +395,13 @@ P2PSocketTcp::P2PSocketTcp(
mojo::PendingRemote<mojom::P2PSocketClient> client,
mojo::PendingReceiver<mojom::P2PSocket> socket,
P2PSocketType type,
const net::NetworkTrafficAnnotationTag& traffic_annotation,
ProxyResolvingClientSocketFactory* proxy_resolving_socket_factory)
: P2PSocketTcpBase(delegate,
std::move(client),
std::move(socket),
type,
traffic_annotation,
proxy_resolving_socket_factory) {
DCHECK(type == P2P_SOCKET_TCP_CLIENT || type == P2P_SOCKET_SSLTCP_CLIENT ||
type == P2P_SOCKET_TLS_CLIENT);
Expand All @@ -427,17 +425,14 @@ bool P2PSocketTcp::ProcessInput(base::span<const uint8_t> input,
return OnPacket(input.subspan(kPacketHeaderSize, packet_size));
}

void P2PSocketTcp::DoSend(
const net::IPEndPoint& to,
base::span<const uint8_t> data,
const rtc::PacketOptions& options,
const net::NetworkTrafficAnnotationTag traffic_annotation) {
void P2PSocketTcp::DoSend(const net::IPEndPoint& to,
base::span<const uint8_t> data,
const rtc::PacketOptions& options) {
int buffer_size = kPacketHeaderSize + data.size();
SendBuffer send_buffer(
options.packet_id,
base::MakeRefCounted<net::DrainableIOBuffer>(
base::MakeRefCounted<net::IOBuffer>(buffer_size), buffer_size),
traffic_annotation);
base::MakeRefCounted<net::IOBuffer>(buffer_size), buffer_size));
*reinterpret_cast<uint16_t*>(send_buffer.buffer->data()) =
base::HostToNet16(data.size());
memcpy(send_buffer.buffer->data() + kPacketHeaderSize, data.data(),
Expand All @@ -458,11 +453,13 @@ P2PSocketStunTcp::P2PSocketStunTcp(
mojo::PendingRemote<mojom::P2PSocketClient> client,
mojo::PendingReceiver<mojom::P2PSocket> socket,
P2PSocketType type,
const net::NetworkTrafficAnnotationTag& traffic_annotation,
ProxyResolvingClientSocketFactory* proxy_resolving_socket_factory)
: P2PSocketTcpBase(delegate,
std::move(client),
std::move(socket),
type,
traffic_annotation,
proxy_resolving_socket_factory) {
DCHECK(type == P2P_SOCKET_STUN_TCP_CLIENT ||
type == P2P_SOCKET_STUN_SSLTCP_CLIENT ||
Expand All @@ -488,11 +485,9 @@ bool P2PSocketStunTcp::ProcessInput(base::span<const uint8_t> input,
return OnPacket(input.subspan(0, packet_size));
}

void P2PSocketStunTcp::DoSend(
const net::IPEndPoint& to,
base::span<const uint8_t> data,
const rtc::PacketOptions& options,
const net::NetworkTrafficAnnotationTag traffic_annotation) {
void P2PSocketStunTcp::DoSend(const net::IPEndPoint& to,
base::span<const uint8_t> data,
const rtc::PacketOptions& options) {
// Each packet is expected to have header (STUN/TURN ChannelData), where
// header contains message type and and length of message.
if (data.size() < kPacketHeaderSize + kPacketLengthOffset) {
Expand All @@ -517,8 +512,7 @@ void P2PSocketStunTcp::DoSend(
SendBuffer send_buffer(
options.packet_id,
base::MakeRefCounted<net::DrainableIOBuffer>(
base::MakeRefCounted<net::IOBuffer>(buffer_size), buffer_size),
traffic_annotation);
base::MakeRefCounted<net::IOBuffer>(buffer_size), buffer_size));
memcpy(send_buffer.buffer->data(), data.data(), data.size());

cricket::ApplyPacketOptions(
Expand Down
37 changes: 15 additions & 22 deletions services/network/p2p/socket_tcp.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) P2PSocketTcpBase : public P2PSocket {
mojo::PendingRemote<mojom::P2PSocketClient> client,
mojo::PendingReceiver<mojom::P2PSocket> socket,
P2PSocketType type,
const net::NetworkTrafficAnnotationTag& traffic_annotation,
ProxyResolvingClientSocketFactory* proxy_resolving_socket_factory);

P2PSocketTcpBase(const P2PSocketTcpBase&) = delete;
Expand All @@ -57,33 +58,26 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) P2PSocketTcpBase : public P2PSocket {

// mojom::P2PSocket implementation:
void Send(base::span<const uint8_t> data,
const P2PPacketInfo& packet_info,
const net::MutableNetworkTrafficAnnotationTag& traffic_annotation)
override;
const P2PPacketInfo& packet_info) override;
void SetOption(P2PSocketOption option, int32_t value) override;

protected:
struct SendBuffer {
SendBuffer();
SendBuffer(int32_t packet_id,
scoped_refptr<net::DrainableIOBuffer> buffer,
const net::NetworkTrafficAnnotationTag traffic_annotation);
SendBuffer(int32_t packet_id, scoped_refptr<net::DrainableIOBuffer> buffer);
SendBuffer(const SendBuffer& rhs);
~SendBuffer();

int32_t rtc_packet_id;
scoped_refptr<net::DrainableIOBuffer> buffer;
net::MutableNetworkTrafficAnnotationTag traffic_annotation;
};

// Derived classes will provide the implementation.
virtual bool ProcessInput(base::span<const uint8_t> input,
size_t* bytes_consumed) = 0;
virtual void DoSend(
const net::IPEndPoint& to,
base::span<const uint8_t> data,
const rtc::PacketOptions& options,
const net::NetworkTrafficAnnotationTag traffic_annotation) = 0;
virtual void DoSend(const net::IPEndPoint& to,
base::span<const uint8_t> data,
const rtc::PacketOptions& options) = 0;

void WriteOrQueue(SendBuffer& send_buffer);
[[nodiscard]] bool OnPacket(base::span<const uint8_t> data);
Expand Down Expand Up @@ -120,6 +114,7 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) P2PSocketTcpBase : public P2PSocket {

bool connected_ = false;
const P2PSocketType type_;
const net::NetworkTrafficAnnotationTag traffic_annotation_;
raw_ptr<ProxyResolvingClientSocketFactory> proxy_resolving_socket_factory_;
};

Expand All @@ -130,6 +125,7 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) P2PSocketTcp : public P2PSocketTcpBase {
mojo::PendingRemote<mojom::P2PSocketClient> client,
mojo::PendingReceiver<mojom::P2PSocket> socket,
P2PSocketType type,
const net::NetworkTrafficAnnotationTag& traffic_annotation,
ProxyResolvingClientSocketFactory* proxy_resolving_socket_factory);

P2PSocketTcp(const P2PSocketTcp&) = delete;
Expand All @@ -140,11 +136,9 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) P2PSocketTcp : public P2PSocketTcpBase {
protected:
bool ProcessInput(base::span<const uint8_t> input,
size_t* bytes_consumed) override;
void DoSend(
const net::IPEndPoint& to,
base::span<const uint8_t> data,
const rtc::PacketOptions& options,
const net::NetworkTrafficAnnotationTag traffic_annotation) override;
void DoSend(const net::IPEndPoint& to,
base::span<const uint8_t> data,
const rtc::PacketOptions& options) override;
};

// P2PSocketStunTcp class provides the framing of STUN messages when used
Expand All @@ -159,6 +153,7 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) P2PSocketStunTcp
mojo::PendingRemote<mojom::P2PSocketClient> client,
mojo::PendingReceiver<mojom::P2PSocket> socket,
P2PSocketType type,
const net::NetworkTrafficAnnotationTag& traffic_annotation,
ProxyResolvingClientSocketFactory* proxy_resolving_socket_factory);

P2PSocketStunTcp(const P2PSocketStunTcp&) = delete;
Expand All @@ -169,11 +164,9 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) P2PSocketStunTcp
protected:
bool ProcessInput(base::span<const uint8_t> input,
size_t* bytes_consumed) override;
void DoSend(
const net::IPEndPoint& to,
base::span<const uint8_t> data,
const rtc::PacketOptions& options,
const net::NetworkTrafficAnnotationTag traffic_annotation) override;
void DoSend(const net::IPEndPoint& to,
base::span<const uint8_t> data,
const rtc::PacketOptions& options) override;

private:
int GetExpectedPacketSize(base::span<const uint8_t> data, int* pad_bytes);
Expand Down

0 comments on commit be6b093

Please sign in to comment.