Skip to content

Commit

Permalink
bpf: fix error propagation for bpf syscalls
Browse files Browse the repository at this point in the history
Currently, interaction with BPF maps via syscalls (open, lookup) might result
in log messages of the following form, where the error detail is `success`:

```
[info][filter] [cilium/conntrack.cc:229] cilium.bpf_metadata: IPv4 conntrack map global lookup failed: Success
```

This is due to the fact that BPF maps are accessed in the starter process. Hence,
the syscalls are also executed in this separate process and the variable `errno` is never set
in the Envoy process where the log is written..

Therefore, this commit fixes the error propagation by setting the variable `errno` after
retrieving the response from the privileged client doing the call to the starter
process.

Fixes: cilium#315
Fixes: cilium#470

Signed-off-by: Marco Hofstetter <[email protected]>
  • Loading branch information
mhofstetter committed Mar 26, 2024
1 parent d5c6260 commit c73651f
Showing 1 changed file with 10 additions and 1 deletion.
11 changes: 10 additions & 1 deletion cilium/bpf.cc
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
#include "cilium/bpf.h"

#include <errno.h>

#include "source/common/common/utility.h"

#include "cilium/privileged_service_client.h"
Expand Down Expand Up @@ -87,17 +89,24 @@ bool Bpf::open(const std::string& path) {
} else if (ret.errno_ == ENOENT && log_on_error) {
ENVOY_LOG(debug, "cilium.bpf_metadata: bpf syscall for map {} failed: {}", path,
Envoy::errorDetails(ret.errno_));
errno = ret.errno_;
} else if (log_on_error) {
ENVOY_LOG(warn, "cilium.bpf_metadata: bpf syscall for map {} failed: {}", path,
Envoy::errorDetails(ret.errno_));
errno = ret.errno_;
}

return false;
}

bool Bpf::lookup(const void* key, void* value) {
auto& cilium_calls = PrivilegedService::Singleton::get();
return cilium_calls.bpf_lookup(fd_, key, key_size_, value, value_size_).return_value_ == 0;
auto result = cilium_calls.bpf_lookup(fd_, key, key_size_, value, value_size_);
if (result.return_value_ < 0) {
errno = result.errno_;
}

return result.return_value_ == 0;
}

} // namespace Cilium
Expand Down

1 comment on commit c73651f

@zviratko
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this fixes cilium#470, does it also fix cilium/cilium#29406 and cilium/cilium#30510 ? I believe some of those are related (or I'm hitting more of them in my setup)

Please sign in to comment.