fix percpu table support in c++ api

The percpu table support in the c++ api was wrong.

This commit creates two new table classes BPFPercpuArrayTable
and BPFPercpuHashTable, it also extends the BPFTable that allows
to access map using strings.
Signed-off-by: Mauricio Vasquez B <mauricio.vasquez@polito.it>

src/cc/CMakeLists.txt

@@ -37,7 +37,7 @@ set(bcc_table_sources table_storage.cc shared_table.cc bpffs_table.cc json_map_d
 set(bcc_util_sources ns_guard.cc common.cc)
 set(bcc_sym_sources bcc_syms.cc bcc_elf.c bcc_perf_map.c bcc_proc.c)
 set(bcc_common_headers libbpf.h perf_reader.h)
-set(bcc_table_headers file_desc.h table_desc.h table_storage.h)
+set(bcc_table_headers common.h file_desc.h table_desc.h table_storage.h)
 set(bcc_api_headers bpf_common.h bpf_module.h bcc_exception.h bcc_syms.h)
 
 if(ENABLE_CLANG_JIT)

src/cc/api/BPF.h

@@ -105,6 +105,14 @@ class BPF {
     return BPFArrayTable<ValueType>({});
   }
 
+  template <class ValueType>
+  BPFPercpuArrayTable<ValueType> get_percpu_array_table(const std::string& name) {
+    TableStorage::iterator it;
+    if (bpf_module_->table_storage().Find(Path({bpf_module_->id(), name}), it))
+      return BPFPercpuArrayTable<ValueType>(it->second);
+    return BPFPercpuArrayTable<ValueType>({});
+  }
+
   template <class KeyType, class ValueType>
   BPFHashTable<KeyType, ValueType> get_hash_table(const std::string& name) {
     TableStorage::iterator it;
@@ -113,6 +121,14 @@ class BPF {
     return BPFHashTable<KeyType, ValueType>({});
   }
 
+  template <class KeyType, class ValueType>
+  BPFPercpuHashTable<KeyType, ValueType> get_percpu_hash_table(const std::string& name) {
+    TableStorage::iterator it;
+    if (bpf_module_->table_storage().Find(Path({bpf_module_->id(), name}), it))
+      return BPFPercpuHashTable<KeyType, ValueType>(it->second);
+    return BPFPercpuHashTable<KeyType, ValueType>({});
+  }
+
   BPFProgTable get_prog_table(const std::string& name);
 
   BPFStackTable get_stack_table(const std::string& name,

src/cc/api/BPFTable.cc

@@ -54,6 +54,31 @@ StatusTuple BPFTable::get_value(const std::string& key_str,
   return leaf_to_string(value, value_str);
 }
 
+StatusTuple BPFTable::get_value(const std::string& key_str,
+                                std::vector<std::string>& value_str) {
+  size_t ncpus = get_possible_cpus().size();
+  char key[desc.key_size];
+  char value[desc.leaf_size * ncpus];
+
+  StatusTuple r(0);
+
+  r = string_to_key(key_str, key);
+  if (r.code() != 0)
+    return r;
+
+  if (!lookup(key, value))
+    return StatusTuple(-1, "error getting value");
+
+  value_str.resize(ncpus);
+
+  for (size_t i = 0; i < ncpus; i++) {
+    r = leaf_to_string(value + i * desc.leaf_size, value_str.at(i));
+    if (r.code() != 0)
+      return r;
+  }
+  return StatusTuple(0);
+}
+
 StatusTuple BPFTable::update_value(const std::string& key_str,
                                    const std::string& value_str) {
   char key[desc.key_size];
@@ -75,6 +100,33 @@ StatusTuple BPFTable::update_value(const std::string& key_str,
   return StatusTuple(0);
 }
 
+StatusTuple BPFTable::update_value(const std::string& key_str,
+                                   const std::vector<std::string>& value_str) {
+  size_t ncpus = get_possible_cpus().size();
+  char key[desc.key_size];
+  char value[desc.leaf_size * ncpus];
+
+  StatusTuple r(0);
+
+  r = string_to_key(key_str, key);
+  if (r.code() != 0)
+    return r;
+
+  if (value_str.size() != ncpus)
+    return StatusTuple(-1, "bad value size");
+
+  for (size_t i = 0; i < ncpus; i++) {
+    r = string_to_leaf(value_str.at(i), value + i * desc.leaf_size);
+    if (r.code() != 0)
+      return r;
+  }
+
+  if (!update(key, value))
+    return StatusTuple(-1, "error updating element");
+
+  return StatusTuple(0);
+}
+
 StatusTuple BPFTable::remove_value(const std::string& key_str) {
   char key[desc.key_size];
 

src/cc/api/BPFTable.h

@@ -29,6 +29,7 @@
 #include "bcc_exception.h"
 #include "bcc_syms.h"
 #include "bpf_module.h"
+#include "common.h"
 #include "libbpf.h"
 #include "perf_reader.h"
 #include "table_desc.h"
@@ -67,28 +68,24 @@ class BPFTableBase {
  protected:
   explicit BPFTableBase(const TableDesc& desc) : desc(desc) {}
 
-  bool lookup(KeyType* key, ValueType* value) {
-    return bpf_lookup_elem(desc.fd, static_cast<void*>(key),
-                           static_cast<void*>(value)) >= 0;
+  bool lookup(void* key, void* value) {
+    return bpf_lookup_elem(desc.fd, key, value) >= 0;
   }
 
-  bool first(KeyType* key) {
-    return bpf_get_first_key(desc.fd, static_cast<void*>(key),
-                             desc.key_size) >= 0;
+  bool first(void* key) {
+    return bpf_get_first_key(desc.fd, key, desc.key_size) >= 0;
   }
 
-  bool next(KeyType* key, KeyType* next_key) {
-    return bpf_get_next_key(desc.fd, static_cast<void*>(key),
-                            static_cast<void*>(next_key)) >= 0;
+  bool next(void* key, void* next_key) {
+    return bpf_get_next_key(desc.fd, key, next_key) >= 0;
   }
 
-  bool update(KeyType* key, ValueType* value) {
-    return bpf_update_elem(desc.fd, static_cast<void*>(key),
-                           static_cast<void*>(value), 0) >= 0;
+  bool update(void* key, void* value) {
+    return bpf_update_elem(desc.fd, key, value, 0) >= 0;
   }
 
-  bool remove(KeyType* key) {
-    return bpf_delete_elem(desc.fd, static_cast<void*>(key)) >= 0;
+  bool remove(void* key) {
+    return bpf_delete_elem(desc.fd, key) >= 0;
   }
 
   const TableDesc& desc;
@@ -99,11 +96,23 @@ class BPFTable : public BPFTableBase<void, void> {
   BPFTable(const TableDesc& desc);
 
   StatusTuple get_value(const std::string& key_str, std::string& value);
+  StatusTuple get_value(const std::string& key_str,
+                        std::vector<std::string>& value);
+
   StatusTuple update_value(const std::string& key_str,
                            const std::string& value_str);
+  StatusTuple update_value(const std::string& key_str,
+                           const std::vector<std::string>& value_str);
+
   StatusTuple remove_value(const std::string& key_str);
 };
 
+template <class ValueType>
+void * get_value_addr(ValueType& t) { return &t; }
+
+template <class ValueType>
+void * get_value_addr(std::vector<ValueType>& t) { return t.data(); }
+
 template <class ValueType>
 class BPFArrayTable : public BPFTableBase<int, ValueType> {
 public:
@@ -114,14 +123,14 @@ public:
       throw std::invalid_argument("Table '" + desc.name + "' is not an array table");
   }
 
-  StatusTuple get_value(const int& index, ValueType& value) {
-    if (!this->lookup(const_cast<int*>(&index), &value))
+  virtual StatusTuple get_value(const int& index, ValueType& value) {
+    if (!this->lookup(const_cast<int*>(&index), get_value_addr(value)))
       return StatusTuple(-1, "Error getting value: %s", std::strerror(errno));
     return StatusTuple(0);
   }
 
-  StatusTuple update_value(const int& index, const ValueType& value) {
-    if (!this->update(const_cast<int*>(&index), const_cast<ValueType*>(&value)))
+  virtual StatusTuple update_value(const int& index, const ValueType& value) {
+    if (!this->update(const_cast<int*>(&index), get_value_addr(const_cast<ValueType&>(value))))
       return StatusTuple(-1, "Error updating value: %s", std::strerror(errno));
     return StatusTuple(0);
   }
@@ -143,6 +152,33 @@ public:
   }
 };
 
+template <class ValueType>
+class BPFPercpuArrayTable : public BPFArrayTable<std::vector<ValueType>> {
+ public:
+  BPFPercpuArrayTable(const TableDesc& desc)
+      : BPFArrayTable<std::vector<ValueType>>(desc),
+      ncpus(get_possible_cpus().size()) {
+    if (desc.type != BPF_MAP_TYPE_PERCPU_ARRAY)
+      throw std::invalid_argument("Table '" + desc.name + "' is not a percpu array table");
+    // leaf structures have to be aligned to 8 bytes as hardcoded in the linux kernel.
+    if (sizeof(ValueType) % 8)
+      throw std::invalid_argument("leaf must be aligned to 8 bytes");
+  }
+
+  StatusTuple get_value(const int& index, std::vector<ValueType>& value) {
+    value.resize(ncpus);
+    return BPFArrayTable<std::vector<ValueType>>::get_value(index, value);
+  }
+
+  StatusTuple update_value(const int& index, const std::vector<ValueType>& value) {
+    if (value.size() != ncpus)
+      return StatusTuple(-1, "bad value size");
+    return BPFArrayTable<std::vector<ValueType>>::update_value(index, value);
+  }
+ private:
+  unsigned int ncpus;
+};
+
 template <class KeyType, class ValueType>
 class BPFHashTable : public BPFTableBase<KeyType, ValueType> {
  public:
@@ -155,19 +191,19 @@ class BPFHashTable : public BPFTableBase<KeyType, ValueType> {
       throw std::invalid_argument("Table '" + desc.name + "' is not a hash table");
   }
 
-  StatusTuple get_value(const KeyType& key, ValueType& value) {
-    if (!this->lookup(const_cast<KeyType*>(&key), &value))
+  virtual StatusTuple get_value(const KeyType& key, ValueType& value) {
+    if (!this->lookup(const_cast<KeyType*>(&key), get_value_addr(value)))
       return StatusTuple(-1, "Error getting value: %s", std::strerror(errno));
     return StatusTuple(0);
   }
 
-  StatusTuple update_value(const KeyType& key, const ValueType& value) {
-    if (!this->update(const_cast<KeyType*>(&key), const_cast<ValueType*>(&value)))
+  virtual StatusTuple update_value(const KeyType& key, const ValueType& value) {
+    if (!this->update(const_cast<KeyType*>(&key), get_value_addr(const_cast<ValueType&>(value))))
       return StatusTuple(-1, "Error updating value: %s", std::strerror(errno));
     return StatusTuple(0);
   }
 
-  StatusTuple remove_value(const KeyType& key) {
+  virtual StatusTuple remove_value(const KeyType& key) {
     if (!this->remove(const_cast<KeyType*>(&key)))
       return StatusTuple(-1, "Error removing value: %s", std::strerror(errno));
     return StatusTuple(0);
@@ -213,6 +249,34 @@ class BPFHashTable : public BPFTableBase<KeyType, ValueType> {
   }
 };
 
+template <class KeyType, class ValueType>
+class BPFPercpuHashTable : public BPFHashTable<KeyType, std::vector<ValueType>> {
+ public:
+  explicit BPFPercpuHashTable(const TableDesc& desc)
+      : BPFHashTable<KeyType, std::vector<ValueType>>(desc),
+      ncpus(get_possible_cpus().size()) {
+    if (desc.type != BPF_MAP_TYPE_PERCPU_HASH &&
+        desc.type != BPF_MAP_TYPE_LRU_PERCPU_HASH)
+      throw std::invalid_argument("Table '" + desc.name + "' is not a percpu hash table");
+    // leaf structures have to be aligned to 8 bytes as hardcoded in the linux kernel.
+    if (sizeof(ValueType) % 8)
+      throw std::invalid_argument("leaf must be aligned to 8 bytes");
+  }
+
+  StatusTuple get_value(const KeyType& key, std::vector<ValueType>& value) {
+    value.resize(ncpus);
+    return BPFHashTable<KeyType, std::vector<ValueType>>::get_value(key, value);
+  }
+
+  StatusTuple update_value(const KeyType& key, const std::vector<ValueType>& value) {
+    if (value.size() != ncpus)
+      return StatusTuple(-1, "bad value size");
+    return BPFHashTable<KeyType, std::vector<ValueType>>::update_value(key, value);
+  }
+ private:
+  unsigned int ncpus;
+};
+
 // From src/cc/export/helpers.h
 static const int BPF_MAX_STACK_DEPTH = 127;
 struct stacktrace_t {

tests/cc/test_array_table.cc

@@ -21,6 +21,8 @@
 #include <random>
 #include <iostream>
 
+#include <linux/version.h>
+
 TEST_CASE("test array table", "[array_table]") {
   const std::string BPF_PROGRAM = R"(
     BPF_TABLE("hash", int, int, myhash, 128);
@@ -91,3 +93,55 @@ TEST_CASE("test array table", "[array_table]") {
     REQUIRE(localtable == offlinetable);
   }
 }
+
+#if LINUX_VERSION_CODE >= KERNEL_VERSION(4, 6, 0)
+TEST_CASE("percpu array table", "[percpu_array_table]") {
+  const std::string BPF_PROGRAM = R"(
+    BPF_TABLE("percpu_hash", int, u64, myhash, 128);
+    BPF_TABLE("percpu_array", int, u64, myarray, 64);
+  )";
+
+  ebpf::BPF bpf;
+  ebpf::StatusTuple res(0);
+  res = bpf.init(BPF_PROGRAM);
+  REQUIRE(res.code() == 0);
+
+  ebpf::BPFPercpuArrayTable<uint64_t> t = bpf.get_percpu_array_table<uint64_t>("myarray");
+  size_t ncpus = ebpf::get_possible_cpus().size();
+
+  SECTION("bad table type") {
+    // try to get table of wrong type
+    auto f1 = [&](){
+      bpf.get_percpu_array_table<uint64_t>("myhash");
+    };
+
+    REQUIRE_THROWS(f1());
+  }
+
+  SECTION("standard methods") {
+    int i;
+    std::vector<uint64_t> v1(ncpus);
+    std::vector<uint64_t> v2;
+
+    for (size_t j = 0; j < ncpus; j++) {
+      v1[j] = 42 * j;
+    }
+
+    i = 1;
+    // update element
+    res = t.update_value(i, v1);
+    REQUIRE(res.code() == 0);
+    res = t.get_value(i, v2);
+    REQUIRE(res.code() == 0);
+    REQUIRE(v2.size() == ncpus);
+    for (size_t j = 0; j < ncpus; j++) {
+      REQUIRE(v2.at(j) == 42 * j);
+    }
+
+    // get non existing element
+    i = 1024;
+    res = t.get_value(i, v2);
+    REQUIRE(res.code() != 0);
+  }
+}
+#endif

tests/cc/test_bpf_table.cc

@@ -67,6 +67,37 @@ TEST_CASE("test bpf table", "[bpf_table]") {
   REQUIRE(res.code() != 0);
 }
 
+#if LINUX_VERSION_CODE >= KERNEL_VERSION(4, 6, 0)
+TEST_CASE("test bpf percpu tables", "[bpf_percpu_table]") {
+  const std::string BPF_PROGRAM = R"(
+    BPF_TABLE("percpu_hash", int, u64, myhash, 128);
+  )";
+
+  ebpf::BPF bpf;
+  ebpf::StatusTuple res(0);
+  res = bpf.init(BPF_PROGRAM);
+  REQUIRE(res.code() == 0);
+
+  ebpf::BPFTable t = bpf.get_table("myhash");
+  size_t ncpus = ebpf::get_possible_cpus().size();
+
+  std::vector<std::string> v1(ncpus);
+  for (size_t i = 0; i < ncpus; i++) {
+    v1.at(i) = std::to_string(42 * i);
+  }
+
+  // update element
+  std::vector<std::string> value;
+  res = t.update_value("0x07", v1);
+  REQUIRE(res.code() == 0);
+  res = t.get_value("0x07", value);
+  REQUIRE(res.code() == 0);
+  for (size_t i = 0; i < ncpus; i++) {
+    REQUIRE(42 * i == std::stoul(value.at(i), nullptr, 16));
+  }
+}
+#endif
+
 TEST_CASE("test bpf hash table", "[bpf_hash_table]") {
   const std::string BPF_PROGRAM = R"(
     BPF_HASH(myhash, int, int, 128);

tests/cc/test_hash_table.cc

@@ -15,6 +15,7 @@
  */
 
 #include "BPF.h"
+#include <linux/version.h>
 
 #include "catch.hpp"
 
@@ -85,3 +86,75 @@ TEST_CASE("test hash table", "[hash_table]") {
     }
   }
 }
+
+#if LINUX_VERSION_CODE >= KERNEL_VERSION(4,6,0)
+TEST_CASE("percpu hash table", "[percpu_hash_table]") {
+  const std::string BPF_PROGRAM = R"(
+    BPF_TABLE("percpu_hash", int, u64, myhash, 128);
+    BPF_TABLE("percpu_array", int, u64, myarray, 64);
+  )";
+
+  ebpf::BPF bpf;
+  ebpf::StatusTuple res(0);
+  res = bpf.init(BPF_PROGRAM);
+  REQUIRE(res.code() == 0);
+
+  ebpf::BPFPercpuHashTable<int, uint64_t> t =
+    bpf.get_percpu_hash_table<int, uint64_t>("myhash");
+  size_t ncpus = ebpf::get_possible_cpus().size();
+
+  SECTION("bad table type") {
+    // try to get table of wrong type
+    auto f1 = [&](){
+      bpf.get_percpu_hash_table<int, uint64_t>("myarray");
+    };
+
+    REQUIRE_THROWS(f1());
+  }
+
+  SECTION("standard methods") {
+    int k;
+    std::vector<uint64_t> v1(ncpus);
+    std::vector<uint64_t> v2;
+
+    for (size_t j = 0; j < ncpus; j++) {
+      v1[j] = 42 * j;
+    }
+
+    k = 1;
+
+    // create new element
+    res = t.update_value(k, v1);
+    REQUIRE(res.code() == 0);
+    res = t.get_value(k, v2);
+    REQUIRE(res.code() == 0);
+    for (size_t j = 0; j < ncpus; j++) {
+      REQUIRE(v2.at(j) == 42 * j);
+    }
+
+    // update existing element
+    for (size_t j = 0; j < ncpus; j++) {
+      v1[j] = 69 * j;
+    }
+    res = t.update_value(k, v1);
+    REQUIRE(res.code() == 0);
+    res = t.get_value(k, v2);
+    REQUIRE(res.code() == 0);
+    for (size_t j = 0; j < ncpus; j++) {
+      REQUIRE(v2.at(j) == 69 * j);
+    }
+
+    // remove existing element
+    res = t.remove_value(k);
+    REQUIRE(res.code() == 0);
+
+    // remove non existing element
+    res = t.remove_value(k);
+    REQUIRE(res.code() != 0);
+
+    // get non existing element
+    res = t.get_value(k, v2);
+    REQUIRE(res.code() != 0);
+  }
+}
+#endif
\ No newline at end of file