bmahler commented on code in PR #511:
URL: https://github.com/apache/mesos/pull/511#discussion_r1520343182
##########
src/linux/ebpf.cpp:
##########
@@ -99,6 +99,48 @@ Try<int> load(const Program& program)
return *fd;
}
+
+// Detach an eBPF program from a target (AKA path) by its attachment type
+// and program id. Returns Nothing() on success or if no program is found.
+Try<Nothing> detach(uint32_t attach_type, int program_id, const string& target)
+{
+ Try<int> target_fd = os::open(target, O_DIRECTORY | O_RDONLY | O_CLOEXEC);
Review Comment:
this is getting leaked without a corresponding os::close
##########
src/linux/ebpf.cpp:
##########
@@ -99,6 +99,48 @@ Try<int> load(const Program& program)
return *fd;
}
+
+// Detach an eBPF program from a target (AKA path) by its attachment type
+// and program id. Returns Nothing() on success or if no program is found.
+Try<Nothing> detach(uint32_t attach_type, int program_id, const string& target)
Review Comment:
what is target here? isn't it a cgroup? in which case, shouldn't we make
this function cgroup namespaced? I guess to be generic this would be taking an
int target which is either a detach location fd or a netdevice ifindex, but
since we're only doing cgroup device stuff, maybe we just keep the code cgroup
specific for now, like we did with attach()?
##########
src/linux/ebpf.hpp:
##########
@@ -61,6 +61,15 @@ namespace cgroups2 {
// device access dynamically.
Try<Nothing> attach(int fd, const std::string& cgroup);
+// Detach a BPF_CGROUP_DEVICE eBPF program from a cgroup, by program id.
+// @param cgroup Absolute path of a cgroup.
+// @param program_id Id of the eBPF program to detach.
Review Comment:
let's clarify that if we don't find the program attached, this returns
Nothing rather than an error
##########
src/linux/ebpf.cpp:
##########
@@ -99,6 +99,48 @@ Try<int> load(const Program& program)
return *fd;
}
+
+// Detach an eBPF program from a target (AKA path) by its attachment type
+// and program id. Returns Nothing() on success or if no program is found.
+Try<Nothing> detach(uint32_t attach_type, int program_id, const string& target)
+{
+ Try<int> target_fd = os::open(target, O_DIRECTORY | O_RDONLY | O_CLOEXEC);
+ if (target_fd.isError()) {
+ return Error("Failed to open '" + target + "': " + target_fd.error());
+ }
+
+ bpf_attr attr;
+ memset(&attr, 0, sizeof(attr));
+ attr.prog_id = program_id;
+
+ Try<int, ErrnoError> program_fd =
+ bpf(BPF_PROG_GET_FD_BY_ID, &attr, sizeof(attr));
Review Comment:
from the docs, it looks like loading a program produces an fd, and unloading
a program is done by closing the fd, so you probably want to keep that in mind
here and figure out the implications of that (do we need an unload function
that will detach and close fds..?, replacement will need to close the previous
fd..? etc)
##########
src/linux/ebpf.hpp:
##########
@@ -61,6 +61,15 @@ namespace cgroups2 {
// device access dynamically.
Try<Nothing> attach(int fd, const std::string& cgroup);
+// Detach a BPF_CGROUP_DEVICE eBPF program from a cgroup, by program id.
+// @param cgroup Absolute path of a cgroup.
+// @param program_id Id of the eBPF program to detach.
Review Comment:
or better yet, we surface this error and no need to update this comment
--
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]
