bmahler commented on code in PR #509:
URL: https://github.com/apache/mesos/pull/509#discussion_r1520006104
##########
src/linux/ebpf.cpp:
##########
@@ -151,6 +163,52 @@ Try<Nothing> attach(int fd, const string& cgroup)
return Nothing();
}
+
+Try<vector<uint32_t>> attached(const string& cgroup)
+{
+ Try<int> cgroup_fd = os::open(cgroup, O_DIRECTORY | O_RDONLY | O_CLOEXEC);
+ if (cgroup_fd.isError()) {
+ return Error("Failed to open '" + cgroup + "': " + cgroup_fd.error());
+ }
+
+ // Program ids are unsigned 32-bit integers. We assume that a maximum
+ // of 32 programs are attached to a cgroup; there should only be 0 or 1 but
+ // we allow for more to be safe. We initialize our query buffer with -1 so
+ // we can distinguish between a returned program id and filler values.
+ //
+ // Example: [1, 7, 3, -1, -1, -1, ..., -1] => [1, 7, 3] are program ids.
+ const int MAX_IDS = 32;
+ vector<int32_t> buffer(MAX_IDS, -1);
+
+ bpf_attr attr;
+ memset(&attr, 0, sizeof(attr));
+ attr.query.target_fd = *cgroup_fd;
+ attr.query.attach_type = BPF_CGROUP_DEVICE;
+ attr.query.prog_cnt = MAX_IDS;
+ attr.query.prog_ids = reinterpret_cast<uint64_t>(buffer.data());
+
+ Try<int, ErrnoError> result = bpf(BPF_PROG_QUERY, &attr, sizeof(attr));
+
Review Comment:
maybe close here instead of both conditionals below?
##########
src/linux/ebpf.cpp:
##########
@@ -53,6 +54,17 @@ Try<int, ErrnoError> bpf(int cmd, bpf_attr* attr, size_t
size)
}
+Try<int> path_to_fd(const string& path)
+{
+ Try<int> fd = os::open(path, O_DIRECTORY | O_RDONLY);
+ if (fd.isError()) {
+ return Error("Failed to attach cgroup program : " + fd.error());
+ }
+
+ return fd.get();
+}
Review Comment:
not needed?
##########
src/linux/ebpf.cpp:
##########
@@ -151,6 +163,52 @@ Try<Nothing> attach(int fd, const string& cgroup)
return Nothing();
}
+
+Try<vector<uint32_t>> attached(const string& cgroup)
+{
+ Try<int> cgroup_fd = os::open(cgroup, O_DIRECTORY | O_RDONLY | O_CLOEXEC);
+ if (cgroup_fd.isError()) {
+ return Error("Failed to open '" + cgroup + "': " + cgroup_fd.error());
+ }
+
+ // Program ids are unsigned 32-bit integers. We assume that a maximum
+ // of 32 programs are attached to a cgroup; there should only be 0 or 1 but
+ // we allow for more to be safe. We initialize our query buffer with -1 so
+ // we can distinguish between a returned program id and filler values.
+ //
+ // Example: [1, 7, 3, -1, -1, -1, ..., -1] => [1, 7, 3] are program ids.
+ const int MAX_IDS = 32;
+ vector<int32_t> buffer(MAX_IDS, -1);
+
+ bpf_attr attr;
+ memset(&attr, 0, sizeof(attr));
+ attr.query.target_fd = *cgroup_fd;
+ attr.query.attach_type = BPF_CGROUP_DEVICE;
+ attr.query.prog_cnt = MAX_IDS;
+ attr.query.prog_ids = reinterpret_cast<uint64_t>(buffer.data());
+
+ Try<int, ErrnoError> result = bpf(BPF_PROG_QUERY, &attr, sizeof(attr));
+
+ if (result.isError()) {
+ os::close(*cgroup_fd);
+ return Error(
+ "BPF syscall to find a program ids attached to '" + cgroup +
+ "' failed: " + result.error().message);
Review Comment:
the caller is going to know that we're trying to find device programs
attached to the given cgroup, i.e. the caller will log something like:
```
Try<x> attached = ebpf::cgroups2::attached("/some/cgroup/path");
if error:
return Error("Failed to find bpf programs attached to '/some/cgroup/path':
" + attached.error());
```
So to compose the messages well, here we can do something like:
```
return result.error();
```
or:
```
return Error("bpf syscall to BPF_PROG_QUERY for BPF_CGROUP_DEVICE programs
failed: " + result.error().message);
```
Since these ^ are the piece of information the caller doesn't know and won't
include when composing.
##########
src/linux/ebpf.hpp:
##########
@@ -61,6 +61,11 @@ namespace cgroups2 {
// device access dynamically.
Try<Nothing> attach(int fd, const std::string& cgroup);
+
+// Get the program ids of all programs that have been attached to a cgroup.
+// @param cgroup Absolute path of a cgroup.
+Try<std::vector<uint32_t>> attached(const std::string& cgroup);
Review Comment:
there appear to be several types of cgroup related bpf programs, but this is
device specific, it would be good to put device specific stuff in a device::
namespace here, don't need to do it in this patch though
##########
src/linux/ebpf.cpp:
##########
@@ -151,6 +163,52 @@ Try<Nothing> attach(int fd, const string& cgroup)
return Nothing();
}
+
+Try<vector<uint32_t>> attached(const string& cgroup)
+{
+ Try<int> cgroup_fd = os::open(cgroup, O_DIRECTORY | O_RDONLY | O_CLOEXEC);
+ if (cgroup_fd.isError()) {
+ return Error("Failed to open '" + cgroup + "': " + cgroup_fd.error());
+ }
+
+ // Program ids are unsigned 32-bit integers. We assume that a maximum
+ // of 32 programs are attached to a cgroup; there should only be 0 or 1 but
+ // we allow for more to be safe. We initialize our query buffer with -1 so
+ // we can distinguish between a returned program id and filler values.
+ //
+ // Example: [1, 7, 3, -1, -1, -1, ..., -1] => [1, 7, 3] are program ids.
+ const int MAX_IDS = 32;
+ vector<int32_t> buffer(MAX_IDS, -1);
Review Comment:
looks like the output size is from attr.query.prog_cnt, strangely enough, we
could simplify things here if we use that rather than inferring from the values
of the ids
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
