Skip to content

Commit

Permalink
add BPF::init_usdt function to init a single USDT so folks can handle…
Browse files Browse the repository at this point in the history
… partial init failure of list of USDTs.

modify BPF::init so that failure doesn't leave the BPF object in a broken state such that subsequent inits will also fail
  • Loading branch information
davemarchevsky authored and yonghong-song committed Nov 14, 2019
1 parent 992e482 commit ccf8261
Show file tree
Hide file tree
Showing 4 changed files with 59 additions and 9 deletions.
2 changes: 2 additions & 0 deletions docs/reference_guide.md
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
35 changes: 26 additions & 9 deletions src/cc/api/BPF.cc
Original file line number Diff line number Diff line change
Expand Up @@ -55,28 +55,45 @@ 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();
const char* flags[flags_len];
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);
};
Expand Down
5 changes: 5 additions & 0 deletions src/cc/api/BPF.h
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down Expand Up @@ -244,6 +246,8 @@ class BPF {
uint64_t& offset_res,
uint64_t symbol_offset = 0);

void init_fail_reset();

int flag_;

void *bsymcache_;
Expand All @@ -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_;
Expand Down
26 changes: 26 additions & 0 deletions tests/cc/test_usdt_probes.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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_;

Expand Down

0 comments on commit ccf8261

Please sign in to comment.