You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by "DevinLeamy (via GitHub)" <gi...@apache.org> on 2024/03/11 16:05:53 UTC
[PR] [ebpf][cgroups2] Fetch the program ids of the eBPF programs attached to a cgroup [mesos]
DevinLeamy opened a new pull request, #509:
URL: https://github.com/apache/mesos/pull/509
Removing programs that are attached to a cgroup is a three step process.
1. We fetch the program ids that are attached to the cgroup.
2. We fetch the file descriptors of the attached programs, using their program ids.
3. We use the file descriptors of the attached programs to detach them.
This patch introduces the function `ebpf::cgroups2::attached()` to carry out the first step. This will be followed by patches for the subsequent two steps.
--
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: reviews-unsubscribe@mesos.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [ebpf][cgroups2] Fetch the program ids of the eBPF programs attached to a cgroup [mesos]
Posted by "DevinLeamy (via GitHub)" <gi...@apache.org>.
DevinLeamy commented on code in PR #509:
URL: https://github.com/apache/mesos/pull/509#discussion_r1520063442
##########
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:
Went with the second one.
--
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: reviews-unsubscribe@mesos.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [ebpf][cgroups2] Fetch the program ids of the eBPF programs attached to a cgroup [mesos]
Posted by "bmahler (via GitHub)" <gi...@apache.org>.
bmahler closed pull request #509: [ebpf][cgroups2] Fetch the program ids of the eBPF programs attached to a cgroup
URL: https://github.com/apache/mesos/pull/509
--
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: reviews-unsubscribe@mesos.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [ebpf][cgroups2] Fetch the program ids of the eBPF programs attached to a cgroup [mesos]
Posted by "bmahler (via GitHub)" <gi...@apache.org>.
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: reviews-unsubscribe@mesos.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org