> On July 10, 2024, 8:42 p.m., Benjamin Mahler wrote: > > src/linux/ebpf.cpp > > Line 122 (original), 130 (patched) > > <https://reviews.apache.org/r/75080/diff/1/?file=2289991#file2289991line130> > > > > 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?) > > Jason Zhou wrote: > I feel like this is implicitly tested in the DeviceControllerTestFixture > by us using this helper function in the attach and detach functions, so I'm > not sure if we need a unit test. I'll make one anyway and we can decide if we > should keep it.
Review: https://reviews.apache.org/r/75083/ - Jason ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/75080/#review226651 ----------------------------------------------------------- On July 11, 2024, 3:51 p.m., Jason Zhou wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/75080/ > ----------------------------------------------------------- > > (Updated July 11, 2024, 3:51 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 files 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/cgroups2.cpp d1fc2638cdf9a07199f90952e04998072021011c > src/linux/ebpf.cpp 3f7f74df25dbf35720cd5f6c19644173552d5b82 > src/tests/containerizer/cgroups2_tests.cpp > cb1e229f7f40aa71f57417c33fccb2cfb313a1f5 > > > Diff: https://reviews.apache.org/r/75080/diff/2/ > > > 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 > >
