Made slight optimizations in Sandbox2's comms.

The optimizations are:
* Reduced the number of calls to `write` (originating from `SendTLV()`) from 3 to 1-2 (depending on size of the payload).
* Reduced the number of calls to `read()` (originating from `RecvTLV()`) from 3 to 2.

PiperOrigin-RevId: 561750509
Change-Id: I81bc092edf602e12c85ee97bd2e77b587b750d65
This commit is contained in:
Jaeden Quintana 2023-08-31 13:49:41 -07:00 committed by Copybara-Service
parent 2c9ac02b68
commit e23acfd7e7
2 changed files with 35 additions and 20 deletions

View File

@ -242,19 +242,26 @@ bool Comms::SendTLV(uint32_t tag, size_t length, const void* value) {
SAPI_RAW_VLOG(3, "Sending a TLV message, tag: 0x%08x, length: %zu", tag, SAPI_RAW_VLOG(3, "Sending a TLV message, tag: 0x%08x, length: %zu", tag,
length); length);
{
absl::MutexLock lock(&tlv_send_transmission_mutex_); // To maintain consistency with `RecvTL()`, we wrap `tag` and `length` in a TL
if (!Send(&tag, sizeof(tag))) { // struct.
return false; const InternalTLV tl = {
} .tag = tag,
if (!Send(&length, sizeof(length))) { .len = length,
return false; };
}
if (length > 0 && !Send(value, length)) { absl::MutexLock lock(&tlv_send_transmission_mutex_);
if (length + sizeof(tl) > kSendTLVTempBufferSize) {
if (!Send(&tl, sizeof(tl))) {
return false; return false;
} }
return Send(value, length);
} }
return true; uint8_t tlv[kSendTLVTempBufferSize];
memcpy(tlv, &tl, sizeof(tl));
memcpy(reinterpret_cast<uint8_t*>(tlv) + sizeof(tl), value, length);
return Send(&tlv, sizeof(tl) + length);
} }
bool Comms::RecvString(std::string* v) { bool Comms::RecvString(std::string* v) {
@ -573,14 +580,13 @@ bool Comms::Recv(void* data, size_t len) {
// Internal helper method (low level). // Internal helper method (low level).
bool Comms::RecvTL(uint32_t* tag, size_t* length) { bool Comms::RecvTL(uint32_t* tag, size_t* length) {
if (!Recv(reinterpret_cast<uint8_t*>(tag), sizeof(*tag))) { InternalTLV tl;
SAPI_RAW_VLOG(2, "RecvTL: Can't read tag"); if (!Recv(reinterpret_cast<uint8_t*>(&tl), sizeof(tl))) {
return false; SAPI_RAW_VLOG(2, "RecvTL: Can't read tag and length");
}
if (!Recv(reinterpret_cast<uint8_t*>(length), sizeof(*length))) {
SAPI_RAW_VLOG(2, "RecvTL: Can't read length for tag %u", *tag);
return false; return false;
} }
*tag = tl.tag;
*length = tl.len;
if (*length > GetMaxMsgSize()) { if (*length > GetMaxMsgSize()) {
SAPI_RAW_LOG(ERROR, "Maximum TLV message size exceeded: (%zu > %zd)", SAPI_RAW_LOG(ERROR, "Maximum TLV message size exceeded: (%zu > %zd)",
*length, GetMaxMsgSize()); *length, GetMaxMsgSize());
@ -622,6 +628,7 @@ bool Comms::RecvTLVGeneric(uint32_t* tag, T* value) {
bool Comms::RecvTLV(uint32_t* tag, size_t* length, void* buffer, bool Comms::RecvTLV(uint32_t* tag, size_t* length, void* buffer,
size_t buffer_size) { size_t buffer_size) {
absl::MutexLock lock(&tlv_recv_transmission_mutex_); absl::MutexLock lock(&tlv_recv_transmission_mutex_);
if (!RecvTL(tag, length)) { if (!RecvTL(tag, length)) {
return false; return false;
} }

View File

@ -84,6 +84,12 @@ class Comms {
// sandbox2::Comms object at the server-side). // sandbox2::Comms object at the server-side).
static constexpr int kSandbox2ClientCommsFD = 1023; static constexpr int kSandbox2ClientCommsFD = 1023;
// Within SendTLV, a stack-allocated buffer is created to contiguously store
// the TLV in order to perform one call to Send.
// If the TLV is larger than the size below, two calls to Send are
// used.
static constexpr size_t kSendTLVTempBufferSize = 1024;
static constexpr DefaultConnectionTag kDefaultConnection = {}; static constexpr DefaultConnectionTag kDefaultConnection = {};
static constexpr const char* kSandbox2CommsFDEnvVar = "SANDBOX2_COMMS_FD"; static constexpr const char* kSandbox2CommsFDEnvVar = "SANDBOX2_COMMS_FD";
@ -229,12 +235,14 @@ class Comms {
// State of the channel (enum), socket will have to be connected later on. // State of the channel (enum), socket will have to be connected later on.
State state_ = State::kUnconnected; State state_ = State::kUnconnected;
// Special struct for passing credentials or FDs. Different from the one above // Special struct for passing credentials or FDs.
// as it inlines the value. This is important as the data is transmitted using // When passing credentials or FDs, it inlines the value. This is important as
// sendmsg/recvmsg instead of send/recv. // the data is transmitted using sendmsg/recvmsg instead of send/recv.
// It is also used when sending/receiving through SendTLV/RecvTLV to reduce
// writes/reads, although the value is written/read separately.
struct ABSL_ATTRIBUTE_PACKED InternalTLV { struct ABSL_ATTRIBUTE_PACKED InternalTLV {
uint32_t tag; uint32_t tag;
uint32_t len; size_t len;
}; };
// Fills sockaddr_un struct with proper values. // Fills sockaddr_un struct with proper values.