BpfMap: shorten error strings to enable SSO (on 64-bit)
This should be a performance improvement, as it avoids both memory allocation (Short String Optimization fits the strings on the heap) and string formatting. Additionally should prevent us from triggering an abort() on OOM as reported on the linked bug: F DEBUG : signal 11 (SIGSEGV), code 1 (SEGV_MAPERR), fault addr 0x0000000000000000 (read) F DEBUG : Cause: null pointer dereference F DEBUG : backtrace: F DEBUG : #00 pc 0000000000000000 <unknown> F DEBUG : #01 pc 000000000001d008 /apex/com.android.tethering/lib64/libbase.so (fmt::v11::vformat(fmt::v11::basic_string_view<char>, fmt::v11::basic_format_args<fmt::v11::context>)+76) (BuildId: d200bca0044b833c9a13a387041b398a) F DEBUG : #02 pc 0000000000007108 /apex/com.android.tethering/lib64/libcom.android.tethering.dns_helper.so (android::bpf::BpfMapRO<unsigned int, UidOwnerValue>::readValue(unsigned int) const+152) (BuildId: b20b032f10bd31bb44f969cef38dcd99) F DEBUG : #03 pc 0000000000006ac8 /apex/com.android.tethering/lib64/libcom.android.tethering.dns_helper.so (android::net::DnsBpfHelper::isUidNetworkingBlocked(unsigned int, bool)+748) (BuildId: b20b032f10bd31bb44f969cef38dcd99) F DEBUG : #04 pc 0000000000008328 /apex/com.android.tethering/lib64/libcom.android.tethering.dns_helper.so (ADnsHelper_isUidNetworkingBlocked.cfi+60) (BuildId: b20b032f10bd31bb44f969cef38dcd99) F DEBUG : #05 pc 00000000001c43b0 /apex/com.android.resolv/lib64/libnetd_resolv.so (android::net::DnsProxyListener::GetAddrInfoHandler::run()+284) (BuildId: 9f306d9e52c85087c00931289aa2d7cd) F DEBUG : #06 pc 00000000001bc888 /apex/com.android.resolv/lib64/libnetd_resolv.so (void* android::netdutils::runAndDelete<android::net::DnsProxyListener::Handler>(void*) (.cfi)+332) (BuildId: 9f306d9e52c85087c00931289aa2d7cd) F DEBUG : #07 pc 0000000000085580 /apex/com.android.runtime/lib64/bionic/libc.so (__pthread_start(void*) (.__uniq.67847048707805468364044055584648682506)+232) (BuildId: 9ea0fa19fb5cc316f79c79f922c5a7dc) F DEBUG : #08 pc 0000000000078248 /apex/com.android.runtime/lib64/bionic/libc.so (__start_thread+64) (BuildId: 9ea0fa19fb5cc316f79c79f922c5a7dc) Bug: 441416613 Test: TreeHugger Flag: EXEMPT BUGFIX Signed-off-by: Maciej Żenczykowski <maze@google.com> Change-Id: Ibdae61b0ec67fd77f0be096ff934c8d424ae1e49
This commit is contained in:
parent
4666361ca8
commit
8f637b873c
1 changed files with 21 additions and 3 deletions
|
|
@ -33,6 +33,7 @@ namespace android {
|
|||
namespace bpf {
|
||||
|
||||
using base::Result;
|
||||
using base::ResultError;
|
||||
using base::unique_fd;
|
||||
using std::function;
|
||||
|
||||
|
|
@ -61,7 +62,24 @@ void Abort(int __unused error, const char* __unused fmt, ...) {
|
|||
abort();
|
||||
}
|
||||
|
||||
#define ERROR_FROM_ERRNO(f) ErrnoErrorf("BpfMap::" f "() failed")
|
||||
// We care about enabling SSO on 64-bit platforms
|
||||
// It turns out that our string implementation can SSO optimize 22 characters + (23rd) trailing NULL
|
||||
//
|
||||
// 32-bit userspace is obsolete, and SSO would require shortening the errors more than is reasonable
|
||||
//
|
||||
// For the curious: the initial u8 is a 7+1 bitfield and stores strlen * 2 + (is_short_flag=0),
|
||||
// though newer versions (_LIBCPP_ABI_VERSION >= 2), reverse the order
|
||||
#ifndef __ILP32__
|
||||
static_assert(sizeof(std::string) == 24);
|
||||
#endif
|
||||
|
||||
// Our string implementation, on 64-bit platforms can fit up to 22 characters with SSO
|
||||
// The common case (ENOENT) shouldn't require heap memory allocation nor string formatting
|
||||
// Other errors will return (string argument passed to ErrnoErrorf) + ": " + strerror(errno)
|
||||
#define ERROR_FROM_ERRNO(f) ({ \
|
||||
static_assert(__builtin_strlen(f ": ENOENT") <= 22, "ERROR_FROM_ERRNO - arg string too long"); \
|
||||
(errno == ENOENT) ? ResultError(f ": ENOENT", ENOENT) : ErrnoErrorf("BpfMap::" f "() failed"); \
|
||||
})
|
||||
|
||||
// This is a class wrapper for eBPF maps. The eBPF map is a special in-kernel
|
||||
// data structure that stores data in <Key, Value> pairs. It can be read/write
|
||||
|
|
@ -179,7 +197,7 @@ class BpfMapRO {
|
|||
uint32_t count = N; // how many to fetch (and possibly delete)
|
||||
int rv = batchLookupAndMaybeDelete(mMapFd, first ? NULL : &batch, &batch, &keys, &values, &count, del);
|
||||
if (rv && errno == ENOSPC) break; // not enough space for full HASH bucket, go around the *outer* loop
|
||||
if (rv && errno != ENOENT) return ERROR_FROM_ERRNO("doBulkLookupAndMaybeDelete");
|
||||
if (rv && errno != ENOENT) return ERROR_FROM_ERRNO("bulkLookup&Del");
|
||||
// count is now how many *were* fetched (and possibly delete)
|
||||
for (unsigned i = 0; i < count; ++i) f(keys[i], values[i]);
|
||||
if (rv) return {}; // ENOENT -> success
|
||||
|
|
@ -364,7 +382,7 @@ class BpfMap : public BpfMapRW<Key, Value> {
|
|||
if (isAtLeastKernelVersion(5, 4, 0)) {
|
||||
Value value;
|
||||
if (!findAndDeleteMapEntry(mMapFd, &key, &value)) return value;
|
||||
if (errno == ENOENT) return ERROR_FROM_ERRNO("readAndDeleteValue");
|
||||
if (errno == ENOENT) return ERROR_FROM_ERRNO("read&DeleteVal");
|
||||
};
|
||||
|
||||
// fallback path in case of weird error and for pre-5.4 kernels
|
||||
|
|
|
|||
Loading…
Reference in a new issue