----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/75080/#review226651 -----------------------------------------------------------
src/linux/ebpf.cpp Line 115 (original), 116 (patched) <https://reviews.apache.org/r/75080/#comment314914> we should always touch the header too, but maybe instead we just make our cgroups functions support both relative and absolute paths by updating cgroups2::path to handle both cases? src/linux/ebpf.cpp Line 122 (original), 130 (patched) <https://reviews.apache.org/r/75080/#comment314913> maybe move this inside the if condition so that it's only used by 1 call? maybe we should add a wrapper in ebpf.hpp? ``` Try<int> program_fd(uint32_t program_id); // this fd has to be closed by caller always? ``` helper could be its own patch (w/ unit test maybe?) src/linux/ebpf.cpp Lines 131 (patched) <https://reviews.apache.org/r/75080/#comment314912> maybe use an Option<> here and below, so that we don't have to worry about uninitialization warnings on the integer? src/linux/ebpf.cpp Lines 128-150 (original), 156-178 (patched) <https://reviews.apache.org/r/75080/#comment314911> adjust the comment now? src/tests/containerizer/cgroups2_tests.cpp Lines 797-805 (patched) <https://reviews.apache.org/r/75080/#comment314910> let's just add a new unit test for this, since we're not testing allow/deny functionality and don't care about the parameterization - Benjamin Mahler On July 10, 2024, 7:17 p.m., Jason Zhou wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/75080/ > ----------------------------------------------------------- > > (Updated July 10, 2024, 7:17 p.m.) > > > Review request for mesos and Benjamin Mahler. > > > Repository: mesos > > > Description > ------- > > Currently, if we try to attach device ebpf files to the same cgroup > multiple times, they will all be attached, and they will all be run > when a device requests access. This conflicts with our design to have > one ebpf file per cgroup that represents all the devices they want to > allow or deny, where that file is updated when the cgroup adds or > removes a device. So we add a patch to atomically replace any existing > ebpf file already attached to our target cgroup using our new ebpf file. > > > Diffs > ----- > > src/linux/ebpf.cpp 3f7f74df25dbf35720cd5f6c19644173552d5b82 > src/tests/containerizer/cgroups2_tests.cpp > cb1e229f7f40aa71f57417c33fccb2cfb313a1f5 > > > Diff: https://reviews.apache.org/r/75080/diff/1/ > > > Testing > ------- > > Added a test to verify that when a we attach a ebpf file to a cgroup that > already has a file attached, the old file is replaced and the new file now > controls device access. Cgroups2 tests passed. > > > Thanks, > > Jason Zhou > >
