Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make bpf2c maps parser match verifier's maps parser #2543

Merged
merged 16 commits into from
Jun 28, 2023
Prev Previous commit
Next Next commit
Cleanup
Signed-off-by: Dave Thaler <dthaler@microsoft.com>
  • Loading branch information
dthaler committed Jun 28, 2023
commit 36976baa83d3f1cb675c67790f96a3356e260766
14 changes: 10 additions & 4 deletions libs/api/Verifier.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -140,17 +140,23 @@ _parse_btf_map_info_and_populate_cache(const ELFIO::elfio& reader, const vector<

// For each map in map_names, find the corresponding map descriptor and cache the map handle.
for (auto& entry : map_names) {
auto& btf_map_descriptor = btf_map_descriptors[btf_map_name_to_index[entry.map_name]];
auto pin_type = _get_pin_type_for_btf_map(btf_data, btf_map_descriptor.original_fd);
uint32_t idx = (uint32_t)btf_map_name_to_index[entry.map_name];
auto& btf_map_descriptor = btf_map_descriptors[idx];
// We temporarily stored BTF type ids in the descriptor's fd fields.
int btf_type_id = btf_map_descriptor.original_fd;
int btf_inner_type_id = btf_map_descriptor.inner_map_fd;

auto pin_type = _get_pin_type_for_btf_map(btf_data, btf_type_id);
cache_map_handle(
ebpf_handle_invalid,
btf_map_descriptor.original_fd, // Actually the BTF ID.
map_idx_to_original_fd(idx),
btf_type_id,
btf_map_descriptor.type,
btf_map_descriptor.key_size,
btf_map_descriptor.value_size,
btf_map_descriptor.max_entries,
(uint32_t)ebpf_fd_invalid,
btf_map_descriptor.inner_map_fd, // Actually the inner map's BTF ID.
btf_inner_type_id,
entry.section_offset,
pin_type);
}
Expand Down
2 changes: 1 addition & 1 deletion libs/api/ebpf_api.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2008,7 +2008,7 @@ _ebpf_pe_get_map_definitions(
map->map_definition.max_entries = entry->definition.max_entries;
map->map_definition.pinning = entry->definition.pinning;
map->map_definition.inner_map_id = entry->definition.inner_id;
map->inner_map_original_fd = map_idx_to_verifier_fd(entry->definition.inner_map_idx);
map->inner_map_original_fd = map_idx_to_original_fd(entry->definition.inner_map_idx);
map->pinned = false;
map->reused = false;
map->pin_path = nullptr;
Expand Down
18 changes: 10 additions & 8 deletions libs/api/windows_platform.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,14 @@
#include <stdexcept>

static bool
is_map_of_maps(ebpf_map_type_t type)
_is_map_of_maps(ebpf_map_type_t type)
{
return (type == BPF_MAP_TYPE_ARRAY_OF_MAPS || type == BPF_MAP_TYPE_HASH_OF_MAPS);
}

// Parse a legacy (non-BTF) maps section.
static void
parse_maps_section_windows(
_parse_maps_section_windows(
std::vector<EbpfMapDescriptor>& verifier_map_descriptors,
const char* data,
size_t map_record_size,
Expand All @@ -42,10 +42,12 @@ parse_maps_section_windows(
ebpf_map_definition_in_file_t s{};
memcpy(&s, data + section_offset, std::min(sizeof(s), map_record_size));

fd_t inner_map_original_fd = is_map_of_maps(s.type) ? map_idx_to_verifier_fd(s.inner_map_idx) : ebpf_fd_invalid;
fd_t inner_map_original_fd =
_is_map_of_maps(s.type) ? map_idx_to_original_fd(s.inner_map_idx) : ebpf_fd_invalid;

cache_map_handle(
ebpf_handle_invalid,
map_idx_to_original_fd(i),
s.id,
s.type,
s.key_size,
Expand All @@ -59,14 +61,14 @@ parse_maps_section_windows(
}

static void
resolve_inner_map_references_windows(std::vector<EbpfMapDescriptor>& verifier_map_descriptors)
_resolve_inner_map_references_windows(std::vector<EbpfMapDescriptor>& verifier_map_descriptors)
{
auto map_descriptors = get_all_map_descriptors();
for (auto& map_descriptor : map_descriptors) {
// Resolve the inner map original fd.
unsigned int inner_map_original_fd = UINT_MAX;
if (is_map_of_maps((ebpf_map_type_t)map_descriptor.verifier_map_descriptor.type)) {
uint32_t inner_map_idx = verifier_fd_to_map_idx(map_descriptor.verifier_map_descriptor.inner_map_fd);
if (_is_map_of_maps((ebpf_map_type_t)map_descriptor.verifier_map_descriptor.type)) {
uint32_t inner_map_idx = original_fd_to_map_idx(map_descriptor.verifier_map_descriptor.inner_map_fd);
if (map_descriptor.inner_id != EBPF_ID_NONE) {
for (auto& map_descriptor2 : map_descriptors) {
if (map_descriptor2.id == map_descriptor.inner_id) {
Expand Down Expand Up @@ -96,7 +98,7 @@ const ebpf_platform_t g_ebpf_platform_windows = {
get_helper_prototype_windows,
is_helper_usable_windows,
sizeof(ebpf_map_definition_in_file_t),
parse_maps_section_windows,
_parse_maps_section_windows,
get_map_descriptor_windows,
get_map_type_windows,
resolve_inner_map_references_windows};
_resolve_inner_map_references_windows};
16 changes: 10 additions & 6 deletions libs/api_common/api_common.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,18 +16,18 @@

// The map file descriptors that appear in eBPF bytecode start at 1,
// in the order the maps appear in the maps sections.
const int VERIFIER_FD_OFFSET = 1;
const int ORIGINAL_FD_OFFSET = 1;

inline fd_t
map_idx_to_verifier_fd(uint32_t idx)
map_idx_to_original_fd(uint32_t idx)
{
return idx + VERIFIER_FD_OFFSET;
return idx + ORIGINAL_FD_OFFSET;
}

inline uint32_t
verifier_fd_to_map_idx(fd_t fd)
original_fd_to_map_idx(fd_t fd)
{
return fd - VERIFIER_FD_OFFSET;
return fd - ORIGINAL_FD_OFFSET;
}

typedef struct _map_cache
Expand All @@ -39,7 +39,10 @@ typedef struct _map_cache
ebpf_pin_type_t pinning;
uint32_t inner_id;

_map_cache() : handle(0), section_offset(0), verifier_map_descriptor(), pinning(PIN_NONE) {}
_map_cache()
: handle(0), id(EBPF_ID_NONE), section_offset(0), verifier_map_descriptor(), pinning(PIN_NONE),
inner_id(EBPF_ID_NONE)
{}

_map_cache(ebpf_handle_t handle, size_t section_offset, EbpfMapDescriptor descriptor, ebpf_pin_type_t pinning)
: handle(handle), section_offset(section_offset), verifier_map_descriptor(descriptor), pinning(pinning)
Expand Down Expand Up @@ -82,6 +85,7 @@ get_file_size(const char* filename, size_t* byte_code_size) noexcept;
void
cache_map_handle(
ebpf_handle_t handle,
uint32_t original_fd,
uint32_t id,
uint32_t type,
uint32_t key_size,
Expand Down
3 changes: 1 addition & 2 deletions libs/api_common/map_descriptors.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,7 @@ cache_map_original_file_descriptor_with_handle(
void
cache_map_handle(
ebpf_handle_t handle,
uint32_t original_fd,
uint32_t id,
uint32_t type,
uint32_t key_size,
Expand All @@ -130,8 +131,6 @@ cache_map_handle(
size_t section_offset,
ebpf_pin_type_t pinning)
{
fd_t original_fd = map_idx_to_verifier_fd((uint32_t)_map_file_descriptors.size());

_map_file_descriptors.emplace_back(
handle,
(id ? id : EBPF_ID_NONE),
Expand Down
6 changes: 3 additions & 3 deletions tests/bpf2c_tests/elf_bpf.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -224,9 +224,9 @@ DECLARE_TEST("invalid_maps1", _test_mode::NoVerify);
DECLARE_TEST("invalid_maps2", _test_mode::NoVerify);
DECLARE_TEST("invalid_maps3", _test_mode::NoVerify);
DECLARE_TEST("map", _test_mode::NoVerify)
DECLARE_TEST("map_in_map", _test_mode::Verify)
DECLARE_TEST("map_in_map_legacy", _test_mode::Verify)
DECLARE_TEST("map_in_map_v2", _test_mode::Verify)
DECLARE_TEST("map_in_map_btf", _test_mode::Verify)
DECLARE_TEST("map_in_map_legacy_id", _test_mode::Verify)
DECLARE_TEST("map_in_map_legacy_idx", _test_mode::Verify)
DECLARE_TEST("map_reuse", _test_mode::Verify)
DECLARE_TEST("map_reuse_2", _test_mode::Verify)
DECLARE_TEST("pidtgid", _test_mode::Verify)
Expand Down
33 changes: 23 additions & 10 deletions tools/bpf2c/bpf_code_generator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -357,9 +357,17 @@ bpf_code_generator::extract_program(const bpf_code_generator::unsafe_string& sec
}
}

// BTF maps sections are identified as any section called ".maps".
// PREVAIL does not support multiple BTF map sections.
static bool
_is_btf_map_section(const std::string& name)
{
return name == ".maps";
}

// Legacy (non-BTF) maps sections are identified as any section called "maps", or matching "maps/<map-name>".
static bool
is_legacy_map_section(const std::string& name)
_is_legacy_map_section(const std::string& name)
{
std::string maps_prefix = "maps/";
return name == "maps" || (name.length() > 5 && name.compare(0, maps_prefix.length(), maps_prefix) == 0);
Expand Down Expand Up @@ -406,11 +414,11 @@ bpf_code_generator::visit_symbols(symbol_visitor_t visitor, const unsafe_string&
}
}

// Parse a BTF maps section.
void
bpf_code_generator::parse()
bpf_code_generator::parse_btf_maps_section(const unsafe_string& name)
{
// Parse the new .maps section if it exists.
auto map_section = get_optional_section(".maps");
auto map_section = get_optional_section(name);
if (map_section) {
auto btf_section = get_required_section(".BTF");
btf_type_data data =
Expand All @@ -434,7 +442,7 @@ bpf_code_generator::parse()
map_names_by_offset[symbol_value] = unsafe_symbol_name;
}
},
".maps");
name);

for (const auto& [offset, unsafe_symbol_name] : map_names_by_offset) {
if (map_name_to_index.find(unsafe_symbol_name.raw()) == map_name_to_index.end()) {
Expand Down Expand Up @@ -471,19 +479,24 @@ bpf_code_generator::parse()
map_definitions[unsafe_symbol_name] = {map_definition, index++};
}
}
}

// Parse any older maps sections.
// Parse all maps sections.
// Parse global data (currently map information) in the eBPF file.
void
bpf_code_generator::parse()
{
for (auto& section : reader.sections) {
std::string name = section->get_name();
if (is_legacy_map_section(name)) {
if (_is_btf_map_section(name)) {
parse_btf_maps_section(name);
} else if (_is_legacy_map_section(name)) {
parse_legacy_maps_section(name);
}
}
}

static std::tuple<std::string, ELFIO::Elf_Half>
get_symbol_name_and_section_index(ELFIO::const_symbol_section_accessor& symbols, ELFIO::Elf_Xword index)
_get_symbol_name_and_section_index(ELFIO::const_symbol_section_accessor& symbols, ELFIO::Elf_Xword index)
{
std::string symbol_name;
ELFIO::Elf64_Addr value{};
Expand All @@ -510,7 +523,7 @@ bpf_code_generator::parse_legacy_maps_section(const unsafe_string& name)
ELFIO::const_symbol_section_accessor symbols{reader, get_required_section(".symtab")};
int map_count = 0;
for (ELFIO::Elf_Xword index = 0; index < symbols.get_symbols_num(); index++) {
auto [symbol_name, section_index] = get_symbol_name_and_section_index(symbols, index);
auto [symbol_name, section_index] = _get_symbol_name_and_section_index(symbols, index);
if ((section_index == map_section->get_index()) && !symbol_name.empty()) {
map_count++;
}
Expand Down
10 changes: 9 additions & 1 deletion tools/bpf2c/bpf_code_generator.h
Original file line number Diff line number Diff line change
Expand Up @@ -240,10 +240,18 @@ class bpf_code_generator
void
parse();

/**
* @brief Parse BTF map information in the eBPF file.
*
* @param[in] section_name Section in the ELF file to parse maps in.
*/
void
parse_btf_maps_section(const unsafe_string& name);

/**
* @brief Parse legacy map information in the eBPF file.
*
* @param[in] section_name Section in the ELF file to parse legacy maps in.
* @param[in] section_name Section in the ELF file to parse maps in.
*/
void
parse_legacy_maps_section(const unsafe_string& name);
Expand Down