add BPF::init_usdt function to init a single USDT so folks can handle partial init...
authorDave Marchevsky <davemarchevsky@gmail.com>
Tue, 30 Jul 2019 18:17:37 +0000 (11:17 -0700)
committeryonghong-song <ys114321@gmail.com>
Thu, 14 Nov 2019 19:12:28 +0000 (11:12 -0800)
modify BPF::init so that failure doesn't leave the BPF object in a broken state such that subsequent inits will also fail

docs/reference_guide.md
src/cc/api/BPF.cc
src/cc/api/BPF.h
tests/cc/test_usdt_probes.cc

index 506c9ab..7f22f1c 100644 (file)
@@ -245,6 +245,8 @@ int do_trace(struct pt_regs *ctx) {
 
 This reads the sixth USDT argument, and then pulls it in as a string to ```path```.
 
+When initializing USDTs via the third argument of ```BPF::init``` in the C API, if any USDT fails to ```init```, entire ```BPF::init``` will fail. If you're OK with some USDTs failing to ```init```, use ```BPF::init_usdt``` before calling ```BPF::init```.
+
 Examples in situ:
 [code](https://github.com/iovisor/bcc/commit/4f88a9401357d7b75e917abd994aa6ea97dda4d3#diff-04a7cad583be5646080970344c48c1f4R24),
 [search /examples](https://github.com/iovisor/bcc/search?q=bpf_usdt_readarg+path%3Aexamples&type=Code),
index a58f23b..66dba74 100644 (file)
@@ -55,18 +55,33 @@ std::string sanitize_str(std::string str, bool (*validator)(char),
   return str;
 }
 
+StatusTuple BPF::init_usdt(const USDT& usdt) {
+  USDT u(usdt);
+  StatusTuple init_stp = u.init();
+  if (init_stp.code() != 0) {
+    return init_stp;
+  }
+
+  usdt_.push_back(std::move(u));
+  all_bpf_program_ += usdt_.back().program_text_;
+  return StatusTuple(0);
+}
+
+void BPF::init_fail_reset() {
+  usdt_.clear();
+  all_bpf_program_ = "";
+}
+
 StatusTuple BPF::init(const std::string& bpf_program,
                       const std::vector<std::string>& cflags,
                       const std::vector<USDT>& usdt) {
-  std::string all_bpf_program;
-
   usdt_.reserve(usdt.size());
   for (const auto& u : usdt) {
-    usdt_.emplace_back(u);
-  }
-  for (auto& u : usdt_) {
-    TRY2(u.init());
-    all_bpf_program += u.program_text_;
+    StatusTuple init_stp = init_usdt(u);
+    if (init_stp.code() != 0) {
+      init_fail_reset();
+      return init_stp;
+    }
   }
 
   auto flags_len = cflags.size();
@@ -74,9 +89,11 @@ StatusTuple BPF::init(const std::string& bpf_program,
   for (size_t i = 0; i < flags_len; i++)
     flags[i] = cflags[i].c_str();
 
-  all_bpf_program += bpf_program;
-  if (bpf_module_->load_string(all_bpf_program, flags, flags_len) != 0)
+  all_bpf_program_ += bpf_program;
+  if (bpf_module_->load_string(all_bpf_program_, flags, flags_len) != 0) {
+    init_fail_reset();
     return StatusTuple(-1, "Unable to initialize BPF program");
+  }
 
   return StatusTuple(0);
 };
index 63a39f4..ff45903 100644 (file)
@@ -58,6 +58,8 @@ class BPF {
                    const std::vector<std::string>& cflags = {},
                    const std::vector<USDT>& usdt = {});
 
+  StatusTuple init_usdt(const USDT& usdt);
+
   ~BPF();
   StatusTuple detach_all();
 
@@ -244,6 +246,8 @@ class BPF {
                                   uint64_t& offset_res,
                                   uint64_t symbol_offset = 0);
 
+  void init_fail_reset();
+
   int flag_;
 
   void *bsymcache_;
@@ -255,6 +259,7 @@ class BPF {
   std::map<std::string, int> funcs_;
 
   std::vector<USDT> usdt_;
+  std::string all_bpf_program_;
 
   std::map<std::string, open_probe_t> kprobes_;
   std::map<std::string, open_probe_t> uprobes_;
index 71c75a1..6015c5d 100644 (file)
@@ -99,6 +99,32 @@ TEST_CASE("test find a probe in our process' shared libs with c++ API", "[usdt]"
   REQUIRE(res.code() == 0);
 }
 
+TEST_CASE("test usdt partial init w/ fail init_usdt", "[usdt]") {
+  ebpf::BPF bpf;
+  ebpf::USDT u(::getpid(), "libbcc_test", "sample_lib_probe_nonexistent", "on_event");
+  ebpf::USDT p(::getpid(), "libbcc_test", "sample_lib_probe_1", "on_event");
+
+  // We should be able to fail initialization and subsequently do bpf.init w/o USDT
+  // successfully
+  auto res = bpf.init_usdt(u);
+  REQUIRE(res.msg() != "");
+  REQUIRE(res.code() != 0);
+
+  // Shouldn't be necessary to re-init bpf object either after failure to init w/
+  // bad USDT
+  res = bpf.init("int on_event() { return 0; }", {}, {u});
+  REQUIRE(res.msg() != "");
+  REQUIRE(res.code() != 0);
+
+  res = bpf.init_usdt(p);
+  REQUIRE(res.msg() == "");
+  REQUIRE(res.code() == 0);
+
+  res = bpf.init("int on_event() { return 0; }", {}, {});
+  REQUIRE(res.msg() == "");
+  REQUIRE(res.code() == 0);
+}
+
 class ChildProcess {
   pid_t pid_;